Skip to content

drivers: add a common driver for ADS1X1X#21694

Merged
crasbe merged 1 commit intoRIOT-OS:masterfrom
baptleduc:ads1x1x-driver-clean
Mar 6, 2026
Merged

drivers: add a common driver for ADS1X1X#21694
crasbe merged 1 commit intoRIOT-OS:masterfrom
baptleduc:ads1x1x-driver-clean

Conversation

@baptleduc
Copy link
Copy Markdown
Contributor

@baptleduc baptleduc commented Sep 4, 2025 •
edited
Loading

Contribution description

While attempting to create a driver for my ADS1115 (this PR), I realized that a driver already exists here. However, the existing driver does not fully support the ADS111x family (datasheet here), which differs from the ADS101x series (datasheet here in several aspects such as bit resolution and maximum sample rate (see page 3 of the datasheet).

I have therefore implemented a common driver for the ADS1x1x family, better documented and inspired by the existing ADS101x driver. This unified driver supports both ADS101x and ADS111x variants, handling their differences internally.

Testing procedure

I adapted the existing test tests/drivers/ads101x to create `tests/drivers/ads111x and compared the outputs to ensure identical behavior and no regressions.

It may be worth adding a helper function to verify the configuration register after each write, to ensure it contains the expected value.

Open questions

  • In a discussion on Element.io, @Enoch247 pointed out that my current family-selection system is exclusive, which prevents supporting a case where both an ADS101x and an ADS111x driver are used in the same application. How should this be handled?
  • Is it necessary to check the return value of i2c_read_regs and i2c_write_regs every time?
  • I am not entirely clear on the difference between the return codes I2C_NODEV and I2C_NOI2C, so I am not sure I have used them correctly throughout the driver.

Issues/PRs references

Follow-up from PR
See the original issue : #21612
Depends on PR to rename ads101x driver to ads1x1x: #21731

@github-actions github-actions bot added Area: tests Area: tests and testing framework Area: drivers Area: Device drivers Area: SAUL Area: Sensor/Actuator Uber Layer labels Sep 4, 2025
@crasbe crasbe linked an issue Sep 4, 2025 that may be closed by this pull request
feat: Add support for ADS1115 ADC (I2C) #21612
Closed
6 tasks
@crasbe crasbe added Type: new feature The issue requests / The PR implemements a new feature for RIOT CI: ready for build If set, CI server will compile all applications for all available boards for the labeled PR labels Sep 4, 2025
@riot-ci
Copy link
Copy Markdown

riot-ci commented Sep 4, 2025 •
edited
Loading

Murdock results

✔️ PASSED

4698cb0 drivers/ads1x1x: add common driver for ads1x1x

Success Failures Total Runtime
11005 0 11005 09m:02s

Artifacts

@baptleduc baptleduc mentioned this pull request Sep 4, 2025
Closed
crasbe
crasbe reviewed Sep 4, 2025
View reviewed changes
Copy link
Copy Markdown
Contributor

@crasbe crasbe left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Please also note the static errors about whitespaces.

@baptleduc baptleduc force-pushed the ads1x1x-driver-clean branch 10 times, most recently from ce1acd9 to 3c14d95 Compare September 9, 2025 12:36
@github-actions github-actions bot added the Area: Kconfig Area: Kconfig integration label Sep 9, 2025
@baptleduc baptleduc force-pushed the ads1x1x-driver-clean branch from c342f4f to 5e3525b Compare September 9, 2025 13:10
@baptleduc baptleduc requested a review from crasbe September 9, 2025 13:23
@Enoch247
Copy link
Copy Markdown
Contributor

Enoch247 commented Sep 9, 2025

I don't see mention of the author's of the existing driver with the rename. Does this new driver really start from scratch with no influences from the existing one?

@Enoch247
Copy link
Copy Markdown
Contributor

Enoch247 commented Sep 9, 2025

In a discussion on Element.io, @Enoch247 pointed out that my current family-selection system is exclusive, which prevents supporting a case where both an ADS101x and an ADS111x driver are used in the same application. How should this be handled?

Doing a git grep MODULE_ADS1 only turned up a handful of matches in the code. I can look at them individually and make some suggestions, but I am out of time for today. Generally though it looked like they just changed a few defines that are only used in the params structures. I think you could change these ADS1X1X_* defines to ADS111X_* and ADS101X_* and allow them to co-exist. The params structs are device board specific. So anyone creating a param struct ought to know what kind of device is actually connected to their board and know what set of defines to be using.

@baptleduc
Copy link
Copy Markdown
Contributor Author

baptleduc commented Sep 9, 2025 •
edited
Loading

In a discussion on Element.io, @Enoch247 pointed out that my current family-selection system is exclusive, which prevents supporting a case where both an ADS101x and an ADS111x driver are used in the same application. How should this be handled?

Doing a git grep MODULE_ADS1 only turned up a handful of matches in the code. I can look at them individually and make some suggestions, but I am out of time for today. Generally though it looked like they just changed a few defines that are only used in the params structures. I think you could change these ADS1X1X_* defines to ADS111X_* and ADS101X_* and allow them to co-exist. The params structs are device board specific. So anyone creating a param struct ought to know what kind of device is actually connected to their board and know what set of defines to be using.

In this case, how should I handle defines that are common to both ads101x and ads111x ? Should I keep them as ADS1X1X_* ?

Also, if I rename these defines to be specific, then functions in ads1x1x.c would need to know exactly which module they are handling, probably requiring #if macros…

Finally, I’m not entirely clear on how the user is expected to select which ADS version they want to use.

It would be really helpful if you could provide a small example to illustrate the intended usage, if possible.

@AnnsAnns
Copy link
Copy Markdown
Member

AnnsAnns commented Jan 29, 2026

will do

AnnsAnns
AnnsAnns reviewed Jan 30, 2026
View reviewed changes
Copy link
Copy Markdown
Member

@AnnsAnns AnnsAnns left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Looks good to me for the most part, just two very minor nitpicks. Thank you for your work on this! (Note, that I don't have the hardware for this therefore did not test this)

@baptleduc baptleduc force-pushed the ads1x1x-driver-clean branch from 4aa3b85 to f1f079f Compare February 24, 2026 13:58
@baptleduc baptleduc requested a review from AnnsAnns February 24, 2026 13:59
@Teufelchen1
Copy link
Copy Markdown
Contributor

Teufelchen1 commented Feb 24, 2026

Nice! I think it is reasonable to squash this down into a single commit?

@baptleduc baptleduc force-pushed the ads1x1x-driver-clean branch from f1f079f to d499d9c Compare February 24, 2026 15:45
@baptleduc baptleduc requested a review from gschorcht as a code owner February 24, 2026 15:45
@github-actions github-actions bot added Area: doc Area: Documentation Platform: ESP Platform: This PR/issue effects ESP-based platforms Area: cpu Area: CPU/MCU ports labels Feb 24, 2026
@baptleduc
Copy link
Copy Markdown
Contributor Author

baptleduc commented Mar 5, 2026

Is there a problem with the CI? Murdock seems stuck

@Teufelchen1 Teufelchen1 added CI: ready for build If set, CI server will compile all applications for all available boards for the labeled PR and removed Area: doc Area: Documentation CI: ready for build If set, CI server will compile all applications for all available boards for the labeled PR Platform: ESP Platform: This PR/issue effects ESP-based platforms Area: cpu Area: CPU/MCU ports labels Mar 6, 2026
@Teufelchen1
Copy link
Copy Markdown
Contributor

Teufelchen1 commented Mar 6, 2026

Not sure whats up with murdock. Maybe it just timed-out, that sometimes happens when shithub is too slow or whatever.

In the meantime, there are two commits that do not belong in this PR. Maybe you need a rebase on the current master? Both superfluous commits are already in the current master.

@baptleduc
drivers/ads1x1x: add common driver for ads1x1x
4698cb0 
Add common driver for ads1x1x family, Kconfig support, documentation and
tests.

Signed-off-by: Baptiste Le Duc <[email protected]>
@baptleduc baptleduc force-pushed the ads1x1x-driver-clean branch from d499d9c to 4698cb0 Compare March 6, 2026 12:51
@baptleduc
Copy link
Copy Markdown
Contributor Author

baptleduc commented Mar 6, 2026

Not sure whats up with murdock. Maybe it just timed-out, that sometimes happens when shithub is too slow or whatever.

In the meantime, there are two commits that do not belong in this PR. Maybe you need a rebase on the current master? Both superfluous commits are already in the current master.

done :)

@Teufelchen1 Teufelchen1 enabled auto-merge March 6, 2026 14:27
@Teufelchen1 Teufelchen1 added this pull request to the merge queue Mar 6, 2026
@github-merge-queue github-merge-queue bot removed this pull request from the merge queue due to failed status checks Mar 6, 2026
@baptleduc
Copy link
Copy Markdown
Contributor Author

baptleduc commented Mar 6, 2026

Is there a reason why github bot removed the PR from the merge queue ?

@crasbe crasbe added this pull request to the merge queue Mar 6, 2026
@crasbe
Copy link
Copy Markdown
Contributor

crasbe commented Mar 6, 2026

Is there a reason why github bot removed the PR from the merge queue ?

No, tflite-micro is one of the things that tends to fail when there's a lot of load on the buildserver.

@baptleduc
Copy link
Copy Markdown
Contributor Author

baptleduc commented Mar 6, 2026

Is there a reason why github bot removed the PR from the merge queue ?

No, tflite-micro is one of the things that tends to fail when there's a lot of load on the buildserver.

Okay thanks !

Merged via the queue into RIOT-OS:master with commit be602ba Mar 6, 2026
26 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@Teufelchen1 Teufelchen1 Teufelchen1 approved these changes

@leandrolanzieri leandrolanzieri Awaiting requested review from leandrolanzieri leandrolanzieri is a code owner

@aabadie aabadie Awaiting requested review from aabadie aabadie is a code owner

@MichelRottleuthner MichelRottleuthner Awaiting requested review from MichelRottleuthner MichelRottleuthner is a code owner

@jia200x jia200x Awaiting requested review from jia200x jia200x is a code owner

@MrKevinWeiss MrKevinWeiss Awaiting requested review from MrKevinWeiss MrKevinWeiss is a code owner

@crasbe crasbe Awaiting requested review from crasbe

@AnnsAnns AnnsAnns Awaiting requested review from AnnsAnns

@gschorcht gschorcht Awaiting requested review from gschorcht

Assignees

No one assigned

Labels

Area: drivers Area: Device drivers Area: Kconfig Area: Kconfig integration Area: SAUL Area: Sensor/Actuator Uber Layer Area: tests Area: tests and testing framework CI: ready for build If set, CI server will compile all applications for all available boards for the labeled PR Type: new feature The issue requests / The PR implemements a new feature for RIOT

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

feat: Add support for ADS1115 ADC (I2C)

6 participants

@baptleduc @riot-ci @Enoch247 @Teufelchen1 @AnnsAnns @crasbe