From 8d127e5f26db68d97c1d94018b8dc43b153a27fd Mon Sep 17 00:00:00 2001 From: Scott Mabin Date: Wed, 20 May 2026 17:39:09 +0200 Subject: [PATCH 1/2] adc: fix calibration, efuse parsing, and register-sequence bugs MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Found by comparing the esp-hal ADC driver against ESP-IDF's components/esp_adc + components/hal + components/efuse. All independently contribute to wrong/garbage readings; none of them needed each other. Calibration: - C6 0 dB curve K0 coefficient was 10x too small (-0.0487... vs -0.487166...). Stale IDF revision; the upstream divisor changed to 1e15 for that one term. - S3 calibration was writing the cal attenuation to a non-existent atten slot (channel 15). The RTC controller falls back to channel 0's atten when all channels are disabled, so write there instead — matches ESP-IDF's `cal_setup`. - RISC-V calibration was passing the channel-15 sentinel through `config_onetime_sample`, which on ADC1 wrote `0x7` instead of the required `0xF` cal-mux value. Split into a dedicated `config_cal_sample`. Efuse parsing: - C2 atten-11dB cal voltage read the sign bit from the wrong variable and had inverted branches. - H2 cal_code was never sign-extended at bit 9 (~500 digi-counts of gain error when that bit was set) and skipped its version check. - H2 init_code's version check was effectively a no-op (`version > 4` with version only ever 0/1). Register sequencing: - C3 ADC2 wrote `channel` to `onetime_channel` instead of `(adc_n << 3) | channel`, so every ADC2 read sampled an ADC1 pad. Now also clears the other unit's onetime bit so alternating ADC1/ADC2 reads don't leave both units enabled. - `start_onetime_sample` had no rising edge — it called `set_bit` on a bit that was usually already set. Replaced with a proper clear -> (C6 delay) -> set, dropping the old broken double-start workaround in `read_oneshot`. - S2/S3 `read_data` returned the 16-bit field unmasked, exposing the ADC2 arbiter/validity flag bits as part of the result. - ESP32 ADC2's switch to RTC controller was missing `sar2_pwdet_force = 0` and `SYSCON.saradc_ctrl.sar2_mux = 1`; without these, after any Wi-Fi/BT init ADC2 stayed muxed to the power-detect path. Also renamed several inline bitmasks to named constants (`ADC_VAL_MASK`, `ONETIME_CHANNEL_MASK`, `ONETIME_UNIT2_BIT`, `ONETIME_CAL_CHANNEL`, `CAL_ATTEN_CHANNEL`) and added a `disable_channels` helper on xtensa to replace the `1 << 15`-mask-truncation trick. --- esp-hal/src/analog/adc/calibration/curve.rs | 4 +- esp-hal/src/analog/adc/esp32.rs | 17 +++- esp-hal/src/analog/adc/riscv.rs | 96 +++++++++++++++++---- esp-hal/src/analog/adc/xtensa.rs | 71 +++++++++++++-- esp-hal/src/efuse/esp32c2/mod.rs | 8 +- esp-hal/src/efuse/esp32h2/mod.rs | 20 +++-- 6 files changed, 180 insertions(+), 36 deletions(-) diff --git a/esp-hal/src/analog/adc/calibration/curve.rs b/esp-hal/src/analog/adc/calibration/curve.rs index 2d69354febd..83cc7029ef2 100644 --- a/esp-hal/src/analog/adc/calibration/curve.rs +++ b/esp-hal/src/analog/adc/calibration/curve.rs @@ -198,11 +198,11 @@ mod impls { ]; - /// Error curve coefficients derived from + /// Error curve coefficients derived from #[cfg(esp32c6)] CURVES_COEFFS1 [ _0dB => [ - -0.0487166399931449, + -0.487166399931449, 0.0006436483033201, 0.0000030410131806, ], diff --git a/esp-hal/src/analog/adc/esp32.rs b/esp-hal/src/analog/adc/esp32.rs index 4b638266d93..c42220111ab 100644 --- a/esp-hal/src/analog/adc/esp32.rs +++ b/esp-hal/src/analog/adc/esp32.rs @@ -5,7 +5,7 @@ use core::{ use super::{AdcConfig, Attenuation}; use crate::{ - peripherals::{ADC1, ADC2, SENS}, + peripherals::{ADC1, ADC2, APB_CTRL, SENS}, private, }; @@ -175,9 +175,18 @@ impl RegisterAccess for ADC2<'_> { } fn clear_dig_force() { - SENS::regs() - .sar_read_ctrl2() - .modify(|_, w| w.sar2_dig_force().clear_bit()); + // Switch ADC2 to RTC-controller mode. ESP-IDF's `adc_ll_set_controller` for + // ADC2 RTC also clears `sar2_pwdet_force` and sets `SYSCON.saradc_ctrl.sar2_mux` + // — without these, ADC2 stays muxed to the power-detect path (especially after + // Wi-Fi/BT initialization) and every conversion returns the PWDET output + // instead of the requested channel. + SENS::regs().sar_read_ctrl2().modify(|_, w| { + w.sar2_dig_force().clear_bit(); + w.sar2_pwdet_force().clear_bit() + }); + APB_CTRL::regs() + .apb_saradc_ctrl() + .modify(|_, w| w.saradc_sar2_mux().set_bit()); } fn set_start_force() { diff --git a/esp-hal/src/analog/adc/riscv.rs b/esp-hal/src/analog/adc/riscv.rs index a34655dc4da..9f0a72ce85d 100644 --- a/esp-hal/src/analog/adc/riscv.rs +++ b/esp-hal/src/analog/adc/riscv.rs @@ -41,12 +41,32 @@ mod calibration; // https://github.com/espressif/esp-idf/blob/903af13e8/components/soc/esp32h2/include/soc/regi2c_saradc.h cfg_if::cfg_if! { if #[cfg(adc_adc1)] { + /// Mask for the 12-bit conversion result returned by the ADC. const ADC_VAL_MASK: u16 = 0xfff; + /// Number of samples averaged during runtime self-calibration. const ADC_CAL_CNT_MAX: u16 = 32; + /// Sentinel passed to [`CalibrationAccess::ADC_CAL_CHANNEL`] from the shared + /// driver. The RISC-V hardware doesn't use this as a physical channel number; + /// instead, calibration writes [`ONETIME_CAL_CHANNEL`] to the `onetime_channel` + /// field and the calibration attenuation to channel 0's atten slot. const ADC_CAL_CHANNEL: u16 = 15; } } +// Encoding of the `APB_SARADC.onetime_sample.onetime_channel` field (4 bits): +// bit 3 – ADC unit selector (clear = ADC1, set = ADC2) +// bits 2:0 – channel index for the selected unit (or 0b111 for the cal mux) +// See `adc_oneshot_ll_set_channel` / `adc_oneshot_ll_disable_channel` in +// `components/hal//include/hal/adc_ll.h` — both encode `(adc_n << 3) | x`. +const ONETIME_CHANNEL_MASK: u8 = 0b0111; +#[cfg(adc_adc2)] +const ONETIME_UNIT2_BIT: u8 = 0b1000; +/// Value written to `onetime_channel` when starting a calibration conversion. +/// Matches ESP-IDF's `adc_oneshot_ll_disable_channel`, which writes +/// `(adc_n << 3) | 0xF` — the OR collapses to `0xF` regardless of unit because +/// `0xF` already has bit 3 set. This selects the on-chip calibration mux. +const ONETIME_CAL_CHANNEL: u8 = 0x0F; + // The number of analog IO pins, and in turn the number of attentuations, // depends on which chip is being used cfg_if::cfg_if! { @@ -74,8 +94,10 @@ where ADCX::enable_vdef(true); - // Start sampling - ADCX::config_onetime_sample(ADC_CAL_CHANNEL as u8, atten as u8); + // Route the SAR to the on-chip calibration mux instead of an external pad. + // The cal mux internally connects to either GND or Vref depending on the + // value programmed by `connect_cal` below. + ADCX::config_cal_sample(atten as u8); // Connect calibration source ADCX::connect_cal(source, true); @@ -113,6 +135,12 @@ pub trait RegisterAccess { /// Configure onetime sampling parameters fn config_onetime_sample(channel: u8, attenuation: u8); + /// Configure onetime sampling to read from the on-chip calibration mux + /// instead of an external pad. Equivalent to ESP-IDF's + /// `adc_oneshot_ll_disable_channel` + `adc_oneshot_ll_set_atten(unit, 0, atten)` + /// pair in `cal_setup`. + fn config_cal_sample(attenuation: u8); + /// Start onetime sampling fn start_onetime_sample(); @@ -136,14 +164,39 @@ pub trait RegisterAccess { impl RegisterAccess for crate::peripherals::ADC1<'_> { fn config_onetime_sample(channel: u8, attenuation: u8) { APB_SARADC::regs().onetime_sample().modify(|_, w| unsafe { + // Make sure only ADC1's onetime-sample bit is set; alternating ADC1/ADC2 + // reads must not leave both units enabled. + #[cfg(adc_adc2)] + w.saradc2_onetime_sample().clear_bit(); + w.saradc1_onetime_sample().set_bit(); + w.onetime_channel() + .bits(channel & ONETIME_CHANNEL_MASK); + w.onetime_atten().bits(attenuation) + }); + } + + fn config_cal_sample(attenuation: u8) { + APB_SARADC::regs().onetime_sample().modify(|_, w| unsafe { + #[cfg(adc_adc2)] + w.saradc2_onetime_sample().clear_bit(); w.saradc1_onetime_sample().set_bit(); - w.onetime_channel().bits(channel); + w.onetime_channel().bits(ONETIME_CAL_CHANNEL); w.onetime_atten().bits(attenuation) }); } fn start_onetime_sample() { - APB_SARADC::regs() + // The hardware requires a 0->1 transition of onetime_start that the ADC + // digital controller can capture on its (slower) clock. ESP-IDF holds the + // bit low for >= 3 ADC controller cycles before the rising edge — see + // `adc_hal_onetime_start` in adc_oneshot_hal.c. + let saradc = APB_SARADC::regs(); + saradc + .onetime_sample() + .modify(|_, w| w.onetime_start().clear_bit()); + #[cfg(esp32c6)] + crate::rom::ets_delay_us(5); + saradc .onetime_sample() .modify(|_, w| w.onetime_start().set_bit()); } @@ -158,7 +211,7 @@ impl RegisterAccess for crate::peripherals::ADC1<'_> { .read() .saradc1_data() .bits() as u16 - & 0xfff + & ADC_VAL_MASK } fn reset() { @@ -231,14 +284,33 @@ impl super::CalibrationAccess for crate::peripherals::ADC1<'_> { impl RegisterAccess for crate::peripherals::ADC2<'_> { fn config_onetime_sample(channel: u8, attenuation: u8) { APB_SARADC::regs().onetime_sample().modify(|_, w| unsafe { + // Make sure only ADC2's onetime-sample bit is set; alternating ADC1/ADC2 + // reads must not leave both units enabled. + w.saradc1_onetime_sample().clear_bit(); w.saradc2_onetime_sample().set_bit(); - w.onetime_channel().bits(channel); + w.onetime_channel() + .bits(ONETIME_UNIT2_BIT | (channel & ONETIME_CHANNEL_MASK)); + w.onetime_atten().bits(attenuation) + }); + } + + fn config_cal_sample(attenuation: u8) { + APB_SARADC::regs().onetime_sample().modify(|_, w| unsafe { + w.saradc1_onetime_sample().clear_bit(); + w.saradc2_onetime_sample().set_bit(); + w.onetime_channel().bits(ONETIME_CAL_CHANNEL); w.onetime_atten().bits(attenuation) }); } fn start_onetime_sample() { - APB_SARADC::regs() + let saradc = APB_SARADC::regs(); + saradc + .onetime_sample() + .modify(|_, w| w.onetime_start().clear_bit()); + #[cfg(esp32c6)] + crate::rom::ets_delay_us(5); + saradc .onetime_sample() .modify(|_, w| w.onetime_start().set_bit()); } @@ -253,7 +325,7 @@ impl RegisterAccess for crate::peripherals::ADC2<'_> { .read() .saradc2_data() .bits() as u16 - & 0xfff + & ADC_VAL_MASK } fn reset() { @@ -399,14 +471,6 @@ where let attenuation = self.attenuations[channel as usize].unwrap() as u8; ADCX::config_onetime_sample(channel, attenuation); ADCX::start_onetime_sample(); - - // see https://github.com/espressif/esp-idf/blob/b4268c874a4cf8fcf7c0c4153cffb76ad2ddda4e/components/hal/adc_oneshot_hal.c#L105-L107 - // the delay might be a bit generous but longer delay seem to not cause problems - #[cfg(esp32c6)] - { - crate::rom::ets_delay_us(40); - ADCX::start_onetime_sample(); - } } // Wait for ADC to finish conversion diff --git a/esp-hal/src/analog/adc/xtensa.rs b/esp-hal/src/analog/adc/xtensa.rs index faa2e43c3bd..a98e9ce7122 100644 --- a/esp-hal/src/analog/adc/xtensa.rs +++ b/esp-hal/src/analog/adc/xtensa.rs @@ -17,12 +17,37 @@ pub(super) const NUM_ATTENS: usize = 10; cfg_if::cfg_if! { if #[cfg(esp32s3)] { - const ADC_VAL_MASK: u16 = 0xfff; - const ADC_CAL_CNT_MAX: u16 = 32; - const ADC_CAL_CHANNEL: u16 = 15; + /// Mask covering the 12-bit conversion result on ESP32-S3. + /// + /// `sar_meas{1,2}_ctrl2.meas_data_sar` is a 16-bit field that also + /// contains arbiter / validity flag bits in the high bits; reads must + /// be masked to the actual converter width to avoid garbage readings + /// (especially on ADC2, where the arbiter sets flags when Wi-Fi takes + /// over). + const ADC_VAL_MASK: u16 = 0x0fff; + } else if #[cfg(esp32s2)] { + /// Mask covering the 13-bit conversion result on ESP32-S2. + const ADC_VAL_MASK: u16 = 0x1fff; } } +#[cfg(esp32s3)] +const ADC_CAL_CNT_MAX: u16 = 32; + +/// Sentinel value for [`CalibrationAccess::ADC_CAL_CHANNEL`]. The xtensa +/// calibration routine never uses this as a channel index — instead it calls +/// [`RegisterAccess::disable_channels`] (matching ESP-IDF's +/// `adc_oneshot_ll_disable_channel`) and writes the calibration attenuation +/// to channel 0's atten slot, since the RTC controller auto-selects channel 0's +/// attenuation when all channels are disabled. +#[cfg(esp32s3)] +const ADC_CAL_CHANNEL: u16 = 15; + +/// Attenuation slot the RTC controller falls back to when no channels are +/// enabled. ESP-IDF's `cal_setup` writes the calibration attenuation here. +#[cfg(esp32s3)] +const CAL_ATTEN_CHANNEL: usize = 0; + impl AdcConfig where ADCX: RegisterAccess, @@ -38,9 +63,17 @@ where ADCX::enable_vdef(true); - // Start sampling - ADCX::set_en_pad(ADCX::ADC_CAL_CHANNEL as u8); - ADCX::set_attenuation(ADCX::ADC_CAL_CHANNEL as usize, atten as u8); + // Match ESP-IDF's `cal_setup` in components/hal/adc_hal_common.c: + // + // adc_oneshot_ll_disable_channel(adc_n); // sar_en_pad = 0 + // adc_oneshot_ll_set_atten(adc_n, 0, atten); // channel 0's atten + // + // The IDF comment notes: "When controlled by RTC controller, when all + // channels are disabled, HW auto selects channel0 atten param." So the + // literal `0` here is a hardware-defined fallback slot, not an arbitrary + // channel choice. + ADCX::disable_channels(); + ADCX::set_attenuation(CAL_ATTEN_CHANNEL, atten as u8); // Connect calibration source ADCX::connect_cal(source, true); @@ -86,6 +119,12 @@ pub trait RegisterAccess { fn set_en_pad(channel: u8); + /// Disable all channels on this ADC unit (write `sar_en_pad = 0`). + /// + /// Used by the calibration setup to detach the SAR from any external pad. + /// Equivalent to ESP-IDF's `adc_oneshot_ll_disable_channel`. + fn disable_channels(); + fn clear_start_sample(); fn start_sample(); @@ -140,6 +179,12 @@ impl RegisterAccess for crate::peripherals::ADC1<'_> { .modify(|_, w| unsafe { w.sar1_en_pad().bits(1 << channel) }); } + fn disable_channels() { + SENS::regs() + .sar_meas1_ctrl2() + .modify(|_, w| unsafe { w.sar1_en_pad().bits(0) }); + } + fn clear_start_sample() { SENS::regs() .sar_meas1_ctrl2() @@ -161,11 +206,15 @@ impl RegisterAccess for crate::peripherals::ADC1<'_> { } fn read_data() -> u16 { + // The data field is 16 bits wide but only `ADC_VAL_MASK` of it carries + // the conversion result; the upper bits are arbiter / validity flags + // (see comment on `ADC_VAL_MASK`). SENS::regs() .sar_meas1_ctrl2() .read() .meas1_data_sar() .bits() + & ADC_VAL_MASK } #[cfg(any(esp32s2, esp32s3))] @@ -250,6 +299,12 @@ impl RegisterAccess for crate::peripherals::ADC2<'_> { .modify(|_, w| unsafe { w.sar2_en_pad().bits(1 << channel) }); } + fn disable_channels() { + SENS::regs() + .sar_meas2_ctrl2() + .modify(|_, w| unsafe { w.sar2_en_pad().bits(0) }); + } + fn clear_start_sample() { SENS::regs() .sar_meas2_ctrl2() @@ -271,11 +326,15 @@ impl RegisterAccess for crate::peripherals::ADC2<'_> { } fn read_data() -> u16 { + // ADC2 specifically can return non-zero flag bits in the upper bits + // when the arbiter takes over for Wi-Fi; an unmasked read produces + // huge bogus values. See comment on `ADC_VAL_MASK`. SENS::regs() .sar_meas2_ctrl2() .read() .meas2_data_sar() .bits() + & ADC_VAL_MASK } #[cfg(any(esp32s2, esp32s3))] diff --git a/esp-hal/src/efuse/esp32c2/mod.rs b/esp-hal/src/efuse/esp32c2/mod.rs index 95dd575fe5d..3695e8446a6 100644 --- a/esp-hal/src/efuse/esp32c2/mod.rs +++ b/esp-hal/src/efuse/esp32c2/mod.rs @@ -114,11 +114,13 @@ pub fn rtc_calib_cal_code(_unit: AdcCalibUnit, atten: Attenuation) -> Option + // ESP-IDF: signed-extended diff_code11 (sign bit 5); out_digi = code0 - signed - 123 + // (so bit 5 set means add the magnitude; bit 5 clear means subtract). let diff_code11: u16 = super::read_field_le(ADC1_CAL_VOL_ATTEN3); - let code11 = if diff_code0 & (1 << 5) != 0 { - code0 - (diff_code11 & 0x1f) + let code11 = if diff_code11 & (1 << 5) != 0 { + code0 + (diff_code11 & 0x1f) } else { - code0 + diff_code11 + code0 - diff_code11 } - 123; Some(code11) diff --git a/esp-hal/src/efuse/esp32h2/mod.rs b/esp-hal/src/efuse/esp32h2/mod.rs index c473f85fd15..5a875573ba5 100644 --- a/esp-hal/src/efuse/esp32h2/mod.rs +++ b/esp-hal/src/efuse/esp32h2/mod.rs @@ -54,7 +54,7 @@ pub fn rtc_calib_version() -> u8 { pub fn rtc_calib_init_code(_unit: AdcCalibUnit, atten: Attenuation) -> Option { let version = rtc_calib_version(); - if version > 4 { + if version != 1 { return None; } @@ -99,18 +99,28 @@ pub fn rtc_calib_cal_mv(_unit: AdcCalibUnit, atten: Attenuation) -> u16 { /// See: #[instability::unstable] pub fn rtc_calib_cal_code(_unit: AdcCalibUnit, atten: Attenuation) -> Option { + let version = rtc_calib_version(); + + if version != 1 { + return None; + } + let cal_code: u16 = super::read_field_le(match atten { Attenuation::_0dB => ADC1_HI_DOUT_ATTEN0, Attenuation::_2p5dB => ADC1_HI_DOUT_ATTEN1, Attenuation::_6dB => ADC1_HI_DOUT_ATTEN2, Attenuation::_11dB => ADC1_HI_DOUT_ATTEN3, }); - let cal_code: u16 = if atten == Attenuation::_6dB { - 2970 + cal_code + + // ESP-IDF sign-extends the 10-bit field on bit 9 before adding the offset. + // Without this, cal_codes with bit 9 set produce 500+ digi-counts of gain error. + let signed = if cal_code & (1 << 9) != 0 { + -((cal_code & !(1 << 9)) as i32) } else { - 2900 + cal_code + cal_code as i32 }; - Some(cal_code) + let chk_offset: i32 = if atten == Attenuation::_6dB { 2970 } else { 2900 }; + Some((chk_offset + signed) as u16) } /// Returns the major hardware revision From bb19659a32cce497ea4f06e1f9b6250309cadd5c Mon Sep 17 00:00:00 2001 From: Scott Mabin Date: Wed, 20 May 2026 17:49:29 +0200 Subject: [PATCH 2/2] hil-test: add ADC<-DAC loopback test on esp32 / esp32s2 Each DAC channel on ESP32 and ESP32-S2 lives on the same pad as one of the ADC2 channels (ESP32: GPIO25/26 -> ADC2_CH8/9; ESP32-S2: GPIO17/18 -> ADC2_CH6/7). This test uses that internal wiring as a no-rig signal source: write known DAC codes, sample the same pad with the ADC, and assert that the reading is in the right ballpark. Cases: - DAC=0 reads in the bottom 10% of full scale - DAC=255 reads in the top 20% - Midpoint (DAC=128) lands in the middle half - Full sweep is monotonic (modulo ~1.2% noise band) and spans >70% FS - Cross-channel independence: driving one DAC must not affect the other pad. Pre-fix on RISC-V the ADC2 channel encoding bug would have failed this; covers the same class of bug on Xtensa. Thresholds are all fractions of ADC_MAX because the resolution differs per chip: ESP32 returns 12-bit (default of its configurable Resolution), ESP32-S2 returns 13-bit unconditionally. Drive-by: ungate CAL_ATTEN_CHANNEL in esp-hal/src/analog/adc/xtensa.rs so the generic adc_calibrate body still name-resolves on ESP32-S2 (it has no CalibrationAccess impl, but the body is still typechecked). --- esp-hal/src/analog/adc/xtensa.rs | 4 +- hil-test/Cargo.toml | 5 + hil-test/src/bin/adc.rs | 335 +++++++++++++++++++++++++++++++ 3 files changed, 343 insertions(+), 1 deletion(-) create mode 100644 hil-test/src/bin/adc.rs diff --git a/esp-hal/src/analog/adc/xtensa.rs b/esp-hal/src/analog/adc/xtensa.rs index a98e9ce7122..dfca970dbe3 100644 --- a/esp-hal/src/analog/adc/xtensa.rs +++ b/esp-hal/src/analog/adc/xtensa.rs @@ -45,7 +45,9 @@ const ADC_CAL_CHANNEL: u16 = 15; /// Attenuation slot the RTC controller falls back to when no channels are /// enabled. ESP-IDF's `cal_setup` writes the calibration attenuation here. -#[cfg(esp32s3)] +/// Referenced from the generic `adc_calibrate` body, so it has to be in +/// scope for any `RegisterAccess` impl — not just chips that actually +/// implement `CalibrationAccess`. const CAL_ATTEN_CHANNEL: usize = 0; impl AdcConfig diff --git a/hil-test/Cargo.toml b/hil-test/Cargo.toml index 74e8119de0b..0527fb9ef78 100644 --- a/hil-test/Cargo.toml +++ b/hil-test/Cargo.toml @@ -7,6 +7,11 @@ publish = false [lib] name = "hil_test" +[[bin]] +name = "adc" +harness = false +required-features = ["unstable"] + [[bin]] name = "alloc_psram" harness = false diff --git a/hil-test/src/bin/adc.rs b/hil-test/src/bin/adc.rs new file mode 100644 index 00000000000..722256d3736 --- /dev/null +++ b/hil-test/src/bin/adc.rs @@ -0,0 +1,335 @@ +//! ADC + DAC loopback test +//! +//! ESP32 and ESP32-S2 are the only chips with both an ADC and a DAC, and +//! conveniently each DAC channel is wired to the same physical pad as one of +//! the ADC2 channels: +//! +//! ESP32: DAC1 / GPIO25 / ADC2_CH8 DAC2 / GPIO26 / ADC2_CH9 +//! ESP32-S2: DAC1 / GPIO17 / ADC2_CH6 DAC2 / GPIO18 / ADC2_CH7 +//! +//! That gives us an internal loopback with no wiring: the DAC drives a known +//! voltage onto the pad, and the ADC samples the same pad. This test relies on +//! that to validate that the ADC driver returns *roughly* the right value for +//! a programmed DAC level. +//! +//! Tolerances are deliberately wide: +//! +//! - the 8-bit DAC has its own non-linearity (~±50 mV near the rails) +//! - on ESP32-S2 there's no efuse-based ADC calibration in esp-hal yet, so +//! raw codes can be offset by several hundred LSB +//! - the DAC drives 0..VDD (~3.3 V) but at 11 dB attenuation the ADC's +//! usable input range tops out near 3.1 V, so high DAC codes saturate + +//% CHIPS: esp32 esp32s2 +//% FEATURES: unstable + +#![no_std] +#![no_main] + +use hil_test as _; + +/// DAC1 lives on a different GPIO on each chip; this picks the right pad. +#[cfg(esp32)] +macro_rules! dac1_pad { ($p:expr) => { $p.GPIO25 } } +#[cfg(esp32)] +macro_rules! dac2_pad { ($p:expr) => { $p.GPIO26 } } +#[cfg(esp32)] +type Dac1Gpio = esp_hal::peripherals::GPIO25<'static>; +#[cfg(esp32)] +type Dac2Gpio = esp_hal::peripherals::GPIO26<'static>; + +#[cfg(esp32s2)] +macro_rules! dac1_pad { ($p:expr) => { $p.GPIO17 } } +#[cfg(esp32s2)] +macro_rules! dac2_pad { ($p:expr) => { $p.GPIO18 } } +#[cfg(esp32s2)] +type Dac1Gpio = esp_hal::peripherals::GPIO17<'static>; +#[cfg(esp32s2)] +type Dac2Gpio = esp_hal::peripherals::GPIO18<'static>; + +#[embedded_test::tests(default_timeout = 5)] +mod tests { + use esp_hal::{ + Blocking, + analog::{ + adc::{Adc, AdcChannel, AdcConfig, AdcPin, Attenuation}, + dac::Dac, + }, + delay::Delay, + peripherals::{ADC2, DAC1, DAC2}, + }; + + use super::{Dac1Gpio, Dac2Gpio}; + + /// Full-scale code at this chip's native ADC resolution. + /// + /// ESP32's ADC is configurable 9/10/11/12-bit and defaults to 12 (so + /// `0..=4095`). The S2's ADC is fixed at 13-bit (`0..=8191`), and the + /// driver returns the masked raw value. Threshold checks below are all + /// expressed as fractions of `ADC_MAX` so they scale correctly on both. + #[cfg(esp32)] + const ADC_MAX: u16 = 4095; + #[cfg(esp32s2)] + const ADC_MAX: u16 = 8191; + + /// Delay between writing the DAC and sampling the ADC, so the analog + /// output has time to settle on the pad. + const SETTLE_US: u32 = 200; + + /// How many oneshot samples to average per measurement. Without calibration + /// on the S2 (and on ESP32 ADC2 in general), single-sample jitter can be + /// tens of LSB; averaging knocks that down so the thresholds below stay + /// tight enough to be useful. + const AVG_SAMPLES: u32 = 16; + + /// Maximum allowed backward step between consecutive sweep readings. The + /// ADC's full-scale code differs by 2x between ESP32 (12-bit) and S2 + /// (13-bit), so express the tolerance as a fraction of full scale — + /// roughly 1.2% — rather than as a raw LSB count. + const NOISE_TOLERANCE: u16 = ADC_MAX / 80; + + struct Context { + delay: Delay, + dac1: Dac<'static, DAC1<'static>>, + dac2: Dac<'static, DAC2<'static>>, + adc: Adc<'static, ADC2<'static>, Blocking>, + adc_pin1: AdcPin>, + adc_pin2: AdcPin>, + } + + impl Context { + fn read_pin1(&mut self) -> u16 { + read_avg(&mut self.adc, &mut self.adc_pin1) + } + + fn read_pin2(&mut self) -> u16 { + read_avg(&mut self.adc, &mut self.adc_pin2) + } + } + + fn read_avg( + adc: &mut Adc<'static, ADC2<'static>, Blocking>, + pin: &mut AdcPin>, + ) -> u16 { + let mut sum: u32 = 0; + for _ in 0..AVG_SAMPLES { + sum += nb::block!(adc.read_oneshot(pin)).unwrap() as u32; + } + (sum / AVG_SAMPLES) as u16 + } + + #[init] + fn init() -> Context { + let peripherals = esp_hal::init(esp_hal::Config::default()); + + // The DAC consumes the GPIO and puts it in analog mode. The ADC needs + // a handle to the same pad — `set_analog` is idempotent, and the pad's + // analog mux physically connects both the DAC output driver and the + // ADC input sampler, so the two `set_analog` calls don't conflict. + let dac1 = Dac::new(peripherals.DAC1, dac1_pad!(peripherals)); + let dac2 = Dac::new(peripherals.DAC2, dac2_pad!(peripherals)); + + // SAFETY: we just moved the original handles into the DAC; the pad is + // already in analog mode and not used for anything else. + let adc_pad1 = unsafe { super::steal_dac1_pad() }; + let adc_pad2 = unsafe { super::steal_dac2_pad() }; + + let mut config = AdcConfig::>::new(); + let adc_pin1 = config.enable_pin(adc_pad1, Attenuation::_11dB); + let adc_pin2 = config.enable_pin(adc_pad2, Attenuation::_11dB); + let adc = Adc::new(peripherals.ADC2, config); + + Context { + delay: Delay::new(), + dac1, + dac2, + adc, + adc_pin1, + adc_pin2, + } + } + + // -- DAC1 ---------------------------------------------------------------- + + /// DAC at zero must read low. Catches gross channel-mux errors (e.g. the + /// pre-fix C3 bug where ADC2 sampled an ADC1 pad). + #[test] + fn dac1_zero_reads_low(mut ctx: Context) { + ctx.dac1.write(0); + ctx.delay.delay_micros(SETTLE_US); + + let raw = ctx.read_pin1(); + defmt::info!("DAC1=0 -> ADC = {}", raw); + hil_test::assert!( + raw < ADC_MAX / 10, + "expected DAC=0 to read < {}, got {}", + ADC_MAX / 10, + raw, + ); + } + + /// DAC at full scale must read near full scale. At 11 dB attenuation the + /// ADC saturates somewhere around DAC code 240 (depending on VDD), so we + /// only require >= 80% FS. + #[test] + fn dac1_max_reads_high(mut ctx: Context) { + ctx.dac1.write(255); + ctx.delay.delay_micros(SETTLE_US); + + let raw = ctx.read_pin1(); + defmt::info!("DAC1=255 -> ADC = {}", raw); + let lower = ADC_MAX * 8 / 10; + hil_test::assert!(raw >= lower, "expected >= {}, got {}", lower, raw); + } + + /// Midpoint check — DAC at 0x80 must land in the middle half of the ADC + /// range. Bounds gross gain/offset errors. + #[test] + fn dac1_midpoint_in_middle_half(mut ctx: Context) { + ctx.dac1.write(128); + ctx.delay.delay_micros(SETTLE_US); + + let raw = ctx.read_pin1(); + defmt::info!("DAC1=128 -> ADC = {}", raw); + let lower = ADC_MAX / 4; + let upper = ADC_MAX * 3 / 4; + hil_test::assert!( + raw >= lower && raw <= upper, + "expected midpoint in [{}, {}], got {}", + lower, + upper, + raw, + ); + } + + /// Strongest single signal: ADC reading must increase monotonically with + /// DAC code, and the full sweep must span most of the ADC range. This + /// catches sign flips, wrap-around offsets, wrong channels, etc. + #[test] + fn dac1_sweep_monotonic(mut ctx: Context) { + let steps: [u8; 17] = [ + 0, 16, 32, 48, 64, 80, 96, 112, 128, 144, 160, 176, 192, 208, 224, 240, 255, + ]; + let mut readings = [0u16; 17]; + for (i, &v) in steps.iter().enumerate() { + ctx.dac1.write(v); + ctx.delay.delay_micros(SETTLE_US); + readings[i] = ctx.read_pin1(); + defmt::info!("DAC1={} -> ADC = {}", v, readings[i]); + } + + // Allow a small backward step to absorb residual noise after averaging. + for w in readings.windows(2) { + hil_test::assert!( + w[1] + NOISE_TOLERANCE >= w[0], + "non-monotonic: {} -> {}", + w[0], + w[1], + ); + } + + let span = readings[steps.len() - 1] - readings[0]; + let min_span = ADC_MAX * 7 / 10; + hil_test::assert!( + span > min_span, + "DAC sweep produced only {} LSB span (expected > {})", + span, + min_span, + ); + } + + // -- DAC2 ---------------------------------------------------------------- + // Mirrors DAC1; covers the second channel mapping. + + #[test] + fn dac2_zero_reads_low(mut ctx: Context) { + ctx.dac2.write(0); + ctx.delay.delay_micros(SETTLE_US); + let raw = ctx.read_pin2(); + defmt::info!("DAC2=0 -> ADC = {}", raw); + hil_test::assert!(raw < ADC_MAX / 10, "DAC2=0 raw = {}", raw); + } + + #[test] + fn dac2_max_reads_high(mut ctx: Context) { + ctx.dac2.write(255); + ctx.delay.delay_micros(SETTLE_US); + let raw = ctx.read_pin2(); + defmt::info!("DAC2=255 -> ADC = {}", raw); + hil_test::assert!(raw >= ADC_MAX * 8 / 10, "DAC2=255 raw = {}", raw); + } + + #[test] + fn dac2_sweep_monotonic(mut ctx: Context) { + let steps: [u8; 9] = [0, 32, 64, 96, 128, 160, 192, 224, 255]; + let mut readings = [0u16; 9]; + for (i, &v) in steps.iter().enumerate() { + ctx.dac2.write(v); + ctx.delay.delay_micros(SETTLE_US); + readings[i] = ctx.read_pin2(); + defmt::info!("DAC2={} -> ADC = {}", v, readings[i]); + } + for w in readings.windows(2) { + hil_test::assert!( + w[1] + NOISE_TOLERANCE >= w[0], + "DAC2 non-monotonic: {} -> {}", + w[0], + w[1], + ); + } + let span = readings[steps.len() - 1] - readings[0]; + hil_test::assert!( + span > ADC_MAX * 7 / 10, + "DAC2 sweep span = {} (expected > {})", + span, + ADC_MAX * 7 / 10, + ); + } + + /// Cross-channel independence: writing DAC1 must not move the reading on + /// the DAC2 pad. Catches channel-multiplexing bugs (e.g. en_pad encoded + /// wrong) — without the fix in this PR, the ADC2 driver on RISC-V used to + /// sample the wrong pad entirely. + #[test] + fn channels_are_independent(mut ctx: Context) { + ctx.dac1.write(255); + ctx.dac2.write(0); + ctx.delay.delay_micros(SETTLE_US); + let pin1_hi = ctx.read_pin1(); + let pin2_lo = ctx.read_pin2(); + + ctx.dac1.write(0); + ctx.dac2.write(255); + ctx.delay.delay_micros(SETTLE_US); + let pin1_lo = ctx.read_pin1(); + let pin2_hi = ctx.read_pin2(); + + defmt::info!( + "pin1_hi={} pin1_lo={} pin2_hi={} pin2_lo={}", + pin1_hi, + pin1_lo, + pin2_hi, + pin2_lo, + ); + + // Each pad must show a clear separation between its DAC-driven-high + // and DAC-driven-low state, even when the OTHER pad is driven the + // opposite way. + hil_test::assert!(pin1_hi > pin1_lo + ADC_MAX / 2, "pin1 didn't separate"); + hil_test::assert!(pin2_hi > pin2_lo + ADC_MAX / 2, "pin2 didn't separate"); + } +} + +unsafe fn steal_dac1_pad() -> Dac1Gpio { + #[cfg(esp32)] + return unsafe { esp_hal::peripherals::GPIO25::steal() }; + #[cfg(esp32s2)] + return unsafe { esp_hal::peripherals::GPIO17::steal() }; +} + +unsafe fn steal_dac2_pad() -> Dac2Gpio { + #[cfg(esp32)] + return unsafe { esp_hal::peripherals::GPIO26::steal() }; + #[cfg(esp32s2)] + return unsafe { esp_hal::peripherals::GPIO18::steal() }; +}