overrun at invalid diffs, rename clear to reset, simplify dma_sync method

6 files changed, 50 insertions, 54 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 wrap_count = if pos >= fst_pos {
-            fst_count
-        } else {
-            fst_count + dma.reset_complete_count()
-        };
-
-        self.pos = pos;
-        self.completion_count += wrap_count;
+        let first_pos = cap - dma.get_remaining_transfers();
+        self.complete_count += dma.reset_complete_count();
+        self.pos = cap - dma.get_remaining_transfers();
+
+        // 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;