diff --git a/esp-hal/src/rmt.rs b/esp-hal/src/rmt.rs index 67c3ce5d04e..6809932b5cb 100644 --- a/esp-hal/src/rmt.rs +++ b/esp-hal/src/rmt.rs @@ -1000,6 +1000,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, @@ -1019,63 +1023,78 @@ 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(); - - while !self.remaining_data.is_empty() { - // 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 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)); - } - } + + let status = raw.get_tx_status(); + if status == Some(Event::Threshold) { 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 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() + ); } + } - // 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. + #[doc(hidden)] + #[cfg_attr(place_rmt_driver_in_ram, ram)] + 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, ram)] + 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 { - if raw.is_error() { - return Err((Error::TransmissionError, self.channel)); - } - - if raw.is_tx_done() { - break; + match self.poll_internal() { + 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. + break Err((Error::TransmissionError, self.channel)); + } else { + break Ok(self.channel); + } + } + _ => continue, } } - - Ok(self.channel) } } @@ -1094,16 +1113,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. @@ -1122,16 +1137,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 @@ -1321,33 +1332,57 @@ 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. + #[doc(hidden)] + #[cfg_attr(place_rmt_driver_in_ram, ram)] + 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, ram)] + 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 } } @@ -1400,16 +1435,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, } } } @@ -1443,13 +1478,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 } } @@ -1465,15 +1494,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, } } } @@ -1507,11 +1537,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(); @@ -1521,9 +1549,9 @@ where for (idx, entry) in data.iter_mut().take(len).enumerate() { *entry = unsafe { ptr.add(idx).read_volatile().into() }; } - - Ok(()) } + + result } } @@ -1578,8 +1606,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 { @@ -1609,9 +1635,9 @@ pub trait TxChannelInternal: ChannelInternal { fn start_tx(&self); - fn is_tx_done(&self) -> bool; - - fn is_tx_threshold_set(&self) -> bool; + // 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 reset_tx_threshold_set(&self); @@ -1677,7 +1703,9 @@ pub trait RxChannelInternal: ChannelInternal { fn start_rx(&self); - fn is_rx_done(&self) -> bool; + // 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 start_receive(&self) { self.clear_rx_interrupts(); @@ -1862,19 +1890,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 @@ -1962,15 +1977,20 @@ mod chip_specific { } #[inline] - fn is_tx_done(&self) -> bool { + fn get_tx_status(&self) -> Option { let rmt = crate::peripherals::RMT::regs(); - rmt.int_raw().read().ch_tx_end(self.channel()).bit() - } + let reg = rmt.int_raw().read(); + let ch = self.channel(); - #[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() + 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] @@ -2084,10 +2104,20 @@ mod chip_specific { } #[inline] - fn is_rx_done(&self) -> bool { + fn get_rx_status(&self) -> Option { let rmt = crate::peripherals::RMT::regs(); + let reg = rmt.int_raw().read(); let ch_idx = ch_idx(self); - rmt.int_raw().read().ch_rx_end(ch_idx).bit() + + 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] @@ -2218,12 +2248,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 @@ -2316,15 +2340,20 @@ mod chip_specific { } #[inline] - fn is_tx_done(&self) -> bool { + fn get_tx_status(&self) -> Option { let rmt = crate::peripherals::RMT::regs(); - rmt.int_raw().read().ch_tx_end(self.channel()).bit() - } + let reg = rmt.int_raw().read(); + let ch = self.channel(); - #[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() + 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] @@ -2433,9 +2462,18 @@ mod chip_specific { } #[inline] - fn is_rx_done(&self) -> bool { + fn get_rx_status(&self) -> Option { let rmt = crate::peripherals::RMT::regs(); - rmt.int_raw().read().ch_rx_end(self.channel()).bit() + 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] diff --git a/hil-test/tests/rmt.rs b/hil-test/tests/rmt.rs index 7fa8d5e3bbd..9f3c1725bdb 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]