Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Added Comparator functionality #98
Added Comparator functionality #98
Changes from 4 commits
017fb9f
c84a39d
7d98fde
0aa1cd7
29ee801
a730639
f74e4f9
4ba6144
8ddd81c
657b768
f7fd261
3e31097
File filter
Filter by extension
Conversations
Jump to
There are no files selected for viewing
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Why is the low threshold default higher than the default high threshold?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This is tricky because the values are in 2's complement, but also they are 12-bit values stored in 16-bit registers where the 4 LSBs are 0. Because of this, 0x8000 is the lowest possible value (decimal -32768 in 16-bit or really -2048 in 12-bit) and 0x7FF0 is the highest possible value (decimal 32752 in 16-bit or really 2047 in 12-bit).
I expanded the comments to hopefully explain this better.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Ah! I think it'll be clearer to set the defaults to the actual values and have the properties do the conversion and masking.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I set the defaults to the actual (12-bit signed) values so that it is easier to read, and moved the 2's complement and bit shift to 16-bit to the setter functions.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Please use 16-bit values because this library is also for the ADS1115 which is 16-bit. In the math you can do
self.bits
to read if it is 12 or 16 bit.There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I tried to copy the handling of the comparator low/high threshold properties just like the other properties, such as data rate. The subclasses handle the lower level chip differences (12 vs 16 bits, default settings, etc) and pass those back to the higher level ads1x15 class.
Also, the ADC data in the analog_in class is handled as a signed 16-bit integer, so I felt it was appropriate to similarly handle the comparator threshold values as signed 16-bit integers as well.