You signed in with another tab or window. Reload to refresh your session.You signed out in another tab or window. Reload to refresh your session.You switched accounts on another tab or window. Reload to refresh your session.Dismiss alert
Here are a few items found during an automated review:
Issues
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.
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.
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.
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.
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.
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.
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.
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.
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.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
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.
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