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

ADC Calibration #326

Open
4 of 7 tasks
sourcebox opened this issue Dec 23, 2022 · 26 comments
Open
4 of 7 tasks

ADC Calibration #326

sourcebox opened this issue Dec 23, 2022 · 26 comments
Labels
peripheral:adc ADC peripheral

Comments

@sourcebox
Copy link

sourcebox commented Dec 23, 2022

On ESP32-S3 with an attenuation of 11dB, the readings saturate at about 1.5V. I have a correcttly working project for the same board using the IDF with C. In this project, the resolution is set to 12-bit whereas the setting in esp-hal is fixed at 13-bit. It would be nice to also have a configurable setting for the resolution.


Edit by @jessebraham

Status of ADC calibration is as follows:

  • ESP32
  • ESP32-C2
  • ESP32-C3
  • ESP32-C6
  • ESP32-H2
  • ESP32-S2
  • ESP32-S3
@jessebraham jessebraham added the bug Something isn't working label Jan 2, 2023
@MabezDev
Copy link
Member

MabezDev commented Jan 3, 2023

I don't believe we do any ADC calibration so that would be another thing to fix in esp-hal.

@bjoernQ
Copy link
Contributor

bjoernQ commented Jan 30, 2023

So, I double checked some things

Having 13-bit on ESP32-S3 is wrong since it only supports 12-bit - but setting the bit-width is a no-op here - so it's not the cause of the problems.

I compared the readings to esp-idf and certainly the esp-idf readings are correct while esp-hal shows the described behavior.

But if I change components/esp_adc/adc_oneshot.c and remove the code guarded by SOC_ADC_CALIBRATION_V1_SUPPORTED in adc_oneshot_read I get very similar readings.

So, the real problem is actually the uncalibrated reads

@dimpolo
Copy link
Contributor

dimpolo commented Feb 17, 2023

So I tried to understand this whole calibration thing but at this point I give up.

I found these files:
https://github.com/espressif/esp-idf/blob/master/components/hal/esp32s3/include/hal/adc_ll.h
https://github.com/espressif/esp-idf/blob/master/components/soc/esp32s3/include/soc/regi2c_saradc.h

The code seems to:

  • write to some I2C registers?
  • call abort? line
  • have random looking register offsets (see regi2c_saradc.h)
  • maybe communicate with the ULP coprocessor?
  • summon Cthulu?

Is any of this documented somewhere or is there someone knowledgeable about it I could ask?
What are the steps required for calibration?

@bjoernQ
Copy link
Contributor

bjoernQ commented Feb 20, 2023

I also had a look and it's unfortunately under-documented in the TRM

Another source to look at would be the NuttX implementation e.g. https://github.com/apache/nuttx/blob/master/arch/risc-v/src/esp32c3/esp32c3_adc.c for ESP32-C3. It might help a bit to see what is really needed and what not

@dimpolo
Copy link
Contributor

dimpolo commented Feb 20, 2023

Thanks for the link, it does indeed help!

I started writing some code to test things:

// constants taken from https://github.com/espressif/esp-idf/blob/045163a2ec99eb3cb7cc69e2763afd145156c4cf/components/soc/esp32s3/include/soc/regi2c_saradc.h
const I2C_SAR_ADC: u32 = 0x69;
const I2C_SAR_ADC_HOSTID: u32 = 1;

const ADC_SAR1_INITIAL_CODE_HIGH_ADDR: u32 = 0x1;
const ADC_SAR1_INITIAL_CODE_HIGH_ADDR_MSB: u32 = 0x3;
const ADC_SAR1_INITIAL_CODE_HIGH_ADDR_LSB: u32 = 0x0;

const ADC_SAR1_INITIAL_CODE_LOW_ADDR: u32 = 0x0;
const ADC_SAR1_INITIAL_CODE_LOW_ADDR_MSB: u32 = 0x7;
const ADC_SAR1_INITIAL_CODE_LOW_ADDR_LSB: u32 = 0x0;

pub fn adc1_set_calibration(val: u16) {
    let [val_h, val_l] = val.to_be_bytes();

    unsafe {
        regi2c_write_mask!(I2C_SAR_ADC, ADC_SAR1_INITIAL_CODE_HIGH_ADDR, val_h as u32);
        regi2c_write_mask!(I2C_SAR_ADC, ADC_SAR1_INITIAL_CODE_LOW_ADDR, val_l as u32);
    }
}

Calling this function with different values does indeed change the ADC voltage range.

Could you maybe help me understand some things here?
What does regi2c_write_mask do?
Does it really have something to do with an I2C bus?

I can see that the macro expands to a function call of a function that resides somewhere in ROM. But I can't find anything about what the function does and what kind of value it expects.

@bjoernQ
Copy link
Contributor

bjoernQ commented Feb 21, 2023

It's unfortunately undocumented as far as I can see

I think there is some I2C bus and various things are controlled by it (obviously something about the ADC and also other things https://github.com/cnlohr/esp32-c3-cntest#process-for-figuring-out-i2c-and-apll )

For ESP32-C6 those functions are not used from ROM but they are in esp-idf, eg. here https://github.com/espressif/esp-idf/blob/045163a2ec99eb3cb7cc69e2763afd145156c4cf/components/esp_rom/patches/esp_rom_regi2c_esp32c6.c#L172-L195

While that is for ESP32-C6 it would look similar for other chips, I guess. At least it gives an impression of what the ROM function does

@dimpolo
Copy link
Contributor

dimpolo commented Feb 21, 2023

Interesting info.

Here's what I found out today:

From trying out different values I found that you want to call adc1_set_calibration with the uncalibrated ADC value at 0V.

Supposedly you can recover this value from some Efuse bits or you can perform your own measurements using some internal GND connection of the ADC.

The value differs between ADC1 and ADC2 and different Attenuations. The IDF calls adc1_set_calibration every time a different attenuation is used.

I wrote some helper functions over at:
dimpolo@f794324

The efuse data seems to be stored in a very ... unusual... format.
https://github.com/espressif/esp-idf/blob/7052a14d61d98dd387c4d99f435c42370b282fa6/components/efuse/esp32s3/esp_efuse_rtc_calib.c#L28-L61

The offsets I read from the Efuses don't match up with good offsets which I determined experimentally. Maybe I messed up translating the code somewhere or I'm still missing some info.

I'm not sure I'll continue my investigation at this point.
Reading undocumented Efuses so one can use undocumented ROM functions to write undocumented ADC registers over some undocumented internal I2C bus is rather exhausting.

@bjoernQ
Copy link
Contributor

bjoernQ commented Feb 21, 2023

That's already awesome! Thanks for sharing your findings.

I definitely understand you might not feel like continuing this journey but thankfully you shared your findings in a very accessible way so even if you don't want to continue here someone can continue from here

Thanks again!

@noonien
Copy link

noonien commented Mar 21, 2023

Do ADC calibrations need to be made on each boot?

@JurajSadel
Copy link
Contributor

I'll try to look into the configuration process, hopefully we can make some progress here 😅

@jneem
Copy link
Contributor

jneem commented Apr 6, 2023

I haven't tried reading efuse yet, but on an esp32c3 I've had some success imitating the "Calibrate based on GND voltage" approach in the NuttX implementation.

@noonien as @dimpolo pointed out above, you need to reset the calibration whenever the attenuation changes. Since the attenuation is only known in AdcConfig::enable_pin, I think that's where the calibration will have to be done.

@JurajSadel
Copy link
Contributor

Just a quick update, I'm able to get raw values in the range of 0 - 4095 and not just ~2200-4095 on C3 (tested with joystick). I still have to implement the approximation functions from raw values to voltage and right now, I'm working with hardcoded addresses/values etc. But I hope we are very close to the correct calibration on C3 at least.

@sourcebox
Copy link
Author

Regarding the conversion to voltages: since I need to scale the settings differently anyway, it would be nice to also have access to the raw values.

@elpiel
Copy link
Contributor

elpiel commented May 7, 2023

You could probably use a type, e.g. Measurement<R = ()>, where () is no scale factor and use R for each different scale (u32, f32, etc.).

I'm still closely following this issue for the @AeroRust workshop @JurajSadel and I'm really happy that you did such a progress on it.

@elpiel
Copy link
Contributor

elpiel commented May 13, 2023

Hello,
Is there any progress on the calibration?

@bjoernQ
Copy link
Contributor

bjoernQ commented May 15, 2023

I think @JurajSadel made some good progress in getting it to work for ESP32-C3 (which is IIRC what you urgently need). Maybe there is a branch you can already test

@elpiel
Copy link
Contributor

elpiel commented May 17, 2023

Great! If there's a branch I would like to try it out.

Edit: Found it

https://github.com/JurajSadel/esp-hal/tree/feature/adc_calibration

@JurajSadel
Copy link
Contributor

JurajSadel commented May 18, 2023

Hello @elpiel, sorry for the late reply, I created a draft PR and made a note of the current status.

@katyo
Copy link
Contributor

katyo commented May 20, 2023

Currently ESP-IDF implements two-step raw to voltage conversion with curve fitting (at least for ESP32-C3 and ESP32-S3)
(see components/esp_adc/adc_cali_curve_fitting.c)

The curve coefficients hardcoded in source for each attenuation and differs for C3 and S3 (see esp32c3/curve_fitting_coefficients.c and esp32s3/curve_fitting_coefficients.c).

Original implementation far from ideal. First, it uses decimal fixed-point arithmetic in hot code path (for raw value to voltage convarsion) which requires decimal division by 1eN (where N is 15 or 16) for each term (3..=5).
Second, the both numerator and denominator present in a table as u64 constants, also signs of terms present in a second table as a i32 constants. It is typical case of overengineering because the i64 is fairly wide to represent both value and sign as a binary fixed-point numbers for each coefficient. Also we need only numerators because we may use common denominator for all coefficients (say 2^60 instead of 1e16). This can help to reduce table size and optimize arithmetic in a hot code path.

My quick solution for coefficients table looks like so:

macro_rules! curve_fitting_coeff_tables {
    ($($name:ident: $mul:path [ $([ $($val:literal,)* ],)* ];)*) => {
        $(
            const $name: [[i64; TERM_MAX]; COEFF_GROUP_NUM] = [
                $([
                    $(($val as f64 * $mul as f64) as i64,)*
                ],)*
            ];
        )*
    };
}

const COEFF_GROUP_NUM: usize = 4;
const TERM_MAX: usize = 5;
const ERROR_COEF_MUL: u64 = 1 << 60;

curve_fitting_coeff_tables! {
    C3_ERROR_COEF_ATTEN: ERROR_COEF_MUL [
        // atten0
        [
            -0.2259664705000430,
            -0.0007265418501948,
            0.0000109410402681,
            0, // unused
            0, // unused
        ],
        // atten1
        [
            0.4229623392600516,
            -0.0000731527490903,
            0.0000088166562521,
            0, // unused
            0, // unused
        ],
        // atten2
        [
            -1.0178592392364350,
            -0.0097159265299153,
            0.0000149794028038,
            0, // unused
            0, // unused
        ],
        // atten3
        [
            -1.4912262772850453,
            -0.0228549975564099,
            0.0000356391935717,
            -0.0000000179964582,
            0.0000000000042046,
        ],
    ];
}

@katyo
Copy link
Contributor

katyo commented May 20, 2023

This is my attempt: adc_calibration (PoC and WiP).
It based on @JurajSadel's work.
At the moment the curve fitting derived from IDF, no optimizations applied yet.
Also needed some usage example.

I tested this code with potentiometers and thermistors. Seems it works pretty fine.

@bugadani
Copy link
Contributor

bugadani commented May 20, 2023

As everyone seems to be working on either C3 specific code, or generic (like the curve fitting), I've started poking at the S3. This thing really isn't documented at all. (Maybe there's hope for a newer TRM, as the last revision is March of 2023?)

My work on S3 calibration is here. I'm using the Efuse maps from @dimpolo as they are a lot more convenient than me looking up bits from here and there, pieces of the C3 PR, and the curve fitting by @katyo looks like a good finishing touch.

As it turns out, my S3 came with the offsets calibrated, and the self-calibration code came from the C3 impl, and I think only the efuse reading part is working. I'll need to circle back to this.

What's there: before every conversion, the software loads calibration params from global memory. This is missing in the PR from @JurajSadel which assumes a single calibration value.

I'm confident we'll get to the end of this eventually 😂

Edit: as a fun side note, it looks like my ADC reading code is really sensitive (as in, different amounts of error in the returned voltage) to the time between reads. I suspect something is not correctly stopped in the driver.

@katyo
Copy link
Contributor

katyo commented May 22, 2023

Looks like curve fitting from IDF have a problem with zero term.

/**
     * For atten0 ~ 2:
     * error = (K0 * X^0) + (K1 * X^1) + (K2 * X^2);
     *
     * For atten3:
     * error = (K0 * X^0) + (K1 * X^1)  + (K2 * X^2) + (K3 * X^3) + (K4 * X^4);
     */
    variable[0] = 1;
    coeff = (*param->coeff)[atten][0][0];
    term[0] = variable[0] * coeff / (*param->coeff)[atten][0][1];
    error = (int32_t)term[0] * (*param->sign)[atten][0];

adc_cali_curve_fitting.c#L207-L217
Here term[0] is always 0 or 1 because variable[0] equals to 1 and param->coeff[atten][0][0] is always less than param->coeff[atten][0][1].

For example (esp32c3):

1 * 225966470500043ull / 10000000000000000ull == 0 // 0dB
1 * 4229623392600516ull / 10000000000000000ull == 0 // 2.5dB
1 * 1017859239236435ull / 1000000000000000ull == 1 // 6dB
1 * 14912262772850453ull / 10000000000000000ull == 1 // 11dB

I'm not sure if this was done on purpose.

If represent raw ADC samples as a binary fixed-point values (i.e. when raw range 0..2^12 corresponds to range 0.0..1.0) then x^0 (i.e. variable[0] in original code) actually should be equals to 2^12 not an 1.

I planned some experiments to find coefficients myself and compare it with original implementation.

@bugadani
Copy link
Contributor

bugadani commented May 22, 2023

@katyo if I am interpreting the C code and examples correctly, curve fitting is not actually used by the ADC driver, rather it's an optional feature for users. Is this correct? I don't actaully see any mention of it inside the conversion functions, and the examples seem to use the curve-fitting calibration externally.

Because, if this is the case, I'm wondering whether curve fitting should be included in the HAL or provided separately. Alternatively, whether the HAL should provide one read function each for raw and curve-fitted conversions.

@katyo
Copy link
Contributor

katyo commented May 23, 2023

Actually curve fitting consist of two steps: linear fitting and correction using error curve.

First (linear fitting) step uses reference point which represented as a mean ADC sample for some voltage in a middle of range.

That ADC sample value stores in efuse registers as a 10-bit signed int (actual_value = 2000 + stored_value), the voltage for reference point is hardcoded constant.

For example (esp32c3):

  • 400mV for 0dB (0..800mV)
  • 550mV for 2.5dB (0..1100mV)
  • 750mV for 6dB (0..1350mV)
  • 1370mV for 11dB (0..2600mV)

Second (curve fitting) step uses hardcoded coefficients (specific for chip model) to do error correction for measured value.

Because each step above is specific for device model I think it should be a part of HAL.

My implementation is flexible as to which calibration steps will be applied for each input. You can combine it using tuples:

  • AdcCalBasic is a basic step which is a zero bias correction
  • AdcCalLine is a linear fitting step
  • AdcCalCurve is a curve fitting step

Zero bias correction uses initial values stored in efuse registers for each attenuation but in case when storead values is not a valid this values can be measured by connecting input to ground internally.

NOTE: The implementation from IDF provides values in millivolts. I would like fix it to get calibrated raw ADC values in range 0..2^12-1 instead.

@katyo katyo mentioned this issue May 23, 2023
8 tasks
@jessebraham jessebraham added the peripheral:adc ADC peripheral label Sep 19, 2023
@MabezDev MabezDev changed the title ADC input voltage range not correct ADC Calibration Feb 19, 2024
@jessebraham jessebraham added status:needs-attention This should be prioritized and removed bug Something isn't working labels Mar 14, 2024
@jessebraham
Copy link
Member

Implemented calibration for ESP32-H2 today. Doesn't appear to be working correctly unfortunately, so will debug it next week and open a PR when it's ready.

@SergioGasquez
Copy link
Member

SergioGasquez commented Apr 29, 2024

I tried following @jessebraham work on H2 calibration but my H2 revision does not support adc calibration. Will try to get a new devkit in two weeks.

Edit1: WIP ADC cal for H2 branch: https://github.com/SergioGasquez/esp-hal/tree/feat/adc-cal-h2
Edit2: Some very early S2 WIP ADC Cal branch: https://github.com/SergioGasquez/esp-hal/tree/feat/adc-cal-s2
Edit3: Will stop working on this for now, leaving the noisy branches in this comment

@SergioGasquez SergioGasquez removed their assignment Jun 12, 2024
@tom-borcin tom-borcin removed the status:needs-attention This should be prioritized label Jul 15, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
peripheral:adc ADC peripheral
Projects
Status: In Progress
Development

No branches or pull requests