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

Implement calibrated ADC API for S3 #641

Merged
merged 9 commits into from
Jul 24, 2023
Merged

Conversation

bugadani
Copy link
Contributor

@bugadani bugadani commented Jul 5, 2023

Thank you!

Thank you for your contribution.
Please make sure that your submission includes the following:

Must

  • The code compiles without errors or warnings.
  • All examples work.
  • cargo fmt was run.
  • Your changes were added to the CHANGELOG.md in the proper section.
  • You updated existing examples or added examples (if applicable).
  • Added examples are checked in CI

Nice to have

  • You add a description of your work to this PR.
  • You added proper docs for your newly added features and code.

This PR implements ADC calibration on top of #555 sor the S3. The PR also defines the constants for the S2, but does not implement the example, the efuse functions or calibration types.

cc #326

@bugadani bugadani force-pushed the s3_calibration branch 19 times, most recently from b8a3afd to 5a89305 Compare July 5, 2023 18:42
@bugadani
Copy link
Contributor Author

bugadani commented Jul 5, 2023

I would appreciate if someone who has the tools could test the example. I just picked some number out of thin air so it would be good to know if those are at least in the ballpark of reality.

@bugadani bugadani marked this pull request as ready for review July 5, 2023 19:19
@bjoernQ
Copy link
Contributor

bjoernQ commented Jul 6, 2023

Code looks good to me and the raw values range also looks good. As you said would be good if someone with the appropriate equipment could verify the numbers

@bugadani
Copy link
Contributor Author

I would appreciate if we could push this PR forward. The accuracy of the assumed VRef and other parameters may be improved later. The PR works well enough and the measured values are in the ballpark of reality (I just can't say for sure because my hardware's 1MOhm/1MOhm divider just doesn't play nice with the ADC's input impedance).

Copy link
Member

@MabezDev MabezDev left a comment

Choose a reason for hiding this comment

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

LGTM, thanks for working on this!

@MabezDev MabezDev merged commit 2472b6d into esp-rs:main Jul 24, 2023
15 checks passed
@bugadani bugadani deleted the s3_calibration branch July 24, 2023 16:47
playfulFence pushed a commit to playfulFence/esp-hal that referenced this pull request Sep 26, 2023
* adc_cal: s3: Add efuse functions for reading calibration

* Add changelog entry

* Implement calibrated ADC API for S3

* adc_cal: s3: Add calibrated ADC reading example

* Clean up

* Prefer where clauses

* Clean up unnecessary unsafe blocks

* Fix autolinks

---------

Co-authored-by: Scott Mabin <scott@mabez.dev>
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.

3 participants