Skip to content

Initial support for ADS1015 (4-channel 12-bit ADC) - #887

Open
conejoninja wants to merge 2 commits into
tinygo-org:devfrom
conejoninja:ads1015
Open

Initial support for ADS1015 (4-channel 12-bit ADC)#887
conejoninja wants to merge 2 commits into
tinygo-org:devfrom
conejoninja:ads1015

Conversation

@conejoninja

Copy link
Copy Markdown
Member

Initial support for ADS1015 (4-channel 12-bit ADC) used in the NiceCtrlr

It's been a while since I made a driver, so I'm a bit rusty, let me know if something is missing.

Disclaimer: I used the assistance of ClaudeCode

@deadprogram

Copy link
Copy Markdown
Member

Thanks for working on this @conejoninja

Here are a few items found during an automated review:

Issues

  1. Read() waits too little in continuous mode (ads1015.go). The code sleeps conversionPollInterval (1 ms) after a mux change. One conversion needs 7.8 ms at 128 SPS and 4 ms at 250 SPS, thus Value() can return the result of the previous mux setting. Calculate the delay from d.config.DataRate, or poll.

  2. Ready() doc is not correct (ads1015.go). The comment says "In continuous mode it always reports true". The OS bit reads 0 while a conversion is in progress, and in continuous mode the device converts continuously, thus the function usually returns false. The code is not affected, because Read() does not call Ready() in continuous mode, but the doc can mislead.

  3. ToVoltage() divides by 2047 (ads1015.go). The datasheet gives an LSB of FS/2048 (3 mV at +/-6.144 V). The divisor 2047 adds an error of approximately 0.05% at full scale. The RobTillaart library does the same, thus this is possibly intentional. If it is, add a short note.

  4. Possible race in single-shot mode (ads1015.go). Read() polls Ready() immediately after startConversion(). If the device did not yet clear the OS bit, the first poll can report ready and Value() returns the previous result. One sleep before the first poll prevents this.

  5. Dead commented-out constants (registers.go). The two Gain0256mV lines (0b110, 0b111) and the second DataRate3300SPS line are commented-out code. Keep only a one-line note that these codes repeat the last setting.

  6. Address comment does not agree with the code (registers.go). The comment says "see the datasheet for the other three options (0x49, 0x4A, 0x4B)", but Address2, Address3, and Address4 define them on the next lines. Also, names that show the ADDR pin connection (for example AddressADDRtoVDD) are more clear than a number.

  7. Magic value (ads1015.go). SetThresholdHigh(-32768) and SetThresholdLow(0) are the special conversion-ready values. Named constants make the intent more clear than the literal.

  8. No unit test. 27 of 137 driver directories have tests, thus this is not a repo rule. A small fake I2C bus can test Value() sign extension, ToVoltage(), and the config word that startConversion() writes.

Item 1 is the only defect that gives incorrect data. Items 5 and 6 are quick cleanups.

@conejoninja

Copy link
Copy Markdown
Member Author

Thanks, issues has been addressed (I think). Claude helped me (a lot) with the tests, if that's not allowed, let me know and I'll rewrite them again.

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

Successfully merging this pull request may close these issues.

2 participants