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

cpu/sam0_common: adc: implement 16 bit mode by oversampling #19165

Merged
merged 1 commit into from
Feb 24, 2023

Conversation

benpicco
Copy link
Contributor

@benpicco benpicco commented Jan 18, 2023

Contribution description

The precision of measurements can be increased by taking multiple measurements.
For this, the ADC implements an summation/averaging mode in hardware.

The best way I found to expose this in our ADC API is via the 16 bit resolution mode.
We can take up to 16 samples without exceeding the internal ADC register (after which 'automatic right shifts' happen - the result gets truncated).

Testing procedure

Issues/PRs references

@github-actions github-actions bot added Area: cpu Area: CPU/MCU ports Platform: ARM Platform: This PR/issue effects ARM-based platforms labels Jan 18, 2023
@benpicco benpicco force-pushed the cpu/sam0_adc-avg branch 3 times, most recently from 9e3bd93 to ab81e33 Compare January 19, 2023 12:28
@benpicco benpicco changed the title cpu/sam0_common: add option for taking multiple samples cpu/sam0_common: implement 16 bit mode by oversampling Jan 19, 2023
@benpicco benpicco marked this pull request as ready for review January 30, 2023 17:36
Copy link
Member

@dylad dylad left a comment

Choose a reason for hiding this comment

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

Overall changes looks good to me.
But using tests/periph_adc with RES set to ADC_RES_16BIT doesn't work as expected.
Works fine as long as the ADC line is below VCC_REF / 2 but it returns an error when we're above VCC_REF / 2.

I guess this is because this variable is set as int16_t

@benpicco benpicco changed the title cpu/sam0_common: implement 16 bit mode by oversampling cpu/sam0_common: adc: implement 16 bit mode by oversampling Feb 21, 2023
@benpicco
Copy link
Contributor Author

Good catch!

@dylad
Copy link
Member

dylad commented Feb 23, 2023

Works as expected now !
Please squash.

@benpicco benpicco added the CI: ready for build If set, CI server will compile all applications for all available boards for the labeled PR label Feb 23, 2023
Copy link
Member

@dylad dylad left a comment

Choose a reason for hiding this comment

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

ACK

@dylad
Copy link
Member

dylad commented Feb 23, 2023

bors merge

@bors
Copy link
Contributor

bors bot commented Feb 23, 2023

🕐 Waiting for PR status (GitHub check) to be set, probably by CI. Bors will automatically try to run when all required PR statuses are set.

@riot-ci
Copy link

riot-ci commented Feb 23, 2023

Murdock results

✔️ PASSED

470bee5 cpu/sam0_common: implement 16 bit mode by oversampling

Success Failures Total Runtime
6865 0 6865 11m:10s

Artifacts

@benpicco
Copy link
Contributor Author

bors merge

@bors
Copy link
Contributor

bors bot commented Feb 23, 2023

Already running a review

@bors
Copy link
Contributor

bors bot commented Feb 24, 2023

Build succeeded:

@bors bors bot merged commit 0dfc05c into RIOT-OS:master Feb 24, 2023
@benpicco benpicco deleted the cpu/sam0_adc-avg branch February 24, 2023 10:16
@MrKevinWeiss MrKevinWeiss added this to the Release 2023.04 milestone Apr 25, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Area: cpu Area: CPU/MCU ports CI: ready for build If set, CI server will compile all applications for all available boards for the labeled PR Platform: ARM Platform: This PR/issue effects ARM-based platforms
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants