new hook works well, bugfix for seq counters

satrs/src/pus/mod.rs

@@ -1439,18 +1439,36 @@ pub mod tests {
     pub(crate) struct CommonTmInfo {
         pub subservice: u8,
         pub apid: u16,
+        pub seq_count: u16,
         pub msg_counter: u16,
         pub dest_id: u16,
         pub time_stamp: [u8; 7],
     }
 
     impl CommonTmInfo {
+        pub fn new_zero_seq_count(
+            subservice: u8,
+            apid: u16,
+            dest_id: u16,
+            time_stamp: [u8; 7],
+        ) -> Self {
+            Self {
+                subservice,
+                apid,
+                seq_count: 0,
+                msg_counter: 0,
+                dest_id,
+                time_stamp,
+            }
+        }
+
         pub fn new_from_tm(tm: &PusTmCreator) -> Self {
             let mut time_stamp = [0; 7];
             time_stamp.clone_from_slice(&tm.timestamp()[0..7]);
             Self {
                 subservice: PusPacket::subservice(tm),
                 apid: tm.apid(),
+                seq_count: tm.seq_count(),
                 msg_counter: tm.msg_counter(),
                 dest_id: tm.dest_id(),
                 time_stamp,

satrs/src/pus/verification.rs

@@ -915,10 +915,17 @@ pub mod alloc_mod {
         }
     }
 
+    /// This trait allows hooking into the TM generation process of the [VerificationReporter].
+    ///
+    /// The [Self::modify_tm] function is called before the TM is sent. This allows users to change
+    /// fields like the message count or sequence counter before the TM is sent.
     pub trait VerificationHookProvider {
         fn modify_tm(&self, tm: &mut PusTmCreator);
     }
 
+    /// [VerificationHookProvider] which does nothing. This is the default hook variant for
+    /// the [VerificationReporter], assuming that any necessary packet manipulation is performed by
+    /// a centralized TM funnel or inlet.
     #[derive(Default, Copy, Clone)]
     pub struct DummyVerificationHook {}
 
@@ -937,8 +944,6 @@ pub mod alloc_mod {
     pub struct VerificationReporter<
         VerificationHook: VerificationHookProvider = DummyVerificationHook,
     > {
-        // TODO: We could add a hook object which allows users to manipulate the verification
-        // report TM before it is sent..
         source_data_buf: RefCell<alloc::vec::Vec<u8>>,
         pub reporter_creator: VerificationReportCreator,
         pub tm_hook: VerificationHook,
@@ -962,6 +967,8 @@ pub mod alloc_mod {
     }
 
     impl<VerificationHook: VerificationHookProvider> VerificationReporter<VerificationHook> {
+        /// The provided [VerificationHookProvider] can be used to modify a verification packet
+        /// before it is sent.
         pub fn new_with_hook(cfg: &VerificationReporterCfg, tm_hook: VerificationHook) -> Self {
             let reporter = VerificationReportCreator::new(cfg.apid).unwrap();
             Self {
@@ -993,7 +1000,9 @@ pub mod alloc_mod {
         }
     }
 
-    impl VerificationReportingProvider for VerificationReporter {
+    impl<VerificationHook: VerificationHookProvider> VerificationReportingProvider
+        for VerificationReporter<VerificationHook>
+    {
         delegate!(
             to self.reporter_creator {
                 fn set_apid(&mut self, apid: Apid);
@@ -1626,6 +1635,7 @@ pub mod tests {
     };
     use crate::pus::{ChannelWithId, MpscTmInSharedPoolSender, PusTmVariant};
     use crate::request::MessageMetadata;
+    use crate::seq_count::{CcsdsSimpleSeqCountProvider, SequenceCountProviderCore};
     use crate::tmtc::tm_helper::SharedTmPool;
     use crate::ComponentId;
     use alloc::format;
@@ -1643,7 +1653,8 @@ pub mod tests {
     use std::vec::Vec;
 
     use super::{
-        TcStateAccepted, TcStateStarted, VerificationReportingProvider, WasAtLeastAccepted,
+        DummyVerificationHook, SeqCountProviderSimple, TcStateAccepted, TcStateStarted,
+        VerificationHookProvider, VerificationReportingProvider, WasAtLeastAccepted,
     };
 
     fn is_send<T: Send>(_: &T) {}
@@ -1705,10 +1716,25 @@ pub mod tests {
         }
     }
 
-    struct VerificationReporterTestbench {
+    #[derive(Default)]
+    pub struct SequenceCounterHook {
+        pub seq_counter: CcsdsSimpleSeqCountProvider,
+        pub msg_counter: SeqCountProviderSimple<u16>,
+    }
+
+    impl VerificationHookProvider for SequenceCounterHook {
+        fn modify_tm(&self, tm: &mut spacepackets::ecss::tm::PusTmCreator) {
+            tm.set_seq_count(self.seq_counter.get_and_increment());
+            tm.set_msg_counter(self.msg_counter.get_and_increment());
+        }
+    }
+
+    struct VerificationReporterTestbench<
+        VerificationHook: VerificationHookProvider = DummyVerificationHook,
+    > {
         pub id: ComponentId,
         sender: TestSender,
-        reporter: VerificationReporter,
+        reporter: VerificationReporter<VerificationHook>,
         pub request_id: RequestId,
         tc: Vec<u8>,
     }
@@ -1718,9 +1744,16 @@ pub mod tests {
         VerificationReporter::new(&cfg)
     }
 
-    impl VerificationReporterTestbench {
+    fn reporter_with_hook<VerificationHook: VerificationHookProvider>(
-        fn new(id: ComponentId, tc: PusTcCreator) -> Self {
+        hook: VerificationHook,
-            let reporter = base_reporter();
+    ) -> VerificationReporter<VerificationHook> {
+        let cfg = VerificationReporterCfg::new(TEST_APID, 1, 2, 8).unwrap();
+        VerificationReporter::new_with_hook(&cfg, hook)
+    }
+
+    impl<VerificiationHook: VerificationHookProvider> VerificationReporterTestbench<VerificiationHook> {
+        fn new_with_hook(id: ComponentId, tc: PusTcCreator, tm_hook: VerificiationHook) -> Self {
+            let reporter = reporter_with_hook(tm_hook);
             Self {
                 id,
                 sender: TestSender::default(),
@@ -1812,12 +1845,82 @@ pub mod tests {
                 .completion_failure(self.id, &self.sender, token, params)
         }
 
+        fn completion_success_check(&mut self, incrementing_couters: bool) {
+            assert_eq!(self.sender.service_queue.borrow().len(), 3);
+            let mut current_seq_count = 0;
+            let cmp_info = TmInfo {
+                requestor: MessageMetadata::new(self.request_id.into(), self.id),
+                common: CommonTmInfo {
+                    subservice: 1,
+                    apid: TEST_APID,
+                    seq_count: current_seq_count,
+                    msg_counter: current_seq_count,
+                    dest_id: self.reporter.dest_id(),
+                    time_stamp: EMPTY_STAMP,
+                },
+                additional_data: None,
+            };
+            let mut info = self.sender.service_queue.borrow_mut().pop_front().unwrap();
+            assert_eq!(info, cmp_info);
+
+            if incrementing_couters {
+                current_seq_count += 1;
+            }
+
+            let cmp_info = TmInfo {
+                requestor: MessageMetadata::new(self.request_id.into(), self.id),
+                common: CommonTmInfo {
+                    subservice: 3,
+                    apid: TEST_APID,
+                    msg_counter: current_seq_count,
+                    seq_count: current_seq_count,
+                    dest_id: self.reporter.dest_id(),
+                    time_stamp: [0, 1, 0, 1, 0, 1, 0],
+                },
+                additional_data: None,
+            };
+            info = self.sender.service_queue.borrow_mut().pop_front().unwrap();
+            assert_eq!(info, cmp_info);
+
+            if incrementing_couters {
+                current_seq_count += 1;
+            }
+            let cmp_info = TmInfo {
+                requestor: MessageMetadata::new(self.request_id.into(), self.id),
+                common: CommonTmInfo {
+                    subservice: 7,
+                    apid: TEST_APID,
+                    msg_counter: current_seq_count,
+                    seq_count: current_seq_count,
+                    dest_id: self.reporter.dest_id(),
+                    time_stamp: EMPTY_STAMP,
+                },
+                additional_data: None,
+            };
+            info = self.sender.service_queue.borrow_mut().pop_front().unwrap();
+            assert_eq!(info, cmp_info);
+        }
+    }
+
+    impl VerificationReporterTestbench<DummyVerificationHook> {
+        fn new(id: ComponentId, tc: PusTcCreator) -> Self {
+            let reporter = base_reporter();
+            Self {
+                id,
+                sender: TestSender::default(),
+                reporter,
+                request_id: RequestId::new(&tc),
+                tc: tc.to_vec().unwrap(),
+            }
+        }
+
         fn acceptance_check(&self, time_stamp: &[u8; 7]) {
             let cmp_info = TmInfo {
                 requestor: MessageMetadata::new(self.request_id.into(), self.id),
                 common: CommonTmInfo {
                     subservice: 1,
                     apid: TEST_APID,
+                    seq_count: 0,
                     msg_counter: 0,
                     dest_id: self.reporter.dest_id(),
                     time_stamp: *time_stamp,
@@ -1835,6 +1938,7 @@ pub mod tests {
                 requestor: MessageMetadata::new(self.request_id.into(), self.id),
                 common: CommonTmInfo {
                     subservice: 2,
+                    seq_count: 0,
                     apid: TEST_APID,
                     msg_counter: 0,
                     dest_id: self.reporter.dest_id(),
@@ -1853,13 +1957,7 @@ pub mod tests {
             assert_eq!(srv_queue.len(), 2);
             let mut cmp_info = TmInfo {
                 requestor: MessageMetadata::new(self.request_id.into(), self.id),
-                common: CommonTmInfo {
+                common: CommonTmInfo::new_zero_seq_count(1, TEST_APID, 0, EMPTY_STAMP),
-                    subservice: 1,
-                    apid: TEST_APID,
-                    msg_counter: 0,
-                    dest_id: 0,
-                    time_stamp: EMPTY_STAMP,
-                },
                 additional_data: None,
             };
             let mut info = srv_queue.pop_front().unwrap();
@@ -1867,13 +1965,7 @@ pub mod tests {
 
             cmp_info = TmInfo {
                 requestor: MessageMetadata::new(self.request_id.into(), self.id),
-                common: CommonTmInfo {
+                common: CommonTmInfo::new_zero_seq_count(4, TEST_APID, 0, EMPTY_STAMP),
-                    subservice: 4,
-                    apid: TEST_APID,
-                    msg_counter: 0,
-                    dest_id: 0,
-                    time_stamp: EMPTY_STAMP,
-                },
                 additional_data: Some([&[22], fail_data_raw.as_slice()].concat().to_vec()),
             };
             info = srv_queue.pop_front().unwrap();
@@ -1883,13 +1975,7 @@ pub mod tests {
         fn step_success_check(&mut self, time_stamp: &[u8; 7]) {
             let mut cmp_info = TmInfo {
                 requestor: MessageMetadata::new(self.request_id.into(), self.id),
-                common: CommonTmInfo {
+                common: CommonTmInfo::new_zero_seq_count(1, TEST_APID, 0, *time_stamp),
-                    subservice: 1,
-                    apid: TEST_APID,
-                    msg_counter: 0,
-                    dest_id: self.reporter.dest_id(),
-                    time_stamp: *time_stamp,
-                },
                 additional_data: None,
             };
             let mut srv_queue = self.sender.service_queue.borrow_mut();
@@ -1897,39 +1983,21 @@ pub mod tests {
             assert_eq!(info, cmp_info);
             cmp_info = TmInfo {
                 requestor: MessageMetadata::new(self.request_id.into(), self.id),
-                common: CommonTmInfo {
+                common: CommonTmInfo::new_zero_seq_count(3, TEST_APID, 0, *time_stamp),
-                    subservice: 3,
-                    apid: TEST_APID,
-                    msg_counter: 0,
-                    dest_id: self.reporter.dest_id(),
-                    time_stamp: *time_stamp,
-                },
                 additional_data: None,
             };
             info = srv_queue.pop_front().unwrap();
             assert_eq!(info, cmp_info);
             cmp_info = TmInfo {
                 requestor: MessageMetadata::new(self.request_id.into(), self.id),
-                common: CommonTmInfo {
+                common: CommonTmInfo::new_zero_seq_count(5, TEST_APID, 0, *time_stamp),
-                    subservice: 5,
-                    apid: TEST_APID,
-                    msg_counter: 0,
-                    dest_id: self.reporter.dest_id(),
-                    time_stamp: *time_stamp,
-                },
                 additional_data: Some([0].to_vec()),
             };
             info = srv_queue.pop_front().unwrap();
             assert_eq!(info, cmp_info);
             cmp_info = TmInfo {
                 requestor: MessageMetadata::new(self.request_id.into(), self.id),
-                common: CommonTmInfo {
+                common: CommonTmInfo::new_zero_seq_count(5, TEST_APID, 0, *time_stamp),
-                    subservice: 5,
-                    apid: TEST_APID,
-                    msg_counter: 0,
-                    dest_id: 0,
-                    time_stamp: *time_stamp,
-                },
                 additional_data: Some([1].to_vec()),
             };
             info = srv_queue.pop_front().unwrap();
@@ -1940,13 +2008,12 @@ pub mod tests {
             assert_eq!(self.sender.service_queue.borrow().len(), 4);
             let mut cmp_info = TmInfo {
                 requestor: MessageMetadata::new(self.request_id.into(), self.id),
-                common: CommonTmInfo {
+                common: CommonTmInfo::new_zero_seq_count(
-                    subservice: 1,
+                    1,
-                    apid: TEST_APID,
+                    TEST_APID,
-                    msg_counter: 0,
+                    self.reporter.dest_id(),
-                    dest_id: self.reporter.dest_id(),
+                    EMPTY_STAMP,
-                    time_stamp: EMPTY_STAMP,
+                ),
-                },
                 additional_data: None,
             };
             let mut info = self.sender.service_queue.borrow_mut().pop_front().unwrap();
@@ -1954,13 +2021,12 @@ pub mod tests {
 
             cmp_info = TmInfo {
                 requestor: MessageMetadata::new(self.request_id.into(), self.id),
-                common: CommonTmInfo {
+                common: CommonTmInfo::new_zero_seq_count(
-                    subservice: 3,
+                    3,
-                    apid: TEST_APID,
+                    TEST_APID,
-                    msg_counter: 0,
+                    self.reporter.dest_id(),
-                    dest_id: self.reporter.dest_id(),
+                    [0, 1, 0, 1, 0, 1, 0],
-                    time_stamp: [0, 1, 0, 1, 0, 1, 0],
+                ),
-                },
                 additional_data: None,
             };
             info = self.sender.service_queue.borrow_mut().pop_front().unwrap();
@@ -1968,13 +2034,12 @@ pub mod tests {
 
             cmp_info = TmInfo {
                 requestor: MessageMetadata::new(self.request_id.into(), self.id),
-                common: CommonTmInfo {
+                common: CommonTmInfo::new_zero_seq_count(
-                    subservice: 5,
+                    5,
-                    apid: TEST_APID,
+                    TEST_APID,
-                    msg_counter: 0,
+                    self.reporter.dest_id(),
-                    dest_id: self.reporter.dest_id(),
+                    EMPTY_STAMP,
-                    time_stamp: EMPTY_STAMP,
+                ),
-                },
                 additional_data: Some([0].to_vec()),
             };
             info = self.sender.service_queue.get_mut().pop_front().unwrap();
@@ -1982,13 +2047,12 @@ pub mod tests {
 
             cmp_info = TmInfo {
                 requestor: MessageMetadata::new(self.request_id.into(), self.id),
-                common: CommonTmInfo {
+                common: CommonTmInfo::new_zero_seq_count(
-                    subservice: 6,
+                    6,
-                    apid: TEST_APID,
+                    TEST_APID,
-                    msg_counter: 0,
+                    self.reporter.dest_id(),
-                    dest_id: self.reporter.dest_id(),
+                    EMPTY_STAMP,
-                    time_stamp: EMPTY_STAMP,
+                ),
-                },
                 additional_data: Some(
                     [
                         [1].as_slice(),
@@ -2008,13 +2072,12 @@ pub mod tests {
 
             let mut cmp_info = TmInfo {
                 requestor: MessageMetadata::new(self.request_id.into(), self.id),
-                common: CommonTmInfo {
+                common: CommonTmInfo::new_zero_seq_count(
-                    subservice: 1,
+                    1,
-                    apid: TEST_APID,
+                    TEST_APID,
-                    msg_counter: 0,
+                    self.reporter.dest_id(),
-                    dest_id: self.reporter.dest_id(),
+                    EMPTY_STAMP,
-                    time_stamp: EMPTY_STAMP,
+                ),
-                },
                 additional_data: None,
             };
             let mut info = self.sender.service_queue.get_mut().pop_front().unwrap();
@@ -2022,13 +2085,12 @@ pub mod tests {
 
             cmp_info = TmInfo {
                 requestor: MessageMetadata::new(self.request_id.into(), self.id),
-                common: CommonTmInfo {
+                common: CommonTmInfo::new_zero_seq_count(
-                    subservice: 3,
+                    3,
-                    apid: TEST_APID,
+                    TEST_APID,
-                    msg_counter: 0,
+                    self.reporter.dest_id(),
-                    dest_id: 0,
+                    [0, 1, 0, 1, 0, 1, 0],
-                    time_stamp: [0, 1, 0, 1, 0, 1, 0],
+                ),
-                },
                 additional_data: None,
             };
             info = self.sender.service_queue.get_mut().pop_front().unwrap();
@@ -2036,62 +2098,17 @@ pub mod tests {
 
             cmp_info = TmInfo {
                 requestor: MessageMetadata::new(self.request_id.into(), self.id),
-                common: CommonTmInfo {
+                common: CommonTmInfo::new_zero_seq_count(
-                    subservice: 8,
+                    8,
-                    apid: TEST_APID,
+                    TEST_APID,
-                    msg_counter: 0,
+                    self.reporter.dest_id(),
-                    dest_id: self.reporter.dest_id(),
+                    EMPTY_STAMP,
-                    time_stamp: EMPTY_STAMP,
+                ),
-                },
                 additional_data: Some([0, 0, 0x10, 0x20].to_vec()),
             };
             info = self.sender.service_queue.get_mut().pop_front().unwrap();
             assert_eq!(info, cmp_info);
         }
-
-        fn completion_success_check(&mut self) {
-            assert_eq!(self.sender.service_queue.borrow().len(), 3);
-            let cmp_info = TmInfo {
-                requestor: MessageMetadata::new(self.request_id.into(), self.id),
-                common: CommonTmInfo {
-                    subservice: 1,
-                    apid: TEST_APID,
-                    msg_counter: 0,
-                    dest_id: self.reporter.dest_id(),
-                    time_stamp: EMPTY_STAMP,
-                },
-                additional_data: None,
-            };
-            let mut info = self.sender.service_queue.borrow_mut().pop_front().unwrap();
-            assert_eq!(info, cmp_info);
-
-            let cmp_info = TmInfo {
-                requestor: MessageMetadata::new(self.request_id.into(), self.id),
-                common: CommonTmInfo {
-                    subservice: 3,
-                    apid: TEST_APID,
-                    msg_counter: 0,
-                    dest_id: self.reporter.dest_id(),
-                    time_stamp: [0, 1, 0, 1, 0, 1, 0],
-                },
-                additional_data: None,
-            };
-            info = self.sender.service_queue.borrow_mut().pop_front().unwrap();
-            assert_eq!(info, cmp_info);
-            let cmp_info = TmInfo {
-                requestor: MessageMetadata::new(self.request_id.into(), self.id),
-                common: CommonTmInfo {
-                    subservice: 7,
-                    apid: TEST_APID,
-                    msg_counter: 0,
-                    dest_id: self.reporter.dest_id(),
-                    time_stamp: EMPTY_STAMP,
-                },
-                additional_data: None,
-            };
-            info = self.sender.service_queue.borrow_mut().pop_front().unwrap();
-            assert_eq!(info, cmp_info);
-        }
     }
 
     fn create_generic_ping() -> PusTcCreator<'static> {
@@ -2199,13 +2216,7 @@ pub mod tests {
             .expect("sending acceptance failure failed");
         let cmp_info = TmInfo {
             requestor: MessageMetadata::new(testbench.request_id.into(), testbench.id),
-            common: CommonTmInfo {
+            common: CommonTmInfo::new_zero_seq_count(2, TEST_APID, 0, EMPTY_STAMP),
-                subservice: 2,
-                apid: TEST_APID,
-                msg_counter: 0,
-                dest_id: 0,
-                time_stamp: EMPTY_STAMP,
-            },
             additional_data: Some([10, 0, 0, 0, 12].to_vec()),
         };
         let mut service_queue = testbench.sender.service_queue.borrow_mut();
@@ -2336,6 +2347,26 @@ pub mod tests {
         testbench
             .completion_success(started_token, &EMPTY_STAMP)
             .expect("Sending completion success failed");
-        testbench.completion_success_check();
+        testbench.completion_success_check(false);
+    }
+
+    #[test]
+    fn test_packet_manipulation() {
+        let mut testbench = VerificationReporterTestbench::new_with_hook(
+            TEST_COMPONENT_ID.id(),
+            create_generic_ping(),
+            SequenceCounterHook::default(),
+        );
+        let token = testbench.init();
+        let accepted_token = testbench
+            .acceptance_success(token, &EMPTY_STAMP)
+            .expect("Sending acceptance success failed");
+        let started_token = testbench
+            .start_success(accepted_token, &[0, 1, 0, 1, 0, 1, 0])
+            .expect("Sending start success failed");
+        testbench
+            .completion_success(started_token, &EMPTY_STAMP)
+            .expect("Sending completion success failed");
+        testbench.completion_success_check(true);
     }
 }

satrs/src/seq_count.rs

@@ -32,7 +32,7 @@ dyn_clone::clone_trait_object!(SequenceCountProvider<u16>);
 #[cfg(feature = "alloc")]
 impl<T, Raw> SequenceCountProvider<Raw> for T where T: SequenceCountProviderCore<Raw> + Clone {}
 
-#[derive(Default, Clone)]
+#[derive(Clone)]
 pub struct SeqCountProviderSimple<T: Copy> {
     seq_count: Cell<T>,
     max_val: T,
@@ -43,13 +43,12 @@ macro_rules! impl_for_primitives {
         $(
             paste! {
                 impl SeqCountProviderSimple<$ty> {
-                    pub fn [<new_ $ty _max_val>](max_val: $ty) -> Self {
+                    pub fn [<new_custom_max_val_ $ty>](max_val: $ty) -> Self {
                         Self {
                             seq_count: Cell::new(0),
                             max_val,
                         }
                     }
-
                     pub fn [<new_ $ty>]() -> Self {
                         Self {
                             seq_count: Cell::new(0),
@@ -58,6 +57,12 @@ macro_rules! impl_for_primitives {
                     }
                 }
 
+                impl Default for SeqCountProviderSimple<$ty> {
+                    fn default() -> Self {
+                        Self::[<new_ $ty>]()
+                    }
+                }
+
                 impl SequenceCountProviderCore<$ty> for SeqCountProviderSimple<$ty> {
                     fn get(&self) -> $ty {
                         self.seq_count.get()
@@ -86,21 +91,16 @@ macro_rules! impl_for_primitives {
 impl_for_primitives!(u8, u16, u32, u64,);
 
 /// This is a sequence count provider which wraps around at [MAX_SEQ_COUNT].
+#[derive(Clone)]
 pub struct CcsdsSimpleSeqCountProvider {
     provider: SeqCountProviderSimple<u16>,
 }
 
-impl CcsdsSimpleSeqCountProvider {
-    pub fn new() -> Self {
-        Self {
-            provider: SeqCountProviderSimple::new_u16_max_val(MAX_SEQ_COUNT),
-        }
-    }
-}
-
 impl Default for CcsdsSimpleSeqCountProvider {
     fn default() -> Self {
-        Self::new()
+        Self {
+            provider: SeqCountProviderSimple::new_custom_max_val_u16(MAX_SEQ_COUNT),
+        }
     }
 }
 
@@ -187,7 +187,7 @@ mod tests {
 
     #[test]
     fn test_u8_counter() {
-        let u8_counter = SeqCountProviderSimple::new_u8();
+        let u8_counter = SeqCountProviderSimple::<u8>::default();
         assert_eq!(u8_counter.get(), 0);
         assert_eq!(u8_counter.get_and_increment(), 0);
         assert_eq!(u8_counter.get_and_increment(), 1);