From 68637ebdedd6f5f68e4776d2cd12e72cc1e5b74e Mon Sep 17 00:00:00 2001 From: wisp3rwind <17089248+wisp3rwind@users.noreply.github.com> Date: Wed, 18 Jun 2025 17:07:52 +0200 Subject: [PATCH 1/8] RMT: add get_tx_status, get_rx_status methods Unused right now; these methods allow to look at all flags without several volatile reads (in contrast to calling is_tx_done/is_rx_done, is_tx_threshold_set, is_error individually). `match`ing on their result leads to very idiomatic code, and also result in blocking and async code that is very similar. The #[inline] attribute might not be required, but better be sure: We really don't want the Option to actually be constructed, but rather that the compiler completely optimized it away in favor of direct bit tests in the calling function. --- esp-hal/src/rmt.rs | 74 ++++++++++++++++++++++++++++++++++++++++++++++ 1 file changed, 74 insertions(+) diff --git a/esp-hal/src/rmt.rs b/esp-hal/src/rmt.rs index 6b40da89af..aae234146b 100644 --- a/esp-hal/src/rmt.rs +++ b/esp-hal/src/rmt.rs @@ -1588,6 +1588,10 @@ pub trait TxChannelInternal: ChannelInternal { fn start_tx(&self); + // Return the first flag that is set of, in order of decreasing priority, + // Event::Error, Event::End, Event::Threshold + fn get_tx_status(&self) -> Option; + fn is_tx_done(&self) -> bool; fn is_tx_threshold_set(&self) -> bool; @@ -1656,6 +1660,10 @@ pub trait RxChannelInternal: ChannelInternal { fn start_rx(&self); + // Return the first flag that is set of, in order of decreasing priority, + // Event::Error, Event::End, Event::Threshold + fn get_rx_status(&self) -> Option; + fn is_rx_done(&self) -> bool; fn start_receive(&self) { @@ -1940,6 +1948,23 @@ mod chip_specific { self.update(); } + #[inline] + fn get_tx_status(&self) -> Option { + let rmt = crate::peripherals::RMT::regs(); + let reg = rmt.int_raw().read(); + let ch = self.channel(); + + if reg.ch_tx_end(ch).bit() { + Some(Event::End) + } else if reg.ch_tx_err(ch).bit() { + Some(Event::Error) + } else if reg.ch_tx_thr_event(ch).bit() { + Some(Event::Threshold) + } else { + None + } + } + #[inline] fn is_tx_done(&self) -> bool { let rmt = crate::peripherals::RMT::regs(); @@ -2062,6 +2087,23 @@ mod chip_specific { }); } + #[inline] + fn get_rx_status(&self) -> Option { + let rmt = crate::peripherals::RMT::regs(); + let reg = rmt.int_raw().read(); + let ch_idx = ch_idx(self); + + if reg.ch_rx_end(ch_idx).bit() { + Some(Event::End) + } else if reg.ch_rx_err(ch_idx).bit() { + Some(Event::Error) + } else if reg.ch_rx_thr_event(ch_idx).bit() { + Some(Event::Threshold) + } else { + None + } + } + #[inline] fn is_rx_done(&self) -> bool { let rmt = crate::peripherals::RMT::regs(); @@ -2294,6 +2336,23 @@ mod chip_specific { }); } + #[inline] + fn get_tx_status(&self) -> Option { + let rmt = crate::peripherals::RMT::regs(); + let reg = rmt.int_raw().read(); + let ch = self.channel(); + + if reg.ch_tx_end(ch).bit() { + Some(Event::End) + } else if reg.ch_err(ch).bit() { + Some(Event::Error) + } else if reg.ch_tx_thr_event(ch).bit() { + Some(Event::Threshold) + } else { + None + } + } + #[inline] fn is_tx_done(&self) -> bool { let rmt = crate::peripherals::RMT::regs(); @@ -2411,6 +2470,21 @@ mod chip_specific { }); } + #[inline] + fn get_rx_status(&self) -> Option { + let rmt = crate::peripherals::RMT::regs(); + let reg = rmt.int_raw().read(); + let ch = self.channel(); + + if reg.ch_rx_end(ch).bit() { + Some(Event::End) + } else if reg.ch_err(ch).bit() { + Some(Event::Error) + } else { + None + } + } + #[inline] fn is_rx_done(&self) -> bool { let rmt = crate::peripherals::RMT::regs(); From 0b71fb4f9b11cb95bc53d7b372d2fad90f8ff4a0 Mon Sep 17 00:00:00 2001 From: wisp3rwind <17089248+wisp3rwind@users.noreply.github.com> Date: Wed, 18 Jun 2025 17:10:04 +0200 Subject: [PATCH 2/8] RMT: optimize RmtTxFuture to avoid repeated register reads --- esp-hal/src/rmt.rs | 18 ++++++------------ 1 file changed, 6 insertions(+), 12 deletions(-) diff --git a/esp-hal/src/rmt.rs b/esp-hal/src/rmt.rs index aae234146b..abbf66df6f 100644 --- a/esp-hal/src/rmt.rs +++ b/esp-hal/src/rmt.rs @@ -1379,16 +1379,16 @@ impl core::future::Future for RmtTxFuture where Raw: TxChannelInternal, { - type Output = (); + type Output = Result<(), Error>; #[cfg_attr(place_rmt_driver_in_ram, ram)] fn poll(self: Pin<&mut Self>, ctx: &mut Context<'_>) -> Poll { WAKER[self.raw.channel() as usize].register(ctx.waker()); - if self.raw.is_error() || self.raw.is_tx_done() { - Poll::Ready(()) - } else { - Poll::Pending + match self.raw.get_tx_status() { + Some(Event::Error) => Poll::Ready(Err(Error::TransmissionError)), + Some(Event::End) => Poll::Ready(Ok(())), + _ => Poll::Pending, } } } @@ -1422,13 +1422,7 @@ where raw.listen_tx_interrupt(Event::End | Event::Error); raw.start_send(data, false, 0)?; - (RmtTxFuture { raw }).await; - - if raw.is_error() { - Err(Error::TransmissionError) - } else { - Ok(()) - } + (RmtTxFuture { raw }).await } } From c999ed3332d71091379d46fd9e210d418eb67597 Mon Sep 17 00:00:00 2001 From: wisp3rwind <17089248+wisp3rwind@users.noreply.github.com> Date: Sat, 28 Jun 2025 17:34:57 +0200 Subject: [PATCH 3/8] RMT: optimize RmtRxFuture to avoid repeated register reads --- esp-hal/src/rmt.rs | 21 ++++++++++----------- 1 file changed, 10 insertions(+), 11 deletions(-) diff --git a/esp-hal/src/rmt.rs b/esp-hal/src/rmt.rs index abbf66df6f..16aceb32d3 100644 --- a/esp-hal/src/rmt.rs +++ b/esp-hal/src/rmt.rs @@ -1438,15 +1438,16 @@ impl core::future::Future for RmtRxFuture where Raw: RxChannelInternal, { - type Output = (); + type Output = Result<(), Error>; #[cfg_attr(place_rmt_driver_in_ram, ram)] fn poll(self: Pin<&mut Self>, ctx: &mut Context<'_>) -> Poll { WAKER[self.raw.channel() as usize].register(ctx.waker()); - if self.raw.is_error() || self.raw.is_rx_done() { - Poll::Ready(()) - } else { - Poll::Pending + + match self.raw.get_rx_status() { + Some(Event::Error) => Poll::Ready(Err(Error::ReceiverError)), + Some(Event::End) => Poll::Ready(Ok(())), + _ => Poll::Pending, } } } @@ -1480,11 +1481,9 @@ where raw.listen_rx_interrupt(Event::End | Event::Error); raw.start_receive(); - (RmtRxFuture { raw }).await; + let result = (RmtRxFuture { raw }).await; - if raw.is_error() { - Err(Error::ReceiverError) - } else { + if result.is_ok() { raw.stop_rx(); raw.clear_rx_interrupts(); raw.update(); @@ -1494,9 +1493,9 @@ where for (idx, entry) in data.iter_mut().take(len).enumerate() { *entry = unsafe { ptr.add(idx).read_volatile().into() }; } - - Ok(()) } + + result } } From acad11fb9282df65bf21c59cfa67d989d5f47a46 Mon Sep 17 00:00:00 2001 From: wisp3rwind <17089248+wisp3rwind@users.noreply.github.com> Date: Wed, 18 Jun 2025 17:12:03 +0200 Subject: [PATCH 4/8] RMT: Optimize ContinuousTxTransaction by avoiding repeated register reads --- esp-hal/src/rmt.rs | 24 ++++++++---------------- 1 file changed, 8 insertions(+), 16 deletions(-) diff --git a/esp-hal/src/rmt.rs b/esp-hal/src/rmt.rs index 16aceb32d3..9d47fdefea 100644 --- a/esp-hal/src/rmt.rs +++ b/esp-hal/src/rmt.rs @@ -1073,16 +1073,12 @@ impl ContinuousTxTransaction { raw.update(); loop { - if raw.is_error() { - return Err((Error::TransmissionError, self.channel)); - } - - if raw.is_tx_done() { - break; + match raw.get_tx_status() { + Some(Event::Error) => break Err((Error::TransmissionError, self.channel)), + Some(Event::End) => break Ok(self.channel), + _ => continue, } } - - Ok(self.channel) } /// Stop transaction as soon as possible. @@ -1101,16 +1097,12 @@ impl ContinuousTxTransaction { } loop { - if raw.is_error() { - return Err((Error::TransmissionError, self.channel)); - } - - if raw.is_tx_done() { - break; + match raw.get_tx_status() { + Some(Event::Error) => break Err((Error::TransmissionError, self.channel)), + Some(Event::End) => break Ok(self.channel), + _ => continue, } } - - Ok(self.channel) } /// Check if the `loopcount` interrupt bit is set From fc903814a4ad0f754d9d63f21241fde67760dddd Mon Sep 17 00:00:00 2001 From: wisp3rwind <17089248+wisp3rwind@users.noreply.github.com> Date: Sat, 28 Jun 2025 16:18:47 +0200 Subject: [PATCH 5/8] RMT: Optimize/simplify SingleShotTxTransaction (1/2) by merging two loops into one --- esp-hal/src/rmt.rs | 64 +++++++++++++++++++++------------------------- 1 file changed, 29 insertions(+), 35 deletions(-) diff --git a/esp-hal/src/rmt.rs b/esp-hal/src/rmt.rs index 9d47fdefea..d99d8a5a14 100644 --- a/esp-hal/src/rmt.rs +++ b/esp-hal/src/rmt.rs @@ -1004,13 +1004,17 @@ where let raw = self.channel.raw; let memsize = raw.memsize().codes(); - while !self.remaining_data.is_empty() { + loop { // wait for TX-THR while !raw.is_tx_threshold_set() { if raw.is_tx_done() { - // Unexpectedly done, even though we have data left: For example, this could - // happen if there is a stop code inside the data and not just at the end. - return Err((Error::TransmissionError, self.channel)); + if !self.remaining_data.is_empty() { + // Unexpectedly done, even though we have data left: For example, this could + // happen if there is a stop code inside the data and not just at the end. + return Err((Error::TransmissionError, self.channel)); + } else { + return Ok(self.channel); + } } if raw.is_error() { // Not sure that this can happen? In any case, be sure that we don't lock up @@ -1020,41 +1024,31 @@ where } raw.reset_tx_threshold_set(); - // re-fill TX RAM - let ptr = unsafe { raw.channel_ram_start().add(self.ram_index) }; - let count = self.remaining_data.len().min(memsize / 2); - let (chunk, remaining) = self.remaining_data.split_at(count); - for (idx, entry) in chunk.iter().enumerate() { - unsafe { - ptr.add(idx).write_volatile(*entry); + if !self.remaining_data.is_empty() { + // re-fill TX RAM + let ptr = unsafe { raw.channel_ram_start().add(self.ram_index) }; + let count = self.remaining_data.len().min(memsize / 2); + let (chunk, remaining) = self.remaining_data.split_at(count); + for (idx, entry) in chunk.iter().enumerate() { + unsafe { + ptr.add(idx).write_volatile(*entry); + } } - } - - // If count == memsize / 2 codes were written, update ram_index as - // - 0 -> memsize / 2 - // - memsize / 2 -> 0 - // Otherwise, for count < memsize / 2, the new position is invalid but the new - // slice is empty and we won't use ram_index again. - self.ram_index = memsize / 2 - self.ram_index; - self.remaining_data = remaining; - debug_assert!( - self.ram_index == 0 - || self.ram_index == memsize / 2 - || self.remaining_data.is_empty() - ); - } - - loop { - if raw.is_error() { - return Err((Error::TransmissionError, self.channel)); - } - if raw.is_tx_done() { - break; + // If count == memsize / 2 codes were written, update ram_index as + // - 0 -> memsize / 2 + // - memsize / 2 -> 0 + // Otherwise, for count < memsize / 2, the new position is invalid but the new + // slice is empty and we won't use ram_index again. + self.ram_index = memsize / 2 - self.ram_index; + self.remaining_data = remaining; + debug_assert!( + self.ram_index == 0 + || self.ram_index == memsize / 2 + || self.remaining_data.is_empty() + ); } } - - Ok(self.channel) } } From 80da9b138f64b01da25249af14e04cd02e9ed6be Mon Sep 17 00:00:00 2001 From: wisp3rwind <17089248+wisp3rwind@users.noreply.github.com> Date: Wed, 18 Jun 2025 17:13:42 +0200 Subject: [PATCH 6/8] RMT: Optimize/simplify SingleShotTxTransaction (2/2) by avoiding repeated register reads --- esp-hal/src/rmt.rs | 68 +++++++++++++++++++++++----------------------- 1 file changed, 34 insertions(+), 34 deletions(-) diff --git a/esp-hal/src/rmt.rs b/esp-hal/src/rmt.rs index d99d8a5a14..a517fe58f7 100644 --- a/esp-hal/src/rmt.rs +++ b/esp-hal/src/rmt.rs @@ -1005,48 +1005,48 @@ where let memsize = raw.memsize().codes(); loop { - // wait for TX-THR - while !raw.is_tx_threshold_set() { - if raw.is_tx_done() { + // Not sure that all the error cases below can happen. However, it's best to + // handle them to be sure that we don't lock up here in case they can happen. + match raw.get_tx_status() { + Some(Event::Error) => break Err((Error::TransmissionError, self.channel)), + Some(Event::End) => { if !self.remaining_data.is_empty() { // Unexpectedly done, even though we have data left: For example, this could // happen if there is a stop code inside the data and not just at the end. - return Err((Error::TransmissionError, self.channel)); + break Err((Error::TransmissionError, self.channel)); } else { - return Ok(self.channel); + break Ok(self.channel); } } - if raw.is_error() { - // Not sure that this can happen? In any case, be sure that we don't lock up - // here in case it can. - return Err((Error::TransmissionError, self.channel)); - } - } - raw.reset_tx_threshold_set(); - - if !self.remaining_data.is_empty() { - // re-fill TX RAM - let ptr = unsafe { raw.channel_ram_start().add(self.ram_index) }; - let count = self.remaining_data.len().min(memsize / 2); - let (chunk, remaining) = self.remaining_data.split_at(count); - for (idx, entry) in chunk.iter().enumerate() { - unsafe { - ptr.add(idx).write_volatile(*entry); + Some(Event::Threshold) => { + raw.reset_tx_threshold_set(); + + if !self.remaining_data.is_empty() { + // re-fill TX RAM + let ptr = unsafe { raw.channel_ram_start().add(self.ram_index) }; + let count = self.remaining_data.len().min(memsize / 2); + let (chunk, remaining) = self.remaining_data.split_at(count); + for (idx, entry) in chunk.iter().enumerate() { + unsafe { + ptr.add(idx).write_volatile(*entry); + } + } + + // If count == memsize / 2 codes were written, update ram_index as + // - 0 -> memsize / 2 + // - memsize / 2 -> 0 + // Otherwise, for count < memsize / 2, the new position is invalid but the new + // slice is empty and we won't use ram_index again. + self.ram_index = memsize / 2 - self.ram_index; + self.remaining_data = remaining; + debug_assert!( + self.ram_index == 0 + || self.ram_index == memsize / 2 + || self.remaining_data.is_empty() + ); } } - - // If count == memsize / 2 codes were written, update ram_index as - // - 0 -> memsize / 2 - // - memsize / 2 -> 0 - // Otherwise, for count < memsize / 2, the new position is invalid but the new - // slice is empty and we won't use ram_index again. - self.ram_index = memsize / 2 - self.ram_index; - self.remaining_data = remaining; - debug_assert!( - self.ram_index == 0 - || self.ram_index == memsize / 2 - || self.remaining_data.is_empty() - ); + _ => continue, } } } From abd310daf1199f4543d69b9c4174775f1f51e431 Mon Sep 17 00:00:00 2001 From: wisp3rwind <17089248+wisp3rwind@users.noreply.github.com> Date: Tue, 20 May 2025 22:37:56 +0200 Subject: [PATCH 7/8] RMT: add poll() method to blocking transactions This allows properly interleaving several such transactions (as the HIL loopback tests do, in fact) or other work. Regarding the implementation of the place_rmt_driver_in_ram feature for these methods, note that `#[ram]` implies `[inline(never)]`. Thus, this uses `#[inline(always)]` for `poll` and `wait` in that case, which are rather simple methods. If putting them in ram actually matters, the calling user code should probably be placed in ram as well, and then, forced inlining of both methods should have the desired effect. `poll_internal` in turn - contains more code and is called from `poll` and `wait` -> it seems sensible that it is not inlined to reduce code size. - includes the hot loop that copies to the hardware buffer -> should be forced to ram so `#[ram]` makes sense. Note that none of this was benchmarked. --- esp-hal/CHANGELOG.md | 1 + esp-hal/src/rmt.rs | 149 +++++++++++++++++++++++++++--------------- hil-test/tests/rmt.rs | 32 ++++----- 3 files changed, 116 insertions(+), 66 deletions(-) diff --git a/esp-hal/CHANGELOG.md b/esp-hal/CHANGELOG.md index 0f101f426e..57fe0433ea 100644 --- a/esp-hal/CHANGELOG.md +++ b/esp-hal/CHANGELOG.md @@ -19,6 +19,7 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0 - `aes::Key` variants have been renamed from bytes to bits (e.g. `Key16 -> Key128`) (#3845) - `aes::Mode` has been replaced by `Operation`. The key length is now solely determined by the key. (#3882) - `Aes::process` has been split into `Aes::encrypt` and `Aes::decrypt` (#3882) +- Blocking RMT transactions can now be `poll`ed without blocking, returning whether they have completed. (#3716) ### Fixed diff --git a/esp-hal/src/rmt.rs b/esp-hal/src/rmt.rs index a517fe58f7..1a72724c71 100644 --- a/esp-hal/src/rmt.rs +++ b/esp-hal/src/rmt.rs @@ -979,6 +979,10 @@ where } /// An in-progress transaction for a single shot TX transaction. +/// +/// If the data size exceeds the size of the internal buffer, `.poll()` or +/// `.wait()` needs to be called before the entire buffer has been sent to avoid +/// underruns. pub struct SingleShotTxTransaction<'a, Raw> where Raw: TxChannelInternal, @@ -998,16 +1002,64 @@ impl SingleShotTxTransaction<'_, Raw> where Raw: TxChannelInternal, { - /// Wait for the transaction to complete #[cfg_attr(place_rmt_driver_in_ram, ram)] - pub fn wait(mut self) -> Result, (Error, Channel)> { + fn poll_internal(&mut self) -> Option { let raw = self.channel.raw; - let memsize = raw.memsize().codes(); + let status = raw.get_tx_status(); + if status == Some(Event::Threshold) { + raw.reset_tx_threshold_set(); + + if !self.remaining_data.is_empty() { + // re-fill TX RAM + let memsize = raw.memsize().codes(); + let ptr = unsafe { raw.channel_ram_start().add(self.ram_index) }; + let count = self.remaining_data.len().min(memsize / 2); + let (chunk, remaining) = self.remaining_data.split_at(count); + for (idx, entry) in chunk.iter().enumerate() { + unsafe { + ptr.add(idx).write_volatile(*entry); + } + } + + // If count == memsize / 2 codes were written, update ram_index as + // - 0 -> memsize / 2 + // - memsize / 2 -> 0 + // Otherwise, for count < memsize / 2, the new position is invalid but the new + // slice is empty and we won't use ram_index again. + self.ram_index = memsize / 2 - self.ram_index; + self.remaining_data = remaining; + debug_assert!( + self.ram_index == 0 + || self.ram_index == memsize / 2 + || self.remaining_data.is_empty() + ); + } + } + + status + } + + /// Check transmission status and write new data to the hardware if + /// necessary. + /// + /// Returns whether transmission has ended (whether successfully or with an + /// error). In that case, a subsequent call to `wait()` returns immediately. + #[cfg_attr(place_rmt_driver_in_ram, inline(always))] + pub fn poll(&mut self) -> bool { + match self.poll_internal() { + Some(Event::Error | Event::End) => true, + Some(Event::Threshold) | None => false, + } + } + + /// Wait for the transaction to complete + #[cfg_attr(place_rmt_driver_in_ram, inline(always))] + pub fn wait(mut self) -> Result, (Error, Channel)> { + // Not sure that all the error cases below can happen. However, it's best to + // handle them to be sure that we don't lock up here in case they can happen. loop { - // Not sure that all the error cases below can happen. However, it's best to - // handle them to be sure that we don't lock up here in case they can happen. - match raw.get_tx_status() { + match self.poll_internal() { Some(Event::Error) => break Err((Error::TransmissionError, self.channel)), Some(Event::End) => { if !self.remaining_data.is_empty() { @@ -1018,34 +1070,6 @@ where break Ok(self.channel); } } - Some(Event::Threshold) => { - raw.reset_tx_threshold_set(); - - if !self.remaining_data.is_empty() { - // re-fill TX RAM - let ptr = unsafe { raw.channel_ram_start().add(self.ram_index) }; - let count = self.remaining_data.len().min(memsize / 2); - let (chunk, remaining) = self.remaining_data.split_at(count); - for (idx, entry) in chunk.iter().enumerate() { - unsafe { - ptr.add(idx).write_volatile(*entry); - } - } - - // If count == memsize / 2 codes were written, update ram_index as - // - 0 -> memsize / 2 - // - memsize / 2 -> 0 - // Otherwise, for count < memsize / 2, the new position is invalid but the new - // slice is empty and we won't use ram_index again. - self.ram_index = memsize / 2 - self.ram_index; - self.remaining_data = remaining; - debug_assert!( - self.ram_index == 0 - || self.ram_index == memsize / 2 - || self.remaining_data.is_empty() - ); - } - } _ => continue, } } @@ -1286,33 +1310,56 @@ pub struct RxTransaction<'a, Raw: RxChannelInternal> { } impl RxTransaction<'_, Raw> { - /// Wait for the transaction to complete #[cfg_attr(place_rmt_driver_in_ram, ram)] - pub fn wait(self) -> Result, (Error, Channel)> { + fn poll_internal(&mut self) -> Option { let raw = self.channel.raw; - loop { - if raw.is_error() { - return Err((Error::ReceiverError, self.channel)); - } + let status = raw.get_rx_status(); + if status == Some(Event::End) { + // Do not clear the interrupt flags here: Subsequent calls of wait() must + // be able to observe them if this is currently called via poll() + raw.stop_rx(); + raw.update(); - if raw.is_rx_done() { - break; + let ptr = raw.channel_ram_start(); + // SAFETY: RxChannel.receive() verifies that the length of self.data does not + // exceed the channel RAM size. + for (idx, entry) in self.data.iter_mut().enumerate() { + *entry = unsafe { ptr.add(idx).read_volatile() }; } } - raw.stop_rx(); - raw.clear_rx_interrupts(); - raw.update(); + status + } - let ptr = raw.channel_ram_start(); - // SAFETY: RxChannel.receive() verifies that the length of self.data does not - // exceed the channel RAM size. - for (idx, entry) in self.data.iter_mut().enumerate() { - *entry = unsafe { ptr.add(idx).read_volatile() }; + /// Check receive status + /// + /// Returns whether reception has ended (whether successfully or with an + /// error). In that case, a subsequent call to `wait()` returns immediately. + #[cfg_attr(place_rmt_driver_in_ram, inline(always))] + pub fn poll(&mut self) -> bool { + match self.poll_internal() { + Some(Event::Error | Event::End) => true, + Some(Event::Threshold) | None => false, } + } - Ok(self.channel) + /// Wait for the transaction to complete + #[cfg_attr(place_rmt_driver_in_ram, inline(always))] + pub fn wait(mut self) -> Result, (Error, Channel)> { + let raw = self.channel.raw; + + let result = loop { + match self.poll_internal() { + Some(Event::Error) => break Err((Error::ReceiverError, self.channel)), + Some(Event::End) => break Ok(self.channel), + _ => continue, + } + }; + + raw.clear_rx_interrupts(); + + result } } diff --git a/hil-test/tests/rmt.rs b/hil-test/tests/rmt.rs index 7fa8d5e3bb..9f3c1725bd 100644 --- a/hil-test/tests/rmt.rs +++ b/hil-test/tests/rmt.rs @@ -79,7 +79,7 @@ fn generate_tx_data(write_end_marker: bool) -> [u32; TX_LEN // Run a test where some data is sent from one channel and looped back to // another one for receive, and verify that the data matches. -fn do_rmt_loopback(tx_memsize: u8, rx_memsize: u8, wait_tx_first: bool) { +fn do_rmt_loopback(tx_memsize: u8, rx_memsize: u8) { use esp_hal::rmt::{RxChannel, TxChannel}; let peripherals = esp_hal::init(esp_hal::Config::default()); @@ -96,17 +96,20 @@ fn do_rmt_loopback(tx_memsize: u8, rx_memsize: u8, wait_tx_ let tx_data: [_; TX_LEN] = generate_tx_data(true); let mut rcv_data: [u32; TX_LEN] = [PulseCode::empty(); TX_LEN]; - let rx_transaction = rx_channel.receive(&mut rcv_data).unwrap(); - let tx_transaction = tx_channel.transmit(&tx_data).unwrap(); + let mut rx_transaction = rx_channel.receive(&mut rcv_data).unwrap(); + let mut tx_transaction = tx_channel.transmit(&tx_data).unwrap(); - if wait_tx_first { - tx_transaction.wait().unwrap(); - rx_transaction.wait().unwrap(); - } else { - rx_transaction.wait().unwrap(); - tx_transaction.wait().unwrap(); + loop { + let tx_done = tx_transaction.poll(); + let rx_done = rx_transaction.poll(); + if tx_done && rx_done { + break; + } } + tx_transaction.wait().unwrap(); + rx_transaction.wait().unwrap(); + // the last two pulse-codes are the ones which wait for the timeout so // they can't be equal assert_eq!(&tx_data[..TX_LEN - 2], &rcv_data[..TX_LEN - 2]); @@ -180,7 +183,7 @@ mod tests { #[test] fn rmt_loopback_simple() { // 20 codes fit a single RAM block - do_rmt_loopback::<20>(1, 1, false); + do_rmt_loopback::<20>(1, 1); } #[test] @@ -191,7 +194,7 @@ mod tests { #[test] fn rmt_loopback_extended_ram() { // 80 codes require two RAM blocks - do_rmt_loopback::<80>(2, 2, false); + do_rmt_loopback::<80>(2, 2); } // FIXME: This test currently fails on esp32 with an rmt::Error::ReceiverError, @@ -204,9 +207,8 @@ mod tests { #[test] fn rmt_loopback_tx_wrap() { // 80 codes require two RAM blocks; thus a tx channel with only 1 block requires - // wrapping. We need to .wait() on the tx transaction first to handle - // this. - do_rmt_loopback::<80>(1, 2, true); + // wrapping. + do_rmt_loopback::<80>(1, 2); } // FIXME: This test can't work right now, because wrapping rx is not @@ -216,7 +218,7 @@ mod tests { // fn rmt_loopback_rx_wrap() { // // 80 codes require two RAM blocks; thus an rx channel with only 1 block // // requires wrapping - // do_rmt_loopback<80>(2, 1, false); + // do_rmt_loopback<80>(2, 1); // } #[test] From 428a94eea55690ea056c360018db3c9b33192899 Mon Sep 17 00:00:00 2001 From: wisp3rwind <17089248+wisp3rwind@users.noreply.github.com> Date: Sat, 28 Jun 2025 17:42:21 +0200 Subject: [PATCH 8/8] RMT: Remove unused low-level methods Their use has entirely be replaced by get_tx_status and get_rx_status --- esp-hal/src/rmt.rs | 64 ---------------------------------------------- 1 file changed, 64 deletions(-) diff --git a/esp-hal/src/rmt.rs b/esp-hal/src/rmt.rs index 1a72724c71..857a01417c 100644 --- a/esp-hal/src/rmt.rs +++ b/esp-hal/src/rmt.rs @@ -1583,8 +1583,6 @@ pub trait ChannelInternal: RawChannelAccess { fn set_memsize(&self, value: MemSize); - fn is_error(&self) -> bool; - #[inline] fn channel_ram_start(&self) -> *mut u32 { unsafe { @@ -1618,10 +1616,6 @@ pub trait TxChannelInternal: ChannelInternal { // Event::Error, Event::End, Event::Threshold fn get_tx_status(&self) -> Option; - fn is_tx_done(&self) -> bool; - - fn is_tx_threshold_set(&self) -> bool; - fn reset_tx_threshold_set(&self); fn set_tx_threshold(&self, threshold: u8); @@ -1690,8 +1684,6 @@ pub trait RxChannelInternal: ChannelInternal { // Event::Error, Event::End, Event::Threshold fn get_rx_status(&self) -> Option; - fn is_rx_done(&self) -> bool; - fn start_receive(&self) { self.clear_rx_interrupts(); self.set_rx_wrap_mode(false); @@ -1875,19 +1867,6 @@ mod chip_specific { .modify(|_, w| unsafe { w.mem_size().bits(blocks) }); } } - - #[inline] - fn is_error(&self) -> bool { - let rmt = crate::peripherals::RMT::regs(); - let int_raw = rmt.int_raw().read(); - let ch_idx = ch_idx(self); - - if A::Dir::is_tx() { - int_raw.ch_tx_err(ch_idx).bit() - } else { - int_raw.ch_rx_err(ch_idx).bit() - } - } } impl TxChannelInternal for A @@ -1991,18 +1970,6 @@ mod chip_specific { } } - #[inline] - fn is_tx_done(&self) -> bool { - let rmt = crate::peripherals::RMT::regs(); - rmt.int_raw().read().ch_tx_end(self.channel()).bit() - } - - #[inline] - fn is_tx_threshold_set(&self) -> bool { - let rmt = crate::peripherals::RMT::regs(); - rmt.int_raw().read().ch_tx_thr_event(self.channel()).bit() - } - #[inline] fn reset_tx_threshold_set(&self) { let rmt = crate::peripherals::RMT::regs(); @@ -2130,13 +2097,6 @@ mod chip_specific { } } - #[inline] - fn is_rx_done(&self) -> bool { - let rmt = crate::peripherals::RMT::regs(); - let ch_idx = ch_idx(self); - rmt.int_raw().read().ch_rx_end(ch_idx).bit() - } - #[inline] fn stop_rx(&self) { let rmt = crate::peripherals::RMT::regs(); @@ -2265,12 +2225,6 @@ mod chip_specific { rmt.chconf0(self.channel() as usize) .modify(|_, w| unsafe { w.mem_size().bits(value.blocks()) }); } - - #[inline] - fn is_error(&self) -> bool { - let rmt = crate::peripherals::RMT::regs(); - rmt.int_raw().read().ch_err(self.channel()).bit() - } } impl TxChannelInternal for A @@ -2379,18 +2333,6 @@ mod chip_specific { } } - #[inline] - fn is_tx_done(&self) -> bool { - let rmt = crate::peripherals::RMT::regs(); - rmt.int_raw().read().ch_tx_end(self.channel()).bit() - } - - #[inline] - fn is_tx_threshold_set(&self) -> bool { - let rmt = crate::peripherals::RMT::regs(); - rmt.int_raw().read().ch_tx_thr_event(self.channel()).bit() - } - #[inline] fn reset_tx_threshold_set(&self) { let rmt = crate::peripherals::RMT::regs(); @@ -2511,12 +2453,6 @@ mod chip_specific { } } - #[inline] - fn is_rx_done(&self) -> bool { - let rmt = crate::peripherals::RMT::regs(); - rmt.int_raw().read().ch_rx_end(self.channel()).bit() - } - #[inline] fn stop_rx(&self) { let rmt = crate::peripherals::RMT::regs();