ADC conversion overflow using curve calibration

[yuri-ncs]

Bug description

When calling adc_val() on a range of raw ADC values (e.g., from 0 to 4096), the code panics with an overflow error. Specifically, the panic occurs at:

panicked at C:\cratepath\esp-hal-0.23.1\src\analog\adc\calibration\curve.rs:96:25: attempt to multiply with overflow

The overflow just happens in the curve fitting calibration, in the snippet below (taken from the esp-hal calibration code), the overflow happens on this line:

err += (var * (*coeff as i64) / COEFF_MUL) as i32;

because var (an i64) accumulates increasingly large powers of the ADC reading (val as i64).

Relevant code and logs

1. adc_val function (from esp-hal):

fn adc_val(&self, val: u16) -> u16 {
    let val = self.line.adc_val(val);

    let err = if val == 0 {
        0
    } else {
        let mut var = 1i64;
        let mut err = (var * self.coeff[0] / COEFF_MUL) as i32;

        for coeff in &self.coeff[1..] {
            var *= val as i64;
            err += (var * (*coeff as i64) / COEFF_MUL) as i32; // overflow happens here
        }

        err
    };

    (val as i32 - err) as u16
}

2. Application code calling adc_val from 0 to 4096:

for i in 0..4096 {
    info!(
        "Raw value: {:?}, Converted Value: {:?}",
        i,
        self.calibration.adc_val(i) // triggers the overflow
    );
}

3. Adc and Calibration initialization code:

• Initialization code

pub struct RawPins {
    pub in1: GpioPin<4>,
    pub in2: GpioPin<5>,
    pub in3: GpioPin<6>,
    pub in4: GpioPin<7>,
    pub in5: GpioPin<8>,
    pub in6: GpioPin<3>,
    pub in7: GpioPin<9>,
    pub in8: GpioPin<10>,
}

pub struct AdcPins {
    pub in1: AdcPin<GpioPin<4>, ADC1>,
    pub in2: AdcPin<GpioPin<5>, ADC1>,
    pub in3: AdcPin<GpioPin<6>, ADC1>,
    pub in4: AdcPin<GpioPin<7>, ADC1>,
    pub in5: AdcPin<GpioPin<8>, ADC1>,
    pub in6: AdcPin<GpioPin<3>, ADC1>,
    pub in7: AdcPin<GpioPin<9>, ADC1>,
    pub in8: AdcPin<GpioPin<10>, ADC1>,
}

pub struct EspAdc {
    pins: AdcPins,
    calibration: AdcCalCurve<ADC1>,
    peripheral: Adc<'static, ADC1>,
}

pub struct Analog {
    pub adc: EspAdc,
}

impl Analog {
    pub fn new(adc: impl Peripheral<P = peripherals::ADC1> + 'static, pins: RawPins) -> Self {
        let mut config: AdcConfig<ADC1> = AdcConfig::<ADC1>::new();

        let adc_pins = AdcPins {
            in1: config.enable_pin_with_cal(pins.in1, Attenuation::_11dB),  //GPIO 4
            in2: config.enable_pin_with_cal(pins.in2, Attenuation::_11dB),  //GPIO 5
            in3: config.enable_pin_with_cal(pins.in3, Attenuation::_11dB),  //GPIO 6
            in4: config.enable_pin_with_cal(pins.in4, Attenuation::_11dB),  //GPIO 7
            in5: config.enable_pin_with_cal(pins.in5, Attenuation::_11dB),  //GPIO 8
            in6: config.enable_pin_with_cal(pins.in6, Attenuation::_11dB),  //GPIO 3
            in7: config.enable_pin_with_cal(pins.in7, Attenuation::_11dB),  //GPIO 9
            in8: config.enable_pin_with_cal(pins.in8, Attenuation::_11dB),  //GPIO 10
        };

        Analog {
            adc: EspAdc {
                pins: adc_pins,
                peripheral: Adc::new(adc, config),
                calibration: AdcCalCurve::<ADC1>::new_cal(Attenuation::_11dB),
            },
        }
    }
}

• Read code

impl EspAdc {
    pub fn read(&mut self, input: InputRead) -> f32 {
        info!("Cal value: {:?}", self.calibration.adc_cal());
        let mut value: u16;

        match input {
            InputRead::IN1 => value = self.peripheral.read_blocking(&mut self.pins.in1),
            InputRead::IN2 => value = self.peripheral.read_blocking(&mut self.pins.in2),
            InputRead::IN3 => value = self.peripheral.read_blocking(&mut self.pins.in3),
            InputRead::IN4 => value = self.peripheral.read_blocking(&mut self.pins.in4),
            InputRead::IN5 => value = self.peripheral.read_blocking(&mut self.pins.in5),
            InputRead::IN6 => value = self.peripheral.read_blocking(&mut self.pins.in6),
            InputRead::IN7 => value = self.peripheral.read_blocking(&mut self.pins.in7),
            InputRead::IN8 => value = self.peripheral.read_blocking(&mut self.pins.in8),
        }

        for i in 0..4096 {
            info!(
                "Raw value: {:?}, Converted Value: {:?}",
                i,
                self.calibration.adc_val(i)
            );
        }

        value = self.calibration.adc_val(value);

        value as f32
    }
}

4. Panic message:

====================== PANIC ======================
panicked at C:\Users\lenovodev\.cargo\registry\src\index.crates.io-6f17d22bba15001f\esp-hal-0.23.1\src\analog\adc\calibration\curve.rs:96:25:
attempt to multiply with overflow

Backtrace:
0x42002b70
0x42002b70 - <esp_hal::analog::adc::implementation::calibration::curve::AdcCalCurve<ADCI> as esp_hal::analog::adc::AdcCalScheme<ADCI>>::adc_val
    at ??:??
0x420033ad
0x420033ad - cm2008_rs::hal::analog::EspAdc::read
    at C:\Users\lenovodev\Documents\marine\CM2008-rs\src\hal\analog.rs:99
0x42003ad6
0x42003ad6 - cm2008_rs::____embassy_main_task::{{closure}}
    at C:\Users\lenovodev\Documents\marine\CM2008-rs\src\main.rs:56
0x4200e74e
0x4200e74e - embassy_executor::raw::SyncExecutor::poll::{{closure}}
    at C:\Users\lenovodev\.cargo\registry\src\index.crates.io-6f17d22bba15001f\embassy-executor-0.7.0\src\raw\mod.rs:430
0x420037d1
0x420037d1 - esp_hal_embassy::executor::thread::Executor::run
    at C:\Users\lenovodev\.cargo\registry\src\index.crates.io-6f17d22bba15001f\esp-hal-embassy-0.6.0\src\executor\thread.rs:106
0x42002a4f
0x42002a4f - cm2008_rs::__xtensa_lx_rt_main
    at C:\Users\lenovodev\Documents\marine\CM2008-rs\src\main.rs:18
0x4200cc0f
0x4200cc0f - Reset
    at C:\Users\lenovodev\.cargo\registry\src\index.crates.io-6f17d22bba15001f\xtensa-lx-rt-0.18.0\src\lib.rs:82
0x40378912
0x40378912 - ESP32Reset
    at C:\Users\lenovodev\.cargo\registry\src\index.crates.io-6f17d22bba15001f\esp-hal-0.23.1\src\soc\esp32s3\mod.rs:165

To Reproduce

1. Use the ADC calibration code (AdcCalCurve) in esp-hal (version 0.23.1) on an ESP32-S3 chip with attenuation set to _11dB.
2. Call adc_val(value) with a value > 3594.
3. Observe that the code eventually panics with an integer overflow in the polynomial evaluation for the calibration.

Expected behavior

The code should not overflow when calculating calibration errors at any valid raw ADC input value. It should handle large multiplication either by preventing overflow or using safer arithmetic for calibration curves.

Environment

• Target device: ESP32-S3 (Chip revision: v0.2)
• Crate name and version: esp-hal 0.23.1
• Bootloader info (from logs): ESP-IDF v5.1-beta1-378-gea5e0ff298-dirt 2nd stage bootloader
• Other relevant details:
  • Panic occurs when reading a value > 3094.
  • Using Rust and Embassy environment.

Cargo.toml

[package]
edition = "2021"
name = "cm2008-rs"
version = "0.1.0"

[[bin]]
name = "cm2008-rs"
path = "./src/main.rs"

[dependencies]
embassy-net = { version = "0.6.0", features = [
    "dhcpv4",
    "medium-ethernet",
    "tcp",
    "udp",
] }
embassy-executor = { version = "0.7.0", features = ["task-arena-size-20480"] }
embassy-time = { version = "0.4.0", features = ["generic-queue-8"] }
esp-hal-embassy = { version = "0.6.0", features = ["esp32s3"] }
embedded-io = "0.6.1"
embedded-io-async = "0.6.1"
esp-alloc = { version = "0.6.0" }
esp-backtrace = { version = "0.15.0", features = [
    "esp32s3",
    "exception-handler",
    "panic-handler",
    "println",
] }
esp-hal = { version = "0.23.1", features = ["esp32s3", "unstable"] }
esp-println = { version = "0.13.0", features = ["esp32s3", "log"] }
esp-wifi = { version = "0.12.0", default-features = false, features = [
    "ble",
    "coex",
    "esp-alloc",
    "esp32s3",
    "log",
    "utils",
    "wifi",
] }
heapless = { version = "0.8.0", default-features = false }
log = { version = "0.4.21" }
smoltcp = { version = "0.12.0", default-features = false, features = [
    "medium-ethernet",
    "multicast",
    "proto-dhcpv4",
    "proto-dns",
    "proto-ipv4",
    "socket-dns",
    "socket-icmp",
    "socket-raw",
    "socket-tcp",
    "socket-udp",
] }
# for more networking protocol support see https://crates.io/crates/edge-net
bleps = { git = "https://github.com/bjoernQ/bleps", package = "bleps", rev = "a5148d8ae679e021b78f53fd33afb8bb35d0b62e", features = [
    "async",
    "macros",
] }
critical-section = "1.2.0"
static_cell = { version = "2.1.0", features = ["nightly"] }
fugit = "0.3.7"
embedded-can = "0.4.1"

[profile.dev]
# Rust debug is too slow.
# For debug builds always builds with some optimization
opt-level = "s"

[profile.release]
codegen-units = 1        # LLVM can perform better optimizations using a single thread
debug = 2
debug-assertions = false
incremental = true
lto = 'fat'
opt-level = 's'
overflow-checks = false

---

[MabezDev]

Thanks for the detailed issue report!

ADC is lacking in many ways, it's driver has many issues associated with it. Unfortunately I don't think we'll tackle this ourselves anytime soon, as our focus currently is a stable release.

A PR to fix this would be really great, if you have the spare time!

[yuri-ncs]

I cant promise anything, but i will give it a try, also, the API-GUIDELINES are missing in the documentation folder

[MabezDev]

Ah sorry, we recently renamed them to DEVELOPER-GUIDLINES.md.