From 2b10caafd488987b6cf9a2cd69820c81d0a0826e Mon Sep 17 00:00:00 2001 From: Alexandros Liarokapis Date: Sun, 15 Sep 2024 18:47:33 +0300 Subject: [PATCH 01/11] stm32: initial support for alternative ringbuffer implementation --- embassy-stm32/Cargo.toml | 2 + embassy-stm32/src/dma/dma_bdma.rs | 4 - embassy-stm32/src/dma/ringbuffer.rs | 668 ------------------ embassy-stm32/src/dma/ringbuffer/mod.rs | 293 ++++++++ embassy-stm32/src/dma/ringbuffer/tests/mod.rs | 165 +++++ .../src/dma/ringbuffer/tests/prop_test/mod.rs | 50 ++ .../dma/ringbuffer/tests/prop_test/reader.rs | 122 ++++ .../dma/ringbuffer/tests/prop_test/writer.rs | 121 ++++ 8 files changed, 753 insertions(+), 672 deletions(-) delete mode 100644 embassy-stm32/src/dma/ringbuffer.rs create mode 100644 embassy-stm32/src/dma/ringbuffer/mod.rs create mode 100644 embassy-stm32/src/dma/ringbuffer/tests/mod.rs create mode 100644 embassy-stm32/src/dma/ringbuffer/tests/prop_test/mod.rs create mode 100644 embassy-stm32/src/dma/ringbuffer/tests/prop_test/reader.rs create mode 100644 embassy-stm32/src/dma/ringbuffer/tests/prop_test/writer.rs diff --git a/embassy-stm32/Cargo.toml b/embassy-stm32/Cargo.toml index 8fc8da006..53ec1b27f 100644 --- a/embassy-stm32/Cargo.toml +++ b/embassy-stm32/Cargo.toml @@ -93,6 +93,8 @@ aligned = "0.4.1" [dev-dependencies] critical-section = { version = "1.1", features = ["std"] } +proptest = "1.5.0" +proptest-state-machine = "0.3.0" [build-dependencies] proc-macro2 = "1.0.36" diff --git a/embassy-stm32/src/dma/dma_bdma.rs b/embassy-stm32/src/dma/dma_bdma.rs index d10b5554f..a43137c6e 100644 --- a/embassy-stm32/src/dma/dma_bdma.rs +++ b/embassy-stm32/src/dma/dma_bdma.rs @@ -763,10 +763,6 @@ impl<'a> DmaCtrl for DmaCtrlImpl<'a> { self.0.get_remaining_transfers() as _ } - fn get_complete_count(&self) -> usize { - STATE[self.0.id as usize].complete_count.load(Ordering::Acquire) - } - fn reset_complete_count(&mut self) -> usize { let state = &STATE[self.0.id as usize]; #[cfg(not(armv6m))] diff --git a/embassy-stm32/src/dma/ringbuffer.rs b/embassy-stm32/src/dma/ringbuffer.rs deleted file mode 100644 index 23f1d67d5..000000000 --- a/embassy-stm32/src/dma/ringbuffer.rs +++ /dev/null @@ -1,668 +0,0 @@ -#![cfg_attr(gpdma, allow(unused))] - -use core::future::poll_fn; -use core::ops::Range; -use core::sync::atomic::{compiler_fence, Ordering}; -use core::task::{Poll, Waker}; - -use super::word::Word; - -/// A "read-only" ring-buffer to be used together with the DMA controller which -/// writes in a circular way, "uncontrolled" to the buffer. -/// -/// A snapshot of the ring buffer state can be attained by setting the `ndtr` field -/// to the current register value. `ndtr` describes the current position of the DMA -/// write. -/// -/// # Buffer layout -/// -/// ```text -/// Without wraparound: With wraparound: -/// -/// + buf +--- NDTR ---+ + buf +---------- NDTR ----------+ -/// | | | | | | -/// v v v v v v -/// +-----------------------------------------+ +-----------------------------------------+ -/// |oooooooooooXXXXXXXXXXXXXXXXoooooooooooooo| |XXXXXXXXXXXXXooooooooooooXXXXXXXXXXXXXXXX| -/// +-----------------------------------------+ +-----------------------------------------+ -/// ^ ^ ^ ^ ^ ^ -/// | | | | | | -/// +- start --+ | +- end ------+ | -/// | | | | -/// +- end --------------------+ +- start ----------------+ -/// ``` -pub struct ReadableDmaRingBuffer<'a, W: Word> { - pub(crate) dma_buf: &'a mut [W], - start: usize, -} - -#[derive(Debug, PartialEq)] -#[cfg_attr(feature = "defmt", derive(defmt::Format))] -pub struct OverrunError; - -pub trait DmaCtrl { - /// Get the NDTR register value, i.e. the space left in the underlying - /// buffer until the dma writer wraps. - fn get_remaining_transfers(&self) -> usize; - - /// Get the transfer completed counter. - /// This counter is incremented by the dma controller when NDTR is reloaded, - /// i.e. when the writing wraps. - fn get_complete_count(&self) -> usize; - - /// Reset the transfer completed counter to 0 and return the value just prior to the reset. - fn reset_complete_count(&mut self) -> usize; - - /// Set the waker for a running poll_fn - fn set_waker(&mut self, waker: &Waker); -} - -impl<'a, W: Word> ReadableDmaRingBuffer<'a, W> { - pub fn new(dma_buf: &'a mut [W]) -> Self { - Self { dma_buf, start: 0 } - } - - /// Reset the ring buffer to its initial state - pub fn clear(&mut self, dma: &mut impl DmaCtrl) { - self.start = 0; - dma.reset_complete_count(); - } - - /// The capacity of the ringbuffer - pub const fn cap(&self) -> usize { - self.dma_buf.len() - } - - /// The current position of the ringbuffer - fn pos(&self, dma: &mut impl DmaCtrl) -> usize { - self.cap() - dma.get_remaining_transfers() - } - - /// Read an exact number of elements from the ringbuffer. - /// - /// Returns the remaining number of elements available for immediate reading. - /// OverrunError is returned if the portion to be read was overwritten by the DMA controller. - /// - /// Async/Wake Behavior: - /// The underlying DMA peripheral only can wake us when its buffer pointer has reached the halfway point, - /// and when it wraps around. This means that when called with a buffer of length 'M', when this - /// ring buffer was created with a buffer of size 'N': - /// - If M equals N/2 or N/2 divides evenly into M, this function will return every N/2 elements read on the DMA source. - /// - Otherwise, this function may need up to N/2 extra elements to arrive before returning. - pub async fn read_exact(&mut self, dma: &mut impl DmaCtrl, buffer: &mut [W]) -> Result { - let mut read_data = 0; - let buffer_len = buffer.len(); - - poll_fn(|cx| { - dma.set_waker(cx.waker()); - - compiler_fence(Ordering::SeqCst); - - match self.read(dma, &mut buffer[read_data..buffer_len]) { - Ok((len, remaining)) => { - read_data += len; - if read_data == buffer_len { - Poll::Ready(Ok(remaining)) - } else { - Poll::Pending - } - } - Err(e) => Poll::Ready(Err(e)), - } - }) - .await - } - - /// Read elements from the ring buffer - /// Return a tuple of the length read and the length remaining in the buffer - /// If not all of the elements were read, then there will be some elements in the buffer remaining - /// The length remaining is the capacity, ring_buf.len(), less the elements remaining after the read - /// OverrunError is returned if the portion to be read was overwritten by the DMA controller. - pub fn read(&mut self, dma: &mut impl DmaCtrl, buf: &mut [W]) -> Result<(usize, usize), OverrunError> { - /* - This algorithm is optimistic: we assume we haven't overrun more than a full buffer and then check - after we've done our work to see we have. This is because on stm32, an interrupt is not guaranteed - to fire in the same clock cycle that a register is read, so checking get_complete_count early does - not yield relevant information. - - Therefore, the only variable we really need to know is ndtr. If the dma has overrun by more than a full - buffer, we will do a bit more work than we have to, but algorithms should not be optimized for error - conditions. - - After we've done our work, we confirm that we haven't overrun more than a full buffer, and also that - the dma has not overrun within the data we could have copied. We check the data we could have copied - rather than the data we actually copied because it costs nothing and confirms an error condition - earlier. - */ - let end = self.pos(dma); - if self.start == end && dma.get_complete_count() == 0 { - // No elements are available in the buffer - Ok((0, self.cap())) - } else if self.start < end { - // The available, unread portion in the ring buffer DOES NOT wrap - // Copy out the elements from the dma buffer - let len = self.copy_to(buf, self.start..end); - - compiler_fence(Ordering::SeqCst); - - /* - first, check if the dma has wrapped at all if it's after end - or more than once if it's before start - - this is in a critical section to try to reduce mushy behavior. - it's not ideal but it's the best we can do - - then, get the current position of of the dma write and check - if it's inside data we could have copied - */ - let (pos, complete_count) = critical_section::with(|_| (self.pos(dma), dma.get_complete_count())); - if (pos >= self.start && pos < end) || (complete_count > 0 && pos >= end) || complete_count > 1 { - Err(OverrunError) - } else { - self.start = (self.start + len) % self.cap(); - - Ok((len, self.cap() - self.start)) - } - } else if self.start + buf.len() < self.cap() { - // The available, unread portion in the ring buffer DOES wrap - // The DMA writer has wrapped since we last read and is currently - // writing (or the next byte added will be) in the beginning of the ring buffer. - - // The provided read buffer is not large enough to include all elements from the tail of the dma buffer. - - // Copy out from the dma buffer - let len = self.copy_to(buf, self.start..self.cap()); - - compiler_fence(Ordering::SeqCst); - - /* - first, check if the dma has wrapped around more than once - - then, get the current position of of the dma write and check - if it's inside data we could have copied - */ - let pos = self.pos(dma); - if pos > self.start || pos < end || dma.get_complete_count() > 1 { - Err(OverrunError) - } else { - self.start = (self.start + len) % self.cap(); - - Ok((len, self.start + end)) - } - } else { - // The available, unread portion in the ring buffer DOES wrap - // The DMA writer has wrapped since we last read and is currently - // writing (or the next byte added will be) in the beginning of the ring buffer. - - // The provided read buffer is large enough to include all elements from the tail of the dma buffer, - // so the next read will not have any unread tail elements in the ring buffer. - - // Copy out from the dma buffer - let tail = self.copy_to(buf, self.start..self.cap()); - let head = self.copy_to(&mut buf[tail..], 0..end); - - compiler_fence(Ordering::SeqCst); - - /* - first, check if the dma has wrapped around more than once - - then, get the current position of of the dma write and check - if it's inside data we could have copied - */ - let pos = self.pos(dma); - if pos > self.start || pos < end || dma.reset_complete_count() > 1 { - Err(OverrunError) - } else { - self.start = head; - Ok((tail + head, self.cap() - self.start)) - } - } - } - /// Copy from the dma buffer at `data_range` into `buf` - fn copy_to(&mut self, buf: &mut [W], data_range: Range) -> usize { - // Limit the number of elements that can be copied - let length = usize::min(data_range.len(), buf.len()); - - // Copy from dma buffer into read buffer - // We need to do it like this instead of a simple copy_from_slice() because - // reading from a part of memory that may be simultaneously written to is unsafe - unsafe { - let dma_buf = self.dma_buf.as_ptr(); - - for i in 0..length { - buf[i] = core::ptr::read_volatile(dma_buf.offset((data_range.start + i) as isize)); - } - } - - length - } -} - -pub struct WritableDmaRingBuffer<'a, W: Word> { - pub(crate) dma_buf: &'a mut [W], - end: usize, -} - -impl<'a, W: Word> WritableDmaRingBuffer<'a, W> { - pub fn new(dma_buf: &'a mut [W]) -> Self { - Self { dma_buf, end: 0 } - } - - /// Reset the ring buffer to its initial state - pub fn clear(&mut self, dma: &mut impl DmaCtrl) { - self.end = 0; - dma.reset_complete_count(); - } - - /// The capacity of the ringbuffer - pub const fn cap(&self) -> usize { - self.dma_buf.len() - } - - /// The current position of the ringbuffer - fn pos(&self, dma: &mut impl DmaCtrl) -> usize { - self.cap() - dma.get_remaining_transfers() - } - - /// Write elements directly to the buffer. This must be done before the DMA is started - /// or after the buffer has been cleared using `clear()`. - pub fn write_immediate(&mut self, buffer: &[W]) -> Result<(usize, usize), OverrunError> { - if self.end != 0 { - return Err(OverrunError); - } - let written = self.copy_from(buffer, 0..self.cap()); - self.end = written % self.cap(); - Ok((written, self.cap() - written)) - } - - /// Write an exact number of elements to the ringbuffer. - pub async fn write_exact(&mut self, dma: &mut impl DmaCtrl, buffer: &[W]) -> Result { - let mut written_data = 0; - let buffer_len = buffer.len(); - - poll_fn(|cx| { - dma.set_waker(cx.waker()); - - compiler_fence(Ordering::SeqCst); - - match self.write(dma, &buffer[written_data..buffer_len]) { - Ok((len, remaining)) => { - written_data += len; - if written_data == buffer_len { - Poll::Ready(Ok(remaining)) - } else { - Poll::Pending - } - } - Err(e) => Poll::Ready(Err(e)), - } - }) - .await - } - - /// Write elements from the ring buffer - /// Return a tuple of the length written and the capacity remaining to be written in the buffer - pub fn write(&mut self, dma: &mut impl DmaCtrl, buf: &[W]) -> Result<(usize, usize), OverrunError> { - let start = self.pos(dma); - if start > self.end { - // The occupied portion in the ring buffer DOES wrap - let len = self.copy_from(buf, self.end..start); - - compiler_fence(Ordering::SeqCst); - - // Confirm that the DMA is not inside data we could have written - let (pos, complete_count) = critical_section::with(|_| (self.pos(dma), dma.get_complete_count())); - if (pos >= self.end && pos < start) || (complete_count > 0 && pos >= start) || complete_count > 1 { - Err(OverrunError) - } else { - self.end = (self.end + len) % self.cap(); - - Ok((len, self.cap() - (start - self.end))) - } - } else if start == self.end && dma.get_complete_count() == 0 { - Ok((0, 0)) - } else if start <= self.end && self.end + buf.len() < self.cap() { - // The occupied portion in the ring buffer DOES NOT wrap - // and copying elements into the buffer WILL NOT cause it to - - // Copy into the dma buffer - let len = self.copy_from(buf, self.end..self.cap()); - - compiler_fence(Ordering::SeqCst); - - // Confirm that the DMA is not inside data we could have written - let pos = self.pos(dma); - if pos > self.end || pos < start || dma.get_complete_count() > 1 { - Err(OverrunError) - } else { - self.end = (self.end + len) % self.cap(); - - Ok((len, self.cap() - (self.end - start))) - } - } else { - // The occupied portion in the ring buffer DOES NOT wrap - // and copying elements into the buffer WILL cause it to - - let tail = self.copy_from(buf, self.end..self.cap()); - let head = self.copy_from(&buf[tail..], 0..start); - - compiler_fence(Ordering::SeqCst); - - // Confirm that the DMA is not inside data we could have written - let pos = self.pos(dma); - if pos > self.end || pos < start || dma.reset_complete_count() > 1 { - Err(OverrunError) - } else { - self.end = head; - - Ok((tail + head, self.cap() - (start - self.end))) - } - } - } - /// Copy into the dma buffer at `data_range` from `buf` - fn copy_from(&mut self, buf: &[W], data_range: Range) -> usize { - // Limit the number of elements that can be copied - let length = usize::min(data_range.len(), buf.len()); - - // Copy into dma buffer from read buffer - // We need to do it like this instead of a simple copy_from_slice() because - // reading from a part of memory that may be simultaneously written to is unsafe - unsafe { - let dma_buf = self.dma_buf.as_mut_ptr(); - - for i in 0..length { - core::ptr::write_volatile(dma_buf.offset((data_range.start + i) as isize), buf[i]); - } - } - - length - } -} -#[cfg(test)] -mod tests { - use core::array; - use std::{cell, vec}; - - use super::*; - - #[allow(dead_code)] - #[derive(PartialEq, Debug)] - enum TestCircularTransferRequest { - GetCompleteCount(usize), - ResetCompleteCount(usize), - PositionRequest(usize), - } - - struct TestCircularTransfer { - len: usize, - requests: cell::RefCell>, - } - - impl DmaCtrl for TestCircularTransfer { - fn get_remaining_transfers(&self) -> usize { - match self.requests.borrow_mut().pop().unwrap() { - TestCircularTransferRequest::PositionRequest(pos) => { - let len = self.len; - - assert!(len >= pos); - - len - pos - } - _ => unreachable!(), - } - } - - fn get_complete_count(&self) -> usize { - match self.requests.borrow_mut().pop().unwrap() { - TestCircularTransferRequest::GetCompleteCount(complete_count) => complete_count, - _ => unreachable!(), - } - } - - fn reset_complete_count(&mut self) -> usize { - match self.requests.get_mut().pop().unwrap() { - TestCircularTransferRequest::ResetCompleteCount(complete_count) => complete_count, - _ => unreachable!(), - } - } - - fn set_waker(&mut self, waker: &Waker) {} - } - - impl TestCircularTransfer { - pub fn new(len: usize) -> Self { - Self { - requests: cell::RefCell::new(vec![]), - len, - } - } - - pub fn setup(&self, mut requests: vec::Vec) { - requests.reverse(); - self.requests.replace(requests); - } - } - - #[test] - fn empty_and_read_not_started() { - let mut dma_buf = [0u8; 16]; - let ringbuf = ReadableDmaRingBuffer::new(&mut dma_buf); - - assert_eq!(0, ringbuf.start); - } - - #[test] - fn can_read() { - let mut dma = TestCircularTransfer::new(16); - - let mut dma_buf: [u8; 16] = array::from_fn(|idx| idx as u8); // 0, 1, ..., 15 - let mut ringbuf = ReadableDmaRingBuffer::new(&mut dma_buf); - - assert_eq!(0, ringbuf.start); - assert_eq!(16, ringbuf.cap()); - - dma.setup(vec![ - TestCircularTransferRequest::PositionRequest(8), - TestCircularTransferRequest::PositionRequest(10), - TestCircularTransferRequest::GetCompleteCount(0), - ]); - let mut buf = [0; 2]; - assert_eq!(2, ringbuf.read(&mut dma, &mut buf).unwrap().0); - assert_eq!([0, 1], buf); - assert_eq!(2, ringbuf.start); - - dma.setup(vec![ - TestCircularTransferRequest::PositionRequest(10), - TestCircularTransferRequest::PositionRequest(12), - TestCircularTransferRequest::GetCompleteCount(0), - ]); - let mut buf = [0; 2]; - assert_eq!(2, ringbuf.read(&mut dma, &mut buf).unwrap().0); - assert_eq!([2, 3], buf); - assert_eq!(4, ringbuf.start); - - dma.setup(vec![ - TestCircularTransferRequest::PositionRequest(12), - TestCircularTransferRequest::PositionRequest(14), - TestCircularTransferRequest::GetCompleteCount(0), - ]); - let mut buf = [0; 8]; - assert_eq!(8, ringbuf.read(&mut dma, &mut buf).unwrap().0); - assert_eq!([4, 5, 6, 7, 8, 9], buf[..6]); - assert_eq!(12, ringbuf.start); - } - - #[test] - fn can_read_with_wrap() { - let mut dma = TestCircularTransfer::new(16); - - let mut dma_buf: [u8; 16] = array::from_fn(|idx| idx as u8); // 0, 1, ..., 15 - let mut ringbuf = ReadableDmaRingBuffer::new(&mut dma_buf); - - assert_eq!(0, ringbuf.start); - assert_eq!(16, ringbuf.cap()); - - /* - Read to close to the end of the buffer - */ - dma.setup(vec![ - TestCircularTransferRequest::PositionRequest(14), - TestCircularTransferRequest::PositionRequest(16), - TestCircularTransferRequest::GetCompleteCount(0), - ]); - let mut buf = [0; 14]; - assert_eq!(14, ringbuf.read(&mut dma, &mut buf).unwrap().0); - assert_eq!(14, ringbuf.start); - - /* - Now, read around the buffer - */ - dma.setup(vec![ - TestCircularTransferRequest::PositionRequest(6), - TestCircularTransferRequest::PositionRequest(8), - TestCircularTransferRequest::ResetCompleteCount(1), - ]); - let mut buf = [0; 6]; - assert_eq!(6, ringbuf.read(&mut dma, &mut buf).unwrap().0); - assert_eq!(4, ringbuf.start); - } - - #[test] - fn can_read_when_dma_writer_is_wrapped_and_read_does_not_wrap() { - let mut dma = TestCircularTransfer::new(16); - - let mut dma_buf: [u8; 16] = array::from_fn(|idx| idx as u8); // 0, 1, ..., 15 - let mut ringbuf = ReadableDmaRingBuffer::new(&mut dma_buf); - - assert_eq!(0, ringbuf.start); - assert_eq!(16, ringbuf.cap()); - - /* - Read to close to the end of the buffer - */ - dma.setup(vec![ - TestCircularTransferRequest::PositionRequest(14), - TestCircularTransferRequest::PositionRequest(16), - TestCircularTransferRequest::GetCompleteCount(0), - ]); - let mut buf = [0; 14]; - assert_eq!(14, ringbuf.read(&mut dma, &mut buf).unwrap().0); - assert_eq!(14, ringbuf.start); - - /* - Now, read to the end of the buffer - */ - dma.setup(vec![ - TestCircularTransferRequest::PositionRequest(6), - TestCircularTransferRequest::PositionRequest(8), - TestCircularTransferRequest::ResetCompleteCount(1), - ]); - let mut buf = [0; 2]; - assert_eq!(2, ringbuf.read(&mut dma, &mut buf).unwrap().0); - assert_eq!(0, ringbuf.start); - } - - #[test] - fn can_read_when_dma_writer_wraps_once_with_same_ndtr() { - let mut dma = TestCircularTransfer::new(16); - - let mut dma_buf: [u8; 16] = array::from_fn(|idx| idx as u8); // 0, 1, ..., 15 - let mut ringbuf = ReadableDmaRingBuffer::new(&mut dma_buf); - - assert_eq!(0, ringbuf.start); - assert_eq!(16, ringbuf.cap()); - - /* - Read to about the middle of the buffer - */ - dma.setup(vec![ - TestCircularTransferRequest::PositionRequest(6), - TestCircularTransferRequest::PositionRequest(6), - TestCircularTransferRequest::GetCompleteCount(0), - ]); - let mut buf = [0; 6]; - assert_eq!(6, ringbuf.read(&mut dma, &mut buf).unwrap().0); - assert_eq!(6, ringbuf.start); - - /* - Now, wrap the DMA controller around - */ - dma.setup(vec![ - TestCircularTransferRequest::PositionRequest(6), - TestCircularTransferRequest::GetCompleteCount(1), - TestCircularTransferRequest::PositionRequest(6), - TestCircularTransferRequest::GetCompleteCount(1), - ]); - let mut buf = [0; 6]; - assert_eq!(6, ringbuf.read(&mut dma, &mut buf).unwrap().0); - assert_eq!(12, ringbuf.start); - } - - #[test] - fn cannot_read_when_dma_writer_overwrites_during_not_wrapping_read() { - let mut dma = TestCircularTransfer::new(16); - - let mut dma_buf: [u8; 16] = array::from_fn(|idx| idx as u8); // 0, 1, ..., 15 - let mut ringbuf = ReadableDmaRingBuffer::new(&mut dma_buf); - - assert_eq!(0, ringbuf.start); - assert_eq!(16, ringbuf.cap()); - - /* - Read a few bytes - */ - dma.setup(vec![ - TestCircularTransferRequest::PositionRequest(2), - TestCircularTransferRequest::PositionRequest(2), - TestCircularTransferRequest::GetCompleteCount(0), - ]); - let mut buf = [0; 6]; - assert_eq!(2, ringbuf.read(&mut dma, &mut buf).unwrap().0); - assert_eq!(2, ringbuf.start); - - /* - Now, overtake the reader - */ - dma.setup(vec![ - TestCircularTransferRequest::PositionRequest(4), - TestCircularTransferRequest::PositionRequest(6), - TestCircularTransferRequest::GetCompleteCount(1), - ]); - let mut buf = [0; 6]; - assert_eq!(OverrunError, ringbuf.read(&mut dma, &mut buf).unwrap_err()); - } - - #[test] - fn cannot_read_when_dma_writer_overwrites_during_wrapping_read() { - let mut dma = TestCircularTransfer::new(16); - - let mut dma_buf: [u8; 16] = array::from_fn(|idx| idx as u8); // 0, 1, ..., 15 - let mut ringbuf = ReadableDmaRingBuffer::new(&mut dma_buf); - - assert_eq!(0, ringbuf.start); - assert_eq!(16, ringbuf.cap()); - - /* - Read to close to the end of the buffer - */ - dma.setup(vec![ - TestCircularTransferRequest::PositionRequest(14), - TestCircularTransferRequest::PositionRequest(16), - TestCircularTransferRequest::GetCompleteCount(0), - ]); - let mut buf = [0; 14]; - assert_eq!(14, ringbuf.read(&mut dma, &mut buf).unwrap().0); - assert_eq!(14, ringbuf.start); - - /* - Now, overtake the reader - */ - dma.setup(vec![ - TestCircularTransferRequest::PositionRequest(8), - TestCircularTransferRequest::PositionRequest(10), - TestCircularTransferRequest::ResetCompleteCount(2), - ]); - let mut buf = [0; 6]; - assert_eq!(OverrunError, ringbuf.read(&mut dma, &mut buf).unwrap_err()); - } -} diff --git a/embassy-stm32/src/dma/ringbuffer/mod.rs b/embassy-stm32/src/dma/ringbuffer/mod.rs new file mode 100644 index 000000000..10a9ff975 --- /dev/null +++ b/embassy-stm32/src/dma/ringbuffer/mod.rs @@ -0,0 +1,293 @@ +#![cfg_attr(gpdma, allow(unused))] + +use core::future::poll_fn; +use core::task::{Poll, Waker}; + +use crate::dma::word::Word; + +pub trait DmaCtrl { + /// Get the NDTR register value, i.e. the space left in the underlying + /// buffer until the dma writer wraps. + fn get_remaining_transfers(&self) -> usize; + + /// Reset the transfer completed counter to 0 and return the value just prior to the reset. + fn reset_complete_count(&mut self) -> usize; + + /// Set the waker for a running poll_fn + fn set_waker(&mut self, waker: &Waker); +} + +#[derive(Debug, PartialEq)] +#[cfg_attr(feature = "defmt", derive(defmt::Format))] +pub struct OverrunError; + +#[derive(Debug, Clone, Copy, Default)] +struct DmaIndex { + completion_count: usize, + pos: usize, +} + +fn pos(cap: usize, dma: &impl DmaCtrl) -> usize { + cap - dma.get_remaining_transfers() +} + +impl DmaIndex { + fn reset(&mut self) { + self.pos = 0; + self.completion_count = 0; + } + + fn as_index(&self, cap: usize, offset: usize) -> usize { + (self.pos + offset) % cap + } + + fn dma_sync(&mut self, cap: usize, dma: &mut impl DmaCtrl) { + let fst_pos = pos(cap, dma); + let fst_count = dma.reset_complete_count(); + let pos = pos(cap, dma); + + let wrap_count = if pos >= fst_pos { + fst_count + } else { + fst_count + dma.reset_complete_count() + }; + + self.pos = pos; + self.completion_count += wrap_count; + } + + fn advance(&mut self, cap: usize, steps: usize) { + let next = self.pos + steps; + self.completion_count += next / cap; + self.pos = next % cap; + } + + fn normalize(lhs: &mut DmaIndex, rhs: &mut DmaIndex) { + let min_count = lhs.completion_count.min(rhs.completion_count); + lhs.completion_count -= min_count; + rhs.completion_count -= min_count; + } + + fn diff(&mut self, cap: usize, rhs: &mut DmaIndex) -> isize { + Self::normalize(self, rhs); + (self.completion_count * cap + self.pos) as isize - (rhs.completion_count * cap + rhs.pos) as isize + } +} + +pub struct ReadableDmaRingBuffer<'a, W: Word> { + dma_buf: &'a mut [W], + write_index: DmaIndex, + read_index: DmaIndex, +} + +impl<'a, W: Word> ReadableDmaRingBuffer<'a, W> { + /// Construct an empty buffer. + pub fn new(dma_buf: &'a mut [W]) -> Self { + Self { + dma_buf, + write_index: Default::default(), + read_index: Default::default(), + } + } + + /// Reset the ring buffer to its initial state + pub fn clear(&mut self, dma: &mut impl DmaCtrl) { + dma.reset_complete_count(); + self.write_index.reset(); + self.update_dma_index(dma); + self.read_index = self.write_index; + } + + /// The capacity of the ringbuffer + pub const fn cap(&self) -> usize { + self.dma_buf.len() + } + + /// Read elements from the ring buffer + /// Return a tuple of the length read and the length remaining in the buffer + /// If not all of the elements were read, then there will be some elements in the buffer remaining + /// The length remaining is the capacity, ring_buf.len(), less the elements remaining after the read + /// OverrunError is returned if the portion to be read was overwritten by the DMA controller. + pub fn read(&mut self, dma: &mut impl DmaCtrl, buf: &mut [W]) -> Result<(usize, usize), OverrunError> { + let readable = self.margin(dma)?.min(buf.len()); + for i in 0..readable { + buf[i] = self.read_buf(i); + } + let available = self.margin(dma)?; + self.read_index.advance(self.cap(), readable); + Ok((readable, available - readable)) + } + + /// Read an exact number of elements from the ringbuffer. + /// + /// Returns the remaining number of elements available for immediate reading. + /// OverrunError is returned if the portion to be read was overwritten by the DMA controller. + /// + /// Async/Wake Behavior: + /// The underlying DMA peripheral only can wake us when its buffer pointer has reached the halfway point, + /// and when it wraps around. This means that when called with a buffer of length 'M', when this + /// ring buffer was created with a buffer of size 'N': + /// - If M equals N/2 or N/2 divides evenly into M, this function will return every N/2 elements read on the DMA source. + /// - Otherwise, this function may need up to N/2 extra elements to arrive before returning. + pub async fn read_exact(&mut self, dma: &mut impl DmaCtrl, buffer: &mut [W]) -> Result { + let mut read_data = 0; + let buffer_len = buffer.len(); + + poll_fn(|cx| { + dma.set_waker(cx.waker()); + + match self.read(dma, &mut buffer[read_data..buffer_len]) { + Ok((len, remaining)) => { + read_data += len; + if read_data == buffer_len { + Poll::Ready(Ok(remaining)) + } else { + Poll::Pending + } + } + Err(e) => Poll::Ready(Err(e)), + } + }) + .await + } + + fn update_dma_index(&mut self, dma: &mut impl DmaCtrl) { + self.write_index.dma_sync(self.cap(), dma) + } + + fn read_buf(&self, offset: usize) -> W { + unsafe { + core::ptr::read_volatile( + self.dma_buf + .as_ptr() + .offset(self.read_index.as_index(self.cap(), offset) as isize), + ) + } + } + + /// Returns available dma samples + fn margin(&mut self, dma: &mut impl DmaCtrl) -> Result { + self.update_dma_index(dma); + + let diff: usize = self + .write_index + .diff(self.cap(), &mut self.read_index) + .try_into() + .unwrap(); + + if diff > self.cap() { + Err(OverrunError) + } else { + Ok(diff) + } + } +} + +pub struct WritableDmaRingBuffer<'a, W: Word> { + dma_buf: &'a mut [W], + read_index: DmaIndex, + write_index: DmaIndex, +} + +impl<'a, W: Word> WritableDmaRingBuffer<'a, W> { + /// Construct a ringbuffer filled with the given buffer data. + pub fn new(dma_buf: &'a mut [W]) -> Self { + let len = dma_buf.len(); + Self { + dma_buf, + read_index: Default::default(), + write_index: DmaIndex { + completion_count: 0, + pos: len, + }, + } + } + + /// Reset the ring buffer to its initial state. The buffer after the reset will be full. + pub fn clear(&mut self, dma: &mut impl DmaCtrl) { + dma.reset_complete_count(); + self.read_index.reset(); + self.update_dma_index(dma); + self.write_index = self.read_index; + self.write_index.advance(self.cap(), self.cap()); + } + + /// Get the capacity of the ringbuffer. + pub const fn cap(&self) -> usize { + self.dma_buf.len() + } + + /// Append data to the ring buffer. + /// Returns a tuple of the data written and the remaining write capacity in the buffer. + pub fn write(&mut self, dma: &mut impl DmaCtrl, buf: &[W]) -> Result<(usize, usize), OverrunError> { + let writable = self.margin(dma)?.min(buf.len()); + for i in 0..writable { + self.write_buf(i, buf[i]); + } + let available = self.margin(dma)?; + self.write_index.advance(self.cap(), writable); + Ok((writable, available - writable)) + } + + /// Write elements directly to the buffer. + pub fn write_immediate(&mut self, buf: &[W]) -> Result<(usize, usize), OverrunError> { + for (i, data) in buf.iter().enumerate() { + self.write_buf(i, *data) + } + let written = buf.len().min(self.cap()); + Ok((written, self.cap() - written)) + } + + /// Write an exact number of elements to the ringbuffer. + pub async fn write_exact(&mut self, dma: &mut impl DmaCtrl, buffer: &[W]) -> Result { + let mut written_data = 0; + let buffer_len = buffer.len(); + + poll_fn(|cx| { + dma.set_waker(cx.waker()); + + match self.write(dma, &buffer[written_data..buffer_len]) { + Ok((len, remaining)) => { + written_data += len; + if written_data == buffer_len { + Poll::Ready(Ok(remaining)) + } else { + Poll::Pending + } + } + Err(e) => Poll::Ready(Err(e)), + } + }) + .await + } + + fn update_dma_index(&mut self, dma: &mut impl DmaCtrl) { + self.read_index.dma_sync(self.cap(), dma); + } + + fn write_buf(&mut self, offset: usize, value: W) { + unsafe { + core::ptr::write_volatile( + self.dma_buf + .as_mut_ptr() + .offset(self.write_index.as_index(self.cap(), offset) as isize), + value, + ) + } + } + + fn margin(&mut self, dma: &mut impl DmaCtrl) -> Result { + self.update_dma_index(dma); + + let diff = self.write_index.diff(self.cap(), &mut self.read_index); + + if diff < 0 { + Err(OverrunError) + } else { + Ok(self.cap().saturating_sub(diff as usize)) + } + } +} + +#[cfg(test)] +mod tests; diff --git a/embassy-stm32/src/dma/ringbuffer/tests/mod.rs b/embassy-stm32/src/dma/ringbuffer/tests/mod.rs new file mode 100644 index 000000000..9768e1df8 --- /dev/null +++ b/embassy-stm32/src/dma/ringbuffer/tests/mod.rs @@ -0,0 +1,165 @@ +use std::{cell, vec}; + +use super::*; + +#[allow(dead_code)] +#[derive(PartialEq, Debug)] +enum TestCircularTransferRequest { + ResetCompleteCount(usize), + PositionRequest(usize), +} + +struct TestCircularTransfer { + len: usize, + requests: cell::RefCell>, +} + +impl DmaCtrl for TestCircularTransfer { + fn get_remaining_transfers(&self) -> usize { + match self.requests.borrow_mut().pop().unwrap() { + TestCircularTransferRequest::PositionRequest(pos) => { + let len = self.len; + + assert!(len >= pos); + + len - pos + } + _ => unreachable!(), + } + } + + fn reset_complete_count(&mut self) -> usize { + match self.requests.get_mut().pop().unwrap() { + TestCircularTransferRequest::ResetCompleteCount(complete_count) => complete_count, + _ => unreachable!(), + } + } + + fn set_waker(&mut self, _waker: &Waker) {} +} + +impl TestCircularTransfer { + pub fn new(len: usize) -> Self { + Self { + requests: cell::RefCell::new(vec![]), + len, + } + } + + pub fn setup(&self, mut requests: vec::Vec) { + requests.reverse(); + self.requests.replace(requests); + } +} + +const CAP: usize = 16; + +#[test] +fn dma_index_dma_sync_syncs_position_to_last_read_if_sync_takes_place_on_same_dma_cycle() { + let mut dma = TestCircularTransfer::new(CAP); + dma.setup(vec![ + TestCircularTransferRequest::PositionRequest(4), + TestCircularTransferRequest::ResetCompleteCount(0), + TestCircularTransferRequest::PositionRequest(7), + ]); + let mut index = DmaIndex::default(); + index.dma_sync(CAP, &mut dma); + assert_eq!(index.completion_count, 0); + assert_eq!(index.pos, 7); +} + +#[test] +fn dma_index_dma_sync_updates_completion_count_properly_if_sync_takes_place_on_same_dma_cycle() { + let mut dma = TestCircularTransfer::new(CAP); + dma.setup(vec![ + TestCircularTransferRequest::PositionRequest(4), + TestCircularTransferRequest::ResetCompleteCount(2), + TestCircularTransferRequest::PositionRequest(7), + ]); + let mut index = DmaIndex::default(); + index.completion_count = 1; + index.dma_sync(CAP, &mut dma); + assert_eq!(index.completion_count, 3); + assert_eq!(index.pos, 7); +} + +#[test] +fn dma_index_dma_sync_syncs_to_last_position_if_reads_occur_on_different_dma_cycles() { + let mut dma = TestCircularTransfer::new(CAP); + dma.setup(vec![ + TestCircularTransferRequest::PositionRequest(10), + TestCircularTransferRequest::ResetCompleteCount(1), + TestCircularTransferRequest::PositionRequest(5), + TestCircularTransferRequest::ResetCompleteCount(0), + ]); + let mut index = DmaIndex::default(); + index.dma_sync(CAP, &mut dma); + assert_eq!(index.completion_count, 1); + assert_eq!(index.pos, 5); +} + +#[test] +fn dma_index_dma_sync_detects_new_cycle_if_later_position_is_less_than_first_and_first_completion_count_occurs_on_first_cycle( +) { + let mut dma = TestCircularTransfer::new(CAP); + dma.setup(vec![ + TestCircularTransferRequest::PositionRequest(10), + TestCircularTransferRequest::ResetCompleteCount(1), + TestCircularTransferRequest::PositionRequest(5), + TestCircularTransferRequest::ResetCompleteCount(1), + ]); + let mut index = DmaIndex::default(); + index.completion_count = 1; + index.dma_sync(CAP, &mut dma); + assert_eq!(index.completion_count, 3); + assert_eq!(index.pos, 5); +} + +#[test] +fn dma_index_dma_sync_detects_new_cycle_if_later_position_is_less_than_first_and_first_completion_count_occurs_on_later_cycle( +) { + let mut dma = TestCircularTransfer::new(CAP); + dma.setup(vec![ + TestCircularTransferRequest::PositionRequest(10), + TestCircularTransferRequest::ResetCompleteCount(2), + TestCircularTransferRequest::PositionRequest(5), + TestCircularTransferRequest::ResetCompleteCount(0), + ]); + let mut index = DmaIndex::default(); + index.completion_count = 1; + index.dma_sync(CAP, &mut dma); + assert_eq!(index.completion_count, 3); + assert_eq!(index.pos, 5); +} + +#[test] +fn dma_index_as_index_returns_index_mod_cap_by_default() { + let index = DmaIndex::default(); + assert_eq!(index.as_index(CAP, 0), 0); + assert_eq!(index.as_index(CAP, 1), 1); + assert_eq!(index.as_index(CAP, 2), 2); + assert_eq!(index.as_index(CAP, 3), 3); + assert_eq!(index.as_index(CAP, 4), 4); + assert_eq!(index.as_index(CAP, CAP), 0); + assert_eq!(index.as_index(CAP, CAP + 1), 1); +} + +#[test] +fn dma_index_advancing_increases_as_index() { + let mut index = DmaIndex::default(); + assert_eq!(index.as_index(CAP, 0), 0); + index.advance(CAP, 1); + assert_eq!(index.as_index(CAP, 0), 1); + index.advance(CAP, 1); + assert_eq!(index.as_index(CAP, 0), 2); + index.advance(CAP, 1); + assert_eq!(index.as_index(CAP, 0), 3); + index.advance(CAP, 1); + assert_eq!(index.as_index(CAP, 0), 4); + index.advance(CAP, CAP - 4); + assert_eq!(index.as_index(CAP, 0), 0); + index.advance(CAP, 1); + assert_eq!(index.as_index(CAP, 0), 1); +} + +mod prop_test; diff --git a/embassy-stm32/src/dma/ringbuffer/tests/prop_test/mod.rs b/embassy-stm32/src/dma/ringbuffer/tests/prop_test/mod.rs new file mode 100644 index 000000000..661fb1728 --- /dev/null +++ b/embassy-stm32/src/dma/ringbuffer/tests/prop_test/mod.rs @@ -0,0 +1,50 @@ +use std::task::Waker; + +use proptest::prop_oneof; +use proptest::strategy::{self, BoxedStrategy, Strategy as _}; +use proptest_state_machine::{prop_state_machine, ReferenceStateMachine, StateMachineTest}; + +use super::*; + +const CAP: usize = 128; + +#[derive(Debug, Default)] +struct DmaMock { + pos: usize, + wraps: usize, +} + +impl DmaMock { + pub fn advance(&mut self, steps: usize) { + let next = self.pos + steps; + self.pos = next % CAP; + self.wraps += next / CAP; + } +} + +impl DmaCtrl for DmaMock { + fn get_remaining_transfers(&self) -> usize { + CAP - self.pos + } + + fn reset_complete_count(&mut self) -> usize { + core::mem::replace(&mut self.wraps, 0) + } + + fn set_waker(&mut self, _waker: &Waker) {} +} + +#[derive(Debug, Clone)] +enum Status { + Available(usize), + Failed, +} + +impl Status { + pub fn new(capacity: usize) -> Self { + Self::Available(capacity) + } +} + +mod reader; +mod writer; diff --git a/embassy-stm32/src/dma/ringbuffer/tests/prop_test/reader.rs b/embassy-stm32/src/dma/ringbuffer/tests/prop_test/reader.rs new file mode 100644 index 000000000..6555ebfb0 --- /dev/null +++ b/embassy-stm32/src/dma/ringbuffer/tests/prop_test/reader.rs @@ -0,0 +1,122 @@ +use core::fmt::Debug; + +use super::*; + +#[derive(Debug, Clone)] +enum ReaderTransition { + Write(usize), + Clear, + ReadUpTo(usize), +} + +struct ReaderSM; + +impl ReferenceStateMachine for ReaderSM { + type State = Status; + type Transition = ReaderTransition; + + fn init_state() -> BoxedStrategy { + strategy::Just(Status::new(0)).boxed() + } + + fn transitions(_state: &Self::State) -> BoxedStrategy { + prop_oneof![ + (1..50_usize).prop_map(ReaderTransition::Write), + (1..50_usize).prop_map(ReaderTransition::ReadUpTo), + strategy::Just(ReaderTransition::Clear), + ] + .boxed() + } + + fn apply(status: Self::State, transition: &Self::Transition) -> Self::State { + match (status, transition) { + (_, ReaderTransition::Clear) => Status::Available(0), + (Status::Available(x), ReaderTransition::Write(y)) => { + if x + y > CAP { + Status::Failed + } else { + Status::Available(x + y) + } + } + (Status::Available(x), ReaderTransition::ReadUpTo(y)) => Status::Available(x.saturating_sub(*y)), + (Status::Failed, _) => Status::Failed, + } + } +} + +struct ReaderSut { + status: Status, + buffer: *mut [u8], + producer: DmaMock, + consumer: ReadableDmaRingBuffer<'static, u8>, +} + +impl Debug for ReaderSut { + fn fmt(&self, f: &mut core::fmt::Formatter) -> core::fmt::Result { + ::fmt(&self.producer, f) + } +} + +struct ReaderTest; + +impl StateMachineTest for ReaderTest { + type SystemUnderTest = ReaderSut; + type Reference = ReaderSM; + + fn init_test(ref_status: &::State) -> Self::SystemUnderTest { + let buffer = Box::into_raw(Box::new([0; CAP])); + ReaderSut { + status: ref_status.clone(), + buffer, + producer: DmaMock::default(), + consumer: ReadableDmaRingBuffer::new(unsafe { &mut *buffer }), + } + } + + fn teardown(state: Self::SystemUnderTest) { + unsafe { + let _ = Box::from_raw(state.buffer); + }; + } + + fn apply( + mut sut: Self::SystemUnderTest, + ref_state: &::State, + transition: ::Transition, + ) -> Self::SystemUnderTest { + match transition { + ReaderTransition::Write(x) => sut.producer.advance(x), + ReaderTransition::Clear => { + sut.consumer.clear(&mut sut.producer); + } + ReaderTransition::ReadUpTo(x) => { + let status = sut.status; + let ReaderSut { + ref mut producer, + ref mut consumer, + .. + } = sut; + let mut buf = vec![0; x]; + let res = consumer.read(producer, &mut buf); + match status { + Status::Available(n) => { + let readable = x.min(n); + + assert_eq!(res.unwrap().0, readable); + } + Status::Failed => assert!(res.is_err()), + } + } + } + + ReaderSut { + status: ref_state.clone(), + ..sut + } + } +} + +prop_state_machine! { + #[test] + fn reader_state_test(sequential 1..20 => ReaderTest); +} diff --git a/embassy-stm32/src/dma/ringbuffer/tests/prop_test/writer.rs b/embassy-stm32/src/dma/ringbuffer/tests/prop_test/writer.rs new file mode 100644 index 000000000..15f54c672 --- /dev/null +++ b/embassy-stm32/src/dma/ringbuffer/tests/prop_test/writer.rs @@ -0,0 +1,121 @@ +use core::fmt::Debug; + +use super::*; + +#[derive(Debug, Clone)] +enum WriterTransition { + Read(usize), + WriteUpTo(usize), + Clear, +} + +struct WriterSM; + +impl ReferenceStateMachine for WriterSM { + type State = Status; + type Transition = WriterTransition; + + fn init_state() -> BoxedStrategy { + strategy::Just(Status::new(CAP)).boxed() + } + + fn transitions(_state: &Self::State) -> BoxedStrategy { + prop_oneof![ + (1..50_usize).prop_map(WriterTransition::Read), + (1..50_usize).prop_map(WriterTransition::WriteUpTo), + strategy::Just(WriterTransition::Clear), + ] + .boxed() + } + + fn apply(status: Self::State, transition: &Self::Transition) -> Self::State { + match (status, transition) { + (_, WriterTransition::Clear) => Status::Available(CAP), + (Status::Available(x), WriterTransition::Read(y)) => { + if x < *y { + Status::Failed + } else { + Status::Available(x - y) + } + } + (Status::Available(x), WriterTransition::WriteUpTo(y)) => Status::Available((x + *y).min(CAP)), + (Status::Failed, _) => Status::Failed, + } + } +} + +struct WriterSut { + status: Status, + buffer: *mut [u8], + producer: WritableDmaRingBuffer<'static, u8>, + consumer: DmaMock, +} + +impl Debug for WriterSut { + fn fmt(&self, f: &mut core::fmt::Formatter) -> core::fmt::Result { + ::fmt(&self.consumer, f) + } +} + +struct WriterTest; + +impl StateMachineTest for WriterTest { + type SystemUnderTest = WriterSut; + type Reference = WriterSM; + + fn init_test(ref_status: &::State) -> Self::SystemUnderTest { + let buffer = Box::into_raw(Box::new([0; CAP])); + WriterSut { + status: ref_status.clone(), + buffer, + producer: WritableDmaRingBuffer::new(unsafe { &mut *buffer }), + consumer: DmaMock::default(), + } + } + + fn teardown(state: Self::SystemUnderTest) { + unsafe { + let _ = Box::from_raw(state.buffer); + }; + } + + fn apply( + mut sut: Self::SystemUnderTest, + ref_status: &::State, + transition: ::Transition, + ) -> Self::SystemUnderTest { + match transition { + WriterTransition::Read(x) => sut.consumer.advance(x), + WriterTransition::Clear => { + sut.producer.clear(&mut sut.consumer); + } + WriterTransition::WriteUpTo(x) => { + let status = sut.status; + let WriterSut { + ref mut producer, + ref mut consumer, + .. + } = sut; + let mut buf = vec![0; x]; + let res = producer.write(consumer, &mut buf); + match status { + Status::Available(n) => { + let writable = x.min(CAP - n.min(CAP)); + assert_eq!(res.unwrap().0, writable); + } + Status::Failed => assert!(res.is_err()), + } + } + } + + WriterSut { + status: ref_status.clone(), + ..sut + } + } +} + +prop_state_machine! { + #[test] + fn writer_state_test(sequential 1..20 => WriterTest); +} From f4ec0cb4d4d16bd4e1c242864c5ca828745704c0 Mon Sep 17 00:00:00 2001 From: Alexandros Liarokapis Date: Mon, 23 Sep 2024 02:01:44 +0300 Subject: [PATCH 02/11] simplify and rename ringbuffer methods, make len available --- embassy-stm32/src/dma/ringbuffer/mod.rs | 92 ++++++++++++------------- 1 file changed, 43 insertions(+), 49 deletions(-) diff --git a/embassy-stm32/src/dma/ringbuffer/mod.rs b/embassy-stm32/src/dma/ringbuffer/mod.rs index 10a9ff975..d4d119a69 100644 --- a/embassy-stm32/src/dma/ringbuffer/mod.rs +++ b/embassy-stm32/src/dma/ringbuffer/mod.rs @@ -27,10 +27,6 @@ struct DmaIndex { pos: usize, } -fn pos(cap: usize, dma: &impl DmaCtrl) -> usize { - cap - dma.get_remaining_transfers() -} - impl DmaIndex { fn reset(&mut self) { self.pos = 0; @@ -42,9 +38,9 @@ impl DmaIndex { } fn dma_sync(&mut self, cap: usize, dma: &mut impl DmaCtrl) { - let fst_pos = pos(cap, dma); + let fst_pos = cap - dma.get_remaining_transfers(); let fst_count = dma.reset_complete_count(); - let pos = pos(cap, dma); + let pos = cap - dma.get_remaining_transfers(); let wrap_count = if pos >= fst_pos { fst_count @@ -68,8 +64,7 @@ impl DmaIndex { rhs.completion_count -= min_count; } - fn diff(&mut self, cap: usize, rhs: &mut DmaIndex) -> isize { - Self::normalize(self, rhs); + fn diff(&self, cap: usize, rhs: &DmaIndex) -> isize { (self.completion_count * cap + self.pos) as isize - (rhs.completion_count * cap + rhs.pos) as isize } } @@ -94,26 +89,39 @@ impl<'a, W: Word> ReadableDmaRingBuffer<'a, W> { pub fn clear(&mut self, dma: &mut impl DmaCtrl) { dma.reset_complete_count(); self.write_index.reset(); - self.update_dma_index(dma); + self.write_index.dma_sync(self.cap(), dma); self.read_index = self.write_index; } - /// The capacity of the ringbuffer + /// Get the full ringbuffer capacity. pub const fn cap(&self) -> usize { self.dma_buf.len() } + /// Get the available readable dma samples. + pub fn len(&self) -> Result { + let diff: usize = self.write_index.diff(self.cap(), &self.read_index).try_into().unwrap(); + + if diff > self.cap() { + Err(OverrunError) + } else { + Ok(diff) + } + } + /// Read elements from the ring buffer /// Return a tuple of the length read and the length remaining in the buffer /// If not all of the elements were read, then there will be some elements in the buffer remaining /// The length remaining is the capacity, ring_buf.len(), less the elements remaining after the read /// OverrunError is returned if the portion to be read was overwritten by the DMA controller. pub fn read(&mut self, dma: &mut impl DmaCtrl, buf: &mut [W]) -> Result<(usize, usize), OverrunError> { - let readable = self.margin(dma)?.min(buf.len()); + self.sync_write_index(dma); + let readable = self.len()?.min(buf.len()); for i in 0..readable { buf[i] = self.read_buf(i); } - let available = self.margin(dma)?; + self.sync_write_index(dma); + let available = self.len()?; self.read_index.advance(self.cap(), readable); Ok((readable, available - readable)) } @@ -151,10 +159,6 @@ impl<'a, W: Word> ReadableDmaRingBuffer<'a, W> { .await } - fn update_dma_index(&mut self, dma: &mut impl DmaCtrl) { - self.write_index.dma_sync(self.cap(), dma) - } - fn read_buf(&self, offset: usize) -> W { unsafe { core::ptr::read_volatile( @@ -165,21 +169,9 @@ impl<'a, W: Word> ReadableDmaRingBuffer<'a, W> { } } - /// Returns available dma samples - fn margin(&mut self, dma: &mut impl DmaCtrl) -> Result { - self.update_dma_index(dma); - - let diff: usize = self - .write_index - .diff(self.cap(), &mut self.read_index) - .try_into() - .unwrap(); - - if diff > self.cap() { - Err(OverrunError) - } else { - Ok(diff) - } + fn sync_write_index(&mut self, dma: &mut impl DmaCtrl) { + self.write_index.dma_sync(self.cap(), dma); + DmaIndex::normalize(&mut self.write_index, &mut self.read_index); } } @@ -207,12 +199,23 @@ impl<'a, W: Word> WritableDmaRingBuffer<'a, W> { pub fn clear(&mut self, dma: &mut impl DmaCtrl) { dma.reset_complete_count(); self.read_index.reset(); - self.update_dma_index(dma); + self.read_index.dma_sync(self.cap(), dma); self.write_index = self.read_index; self.write_index.advance(self.cap(), self.cap()); } - /// Get the capacity of the ringbuffer. + /// Get the remaining writable dma samples. + pub fn len(&self) -> Result { + let diff = self.write_index.diff(self.cap(), &self.read_index); + + if diff < 0 { + Err(OverrunError) + } else { + Ok(self.cap().saturating_sub(diff as usize)) + } + } + + /// Get the full ringbuffer capacity. pub const fn cap(&self) -> usize { self.dma_buf.len() } @@ -220,11 +223,13 @@ impl<'a, W: Word> WritableDmaRingBuffer<'a, W> { /// Append data to the ring buffer. /// Returns a tuple of the data written and the remaining write capacity in the buffer. pub fn write(&mut self, dma: &mut impl DmaCtrl, buf: &[W]) -> Result<(usize, usize), OverrunError> { - let writable = self.margin(dma)?.min(buf.len()); + self.sync_read_index(dma); + let writable = self.len()?.min(buf.len()); for i in 0..writable { self.write_buf(i, buf[i]); } - let available = self.margin(dma)?; + self.sync_read_index(dma); + let available = self.len()?; self.write_index.advance(self.cap(), writable); Ok((writable, available - writable)) } @@ -261,10 +266,6 @@ impl<'a, W: Word> WritableDmaRingBuffer<'a, W> { .await } - fn update_dma_index(&mut self, dma: &mut impl DmaCtrl) { - self.read_index.dma_sync(self.cap(), dma); - } - fn write_buf(&mut self, offset: usize, value: W) { unsafe { core::ptr::write_volatile( @@ -276,16 +277,9 @@ impl<'a, W: Word> WritableDmaRingBuffer<'a, W> { } } - fn margin(&mut self, dma: &mut impl DmaCtrl) -> Result { - self.update_dma_index(dma); - - let diff = self.write_index.diff(self.cap(), &mut self.read_index); - - if diff < 0 { - Err(OverrunError) - } else { - Ok(self.cap().saturating_sub(diff as usize)) - } + fn sync_read_index(&mut self, dma: &mut impl DmaCtrl) { + self.read_index.dma_sync(self.cap(), dma); + DmaIndex::normalize(&mut self.read_index, &mut self.write_index); } } From 85fb890b0025db386459f8b6a573c29f00bf3ed1 Mon Sep 17 00:00:00 2001 From: Alexandros Liarokapis Date: Mon, 23 Sep 2024 02:49:05 +0300 Subject: [PATCH 03/11] add auto-clear functionality to ringbuffer --- embassy-stm32/src/dma/ringbuffer/mod.rs | 56 ++++++++++++------- .../dma/ringbuffer/tests/prop_test/reader.rs | 3 +- .../dma/ringbuffer/tests/prop_test/writer.rs | 3 +- 3 files changed, 40 insertions(+), 22 deletions(-) diff --git a/embassy-stm32/src/dma/ringbuffer/mod.rs b/embassy-stm32/src/dma/ringbuffer/mod.rs index d4d119a69..abc01e3cc 100644 --- a/embassy-stm32/src/dma/ringbuffer/mod.rs +++ b/embassy-stm32/src/dma/ringbuffer/mod.rs @@ -85,7 +85,7 @@ impl<'a, W: Word> ReadableDmaRingBuffer<'a, W> { } } - /// Reset the ring buffer to its initial state + /// Reset the ring buffer to its initial state. pub fn clear(&mut self, dma: &mut impl DmaCtrl) { dma.reset_complete_count(); self.write_index.reset(); @@ -113,17 +113,12 @@ impl<'a, W: Word> ReadableDmaRingBuffer<'a, W> { /// Return a tuple of the length read and the length remaining in the buffer /// If not all of the elements were read, then there will be some elements in the buffer remaining /// The length remaining is the capacity, ring_buf.len(), less the elements remaining after the read - /// OverrunError is returned if the portion to be read was overwritten by the DMA controller. + /// OverrunError is returned if the portion to be read was overwritten by the DMA controller, + /// in which case the rinbuffer will automatically clear itself. pub fn read(&mut self, dma: &mut impl DmaCtrl, buf: &mut [W]) -> Result<(usize, usize), OverrunError> { - self.sync_write_index(dma); - let readable = self.len()?.min(buf.len()); - for i in 0..readable { - buf[i] = self.read_buf(i); - } - self.sync_write_index(dma); - let available = self.len()?; - self.read_index.advance(self.cap(), readable); - Ok((readable, available - readable)) + self.read_raw(dma, buf).inspect_err(|_e| { + self.clear(dma); + }) } /// Read an exact number of elements from the ringbuffer. @@ -159,6 +154,18 @@ impl<'a, W: Word> ReadableDmaRingBuffer<'a, W> { .await } + fn read_raw(&mut self, dma: &mut impl DmaCtrl, buf: &mut [W]) -> Result<(usize, usize), OverrunError> { + self.sync_write_index(dma); + let readable = self.len()?.min(buf.len()); + for i in 0..readable { + buf[i] = self.read_buf(i); + } + self.sync_write_index(dma); + let available = self.len()?; + self.read_index.advance(self.cap(), readable); + Ok((readable, available - readable)) + } + fn read_buf(&self, offset: usize) -> W { unsafe { core::ptr::read_volatile( @@ -222,16 +229,13 @@ impl<'a, W: Word> WritableDmaRingBuffer<'a, W> { /// Append data to the ring buffer. /// Returns a tuple of the data written and the remaining write capacity in the buffer. + /// OverrunError is returned if the portion to be written was previously read by the DMA controller. + /// In this case, the ringbuffer will automatically reset itself, giving a full buffer worth of + /// leeway between the write index and the DMA. pub fn write(&mut self, dma: &mut impl DmaCtrl, buf: &[W]) -> Result<(usize, usize), OverrunError> { - self.sync_read_index(dma); - let writable = self.len()?.min(buf.len()); - for i in 0..writable { - self.write_buf(i, buf[i]); - } - self.sync_read_index(dma); - let available = self.len()?; - self.write_index.advance(self.cap(), writable); - Ok((writable, available - writable)) + self.write_raw(dma, buf).inspect_err(|_e| { + self.clear(dma); + }) } /// Write elements directly to the buffer. @@ -266,6 +270,18 @@ impl<'a, W: Word> WritableDmaRingBuffer<'a, W> { .await } + pub fn write_raw(&mut self, dma: &mut impl DmaCtrl, buf: &[W]) -> Result<(usize, usize), OverrunError> { + self.sync_read_index(dma); + let writable = self.len()?.min(buf.len()); + for i in 0..writable { + self.write_buf(i, buf[i]); + } + self.sync_read_index(dma); + let available = self.len()?; + self.write_index.advance(self.cap(), writable); + Ok((writable, available - writable)) + } + fn write_buf(&mut self, offset: usize, value: W) { unsafe { core::ptr::write_volatile( diff --git a/embassy-stm32/src/dma/ringbuffer/tests/prop_test/reader.rs b/embassy-stm32/src/dma/ringbuffer/tests/prop_test/reader.rs index 6555ebfb0..6e640a813 100644 --- a/embassy-stm32/src/dma/ringbuffer/tests/prop_test/reader.rs +++ b/embassy-stm32/src/dma/ringbuffer/tests/prop_test/reader.rs @@ -38,8 +38,9 @@ impl ReferenceStateMachine for ReaderSM { Status::Available(x + y) } } + (Status::Failed, ReaderTransition::Write(_)) => Status::Failed, (Status::Available(x), ReaderTransition::ReadUpTo(y)) => Status::Available(x.saturating_sub(*y)), - (Status::Failed, _) => Status::Failed, + (Status::Failed, ReaderTransition::ReadUpTo(_)) => Status::Available(0), } } } diff --git a/embassy-stm32/src/dma/ringbuffer/tests/prop_test/writer.rs b/embassy-stm32/src/dma/ringbuffer/tests/prop_test/writer.rs index 15f54c672..c1b3859b2 100644 --- a/embassy-stm32/src/dma/ringbuffer/tests/prop_test/writer.rs +++ b/embassy-stm32/src/dma/ringbuffer/tests/prop_test/writer.rs @@ -38,8 +38,9 @@ impl ReferenceStateMachine for WriterSM { Status::Available(x - y) } } + (Status::Failed, WriterTransition::Read(_)) => Status::Failed, (Status::Available(x), WriterTransition::WriteUpTo(y)) => Status::Available((x + *y).min(CAP)), - (Status::Failed, _) => Status::Failed, + (Status::Failed, WriterTransition::WriteUpTo(_)) => Status::Available(CAP), } } } From 82712252166a274c1499c3680b63f86cdfbb8dfb Mon Sep 17 00:00:00 2001 From: Alexandros Liarokapis Date: Mon, 23 Sep 2024 16:50:26 +0300 Subject: [PATCH 04/11] make len method take mut self and remove sync index calls --- embassy-stm32/src/dma/ringbuffer/mod.rs | 32 ++++++++++--------------- 1 file changed, 12 insertions(+), 20 deletions(-) diff --git a/embassy-stm32/src/dma/ringbuffer/mod.rs b/embassy-stm32/src/dma/ringbuffer/mod.rs index abc01e3cc..76f7f36ec 100644 --- a/embassy-stm32/src/dma/ringbuffer/mod.rs +++ b/embassy-stm32/src/dma/ringbuffer/mod.rs @@ -99,7 +99,10 @@ impl<'a, W: Word> ReadableDmaRingBuffer<'a, W> { } /// Get the available readable dma samples. - pub fn len(&self) -> Result { + pub fn len(&mut self, dma: &mut impl DmaCtrl) -> Result { + self.write_index.dma_sync(self.cap(), dma); + DmaIndex::normalize(&mut self.write_index, &mut self.read_index); + let diff: usize = self.write_index.diff(self.cap(), &self.read_index).try_into().unwrap(); if diff > self.cap() { @@ -155,13 +158,11 @@ impl<'a, W: Word> ReadableDmaRingBuffer<'a, W> { } fn read_raw(&mut self, dma: &mut impl DmaCtrl, buf: &mut [W]) -> Result<(usize, usize), OverrunError> { - self.sync_write_index(dma); - let readable = self.len()?.min(buf.len()); + let readable = self.len(dma)?.min(buf.len()); for i in 0..readable { buf[i] = self.read_buf(i); } - self.sync_write_index(dma); - let available = self.len()?; + let available = self.len(dma)?; self.read_index.advance(self.cap(), readable); Ok((readable, available - readable)) } @@ -175,11 +176,6 @@ impl<'a, W: Word> ReadableDmaRingBuffer<'a, W> { ) } } - - fn sync_write_index(&mut self, dma: &mut impl DmaCtrl) { - self.write_index.dma_sync(self.cap(), dma); - DmaIndex::normalize(&mut self.write_index, &mut self.read_index); - } } pub struct WritableDmaRingBuffer<'a, W: Word> { @@ -212,7 +208,10 @@ impl<'a, W: Word> WritableDmaRingBuffer<'a, W> { } /// Get the remaining writable dma samples. - pub fn len(&self) -> Result { + pub fn len(&mut self, dma: &mut impl DmaCtrl) -> Result { + self.read_index.dma_sync(self.cap(), dma); + DmaIndex::normalize(&mut self.read_index, &mut self.write_index); + let diff = self.write_index.diff(self.cap(), &self.read_index); if diff < 0 { @@ -271,13 +270,11 @@ impl<'a, W: Word> WritableDmaRingBuffer<'a, W> { } pub fn write_raw(&mut self, dma: &mut impl DmaCtrl, buf: &[W]) -> Result<(usize, usize), OverrunError> { - self.sync_read_index(dma); - let writable = self.len()?.min(buf.len()); + let writable = self.len(dma)?.min(buf.len()); for i in 0..writable { self.write_buf(i, buf[i]); } - self.sync_read_index(dma); - let available = self.len()?; + let available = self.len(dma)?; self.write_index.advance(self.cap(), writable); Ok((writable, available - writable)) } @@ -292,11 +289,6 @@ impl<'a, W: Word> WritableDmaRingBuffer<'a, W> { ) } } - - fn sync_read_index(&mut self, dma: &mut impl DmaCtrl) { - self.read_index.dma_sync(self.cap(), dma); - DmaIndex::normalize(&mut self.read_index, &mut self.write_index); - } } #[cfg(test)] From 9c7b296432af40ac44e5e65a3742c691f1414444 Mon Sep 17 00:00:00 2001 From: Alexandros Liarokapis Date: Tue, 24 Sep 2024 00:59:34 +0300 Subject: [PATCH 05/11] overrun at invalid diffs, rename clear to reset, simplify dma_sync method --- embassy-stm32/src/adc/ringbuffered_v2.rs | 3 +- embassy-stm32/src/dma/dma_bdma.rs | 4 +- embassy-stm32/src/dma/ringbuffer/mod.rs | 53 +++++++++---------- embassy-stm32/src/dma/ringbuffer/tests/mod.rs | 22 ++++---- .../dma/ringbuffer/tests/prop_test/reader.rs | 10 ++-- .../dma/ringbuffer/tests/prop_test/writer.rs | 10 ++-- 6 files changed, 49 insertions(+), 53 deletions(-) diff --git a/embassy-stm32/src/adc/ringbuffered_v2.rs b/embassy-stm32/src/adc/ringbuffered_v2.rs index 3b064044e..9fc233222 100644 --- a/embassy-stm32/src/adc/ringbuffered_v2.rs +++ b/embassy-stm32/src/adc/ringbuffered_v2.rs @@ -226,9 +226,8 @@ impl<'d, T: Instance> RingBufferedAdc<'d, T> { /// Turns on ADC if it is not already turned on and starts continuous DMA transfer. pub fn start(&mut self) -> Result<(), OverrunError> { - self.ring_buf.clear(); - self.setup_adc(); + self.ring_buf.clear(); Ok(()) } diff --git a/embassy-stm32/src/dma/dma_bdma.rs b/embassy-stm32/src/dma/dma_bdma.rs index a43137c6e..59ad4d988 100644 --- a/embassy-stm32/src/dma/dma_bdma.rs +++ b/embassy-stm32/src/dma/dma_bdma.rs @@ -833,7 +833,7 @@ impl<'a, W: Word> ReadableRingBuffer<'a, W> { /// Clear all data in the ring buffer. pub fn clear(&mut self) { - self.ringbuf.clear(&mut DmaCtrlImpl(self.channel.reborrow())); + self.ringbuf.reset(&mut DmaCtrlImpl(self.channel.reborrow())); } /// Read elements from the ring buffer @@ -980,7 +980,7 @@ impl<'a, W: Word> WritableRingBuffer<'a, W> { /// Clear all data in the ring buffer. pub fn clear(&mut self) { - self.ringbuf.clear(&mut DmaCtrlImpl(self.channel.reborrow())); + self.ringbuf.reset(&mut DmaCtrlImpl(self.channel.reborrow())); } /// Write elements directly to the raw buffer. diff --git a/embassy-stm32/src/dma/ringbuffer/mod.rs b/embassy-stm32/src/dma/ringbuffer/mod.rs index 76f7f36ec..eb64ad016 100644 --- a/embassy-stm32/src/dma/ringbuffer/mod.rs +++ b/embassy-stm32/src/dma/ringbuffer/mod.rs @@ -23,14 +23,14 @@ pub struct OverrunError; #[derive(Debug, Clone, Copy, Default)] struct DmaIndex { - completion_count: usize, + complete_count: usize, pos: usize, } impl DmaIndex { fn reset(&mut self) { self.pos = 0; - self.completion_count = 0; + self.complete_count = 0; } fn as_index(&self, cap: usize, offset: usize) -> usize { @@ -38,34 +38,31 @@ impl DmaIndex { } fn dma_sync(&mut self, cap: usize, dma: &mut impl DmaCtrl) { - let fst_pos = cap - dma.get_remaining_transfers(); - let fst_count = dma.reset_complete_count(); - let pos = cap - dma.get_remaining_transfers(); + let first_pos = cap - dma.get_remaining_transfers(); + self.complete_count += dma.reset_complete_count(); + self.pos = cap - dma.get_remaining_transfers(); - let wrap_count = if pos >= fst_pos { - fst_count - } else { - fst_count + dma.reset_complete_count() - }; - - self.pos = pos; - self.completion_count += wrap_count; + // If the latter call to get_remaining_transfers() returned a smaller value than the first, the dma + // has wrapped around between calls and we must check if the complete count also incremented. + if self.pos < first_pos { + self.complete_count += dma.reset_complete_count(); + } } fn advance(&mut self, cap: usize, steps: usize) { let next = self.pos + steps; - self.completion_count += next / cap; + self.complete_count += next / cap; self.pos = next % cap; } fn normalize(lhs: &mut DmaIndex, rhs: &mut DmaIndex) { - let min_count = lhs.completion_count.min(rhs.completion_count); - lhs.completion_count -= min_count; - rhs.completion_count -= min_count; + let min_count = lhs.complete_count.min(rhs.complete_count); + lhs.complete_count -= min_count; + rhs.complete_count -= min_count; } fn diff(&self, cap: usize, rhs: &DmaIndex) -> isize { - (self.completion_count * cap + self.pos) as isize - (rhs.completion_count * cap + rhs.pos) as isize + (self.complete_count * cap + self.pos) as isize - (rhs.complete_count * cap + rhs.pos) as isize } } @@ -86,7 +83,7 @@ impl<'a, W: Word> ReadableDmaRingBuffer<'a, W> { } /// Reset the ring buffer to its initial state. - pub fn clear(&mut self, dma: &mut impl DmaCtrl) { + pub fn reset(&mut self, dma: &mut impl DmaCtrl) { dma.reset_complete_count(); self.write_index.reset(); self.write_index.dma_sync(self.cap(), dma); @@ -103,12 +100,12 @@ impl<'a, W: Word> ReadableDmaRingBuffer<'a, W> { self.write_index.dma_sync(self.cap(), dma); DmaIndex::normalize(&mut self.write_index, &mut self.read_index); - let diff: usize = self.write_index.diff(self.cap(), &self.read_index).try_into().unwrap(); + let diff = self.write_index.diff(self.cap(), &self.read_index); - if diff > self.cap() { + if diff < 0 || diff > self.cap() as isize { Err(OverrunError) } else { - Ok(diff) + Ok(diff as usize) } } @@ -117,10 +114,10 @@ impl<'a, W: Word> ReadableDmaRingBuffer<'a, W> { /// If not all of the elements were read, then there will be some elements in the buffer remaining /// The length remaining is the capacity, ring_buf.len(), less the elements remaining after the read /// OverrunError is returned if the portion to be read was overwritten by the DMA controller, - /// in which case the rinbuffer will automatically clear itself. + /// in which case the rinbuffer will automatically reset itself. pub fn read(&mut self, dma: &mut impl DmaCtrl, buf: &mut [W]) -> Result<(usize, usize), OverrunError> { self.read_raw(dma, buf).inspect_err(|_e| { - self.clear(dma); + self.reset(dma); }) } @@ -192,14 +189,14 @@ impl<'a, W: Word> WritableDmaRingBuffer<'a, W> { dma_buf, read_index: Default::default(), write_index: DmaIndex { - completion_count: 0, + complete_count: 0, pos: len, }, } } /// Reset the ring buffer to its initial state. The buffer after the reset will be full. - pub fn clear(&mut self, dma: &mut impl DmaCtrl) { + pub fn reset(&mut self, dma: &mut impl DmaCtrl) { dma.reset_complete_count(); self.read_index.reset(); self.read_index.dma_sync(self.cap(), dma); @@ -214,7 +211,7 @@ impl<'a, W: Word> WritableDmaRingBuffer<'a, W> { let diff = self.write_index.diff(self.cap(), &self.read_index); - if diff < 0 { + if diff < 0 || diff > self.cap() as isize { Err(OverrunError) } else { Ok(self.cap().saturating_sub(diff as usize)) @@ -233,7 +230,7 @@ impl<'a, W: Word> WritableDmaRingBuffer<'a, W> { /// leeway between the write index and the DMA. pub fn write(&mut self, dma: &mut impl DmaCtrl, buf: &[W]) -> Result<(usize, usize), OverrunError> { self.write_raw(dma, buf).inspect_err(|_e| { - self.clear(dma); + self.reset(dma); }) } diff --git a/embassy-stm32/src/dma/ringbuffer/tests/mod.rs b/embassy-stm32/src/dma/ringbuffer/tests/mod.rs index 9768e1df8..1800eae69 100644 --- a/embassy-stm32/src/dma/ringbuffer/tests/mod.rs +++ b/embassy-stm32/src/dma/ringbuffer/tests/mod.rs @@ -64,12 +64,12 @@ fn dma_index_dma_sync_syncs_position_to_last_read_if_sync_takes_place_on_same_dm ]); let mut index = DmaIndex::default(); index.dma_sync(CAP, &mut dma); - assert_eq!(index.completion_count, 0); + assert_eq!(index.complete_count, 0); assert_eq!(index.pos, 7); } #[test] -fn dma_index_dma_sync_updates_completion_count_properly_if_sync_takes_place_on_same_dma_cycle() { +fn dma_index_dma_sync_updates_complete_count_properly_if_sync_takes_place_on_same_dma_cycle() { let mut dma = TestCircularTransfer::new(CAP); dma.setup(vec![ TestCircularTransferRequest::PositionRequest(4), @@ -77,9 +77,9 @@ fn dma_index_dma_sync_updates_completion_count_properly_if_sync_takes_place_on_s TestCircularTransferRequest::PositionRequest(7), ]); let mut index = DmaIndex::default(); - index.completion_count = 1; + index.complete_count = 1; index.dma_sync(CAP, &mut dma); - assert_eq!(index.completion_count, 3); + assert_eq!(index.complete_count, 3); assert_eq!(index.pos, 7); } @@ -94,12 +94,12 @@ fn dma_index_dma_sync_syncs_to_last_position_if_reads_occur_on_different_dma_cyc ]); let mut index = DmaIndex::default(); index.dma_sync(CAP, &mut dma); - assert_eq!(index.completion_count, 1); + assert_eq!(index.complete_count, 1); assert_eq!(index.pos, 5); } #[test] -fn dma_index_dma_sync_detects_new_cycle_if_later_position_is_less_than_first_and_first_completion_count_occurs_on_first_cycle( +fn dma_index_dma_sync_detects_new_cycle_if_later_position_is_less_than_first_and_first_complete_count_occurs_on_first_cycle( ) { let mut dma = TestCircularTransfer::new(CAP); dma.setup(vec![ @@ -109,14 +109,14 @@ fn dma_index_dma_sync_detects_new_cycle_if_later_position_is_less_than_first_and TestCircularTransferRequest::ResetCompleteCount(1), ]); let mut index = DmaIndex::default(); - index.completion_count = 1; + index.complete_count = 1; index.dma_sync(CAP, &mut dma); - assert_eq!(index.completion_count, 3); + assert_eq!(index.complete_count, 3); assert_eq!(index.pos, 5); } #[test] -fn dma_index_dma_sync_detects_new_cycle_if_later_position_is_less_than_first_and_first_completion_count_occurs_on_later_cycle( +fn dma_index_dma_sync_detects_new_cycle_if_later_position_is_less_than_first_and_first_complete_count_occurs_on_later_cycle( ) { let mut dma = TestCircularTransfer::new(CAP); dma.setup(vec![ @@ -126,9 +126,9 @@ fn dma_index_dma_sync_detects_new_cycle_if_later_position_is_less_than_first_and TestCircularTransferRequest::ResetCompleteCount(0), ]); let mut index = DmaIndex::default(); - index.completion_count = 1; + index.complete_count = 1; index.dma_sync(CAP, &mut dma); - assert_eq!(index.completion_count, 3); + assert_eq!(index.complete_count, 3); assert_eq!(index.pos, 5); } diff --git a/embassy-stm32/src/dma/ringbuffer/tests/prop_test/reader.rs b/embassy-stm32/src/dma/ringbuffer/tests/prop_test/reader.rs index 6e640a813..4f3957a68 100644 --- a/embassy-stm32/src/dma/ringbuffer/tests/prop_test/reader.rs +++ b/embassy-stm32/src/dma/ringbuffer/tests/prop_test/reader.rs @@ -5,7 +5,7 @@ use super::*; #[derive(Debug, Clone)] enum ReaderTransition { Write(usize), - Clear, + Reset, ReadUpTo(usize), } @@ -23,14 +23,14 @@ impl ReferenceStateMachine for ReaderSM { prop_oneof![ (1..50_usize).prop_map(ReaderTransition::Write), (1..50_usize).prop_map(ReaderTransition::ReadUpTo), - strategy::Just(ReaderTransition::Clear), + strategy::Just(ReaderTransition::Reset), ] .boxed() } fn apply(status: Self::State, transition: &Self::Transition) -> Self::State { match (status, transition) { - (_, ReaderTransition::Clear) => Status::Available(0), + (_, ReaderTransition::Reset) => Status::Available(0), (Status::Available(x), ReaderTransition::Write(y)) => { if x + y > CAP { Status::Failed @@ -87,8 +87,8 @@ impl StateMachineTest for ReaderTest { ) -> Self::SystemUnderTest { match transition { ReaderTransition::Write(x) => sut.producer.advance(x), - ReaderTransition::Clear => { - sut.consumer.clear(&mut sut.producer); + ReaderTransition::Reset => { + sut.consumer.reset(&mut sut.producer); } ReaderTransition::ReadUpTo(x) => { let status = sut.status; diff --git a/embassy-stm32/src/dma/ringbuffer/tests/prop_test/writer.rs b/embassy-stm32/src/dma/ringbuffer/tests/prop_test/writer.rs index c1b3859b2..15433c0ee 100644 --- a/embassy-stm32/src/dma/ringbuffer/tests/prop_test/writer.rs +++ b/embassy-stm32/src/dma/ringbuffer/tests/prop_test/writer.rs @@ -6,7 +6,7 @@ use super::*; enum WriterTransition { Read(usize), WriteUpTo(usize), - Clear, + Reset, } struct WriterSM; @@ -23,14 +23,14 @@ impl ReferenceStateMachine for WriterSM { prop_oneof![ (1..50_usize).prop_map(WriterTransition::Read), (1..50_usize).prop_map(WriterTransition::WriteUpTo), - strategy::Just(WriterTransition::Clear), + strategy::Just(WriterTransition::Reset), ] .boxed() } fn apply(status: Self::State, transition: &Self::Transition) -> Self::State { match (status, transition) { - (_, WriterTransition::Clear) => Status::Available(CAP), + (_, WriterTransition::Reset) => Status::Available(CAP), (Status::Available(x), WriterTransition::Read(y)) => { if x < *y { Status::Failed @@ -87,8 +87,8 @@ impl StateMachineTest for WriterTest { ) -> Self::SystemUnderTest { match transition { WriterTransition::Read(x) => sut.consumer.advance(x), - WriterTransition::Clear => { - sut.producer.clear(&mut sut.consumer); + WriterTransition::Reset => { + sut.producer.reset(&mut sut.consumer); } WriterTransition::WriteUpTo(x) => { let status = sut.status; From c991ddb76662cc7da5e847f33c1a446f17822887 Mon Sep 17 00:00:00 2001 From: Alexandros Liarokapis Date: Tue, 24 Sep 2024 17:46:53 +0300 Subject: [PATCH 06/11] use request_pause instead of request_stop at adc shutdown --- embassy-stm32/src/adc/ringbuffered_v2.rs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/embassy-stm32/src/adc/ringbuffered_v2.rs b/embassy-stm32/src/adc/ringbuffered_v2.rs index 9fc233222..9c114e463 100644 --- a/embassy-stm32/src/adc/ringbuffered_v2.rs +++ b/embassy-stm32/src/adc/ringbuffered_v2.rs @@ -244,7 +244,7 @@ impl<'d, T: Instance> RingBufferedAdc<'d, T> { /// [`start`]: #method.start pub fn teardown_adc(&mut self) { // Stop the DMA transfer - self.ring_buf.request_stop(); + self.ring_buf.request_pause(); let r = T::regs(); From f0d2ebdc7ead41307155b083790b8450ca2b7eac Mon Sep 17 00:00:00 2001 From: Alexandros Liarokapis Date: Tue, 1 Oct 2024 17:59:04 +0300 Subject: [PATCH 07/11] stm32: fix ringbugger overrun errors due to bad dma wrap-around behavior --- embassy-stm32/src/dma/dma_bdma.rs | 30 ++++--- embassy-stm32/src/dma/ringbuffer/mod.rs | 65 +++++++++------ embassy-stm32/src/dma/ringbuffer/tests/mod.rs | 83 +------------------ embassy-stm32/src/sai/mod.rs | 10 ++- embassy-stm32/src/usart/ringbuffered.rs | 1 - 5 files changed, 72 insertions(+), 117 deletions(-) diff --git a/embassy-stm32/src/dma/dma_bdma.rs b/embassy-stm32/src/dma/dma_bdma.rs index 59ad4d988..3f047a9a4 100644 --- a/embassy-stm32/src/dma/dma_bdma.rs +++ b/embassy-stm32/src/dma/dma_bdma.rs @@ -6,7 +6,7 @@ use core::task::{Context, Poll, Waker}; use embassy_hal_internal::{into_ref, Peripheral, PeripheralRef}; use embassy_sync::waitqueue::AtomicWaker; -use super::ringbuffer::{DmaCtrl, OverrunError, ReadableDmaRingBuffer, WritableDmaRingBuffer}; +use super::ringbuffer::{DmaCtrl, Error, ReadableDmaRingBuffer, WritableDmaRingBuffer}; use super::word::{Word, WordSize}; use super::{AnyChannel, Channel, Dir, Request, STATE}; use crate::interrupt::typelevel::Interrupt; @@ -299,7 +299,6 @@ impl AnyChannel { } else { return; } - state.waker.wake(); } #[cfg(bdma)] @@ -828,7 +827,8 @@ impl<'a, W: Word> ReadableRingBuffer<'a, W> { /// /// You must call this after creating it for it to work. pub fn start(&mut self) { - self.channel.start() + self.channel.start(); + self.clear(); } /// Clear all data in the ring buffer. @@ -840,15 +840,15 @@ impl<'a, W: Word> ReadableRingBuffer<'a, W> { /// Return a tuple of the length read and the length remaining in the buffer /// If not all of the elements were read, then there will be some elements in the buffer remaining /// The length remaining is the capacity, ring_buf.len(), less the elements remaining after the read - /// OverrunError is returned if the portion to be read was overwritten by the DMA controller. - pub fn read(&mut self, buf: &mut [W]) -> Result<(usize, usize), OverrunError> { + /// Error is returned if the portion to be read was overwritten by the DMA controller. + pub fn read(&mut self, buf: &mut [W]) -> Result<(usize, usize), Error> { self.ringbuf.read(&mut DmaCtrlImpl(self.channel.reborrow()), buf) } /// Read an exact number of elements from the ringbuffer. /// /// Returns the remaining number of elements available for immediate reading. - /// OverrunError is returned if the portion to be read was overwritten by the DMA controller. + /// Error is returned if the portion to be read was overwritten by the DMA controller. /// /// Async/Wake Behavior: /// The underlying DMA peripheral only can wake us when its buffer pointer has reached the halfway point, @@ -856,12 +856,17 @@ impl<'a, W: Word> ReadableRingBuffer<'a, W> { /// ring buffer was created with a buffer of size 'N': /// - If M equals N/2 or N/2 divides evenly into M, this function will return every N/2 elements read on the DMA source. /// - Otherwise, this function may need up to N/2 extra elements to arrive before returning. - pub async fn read_exact(&mut self, buffer: &mut [W]) -> Result { + pub async fn read_exact(&mut self, buffer: &mut [W]) -> Result { self.ringbuf .read_exact(&mut DmaCtrlImpl(self.channel.reborrow()), buffer) .await } + /// The current length of the ringbuffer + pub fn len(&mut self) -> Result { + Ok(self.ringbuf.len(&mut DmaCtrlImpl(self.channel.reborrow()))?) + } + /// The capacity of the ringbuffer pub const fn capacity(&self) -> usize { self.ringbuf.cap() @@ -986,23 +991,28 @@ impl<'a, W: Word> WritableRingBuffer<'a, W> { /// Write elements directly to the raw buffer. /// This can be used to fill the buffer before starting the DMA transfer. #[allow(dead_code)] - pub fn write_immediate(&mut self, buf: &[W]) -> Result<(usize, usize), OverrunError> { + pub fn write_immediate(&mut self, buf: &[W]) -> Result<(usize, usize), Error> { self.ringbuf.write_immediate(buf) } /// Write elements from the ring buffer /// Return a tuple of the length written and the length remaining in the buffer - pub fn write(&mut self, buf: &[W]) -> Result<(usize, usize), OverrunError> { + pub fn write(&mut self, buf: &[W]) -> Result<(usize, usize), Error> { self.ringbuf.write(&mut DmaCtrlImpl(self.channel.reborrow()), buf) } /// Write an exact number of elements to the ringbuffer. - pub async fn write_exact(&mut self, buffer: &[W]) -> Result { + pub async fn write_exact(&mut self, buffer: &[W]) -> Result { self.ringbuf .write_exact(&mut DmaCtrlImpl(self.channel.reborrow()), buffer) .await } + /// The current length of the ringbuffer + pub fn len(&mut self) -> Result { + Ok(self.ringbuf.len(&mut DmaCtrlImpl(self.channel.reborrow()))?) + } + /// The capacity of the ringbuffer pub const fn capacity(&self) -> usize { self.ringbuf.cap() diff --git a/embassy-stm32/src/dma/ringbuffer/mod.rs b/embassy-stm32/src/dma/ringbuffer/mod.rs index eb64ad016..a257faa5b 100644 --- a/embassy-stm32/src/dma/ringbuffer/mod.rs +++ b/embassy-stm32/src/dma/ringbuffer/mod.rs @@ -19,9 +19,13 @@ pub trait DmaCtrl { #[derive(Debug, PartialEq)] #[cfg_attr(feature = "defmt", derive(defmt::Format))] -pub struct OverrunError; +pub enum Error { + Overrun, + DmaUnsynced, +} #[derive(Debug, Clone, Copy, Default)] +#[cfg_attr(feature = "defmt", derive(defmt::Format))] struct DmaIndex { complete_count: usize, pos: usize, @@ -38,15 +42,19 @@ impl DmaIndex { } fn dma_sync(&mut self, cap: usize, dma: &mut impl DmaCtrl) { - let first_pos = cap - dma.get_remaining_transfers(); - self.complete_count += dma.reset_complete_count(); - self.pos = cap - dma.get_remaining_transfers(); + // Important! + // The ordering of the first two lines matters! + // If changed, the code will detect a wrong +capacity + // jump at wrap-around. + let count_diff = dma.reset_complete_count(); + let pos = cap - dma.get_remaining_transfers(); + self.pos = if pos < self.pos && count_diff == 0 { + cap - 1 + } else { + pos + }; - // If the latter call to get_remaining_transfers() returned a smaller value than the first, the dma - // has wrapped around between calls and we must check if the complete count also incremented. - if self.pos < first_pos { - self.complete_count += dma.reset_complete_count(); - } + self.complete_count += count_diff; } fn advance(&mut self, cap: usize, steps: usize) { @@ -96,14 +104,18 @@ impl<'a, W: Word> ReadableDmaRingBuffer<'a, W> { } /// Get the available readable dma samples. - pub fn len(&mut self, dma: &mut impl DmaCtrl) -> Result { + pub fn len(&mut self, dma: &mut impl DmaCtrl) -> Result { self.write_index.dma_sync(self.cap(), dma); DmaIndex::normalize(&mut self.write_index, &mut self.read_index); let diff = self.write_index.diff(self.cap(), &self.read_index); - if diff < 0 || diff > self.cap() as isize { - Err(OverrunError) + if diff < 0 { + return Err(Error::DmaUnsynced); + } + + if diff > self.cap() as isize { + Err(Error::Overrun) } else { Ok(diff as usize) } @@ -113,9 +125,9 @@ impl<'a, W: Word> ReadableDmaRingBuffer<'a, W> { /// Return a tuple of the length read and the length remaining in the buffer /// If not all of the elements were read, then there will be some elements in the buffer remaining /// The length remaining is the capacity, ring_buf.len(), less the elements remaining after the read - /// OverrunError is returned if the portion to be read was overwritten by the DMA controller, + /// Error is returned if the portion to be read was overwritten by the DMA controller, /// in which case the rinbuffer will automatically reset itself. - pub fn read(&mut self, dma: &mut impl DmaCtrl, buf: &mut [W]) -> Result<(usize, usize), OverrunError> { + pub fn read(&mut self, dma: &mut impl DmaCtrl, buf: &mut [W]) -> Result<(usize, usize), Error> { self.read_raw(dma, buf).inspect_err(|_e| { self.reset(dma); }) @@ -124,7 +136,7 @@ impl<'a, W: Word> ReadableDmaRingBuffer<'a, W> { /// Read an exact number of elements from the ringbuffer. /// /// Returns the remaining number of elements available for immediate reading. - /// OverrunError is returned if the portion to be read was overwritten by the DMA controller. + /// Error is returned if the portion to be read was overwritten by the DMA controller. /// /// Async/Wake Behavior: /// The underlying DMA peripheral only can wake us when its buffer pointer has reached the halfway point, @@ -132,7 +144,7 @@ impl<'a, W: Word> ReadableDmaRingBuffer<'a, W> { /// ring buffer was created with a buffer of size 'N': /// - If M equals N/2 or N/2 divides evenly into M, this function will return every N/2 elements read on the DMA source. /// - Otherwise, this function may need up to N/2 extra elements to arrive before returning. - pub async fn read_exact(&mut self, dma: &mut impl DmaCtrl, buffer: &mut [W]) -> Result { + pub async fn read_exact(&mut self, dma: &mut impl DmaCtrl, buffer: &mut [W]) -> Result { let mut read_data = 0; let buffer_len = buffer.len(); @@ -154,7 +166,7 @@ impl<'a, W: Word> ReadableDmaRingBuffer<'a, W> { .await } - fn read_raw(&mut self, dma: &mut impl DmaCtrl, buf: &mut [W]) -> Result<(usize, usize), OverrunError> { + fn read_raw(&mut self, dma: &mut impl DmaCtrl, buf: &mut [W]) -> Result<(usize, usize), Error> { let readable = self.len(dma)?.min(buf.len()); for i in 0..readable { buf[i] = self.read_buf(i); @@ -205,14 +217,17 @@ impl<'a, W: Word> WritableDmaRingBuffer<'a, W> { } /// Get the remaining writable dma samples. - pub fn len(&mut self, dma: &mut impl DmaCtrl) -> Result { + pub fn len(&mut self, dma: &mut impl DmaCtrl) -> Result { self.read_index.dma_sync(self.cap(), dma); DmaIndex::normalize(&mut self.read_index, &mut self.write_index); let diff = self.write_index.diff(self.cap(), &self.read_index); - if diff < 0 || diff > self.cap() as isize { - Err(OverrunError) + if diff > self.cap() as isize { + return Err(Error::DmaUnsynced); + } + if diff < 0 { + Err(Error::Overrun) } else { Ok(self.cap().saturating_sub(diff as usize)) } @@ -225,17 +240,17 @@ impl<'a, W: Word> WritableDmaRingBuffer<'a, W> { /// Append data to the ring buffer. /// Returns a tuple of the data written and the remaining write capacity in the buffer. - /// OverrunError is returned if the portion to be written was previously read by the DMA controller. + /// Error is returned if the portion to be written was previously read by the DMA controller. /// In this case, the ringbuffer will automatically reset itself, giving a full buffer worth of /// leeway between the write index and the DMA. - pub fn write(&mut self, dma: &mut impl DmaCtrl, buf: &[W]) -> Result<(usize, usize), OverrunError> { + pub fn write(&mut self, dma: &mut impl DmaCtrl, buf: &[W]) -> Result<(usize, usize), Error> { self.write_raw(dma, buf).inspect_err(|_e| { self.reset(dma); }) } /// Write elements directly to the buffer. - pub fn write_immediate(&mut self, buf: &[W]) -> Result<(usize, usize), OverrunError> { + pub fn write_immediate(&mut self, buf: &[W]) -> Result<(usize, usize), Error> { for (i, data) in buf.iter().enumerate() { self.write_buf(i, *data) } @@ -244,7 +259,7 @@ impl<'a, W: Word> WritableDmaRingBuffer<'a, W> { } /// Write an exact number of elements to the ringbuffer. - pub async fn write_exact(&mut self, dma: &mut impl DmaCtrl, buffer: &[W]) -> Result { + pub async fn write_exact(&mut self, dma: &mut impl DmaCtrl, buffer: &[W]) -> Result { let mut written_data = 0; let buffer_len = buffer.len(); @@ -266,7 +281,7 @@ impl<'a, W: Word> WritableDmaRingBuffer<'a, W> { .await } - pub fn write_raw(&mut self, dma: &mut impl DmaCtrl, buf: &[W]) -> Result<(usize, usize), OverrunError> { + fn write_raw(&mut self, dma: &mut impl DmaCtrl, buf: &[W]) -> Result<(usize, usize), Error> { let writable = self.len(dma)?.min(buf.len()); for i in 0..writable { self.write_buf(i, buf[i]); diff --git a/embassy-stm32/src/dma/ringbuffer/tests/mod.rs b/embassy-stm32/src/dma/ringbuffer/tests/mod.rs index 1800eae69..6fabedb83 100644 --- a/embassy-stm32/src/dma/ringbuffer/tests/mod.rs +++ b/embassy-stm32/src/dma/ringbuffer/tests/mod.rs @@ -2,13 +2,14 @@ use std::{cell, vec}; use super::*; -#[allow(dead_code)] +#[allow(unused)] #[derive(PartialEq, Debug)] enum TestCircularTransferRequest { ResetCompleteCount(usize), PositionRequest(usize), } +#[allow(unused)] struct TestCircularTransfer { len: usize, requests: cell::RefCell>, @@ -39,6 +40,7 @@ impl DmaCtrl for TestCircularTransfer { } impl TestCircularTransfer { + #[allow(unused)] pub fn new(len: usize) -> Self { Self { requests: cell::RefCell::new(vec![]), @@ -46,6 +48,7 @@ impl TestCircularTransfer { } } + #[allow(unused)] pub fn setup(&self, mut requests: vec::Vec) { requests.reverse(); self.requests.replace(requests); @@ -54,84 +57,6 @@ impl TestCircularTransfer { const CAP: usize = 16; -#[test] -fn dma_index_dma_sync_syncs_position_to_last_read_if_sync_takes_place_on_same_dma_cycle() { - let mut dma = TestCircularTransfer::new(CAP); - dma.setup(vec![ - TestCircularTransferRequest::PositionRequest(4), - TestCircularTransferRequest::ResetCompleteCount(0), - TestCircularTransferRequest::PositionRequest(7), - ]); - let mut index = DmaIndex::default(); - index.dma_sync(CAP, &mut dma); - assert_eq!(index.complete_count, 0); - assert_eq!(index.pos, 7); -} - -#[test] -fn dma_index_dma_sync_updates_complete_count_properly_if_sync_takes_place_on_same_dma_cycle() { - let mut dma = TestCircularTransfer::new(CAP); - dma.setup(vec![ - TestCircularTransferRequest::PositionRequest(4), - TestCircularTransferRequest::ResetCompleteCount(2), - TestCircularTransferRequest::PositionRequest(7), - ]); - let mut index = DmaIndex::default(); - index.complete_count = 1; - index.dma_sync(CAP, &mut dma); - assert_eq!(index.complete_count, 3); - assert_eq!(index.pos, 7); -} - -#[test] -fn dma_index_dma_sync_syncs_to_last_position_if_reads_occur_on_different_dma_cycles() { - let mut dma = TestCircularTransfer::new(CAP); - dma.setup(vec![ - TestCircularTransferRequest::PositionRequest(10), - TestCircularTransferRequest::ResetCompleteCount(1), - TestCircularTransferRequest::PositionRequest(5), - TestCircularTransferRequest::ResetCompleteCount(0), - ]); - let mut index = DmaIndex::default(); - index.dma_sync(CAP, &mut dma); - assert_eq!(index.complete_count, 1); - assert_eq!(index.pos, 5); -} - -#[test] -fn dma_index_dma_sync_detects_new_cycle_if_later_position_is_less_than_first_and_first_complete_count_occurs_on_first_cycle( -) { - let mut dma = TestCircularTransfer::new(CAP); - dma.setup(vec![ - TestCircularTransferRequest::PositionRequest(10), - TestCircularTransferRequest::ResetCompleteCount(1), - TestCircularTransferRequest::PositionRequest(5), - TestCircularTransferRequest::ResetCompleteCount(1), - ]); - let mut index = DmaIndex::default(); - index.complete_count = 1; - index.dma_sync(CAP, &mut dma); - assert_eq!(index.complete_count, 3); - assert_eq!(index.pos, 5); -} - -#[test] -fn dma_index_dma_sync_detects_new_cycle_if_later_position_is_less_than_first_and_first_complete_count_occurs_on_later_cycle( -) { - let mut dma = TestCircularTransfer::new(CAP); - dma.setup(vec![ - TestCircularTransferRequest::PositionRequest(10), - TestCircularTransferRequest::ResetCompleteCount(2), - TestCircularTransferRequest::PositionRequest(5), - TestCircularTransferRequest::ResetCompleteCount(0), - ]); - let mut index = DmaIndex::default(); - index.complete_count = 1; - index.dma_sync(CAP, &mut dma); - assert_eq!(index.complete_count, 3); - assert_eq!(index.pos, 5); -} - #[test] fn dma_index_as_index_returns_index_mod_cap_by_default() { let index = DmaIndex::default(); diff --git a/embassy-stm32/src/sai/mod.rs b/embassy-stm32/src/sai/mod.rs index 63f48ace0..7d2f071de 100644 --- a/embassy-stm32/src/sai/mod.rs +++ b/embassy-stm32/src/sai/mod.rs @@ -27,8 +27,14 @@ pub enum Error { } #[cfg(not(gpdma))] -impl From for Error { - fn from(_: ringbuffer::OverrunError) -> Self { +impl From for Error { + fn from(#[allow(unused)] err: ringbuffer::Error) -> Self { + #[cfg(feature = "defmt")] + { + if err == ringbuffer::Error::DmaUnsynced { + defmt::error!("Ringbuffer broken invariants detected!"); + } + } Self::Overrun } } diff --git a/embassy-stm32/src/usart/ringbuffered.rs b/embassy-stm32/src/usart/ringbuffered.rs index 75834bf37..eb2399d9c 100644 --- a/embassy-stm32/src/usart/ringbuffered.rs +++ b/embassy-stm32/src/usart/ringbuffered.rs @@ -83,7 +83,6 @@ impl<'d> RingBufferedUartRx<'d> { // Clear the buffer so that it is ready to receive data compiler_fence(Ordering::SeqCst); self.ring_buf.start(); - self.ring_buf.clear(); let r = self.info.regs; // clear all interrupts and DMA Rx Request From 2ec05da5ddadad9b34c72e8ef1a57a7662a6f0e0 Mon Sep 17 00:00:00 2001 From: Alexandros Liarokapis Date: Wed, 2 Oct 2024 13:10:07 +0300 Subject: [PATCH 08/11] simplify if/else handling on ringbuffer --- embassy-stm32/src/dma/ringbuffer/mod.rs | 11 ++++------- 1 file changed, 4 insertions(+), 7 deletions(-) diff --git a/embassy-stm32/src/dma/ringbuffer/mod.rs b/embassy-stm32/src/dma/ringbuffer/mod.rs index a257faa5b..ac4be3e18 100644 --- a/embassy-stm32/src/dma/ringbuffer/mod.rs +++ b/embassy-stm32/src/dma/ringbuffer/mod.rs @@ -111,10 +111,8 @@ impl<'a, W: Word> ReadableDmaRingBuffer<'a, W> { let diff = self.write_index.diff(self.cap(), &self.read_index); if diff < 0 { - return Err(Error::DmaUnsynced); - } - - if diff > self.cap() as isize { + Err(Error::DmaUnsynced) + } else if diff > self.cap() as isize { Err(Error::Overrun) } else { Ok(diff as usize) @@ -223,11 +221,10 @@ impl<'a, W: Word> WritableDmaRingBuffer<'a, W> { let diff = self.write_index.diff(self.cap(), &self.read_index); - if diff > self.cap() as isize { - return Err(Error::DmaUnsynced); - } if diff < 0 { Err(Error::Overrun) + } else if diff > self.cap() as isize { + Err(Error::DmaUnsynced) } else { Ok(self.cap().saturating_sub(diff as usize)) } From d280b234283d3c65e7ba6087e52005160a538ad2 Mon Sep 17 00:00:00 2001 From: Alexandros Liarokapis Date: Thu, 3 Oct 2024 13:04:15 +0300 Subject: [PATCH 09/11] fix adc/ringbuffered_v2.rs --- embassy-stm32/src/adc/ringbuffered_v2.rs | 4 +++- embassy-stm32/src/dma/ringbuffer/mod.rs | 3 ++- embassy-stm32/src/lib.rs | 3 +++ 3 files changed, 8 insertions(+), 2 deletions(-) diff --git a/embassy-stm32/src/adc/ringbuffered_v2.rs b/embassy-stm32/src/adc/ringbuffered_v2.rs index 9c114e463..3f0c1a57a 100644 --- a/embassy-stm32/src/adc/ringbuffered_v2.rs +++ b/embassy-stm32/src/adc/ringbuffered_v2.rs @@ -6,11 +6,13 @@ use embassy_hal_internal::{into_ref, Peripheral}; use stm32_metapac::adc::vals::SampleTime; use crate::adc::{Adc, AdcChannel, Instance, RxDma}; -use crate::dma::ringbuffer::OverrunError; use crate::dma::{Priority, ReadableRingBuffer, TransferOptions}; use crate::pac::adc::vals; use crate::rcc; +#[cfg_attr(feature = "defmt", derive(defmt::Format))] +pub struct OverrunError; + fn clear_interrupt_flags(r: crate::pac::adc::Adc) { r.sr().modify(|regs| { regs.set_eoc(false); diff --git a/embassy-stm32/src/dma/ringbuffer/mod.rs b/embassy-stm32/src/dma/ringbuffer/mod.rs index ac4be3e18..12d418414 100644 --- a/embassy-stm32/src/dma/ringbuffer/mod.rs +++ b/embassy-stm32/src/dma/ringbuffer/mod.rs @@ -119,7 +119,8 @@ impl<'a, W: Word> ReadableDmaRingBuffer<'a, W> { } } - /// Read elements from the ring buffer + /// Read elements from the ring buffer. + /// /// Return a tuple of the length read and the length remaining in the buffer /// If not all of the elements were read, then there will be some elements in the buffer remaining /// The length remaining is the capacity, ring_buf.len(), less the elements remaining after the read diff --git a/embassy-stm32/src/lib.rs b/embassy-stm32/src/lib.rs index 451f595e0..5f103e652 100644 --- a/embassy-stm32/src/lib.rs +++ b/embassy-stm32/src/lib.rs @@ -296,6 +296,9 @@ mod dual_core { /// It cannot be initialized by the user. The intended use is: /// /// ``` + /// use core::mem::MaybeUninit; + /// use embassy_stm32::{init_secondary, SharedData}; + /// /// #[link_section = ".ram_d3"] /// static SHARED_DATA: MaybeUninit = MaybeUninit::uninit(); /// From 4f810e47f5244b0cb7780ae1eebdba108d9e88c9 Mon Sep 17 00:00:00 2001 From: Alexandros Liarokapis Date: Thu, 3 Oct 2024 21:54:47 +0300 Subject: [PATCH 10/11] enable ci usart tests --- ci.sh | 5 ----- 1 file changed, 5 deletions(-) diff --git a/ci.sh b/ci.sh index ee47ee1c5..aa078be31 100755 --- a/ci.sh +++ b/ci.sh @@ -305,11 +305,6 @@ rm out/tests/stm32f207zg/eth # doesn't work, gives "noise error", no idea why. usart_dma does pass. rm out/tests/stm32u5a5zj/usart -# flaky, probably due to bad ringbuffered dma code. -rm out/tests/stm32l152re/usart_rx_ringbuffered -rm out/tests/stm32f207zg/usart_rx_ringbuffered -rm out/tests/stm32wl55jc/usart_rx_ringbuffered - if [[ -z "${TELEPROBE_TOKEN-}" ]]; then echo No teleprobe token found, skipping running HIL tests exit From 28d03537e958213dab345ad4502dad2b32256135 Mon Sep 17 00:00:00 2001 From: Alexandros Liarokapis Date: Mon, 7 Oct 2024 23:12:35 +0300 Subject: [PATCH 11/11] stm32: Automatically clear on WritableRingBuffer start --- embassy-stm32/src/dma/dma_bdma.rs | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/embassy-stm32/src/dma/dma_bdma.rs b/embassy-stm32/src/dma/dma_bdma.rs index 3f047a9a4..cdc603e2c 100644 --- a/embassy-stm32/src/dma/dma_bdma.rs +++ b/embassy-stm32/src/dma/dma_bdma.rs @@ -980,7 +980,8 @@ impl<'a, W: Word> WritableRingBuffer<'a, W> { /// /// You must call this after creating it for it to work. pub fn start(&mut self) { - self.channel.start() + self.channel.start(); + self.clear(); } /// Clear all data in the ring buffer.