ADC raw values calibration #555

katyo · 2023-05-23T06:25:20Z

Based on #550
Related issue #326

This PR implements ADC calibration based on ESP IDF. The following schemes supported:

Basic calibration which sets initial (zero) bias
Line fitting which corrects gain (also includes basic)
Curve fitting which corrects non-linearity (also inclues basic and line fitting)

First two schemes uses factory values stored in efuse fields when it available.
As a fallback a runtime calibration is made by connecting ADC input to ground or reference voltage internally.

Third scheme uses pre-defined polynomial coefficients derived from IDF.
Also applied useful optimizations for runtime calculations and to coefficients representation to reduce operations and avoid division.

NOTE: Currently calibration supported only for riscv chips because I haven't xtensa-based boards for testing, but it will be relatively easy add support for such chips later.

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.

esp32c3-hal/examples/adc_cal.rs

JurajSadel · 2023-05-25T07:09:25Z

@katyo Thank you very much for working on this, is it something I can help you at this stage?

katyo · 2023-05-25T13:04:35Z

@JurajSadel,
Currently I search a way to avoid breaking changes for an API.
We need find a proper way to select calibration/fitting algorithm when enabling ADC pin.

esp-hal-common/src/soc/esp32c3/efuse.rs

bugadani · 2023-05-28T19:03:21Z

I know my opinion here isn't very important, but I'm pretty happy with where this is heading. From a pure design standpoint, I would be happier if the algorithms weren't tied to the efuses, but having calibration coupled to the ADCs here isn't bad either.

I am slightly worried about the hardcoded constants in curve fitting - how confident can we be that a single set of constants work well across different production runs?

katyo · 2023-05-29T04:47:48Z

@bugadani
I share your concerns about hardcoded curve coefficients which derived from ESP IDF source. I'm far away from chip manufacturing but curve-based error correction gives me best precision at the moment particularly near to top edge of a range. Seems coefficients represents non-linearity of attenuator and convertor itself.

katyo · 2023-06-03T04:00:14Z

Now this PR uses efuse fields access from #567.

katyo · 2023-06-05T18:05:38Z

Because #567 is merged I rebased this pr and mark it ready for review.

JurajSadel

Thank you very much for working on this!

I tried it, the C3 is working fine!

C6 example (adc_cal) is panicking for me with

!! A panic occured in 'C:\Users\JurajSadel\rust_test\esp-hal\esp-hal-common\src\analog\adc\cal_line.rs', at line 34, column 46

PanicInfo {
    payload: Any { .. },
    message: Some(
        No reference point V1 found in efuse,
    ),
    location: Location {
        file: "C:\\Users\\JurajSadel\\rust_test\\esp-hal\\esp-hal-common\\src\\analog\\adc\\cal_line.rs",
        line: 34,
        col: 46,
    },
    can_unwind: true,
}

Backtrace:

No backtrace available - make sure to force frame-pointers. (see https://crates.io/crates/esp-backtrace)

On C2, I think the voltage values are not correct (compared with my toy voltmeter and would be nice if someone could give it a try) and I couldn't compare it with the ESP-IDF because of revision mismatch.

C2 and C6 have a bunch of unused constants warnings, would be nice to clean them as well

katyo · 2023-06-07T15:07:57Z

@JurajSadel Could you test it again?
I fixed calibration data version detection.
Also I added as a fallback the reference voltage calibration for linear scheme. But the last one is not so good because actual reference voltage may varies in range 1.0..=1.2V. This value currently is not present in efuse.

jessebraham · 2023-06-07T17:53:46Z

I've confirmed that the examples build and run successfully on the C2, C3, and C6. The values for the C2 and C6 looked correct, however for me the C3 was reporting closer to 2.8V. Other than that seems good, though.

bjoernQ · 2023-06-09T09:16:16Z

I just tried it and it works fine for me on C2 and C3. However C6 crashes in development mode with this

!! A panic occured in 'C:\projects\review\katyo\esp-hal\esp-hal-common\src\analog\adc\cal_line.rs', at line 73, column 15

PanicInfo {
    payload: Any { .. },
    message: Some(
        attempt to divide by zero,
    ),
    location: Location {
        file: "C:\\projects\\review\\katyo\\esp-hal\\esp-hal-common\\src\\analog\\adc\\cal_line.rs",
        line: 73,
        col: 15,
    },
    can_unwind: true,
}

And in release-mode it certainly shows weird values because of this

katyo · 2023-06-09T10:55:58Z

@bjoernQ Looks strange.

Seems code is zero here:

let gain = ((mv as u32 * GAIN_SCALE * 2 / code as u32 + 1) * 4096 / atten.ref_mv() as u32 + 1) / 2;

May be calibration data is missing (or version check failed)
or (a fallback way) runtime calibration failed.

@bjoernQ Cound you give me a values which return Efuse::get_block_version() call on a C6?

bjoernQ · 2023-06-09T11:01:51Z

Maybe adding some debug or trace logs might help identifying what is going on here. We already have the log dependency in esp-hal-common

katyo · 2023-06-15T07:22:07Z

@elpiel The fact is that ADC input impedance on Espressif SoCs relatively low compared to many other MCUs (KOhms not MOhms).
I recommend you try use 10x-100x lower resistor values and add 10-100nF capacitor to input (after divider). You may need experimenting with it to get stable results.

elpiel · 2023-06-23T05:46:36Z

Testing on the Olimex board with soldered jumpers for the battery meassurement on GPIO 3 indeed shows good results!
I need to double check the values with my multimeter and figure out a way to not charge the battery via USB when measuring it.

https://raw.githubusercontent.com/OLIMEX/ESP32-C3-DevKit-Lipo/main/HARDWARE/ESP32-C3-DevKit-Lipo_Rev_A/ESP32-C3-DevKit-Lipo_Rev_A.pdf

On the embassy project I still get a panic but I will check whether this is because of unhappy embassy crate versions

bugadani

I would like to see the eFuse calibration versions (I commented on a few, but not all) named (a plain 1 is a bit confusing to read), but apart from that, this PR is in a really promising state.

I can see a few grammar issues (mine is probably incorrect, too!), and a few comments that look like they were copied from C or written to explain the C code during development. Really nothing major.

It would be nice if this PR could be merged in the near future 🙏

esp-hal-common/src/analog/adc/cal_curve.rs

esp-hal-common/src/analog/adc/cal_line.rs

bugadani · 2023-06-27T07:27:20Z

esp-hal-common/src/soc/esp32c2/efuse.rs

+    pub fn get_rtc_calib_version() -> u8 {
+        let (major, _minor) = Self::get_block_version();
+        if major == 0 {
+            1


This 1 needs to be a named constant IMO.

I would like avoid introduce new constants.
Ok, I'll think about it.

bugadani · 2023-06-27T07:27:56Z

esp-hal-common/src/soc/esp32c2/efuse.rs

+    pub fn get_rtc_calib_init_code(_unit: u8, atten: Attenuation) -> Option<u16> {
+        let version = Self::get_rtc_calib_version();
+
+        if version != 1 {


This 1 needs to be the same constant as above, I think.

bugadani · 2023-06-27T07:28:32Z

esp-hal-common/src/soc/esp32c3/efuse.rs

+    pub fn get_rtc_calib_cal_code(_unit: u8, atten: Attenuation) -> Option<u16> {
+        let version = Self::get_rtc_calib_version();
+
+        if version != 1 {


esp-hal-common/src/analog/adc/cal_line.rs

esp-hal-common/src/analog/adc/cal_curve.rs

esp-hal-common/src/analog/adc/cal_line.rs

MabezDev · 2023-06-27T11:12:29Z

esp-hal-common/src/soc/esp32c2/efuse.rs

+    pub fn get_rtc_calib_cal_code(_unit: u8, atten: Attenuation) -> Option<u16> {
+        let version = Self::get_rtc_calib_version();
+
+        if version != 1 {


Version here should also be replaced by a const

esp32c2-hal/examples/adc_cal.rs

bugadani · 2023-07-02T07:51:47Z

I just want to say that I'm working on implementing this PR for the S3, just so others don't waste their time on that :) I'll post the results once this one gets merged.

MabezDev

After thinking some more, I don't think the consts are that big of a deal, we can always reference the ESP-IDF code to actually understand what's going on.

I think a quick rebase on main and this should be good to go from my perspective :).

bugadani · 2023-07-03T16:01:59Z

After thinking some more, I don't think the consts are that big of a deal, we can always reference the ESP-IDF code to actually understand what's going on.

Honestly, I think the if major == 0 { 1 } pattern deserves some sort of explanation (be that a comment or a named constant) in the code as it looks like a mistake.

Buuuuut I also don't want to delay the PR 🫣

- `AdcCalScheme<ADCI>` implemented for each calibration scheme (basic, linear, curved) - `AdcCalEfuse` implemented for each ADC unit to get calibration data from efuse bits

Basic calibration is related to setting some initial bias value to ADC unit. Such values usually is stored in efuse bit fields but also can be measured in runtime by connecting ADC input to ground internally.

This scheme also includes basic calibration and implements gain correction based on reference point. Reference point is a pair of reference voltage and corresponding mean raw ADC value. Such raw values usually is stored in efuse bit fields for each supported attenuation. Possibly it also can be measured in runtime by connecting ADC to reference voltage internally.

This scheme also includes basic and linear and implements final polynomial error correction.

This example uses line fitting calibration scheme by default. It periodically prints both raw measured value and computed millivolts.

This example uses curve fitting calibration scheme by default. It periodically prints both raw measured value and computed millivolts.

jessebraham

I think we could introduce some new constants here, as others have suggested, but in the interest of getting this PR merged I'm gonna say LGTM and we can come back to it at a later date. I also noticed that the H2 is missing from this, but again that can wait. I will create an issue to remind us, and may end up just tackling that myself.

Thanks for all your hard work on this, will be great to actually be able to use our ADCs now! 😁

katyo · 2023-07-04T17:20:49Z

@jessebraham Because esp-idf still does not support calibration for H2 so I don't know how we can add support for it just now.
(see efuse/esp32h2/esp_efuse_rtc_calib.c#L13)

* adc_cal: c2: Add efuse functions for reading calibration * adc_cal: c3: Add efuse functions for reading calibration * adc_cal: c6: Add efuse functions for reading calibration * adc_cal: Add extra traits to support calibration - `AdcCalScheme<ADCI>` implemented for each calibration scheme (basic, linear, curved) - `AdcCalEfuse` implemented for each ADC unit to get calibration data from efuse bits * adc_cal: Add basic ADC calibration scheme Basic calibration is related to setting some initial bias value to ADC unit. Such values usually is stored in efuse bit fields but also can be measured in runtime by connecting ADC input to ground internally. * adc_cal: Add line fitting ADC calibration scheme This scheme also includes basic calibration and implements gain correction based on reference point. Reference point is a pair of reference voltage and corresponding mean raw ADC value. Such raw values usually is stored in efuse bit fields for each supported attenuation. Possibly it also can be measured in runtime by connecting ADC to reference voltage internally. * adc_cal: Add curve fitting ADC calibration scheme This scheme also includes basic and linear and implements final polynomial error correction. * adc_cal: riscv: Add ADC calibration implementation for riscv chips * adc_cal: c2: Add calibrated ADC reading example This example uses line fitting calibration scheme by default. It periodically prints both raw measured value and computed millivolts. * adc_cal: c3: Add calibrated ADC reading example This example uses curve fitting calibration scheme by default. It periodically prints both raw measured value and computed millivolts. * adc_cal: c6: Add calibrated ADC reading example This example uses curve fitting calibration scheme by default. It periodically prints both raw measured value and computed millivolts. * adc_cal: riscv: Add changelog entry for ADC calibration

bugadani reviewed May 23, 2023

View reviewed changes

esp32c3-hal/examples/adc_cal.rs Outdated Show resolved Hide resolved

katyo force-pushed the feature/adc_calibration branch 2 times, most recently from d53f1ce to 464e46a Compare May 26, 2023 04:55

katyo changed the title ~~Feature/adc calibration~~ ADC raw values calibration May 27, 2023

katyo force-pushed the feature/adc_calibration branch from 054e556 to 1236249 Compare May 27, 2023 18:24

bugadani reviewed May 28, 2023

View reviewed changes

esp-hal-common/src/soc/esp32c3/efuse.rs Outdated Show resolved Hide resolved

JurajSadel mentioned this pull request May 30, 2023

H2: Add initial ADC support with example #565

Merged

katyo mentioned this pull request May 30, 2023

Unified efuse field access #567

Merged

8 tasks

katyo force-pushed the feature/adc_calibration branch 2 times, most recently from b244042 to d700f09 Compare June 3, 2023 03:57

katyo force-pushed the feature/adc_calibration branch from d700f09 to 4edaa65 Compare June 5, 2023 17:55

katyo marked this pull request as ready for review June 5, 2023 18:05

JurajSadel reviewed Jun 6, 2023

View reviewed changes

katyo force-pushed the feature/adc_calibration branch 2 times, most recently from 062884e to f48ddeb Compare June 7, 2023 15:02

katyo force-pushed the feature/adc_calibration branch 3 times, most recently from 58af83a to 49abd51 Compare June 8, 2023 06:19

JurajSadel mentioned this pull request Jun 12, 2023

C3: ADC raw values calibration #550

Closed

katyo force-pushed the feature/adc_calibration branch from abf5dba to d26c95a Compare June 23, 2023 14:01

bugadani reviewed Jun 27, 2023

View reviewed changes

MabezDev requested changes Jun 27, 2023

View reviewed changes

katyo force-pushed the feature/adc_calibration branch 2 times, most recently from 0039905 to 3f0e414 Compare June 30, 2023 08:41

MabezDev approved these changes Jul 3, 2023

View reviewed changes

katyo added 12 commits July 4, 2023 21:49

adc_cal: c2: Add efuse functions for reading calibration

69208ec

adc_cal: c3: Add efuse functions for reading calibration

bfb094c

adc_cal: c6: Add efuse functions for reading calibration

dfe7744

adc_cal: Add extra traits to support calibration

a5be5a2

- `AdcCalScheme<ADCI>` implemented for each calibration scheme (basic, linear, curved) - `AdcCalEfuse` implemented for each ADC unit to get calibration data from efuse bits

adc_cal: Add basic ADC calibration scheme

7924750

Basic calibration is related to setting some initial bias value to ADC unit. Such values usually is stored in efuse bit fields but also can be measured in runtime by connecting ADC input to ground internally.

adc_cal: Add curve fitting ADC calibration scheme

53e7912

This scheme also includes basic and linear and implements final polynomial error correction.

adc_cal: riscv: Add ADC calibration implementation for riscv chips

5a78b91

adc_cal: c2: Add calibrated ADC reading example

71140d1

This example uses line fitting calibration scheme by default. It periodically prints both raw measured value and computed millivolts.

adc_cal: c3: Add calibrated ADC reading example

15eea18

This example uses curve fitting calibration scheme by default. It periodically prints both raw measured value and computed millivolts.

adc_cal: c6: Add calibrated ADC reading example

9313534

This example uses curve fitting calibration scheme by default. It periodically prints both raw measured value and computed millivolts.

adc_cal: riscv: Add changelog entry for ADC calibration

d2e2816

katyo force-pushed the feature/adc_calibration branch from 3f0e414 to d2e2816 Compare July 4, 2023 16:51

jessebraham approved these changes Jul 4, 2023

View reviewed changes

jessebraham merged commit 74438fc into esp-rs:main Jul 4, 2023

jessebraham mentioned this pull request Jul 4, 2023

[ESP32-H2] Add the adc_cal example #634

Closed

katyo deleted the feature/adc_calibration branch July 4, 2023 17:31

bugadani mentioned this pull request Jul 5, 2023

Implement calibrated ADC API for S3 #641

Merged

8 tasks

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

ADC raw values calibration #555

ADC raw values calibration #555

katyo commented May 23, 2023 •

edited

Loading

JurajSadel commented May 25, 2023

katyo commented May 25, 2023

bugadani commented May 28, 2023

katyo commented May 29, 2023

katyo commented Jun 3, 2023

katyo commented Jun 5, 2023 •

edited

Loading

JurajSadel left a comment •

edited

Loading

katyo commented Jun 7, 2023 •

edited

Loading

jessebraham commented Jun 7, 2023 •

edited

Loading

bjoernQ commented Jun 9, 2023

katyo commented Jun 9, 2023 •

edited

Loading

bjoernQ commented Jun 9, 2023

katyo commented Jun 15, 2023

elpiel commented Jun 23, 2023

bugadani left a comment •

edited

Loading

bugadani Jun 27, 2023

katyo Jun 30, 2023

bugadani Jun 27, 2023

bugadani Jun 27, 2023

MabezDev Jun 27, 2023

bugadani commented Jul 2, 2023 •

edited

Loading

MabezDev left a comment

bugadani commented Jul 3, 2023 •

edited

Loading

jessebraham left a comment

katyo commented Jul 4, 2023

ADC raw values calibration #555

ADC raw values calibration #555

Conversation

katyo commented May 23, 2023 • edited Loading

Must

Nice to have

JurajSadel commented May 25, 2023

katyo commented May 25, 2023

bugadani commented May 28, 2023

katyo commented May 29, 2023

katyo commented Jun 3, 2023

katyo commented Jun 5, 2023 • edited Loading

JurajSadel left a comment • edited Loading

Choose a reason for hiding this comment

katyo commented Jun 7, 2023 • edited Loading

jessebraham commented Jun 7, 2023 • edited Loading

bjoernQ commented Jun 9, 2023

katyo commented Jun 9, 2023 • edited Loading

bjoernQ commented Jun 9, 2023

katyo commented Jun 15, 2023

elpiel commented Jun 23, 2023

bugadani left a comment • edited Loading

Choose a reason for hiding this comment

bugadani Jun 27, 2023

Choose a reason for hiding this comment

katyo Jun 30, 2023

Choose a reason for hiding this comment

bugadani Jun 27, 2023

Choose a reason for hiding this comment

bugadani Jun 27, 2023

Choose a reason for hiding this comment

MabezDev Jun 27, 2023

Choose a reason for hiding this comment

bugadani commented Jul 2, 2023 • edited Loading

MabezDev left a comment

Choose a reason for hiding this comment

bugadani commented Jul 3, 2023 • edited Loading

jessebraham left a comment

Choose a reason for hiding this comment

katyo commented Jul 4, 2023

katyo commented May 23, 2023 •

edited

Loading

katyo commented Jun 5, 2023 •

edited

Loading

JurajSadel left a comment •

edited

Loading

katyo commented Jun 7, 2023 •

edited

Loading

jessebraham commented Jun 7, 2023 •

edited

Loading

katyo commented Jun 9, 2023 •

edited

Loading

bugadani left a comment •

edited

Loading

bugadani commented Jul 2, 2023 •

edited

Loading

bugadani commented Jul 3, 2023 •

edited

Loading