Prefer reporting an error instead of triggering an assert

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)]

esp-hal/src/rmt/reader.rs

@@ -39,15 +39,21 @@ 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,
         data: &mut &mut [PulseCode],
         raw: DynChannelAccess<Rx>,
         final_: bool,
-    ) {
+    ) -> bool {
         if self.state != ReaderState::Active {
-            return;
+            return true;
         }
 
         let ram_start = raw.channel_ram_start();
@@ -62,8 +68,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 +79,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 +146,7 @@ impl RmtReader {
         }
 
         debug_assert!(self.offset == 0 || self.offset as usize == memsize / 2);
+
+        true
     }
 }

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 {

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,49 @@ mod tests {
             "tx with loopcount 0 did not complete immediately"
         );
     }
+
+    // Regression test for esp-rs/esp-hal#4697
+    //
+    // 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);
+
+        // 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];
+                for _ in 0..3 {
+                    assert!(rx_channel.receive(&mut data).await.is_err());
+                }
+            },
+            async {
+                for _ in 0..1000 {
+                    tx.toggle();
+                }
+            },
+        )
+        .await;
+    }
 }