Skip to content

Constants are possibly incorrect #5

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

Closed
ghost opened this issue Apr 19, 2018 · 4 comments
Closed

Constants are possibly incorrect #5

ghost opened this issue Apr 19, 2018 · 4 comments

Comments

@ghost
Copy link

ghost commented Apr 19, 2018

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
Copy link
Contributor

kattni commented Dec 3, 2018

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
Copy link
Contributor

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
Copy link
Contributor

kattni commented Mar 18, 2019

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

@sommersoft
Copy link
Collaborator

Fixed by #9. Thanks again @barbudor!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

3 participants