mavlink: harden production reception and transmission

libs/kaptan-mavlink/src/transmitter.rs

@@ -54,20 +54,66 @@ where
         class: FrameClass,
         frame: MavlinkFrame,
     ) -> Result<EnqueueResult, MavlinkTransmitFault<Memory::Error, Engine::StartError>> {
-        let admission = self.transmit.enqueue(PendingFrame { class, frame });
-        self.start_next().map(|()| admission)
+        let admission = self.enqueue_frame(class, frame);
+        self.start_pending().map(|()| admission)
+    }
+
+    /// Admits a frame without attempting to start the DMA engine.
+    ///
+    /// This split lifecycle is for hardware routes whose terminal DMA IRQ can
+    /// arrive a few bus cycles before the stream's `EN` bit reads clear. Such
+    /// a route may acknowledge a completion, retain the selected frame, and
+    /// retry [`Self::start_pending`] from its next bounded service turn rather
+    /// than treating that transient `Busy` observation as a transport fault.
+    /// Callers still own the policy for a queue-full result.
+    pub fn enqueue_frame(&mut self, class: FrameClass, frame: MavlinkFrame) -> EnqueueResult {
+        self.transmit.enqueue(PendingFrame { class, frame })
+    }
+
+    /// Attempts to start one pending DMA frame.
+    ///
+    /// A rejected engine start leaves the exact frame reserved for a later
+    /// retry. This method therefore does not discard or overwrite DMA source
+    /// storage on a transient hardware-not-ready result.
+    pub fn start_pending(
+        &mut self,
+    ) -> Result<(), MavlinkTransmitFault<Memory::Error, Engine::StartError>> {
+        self.start_next()
     }
 
     /// Releases a terminal DMA frame and opportunistically starts the next.
-    pub fn on_dma_complete(&mut self, completion: TxCompletion) {
+    ///
+    /// The returned class is the frame that reached a terminal DMA result,
+    /// not a class selected from newly pending work. A policy can therefore
+    /// distinguish completion classes without inferring them from newly
+    /// pending work.
+    pub fn on_dma_complete(
+        &mut self,
+        completion: TxCompletion,
+    ) -> Result<Option<FrameClass>, MavlinkTransmitFault<Memory::Error, Engine::StartError>> {
+        let completed = self.complete_dma(completion)?;
+        if completion == TxCompletion::Complete {
+            self.start_pending()?;
+        }
+        Ok(completed)
+    }
+
+    /// Releases the active frame after a terminal DMA result without starting
+    /// another one.
+    ///
+    /// Most routes use [`Self::on_dma_complete`], which preserves legacy
+    /// immediate restart behavior. A route whose hardware exposes terminal
+    /// status before its enable bit is observably low uses this method and
+    /// calls [`Self::start_pending`] from a later bounded service turn.
+    pub fn complete_dma(
+        &mut self,
+        completion: TxCompletion,
+    ) -> Result<Option<FrameClass>, MavlinkTransmitFault<Memory::Error, Engine::StartError>> {
         match completion {
-            TxCompletion::NotPending => {}
-            TxCompletion::Complete => {
-                let _ = self.transmit.complete();
-                let _ = self.start_next();
-            }
+            TxCompletion::NotPending => Ok(None),
+            TxCompletion::Complete => Ok(self.transmit.complete().map(|frame| frame.class)),
             TxCompletion::TransferFault | TxCompletion::Halted => {
-                let _ = self.transmit.fail();
+                Ok(self.transmit.fail().map(|frame| frame.class))
             }
         }
     }
@@ -89,7 +135,7 @@ where
 
 #[cfg(test)]
 mod tests {
-    use super::{EnqueueResult, FrameClass, MavlinkTransmitter};
+    use super::{EnqueueResult, FrameClass, MavlinkTransmitFault, MavlinkTransmitter};
     use crate::{encoder::MavlinkEncoder, message::MavState};
     use kaptan_dma::{DmaReadableBuffer, DmaTxBufferFactory, DmaTxEngine, TxCompletion};
 
@@ -114,6 +160,44 @@ mod tests {
         }
     }
 
+    struct RecordingEngine {
+        started_sequences: [u8; 4],
+        starts: usize,
+    }
+
+    #[derive(Clone, Copy, Debug, Eq, PartialEq)]
+    enum RetryError {
+        Busy,
+    }
+
+    struct BusyOnceEngine {
+        busy_once: bool,
+        starts: usize,
+    }
+
+    impl DmaTxEngine for BusyOnceEngine {
+        type StartError = RetryError;
+
+        fn start(&mut self, _: DmaReadableBuffer<'_>) -> Result<(), Self::StartError> {
+            if self.busy_once {
+                self.busy_once = false;
+                return Err(RetryError::Busy);
+            }
+            self.starts += 1;
+            Ok(())
+        }
+    }
+
+    impl DmaTxEngine for RecordingEngine {
+        type StartError = core::convert::Infallible;
+
+        fn start(&mut self, buffer: DmaReadableBuffer<'_>) -> Result<(), Self::StartError> {
+            self.started_sequences[self.starts] = buffer.bytes()[4];
+            self.starts += 1;
+            Ok(())
+        }
+    }
+
     #[test]
     fn transfer_fault_drops_only_the_active_frame() {
         let mut encoder = MavlinkEncoder::new(1, 1);
@@ -132,7 +216,9 @@ mod tests {
             .unwrap();
         assert!(transmitter.transmit.is_active());
 
-        transmitter.on_dma_complete(TxCompletion::TransferFault);
+        transmitter
+            .on_dma_complete(TxCompletion::TransferFault)
+            .unwrap();
         assert!(!transmitter.transmit.is_active());
     }
 
@@ -169,4 +255,70 @@ mod tests {
             EnqueueResult::CriticalQueueFull
         );
     }
+
+    #[test]
+    fn terminal_completion_can_defer_a_transient_engine_busy_without_losing_frame_order() {
+        let mut encoder = MavlinkEncoder::new(1, 1);
+        let mut transmitter = MavlinkTransmitter::<Memory, BusyOnceEngine, 2>::new(
+            Memory,
+            BusyOnceEngine {
+                busy_once: false,
+                starts: 0,
+            },
+        );
+        let state = MavState::MAV_STATE_STANDBY;
+
+        transmitter.enqueue_frame(FrameClass::Critical, encoder.heartbeat(state, false));
+        transmitter.start_pending().unwrap();
+        assert_eq!(transmitter.engine.starts, 1);
+        transmitter.enqueue_frame(FrameClass::Critical, encoder.heartbeat(state, false));
+
+        // Model the STM32 case: TCIF is acknowledged but CR.EN still reports
+        // busy for the first attempt to start the successor. `complete_dma`
+        // deliberately does not start it in the terminal IRQ path.
+        assert_eq!(
+            transmitter.complete_dma(TxCompletion::Complete).unwrap(),
+            Some(FrameClass::Critical)
+        );
+        assert_eq!(transmitter.engine.starts, 1);
+        transmitter.engine.busy_once = true;
+        assert!(matches!(
+            transmitter.start_pending(),
+            Err(MavlinkTransmitFault::Engine(RetryError::Busy))
+        ));
+        assert!(!transmitter.transmit.is_active());
+
+        transmitter.start_pending().unwrap();
+        assert!(transmitter.transmit.is_active());
+        assert_eq!(transmitter.engine.starts, 2);
+    }
+
+    #[test]
+    fn completion_retry_starts_pending_best_effort_before_later_critical_frame() {
+        let mut encoder = MavlinkEncoder::new(1, 1);
+        let mut transmitter = MavlinkTransmitter::<Memory, RecordingEngine, 3>::new(
+            Memory,
+            RecordingEngine {
+                started_sequences: [0; 4],
+                starts: 0,
+            },
+        );
+        let state = MavState::MAV_STATE_STANDBY;
+
+        // A critical frame is active. A best-effort sample arrives behind it
+        // before DMA completion.
+        transmitter.enqueue_frame(FrameClass::Critical, encoder.heartbeat(state, false));
+        transmitter.start_pending().unwrap();
+        transmitter.enqueue_frame(FrameClass::BestEffort, encoder.heartbeat(state, false));
+
+        // The completion worker performs its bounded retry before the next
+        // periodic sender can enqueue fresh critical traffic. The existing
+        // best-effort frame therefore cannot starve behind later traffic.
+        transmitter.complete_dma(TxCompletion::Complete).unwrap();
+        transmitter.start_pending().unwrap();
+        transmitter.enqueue_frame(FrameClass::Critical, encoder.heartbeat(state, false));
+
+        assert_eq!(transmitter.engine.starts, 2);
+        assert_eq!(&transmitter.engine.started_sequences[..2], &[0, 1]);
+    }
 }