From 52d16e0fe72dde704f57441d513e17a2559cc7b6 Mon Sep 17 00:00:00 2001 From: Robin Mueller Date: Tue, 2 Apr 2024 19:21:01 +0200 Subject: [PATCH] new hook works well, bugfix for seq counters --- satrs/src/pus/mod.rs | 18 ++ satrs/src/pus/verification.rs | 335 +++++++++++++++++++--------------- satrs/src/seq_count.rs | 26 +-- 3 files changed, 214 insertions(+), 165 deletions(-) diff --git a/satrs/src/pus/mod.rs b/satrs/src/pus/mod.rs index 343b8ab..13f30e9 100644 --- a/satrs/src/pus/mod.rs +++ b/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, diff --git a/satrs/src/pus/verification.rs b/satrs/src/pus/verification.rs index f8ac3da..86664b8 100644 --- a/satrs/src/pus/verification.rs +++ b/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>, pub reporter_creator: VerificationReportCreator, pub tm_hook: VerificationHook, @@ -962,6 +967,8 @@ pub mod alloc_mod { } impl VerificationReporter { + /// 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 VerificationReportingProvider + for VerificationReporter + { 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) {} @@ -1705,10 +1716,25 @@ pub mod tests { } } - struct VerificationReporterTestbench { + #[derive(Default)] + pub struct SequenceCounterHook { + pub seq_counter: CcsdsSimpleSeqCountProvider, + pub msg_counter: SeqCountProviderSimple, + } + + 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, pub request_id: RequestId, tc: Vec, } @@ -1718,9 +1744,16 @@ pub mod tests { VerificationReporter::new(&cfg) } - impl VerificationReporterTestbench { - fn new(id: ComponentId, tc: PusTcCreator) -> Self { - let reporter = base_reporter(); + fn reporter_with_hook( + hook: VerificationHook, + ) -> VerificationReporter { + let cfg = VerificationReporterCfg::new(TEST_APID, 1, 2, 8).unwrap(); + VerificationReporter::new_with_hook(&cfg, hook) + } + + impl VerificationReporterTestbench { + 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 { + 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 { - subservice: 1, - apid: TEST_APID, - msg_counter: 0, - dest_id: 0, - time_stamp: EMPTY_STAMP, - }, + common: CommonTmInfo::new_zero_seq_count(1, TEST_APID, 0, 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 { - subservice: 4, - apid: TEST_APID, - msg_counter: 0, - dest_id: 0, - time_stamp: EMPTY_STAMP, - }, + common: CommonTmInfo::new_zero_seq_count(4, TEST_APID, 0, 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 { - subservice: 1, - apid: TEST_APID, - msg_counter: 0, - dest_id: self.reporter.dest_id(), - time_stamp: *time_stamp, - }, + common: CommonTmInfo::new_zero_seq_count(1, TEST_APID, 0, *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 { - subservice: 3, - apid: TEST_APID, - msg_counter: 0, - dest_id: self.reporter.dest_id(), - time_stamp: *time_stamp, - }, + common: CommonTmInfo::new_zero_seq_count(3, TEST_APID, 0, *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 { - subservice: 5, - apid: TEST_APID, - msg_counter: 0, - dest_id: self.reporter.dest_id(), - time_stamp: *time_stamp, - }, + common: CommonTmInfo::new_zero_seq_count(5, TEST_APID, 0, *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 { - subservice: 5, - apid: TEST_APID, - msg_counter: 0, - dest_id: 0, - time_stamp: *time_stamp, - }, + common: CommonTmInfo::new_zero_seq_count(5, TEST_APID, 0, *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 { - subservice: 1, - apid: TEST_APID, - msg_counter: 0, - dest_id: self.reporter.dest_id(), - time_stamp: EMPTY_STAMP, - }, + common: CommonTmInfo::new_zero_seq_count( + 1, + TEST_APID, + self.reporter.dest_id(), + 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 { - subservice: 3, - apid: TEST_APID, - msg_counter: 0, - dest_id: self.reporter.dest_id(), - time_stamp: [0, 1, 0, 1, 0, 1, 0], - }, + common: CommonTmInfo::new_zero_seq_count( + 3, + TEST_APID, + self.reporter.dest_id(), + [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 { - subservice: 5, - apid: TEST_APID, - msg_counter: 0, - dest_id: self.reporter.dest_id(), - time_stamp: EMPTY_STAMP, - }, + common: CommonTmInfo::new_zero_seq_count( + 5, + TEST_APID, + self.reporter.dest_id(), + 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 { - subservice: 6, - apid: TEST_APID, - msg_counter: 0, - dest_id: self.reporter.dest_id(), - time_stamp: EMPTY_STAMP, - }, + common: CommonTmInfo::new_zero_seq_count( + 6, + TEST_APID, + self.reporter.dest_id(), + 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 { - subservice: 1, - apid: TEST_APID, - msg_counter: 0, - dest_id: self.reporter.dest_id(), - time_stamp: EMPTY_STAMP, - }, + common: CommonTmInfo::new_zero_seq_count( + 1, + TEST_APID, + self.reporter.dest_id(), + 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 { - subservice: 3, - apid: TEST_APID, - msg_counter: 0, - dest_id: 0, - time_stamp: [0, 1, 0, 1, 0, 1, 0], - }, + common: CommonTmInfo::new_zero_seq_count( + 3, + TEST_APID, + self.reporter.dest_id(), + [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 { - subservice: 8, - apid: TEST_APID, - msg_counter: 0, - dest_id: self.reporter.dest_id(), - time_stamp: EMPTY_STAMP, - }, + common: CommonTmInfo::new_zero_seq_count( + 8, + TEST_APID, + self.reporter.dest_id(), + 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 { - subservice: 2, - apid: TEST_APID, - msg_counter: 0, - dest_id: 0, - time_stamp: EMPTY_STAMP, - }, + common: CommonTmInfo::new_zero_seq_count(2, TEST_APID, 0, 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); } } diff --git a/satrs/src/seq_count.rs b/satrs/src/seq_count.rs index c60c8f7..b4539b0 100644 --- a/satrs/src/seq_count.rs +++ b/satrs/src/seq_count.rs @@ -32,7 +32,7 @@ dyn_clone::clone_trait_object!(SequenceCountProvider); #[cfg(feature = "alloc")] impl SequenceCountProvider for T where T: SequenceCountProviderCore + Clone {} -#[derive(Default, Clone)] +#[derive(Clone)] pub struct SeqCountProviderSimple { seq_count: Cell, max_val: T, @@ -43,13 +43,12 @@ macro_rules! impl_for_primitives { $( paste! { impl SeqCountProviderSimple<$ty> { - pub fn [](max_val: $ty) -> Self { + pub fn [](max_val: $ty) -> Self { Self { seq_count: Cell::new(0), max_val, } } - pub fn []() -> 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::[]() + } + } + 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, } -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::::default(); assert_eq!(u8_counter.get(), 0); assert_eq!(u8_counter.get_and_increment(), 0); assert_eq!(u8_counter.get_and_increment(), 1);