From a386ee78f24a4a28274bb604066fe25a21cc1e5c Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Bj=C3=B6rn=20Quentin?= Date: Thu, 9 Jul 2026 13:20:43 +0200 Subject: [PATCH 1/6] Prefer reporting an error instead of triggering an assert --- esp-hal/src/rmt.rs | 16 ++++++++++------ esp-hal/src/rmt/reader.rs | 15 +++++++++------ examples/async/embassy_rmt_rx/src/main.rs | 6 +++++- 3 files changed, 24 insertions(+), 13 deletions(-) diff --git a/esp-hal/src/rmt.rs b/esp-hal/src/rmt.rs index eea025cd407..764ab3aded4 100644 --- a/esp-hal/src/rmt.rs +++ b/esp-hal/src/rmt.rs @@ -1863,7 +1863,9 @@ impl<'ch> RxTransaction<'ch, '_> { // `RmtReader::read()` is safe to call even if `poll_internal` is called repeatedly // after the receiver finished since it returns immediately if already done. - self.reader.read(&mut self.data, raw, true); + if !self.reader.read(&mut self.data, raw, true) { + return Some(Event::Error); + } } #[cfg(rmt_has_rx_wrap)] Some(Event::Threshold) => { @@ -2077,11 +2079,13 @@ impl core::future::Future for RxFuture<'_> { // might not be desired. raw.stop_rx(false); - this.reader.read(&mut this.data, raw, true); - - match ev { - Event::Error => Err(Error::ReceiverError), - _ => Ok(this.reader.total), + if !this.reader.read(&mut this.data, raw, true) { + Err(Error::ReceiverError) + } else { + match ev { + Event::Error => Err(Error::ReceiverError), + _ => Ok(this.reader.total), + } } } #[cfg(rmt_has_rx_wrap)] diff --git a/esp-hal/src/rmt/reader.rs b/esp-hal/src/rmt/reader.rs index 674be47cdca..c4042d6ac46 100644 --- a/esp-hal/src/rmt/reader.rs +++ b/esp-hal/src/rmt/reader.rs @@ -45,9 +45,9 @@ impl RmtReader { data: &mut &mut [PulseCode], raw: DynChannelAccess, final_: bool, - ) { + ) -> bool { if self.state != ReaderState::Active { - return; + return true; } let ram_start = raw.channel_ram_start(); @@ -62,8 +62,7 @@ impl RmtReader { // => If both are the same, we're done, max_count = 0 let max_count = (if offset <= hw_offset { 0 } else { memsize }) + hw_offset - offset; - debug_assert!( - max_count == 0 && self.total == 0 + if !(max_count == 0 && self.total == 0 // We always enable wrapping if it is available. If it's unavailable, rx might // stop when the buffer is full, without an end marker present! // (Checking for two value of hw_offset here, because it's not documented what @@ -74,8 +73,10 @@ impl RmtReader { .add(hw_offset.checked_sub(1).unwrap_or(memsize - 1)) .read_volatile() } - .is_end_marker() - ); + .is_end_marker()) + { + return false; + } max_count } else { @@ -139,5 +140,7 @@ impl RmtReader { } debug_assert!(self.offset == 0 || self.offset as usize == memsize / 2); + + true } } diff --git a/examples/async/embassy_rmt_rx/src/main.rs b/examples/async/embassy_rmt_rx/src/main.rs index 3f4acdf76a2..2614eddc609 100644 --- a/examples/async/embassy_rmt_rx/src/main.rs +++ b/examples/async/embassy_rmt_rx/src/main.rs @@ -80,7 +80,11 @@ async fn main(spawner: Spawner) { loop { println!("receive"); - channel.receive(&mut data).await.unwrap(); + if channel.receive(&mut data).await.is_err() { + println!("receive error"); + continue; + } + let mut total = 0usize; for entry in &data[..data.len()] { if entry.length1() == 0 { From 2de78c49bb47b242d492c5fdbe90ccb7b3c341f8 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Bj=C3=B6rn=20Quentin?= Date: Fri, 10 Jul 2026 16:05:36 +0200 Subject: [PATCH 2/6] HIL test --- hil-test/src/bin/rmt.rs | 40 +++++++++++++++++++++++++++++++++++++++- 1 file changed, 39 insertions(+), 1 deletion(-) diff --git a/hil-test/src/bin/rmt.rs b/hil-test/src/bin/rmt.rs index c02d4d1ff59..ca5d3314132 100644 --- a/hil-test/src/bin/rmt.rs +++ b/hil-test/src/bin/rmt.rs @@ -461,6 +461,7 @@ cfg_select! { struct Context { rmt: RMT<'static>, pin: AnyPin<'static>, + pin2: AnyPin<'static>, } impl Context { @@ -505,6 +506,7 @@ impl Context { #[embedded_test::tests(default_timeout = 1, executor = hil_test::Executor::new())] mod tests { + use esp_hal::gpio::Output; #[allow(unused_imports)] use hil_test::{assert, assert_eq}; @@ -523,11 +525,13 @@ mod tests { esp_rtos::start(timg0.timer0, software_interrupt.software_interrupt0); - let pin = AnyPin::from(hil_test::common_test_pins!(peripherals).1); + let pins = hil_test::common_test_pins!(peripherals); + let (pin, pin2) = (AnyPin::from(pins.1), AnyPin::from(pins.0)); Context { rmt: peripherals.RMT, pin, + pin2, } } @@ -1229,4 +1233,38 @@ mod tests { "tx with loopcount 0 did not complete immediately" ); } + + // This test is timing dependent and tests ESP32-specific behavior + #[cfg(esp32)] + #[test] + async fn rmt_check_regession_4697(mut ctx: Context) { + let rmt = Rmt::new(ctx.rmt.reborrow(), FREQ).unwrap().into_async(); + + let (rx, tx) = (ctx.pin, ctx.pin2); + let mut tx = Output::new(tx, Level::Low, Default::default()); + + let mut rx_channel = rx_channel_creator!(rmt) + .configure_rx( + &RxChannelConfig::default() + .with_clk_divider(255) + .with_idle_threshold(10000), + ) + .unwrap() + .with_pin(rx); + + embassy_futures::join::join( + async { + let mut data = [PulseCode::default(); 10]; + for _ in 0..3 { + assert!(rx_channel.receive(&mut data).await.is_err()); + } + }, + async { + for _ in 0..1000 { + tx.toggle(); + } + }, + ) + .await; + } } From 3548e67977460366a92357324826028e466aa386 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Bj=C3=B6rn=20Quentin?= Date: Fri, 10 Jul 2026 16:17:42 +0200 Subject: [PATCH 3/6] Explain --- esp-hal/src/rmt/reader.rs | 5 +++++ 1 file changed, 5 insertions(+) diff --git a/esp-hal/src/rmt/reader.rs b/esp-hal/src/rmt/reader.rs index c4042d6ac46..bbe03384f74 100644 --- a/esp-hal/src/rmt/reader.rs +++ b/esp-hal/src/rmt/reader.rs @@ -39,6 +39,11 @@ impl RmtReader { // // If `final_` is set, read a full buffer length, potentially wrapping around. Otherwise, fetch // half the buffer's length. + // + // The return value is only valuable if `final_` is set. + // In that case `true` indicates that the read completed successfully, while `false` indicates that + // the read was incomplete and an error occurred. + // This was only observed on ESP32 (and probably ESP32-S2) - other chips indicated an error in that case. #[cfg_attr(place_rmt_driver_in_ram, ram)] pub(super) fn read( &mut self, From 3121180ec24ad3e0a44cda83b3147b92c73a7677 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Bj=C3=B6rn=20Quentin?= Date: Fri, 10 Jul 2026 16:34:24 +0200 Subject: [PATCH 4/6] fmt --- esp-hal/src/rmt/reader.rs | 7 ++++--- 1 file changed, 4 insertions(+), 3 deletions(-) diff --git a/esp-hal/src/rmt/reader.rs b/esp-hal/src/rmt/reader.rs index bbe03384f74..36c822091b3 100644 --- a/esp-hal/src/rmt/reader.rs +++ b/esp-hal/src/rmt/reader.rs @@ -41,9 +41,10 @@ impl RmtReader { // half the buffer's length. // // The return value is only valuable if `final_` is set. - // In that case `true` indicates that the read completed successfully, while `false` indicates that - // the read was incomplete and an error occurred. - // This was only observed on ESP32 (and probably ESP32-S2) - other chips indicated an error in that case. + // In that case `true` indicates that the read completed successfully, while `false` indicates + // that the read was incomplete and an error occurred. + // This was only observed on ESP32 (and probably ESP32-S2) - other chips indicated an error in + // that case. #[cfg_attr(place_rmt_driver_in_ram, ram)] pub(super) fn read( &mut self, From 60e4d5e428040f5daf5ac279d26675b848c68213 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Bj=C3=B6rn=20Quentin?= Date: Fri, 10 Jul 2026 17:06:48 +0200 Subject: [PATCH 5/6] More comments --- hil-test/src/bin/rmt.rs | 11 +++++++++++ 1 file changed, 11 insertions(+) diff --git a/hil-test/src/bin/rmt.rs b/hil-test/src/bin/rmt.rs index ca5d3314132..5c9c03cec86 100644 --- a/hil-test/src/bin/rmt.rs +++ b/hil-test/src/bin/rmt.rs @@ -1234,6 +1234,8 @@ mod tests { ); } + // Regression test for esp-rs/esp-hal#4697 + // // This test is timing dependent and tests ESP32-specific behavior #[cfg(esp32)] #[test] @@ -1252,6 +1254,15 @@ mod tests { .unwrap() .with_pin(rx); + // This suspiciously looking code is to drive the hardware into the state that caused the + // originally failed assertion. We want the async part to go into the first receive + // phase - then the second future will start "spamming" RMT with pulses. + // While not really explainable from any documentation, this turned out to drive the + // hardware into an unexpected state. + // + // With the previous assert in place, interestingly the first two receive calls failed as + // expected but then we ran into the issue where the assert failed instead of seeing + // the error bit set. embassy_futures::join::join( async { let mut data = [PulseCode::default(); 10]; From 1ea8f83f35d5c922dbbca4dba7d20e09894eb177 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Bj=C3=B6rn=20Quentin?= Date: Fri, 10 Jul 2026 17:45:14 +0200 Subject: [PATCH 6/6] SCK Spam --- hil-test/src/bin/rmt.rs | 27 ++++++++++++++++++--------- 1 file changed, 18 insertions(+), 9 deletions(-) diff --git a/hil-test/src/bin/rmt.rs b/hil-test/src/bin/rmt.rs index 5c9c03cec86..9b885d6e9dc 100644 --- a/hil-test/src/bin/rmt.rs +++ b/hil-test/src/bin/rmt.rs @@ -506,7 +506,6 @@ impl Context { #[embedded_test::tests(default_timeout = 1, executor = hil_test::Executor::new())] mod tests { - use esp_hal::gpio::Output; #[allow(unused_imports)] use hil_test::{assert, assert_eq}; @@ -1236,14 +1235,14 @@ mod tests { // Regression test for esp-rs/esp-hal#4697 // - // This test is timing dependent and tests ESP32-specific behavior + // This tests ESP32-specific (mis-)behavior #[cfg(esp32)] #[test] async fn rmt_check_regession_4697(mut ctx: Context) { let rmt = Rmt::new(ctx.rmt.reborrow(), FREQ).unwrap().into_async(); let (rx, tx) = (ctx.pin, ctx.pin2); - let mut tx = Output::new(tx, Level::Low, Default::default()); + // let mut tx = Output::new(tx, Level::Low, Default::default()); let mut rx_channel = rx_channel_creator!(rmt) .configure_rx( @@ -1254,9 +1253,21 @@ mod tests { .unwrap() .with_pin(rx); - // This suspiciously looking code is to drive the hardware into the state that caused the - // originally failed assertion. We want the async part to go into the first receive - // phase - then the second future will start "spamming" RMT with pulses. + let mut spi = esp_hal::spi::master::Spi::new( + unsafe { esp_hal::peripherals::SPI2::steal() }, + Default::default(), + ) + .unwrap() + .with_sck(tx) + .into_async(); + + let mut buf = [0u8; 1000]; + + // This code is to drive the hardware into the state that caused the + // originally failed assertion. + // + // We are "spamming" RMT with pulses by feeding it the SPI SCK. + // // While not really explainable from any documentation, this turned out to drive the // hardware into an unexpected state. // @@ -1271,9 +1282,7 @@ mod tests { } }, async { - for _ in 0..1000 { - tx.toggle(); - } + spi.transfer_in_place_async(&mut buf).await.unwrap(); }, ) .await;