Skip to content
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

saul: initial import of saul_bat_voltage module #21018

Merged
merged 6 commits into from
Nov 30, 2024

Conversation

miri64
Copy link
Member

@miri64 miri64 commented Nov 21, 2024

Contribution description

This exposes the battery measuring capabilities for feather-like boards (see, e.g., https://learn.adafruit.com/introducing-the-adafruit-nrf52840-feather/power-management-2#measuring-battery-3122383) into SAUL so that the output value is the voltage and provides support for 4 7 boards:

  • feather-nrf52840-sense ( ✅ tested )
  • feather-nrf52840 ( ✅ tested )
  • particle-xenon ( ✅ tested )
  • particle-argon ( 🟥 not tested, don't own the board )
  • feather-m0, as well as its derivates, feather-m0-lora and feather-m0-wifi, ( 🟨 tested, but not working as expected1 ), see saul: initial import of saul_bat_voltage module #21018 (comment)

Testing procedure

Flash example/saul to one (or ideally all) of the supported boards and use the CLI to read from the BAT registry entry. Here is a handy check list so reviewers might distribute the job among themselves. The bold ones are particularly important as I was unable to test them properly myself.

  • feather-nrf52840-sense
  • feather-nrf52840
  • particle-xenon
  • particle-argon
  • feather-m0, feather-m0-lora, or feather-m0-wifi

Issues/PRs references

Used for testing the power consumption of #20996.

Footnotes

  1. not sure if the board I used for testing is broken. It used to work 5 years ago, here is the app where I took the config from. Sadly, I did not find the LARP prop I integrated the board I used for that into.

@github-actions github-actions bot added Area: build system Area: Build system Area: drivers Area: Device drivers Area: boards Area: Board ports Area: SAUL Area: Sensor/Actuator Uber Layer labels Nov 21, 2024
maribu
maribu previously approved these changes Nov 21, 2024
Copy link
Member

@maribu maribu left a comment

Choose a reason for hiding this comment

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

code looks good to me. I think @chrysn has plenty of particle boards, maybe also the one that still needs testing?

@miri64 miri64 dismissed maribu’s stale review November 21, 2024 13:01

Was only soft-ACK as none of the requested tests seem to have been run.

@miri64 miri64 force-pushed the saul_bat_voltage/feat/initial branch from db0ad27 to 291858d Compare November 21, 2024 13:02
@miri64 miri64 added the Type: new feature The issue requests / The PR implemements a new feature for RIOT label Nov 21, 2024
@miri64
Copy link
Member Author

miri64 commented Nov 21, 2024

  1. not sure if the board I used for testing is broken. It used to work 5 years ago, here is the app where I took the config from. Sadly, I did not find the LARP prop I integrated the board I used for that into.

I just remembered, that I also have a feather-m0-lora here. Same problem as with the bare feather-m0, so its definitely not the board: ADC line 7 samples constantly to -1, which results in -6mV being returned.

@miri64 miri64 force-pushed the saul_bat_voltage/feat/initial branch from 291858d to 745aca6 Compare November 21, 2024 13:44
@miri64
Copy link
Member Author

miri64 commented Nov 21, 2024

I am removing the feather-m0-part for now from this PR, so this can go ahead with the working configurations.

@miri64
Copy link
Member Author

miri64 commented Nov 21, 2024

Moved to #21022

@chrysn
Copy link
Member

chrysn commented Nov 22, 2024

I only have particle-xenon, but it works fine there. It reads a voltage of 4.458V, no matter whether the battery is connected or not (probably b/c I can't use serial w/o also powering the device). All my boards are xenon, but my understanding is that the differences between the particle boards are quite minimal.

@miri64 miri64 force-pushed the saul_bat_voltage/feat/initial branch from 745aca6 to d892c2c Compare November 22, 2024 17:42
@miri64
Copy link
Member Author

miri64 commented Nov 22, 2024

It reads a voltage of 4.458V, no matter whether the battery is connected or not (probably b/c I can't use serial w/o also powering the device).

Here's some proof, that the value actually changes over time ;-)

grafik

@miri64 miri64 added the CI: ready for build If set, CI server will compile all applications for all available boards for the labeled PR label Nov 22, 2024
@riot-ci
Copy link

riot-ci commented Nov 22, 2024

Murdock results

✔️ PASSED

eddd93e particle-{argon,boron}: note that board_bat_voltage is untested

Success Failures Total Runtime
10242 0 10246 17m:38s

Artifacts

@mguetschow
Copy link
Contributor

Successfully tested on feather-nrf52840: With a 600mAh battery connected and one LED turned on, the reported voltage dropped from around 4100mV to 3600mV over night.

@mguetschow
Copy link
Contributor

Successfully tested on feather-nrf52840: With a 600mAh battery connected and one LED turned on, the reported voltage dropped from around 4100mV to 3600mV over night.

The feather-nrf52840-sense also reports 3600mV with the same battery attached.

@miri64
Copy link
Member Author

miri64 commented Nov 28, 2024

Anyone has an Argon lying around? Otherwise, can we just assume that it works, as they use the common interface?

@maribu
Copy link
Member

maribu commented Nov 29, 2024

Compromise: Let's merge this as is, but document that we did not test on Argon specifically, but are optimistic that it will just work due to the similarity? A one-line comment would be totally sufficient.

@miri64
Copy link
Member Author

miri64 commented Nov 29, 2024

Let's merge this as is, but document that we did not test on Argon specifically, but are optimistic that it will just work due to the similarity? A one-line comment would be totally sufficient.

Where should this doc go then? To the header file of saul_bat_voltage or to the particle-argon doc?

@github-actions github-actions bot added Area: doc Area: Documentation Area: tools Area: Supplementary tools labels Nov 29, 2024
@miri64 miri64 force-pushed the saul_bat_voltage/feat/initial branch from 9f8d4dc to df79e1a Compare November 29, 2024 11:34
@miri64 miri64 force-pushed the saul_bat_voltage/feat/initial branch from df79e1a to eddd93e Compare November 29, 2024 11:34
@miri64
Copy link
Member Author

miri64 commented Nov 29, 2024

Where should this doc go then? To the header file of saul_bat_voltage or to the particle-argon doc?

Done for the board(s). There is also the particle-boron, I noticed that only now.

@maribu maribu added this pull request to the merge queue Nov 29, 2024
@benpicco benpicco removed this pull request from the merge queue due to a manual request Nov 29, 2024
@benpicco benpicco added this pull request to the merge queue Nov 29, 2024
@maribu
Copy link
Member

maribu commented Nov 29, 2024

Let's prioritize the release.

@maribu maribu removed this pull request from the merge queue due to a manual request Nov 29, 2024
@maribu maribu added this pull request to the merge queue Nov 29, 2024
@github-merge-queue github-merge-queue bot removed this pull request from the merge queue due to failed status checks Nov 29, 2024
@miri64 miri64 added this pull request to the merge queue Nov 29, 2024
@miri64 miri64 removed this pull request from the merge queue due to a manual request Nov 29, 2024
@miri64 miri64 added this pull request to the merge queue Nov 29, 2024
Merged via the queue into RIOT-OS:master with commit d6f4637 Nov 30, 2024
25 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Area: boards Area: Board ports Area: build system Area: Build system Area: doc Area: Documentation Area: drivers Area: Device drivers Area: SAUL Area: Sensor/Actuator Uber Layer Area: tools Area: Supplementary tools 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
Development

Successfully merging this pull request may close these issues.

5 participants