adafruit / Adafruit_CircuitPython_INA219

circuit python driver for INA219 current sensor
MIT License
26 stars 26 forks source link

Constants are possibly incorrect #5

Closed ghost closed 5 years ago

ghost commented 6 years ago

I think these are wrong:

_CONFIG_BADCRES_MASK             = const(0x0780)  # Bus ADC Resolution Mask
_CONFIG_BADCRES_9BIT             = const(0x0080)  # 9-bit bus res = 0..511
_CONFIG_BADCRES_10BIT            = const(0x0100)  # 10-bit bus res = 0..1023
_CONFIG_BADCRES_11BIT            = const(0x0200)  # 11-bit bus res = 0..2047
_CONFIG_BADCRES_12BIT            = const(0x0400)  # 12-bit bus res = 0..4097

Based on Table 5 in the IN219 datasheet, they should probably be the following, inserting 0 for all X (don't-cares), and changing 4097 to 4095 in the comment.

_CONFIG_BADCRES_MASK             = const(0x0780)  # Bus ADC Resolution Mask
_CONFIG_BADCRES_9BIT             = const(0x0000)  # 9-bit bus res = 0..511
_CONFIG_BADCRES_10BIT            = const(0x0080)  # 10-bit bus res = 0..1023
_CONFIG_BADCRES_11BIT            = const(0x0100)  # 11-bit bus res = 0..2047
_CONFIG_BADCRES_12BIT            = const(0x0180)  # 12-bit bus res = 0..4095

Also, for the "BADCRES" bits, you've left off the rest of Table 5, even though it is included for the "SADCRES" bits. The averaging features area available for both. Perhaps all those constants should be refactored to use offsets so they can be shared between BADCRES and SADCRES.

kattni commented 5 years ago

It looks like several of those constants aren't used, which may explain why not all of Table 5 was included. Have you tested these changes? Would you be interested in putting in a PR with the changes you suggest? Thanks!

barbudor commented 5 years ago

Hi @kattni , I've fixed the values to match the datasheet(as identified by user above) and added the missing ones. I also added new properties to access the config register and its fields. I would like to submit a PR for the above. Best regards

kattni commented 5 years ago

@barbudor Thank you for submitting the PR! I'm going to request a review from our group.

sommersoft commented 5 years ago

Fixed by #9. Thanks again @barbudor!