add some more tests and TODOs

coverage.py

@@ -47,7 +47,7 @@ def main():
     parser.add_argument(
         "-p",
         "--package",
-        choices=["satrs", "satrs-minisim"],
+        choices=["satrs", "satrs-minisim", "satrs-example"],
         default="satrs",
         help="Choose project to generate coverage for",
     )

satrs-example/src/pus/action.rs

@@ -137,11 +137,9 @@ impl PusReplyHandler<ActivePusActionRequestStd, ActionReplyPusWithActionId> for
 }
 
 #[derive(Default)]
-pub struct ExampleActionRequestConverter {}
+pub struct ActionRequestConverter {}
 
-impl PusTcToRequestConverter<ActivePusActionRequestStd, ActionRequest>
+impl PusTcToRequestConverter<ActivePusActionRequestStd, ActionRequest> for ActionRequestConverter {
-    for ExampleActionRequestConverter
-{
     type Error = GenericConversionError;
 
     fn convert(
@@ -228,7 +226,7 @@ pub fn create_action_service_static(
             verif_reporter.clone(),
             EcssTcInSharedStoreConverter::new(tc_pool.clone(), 2048),
         ),
-        ExampleActionRequestConverter::default(),
+        ActionRequestConverter::default(),
         // TODO: Implementation which does not use run-time allocation? Maybe something like
         // a bounded wrapper which pre-allocates using [HashMap::with_capacity]..
         DefaultActiveActionRequestMap::default(),
@@ -272,7 +270,7 @@ pub fn create_action_service_dynamic(
             verif_reporter.clone(),
             EcssTcInVecConverter::default(),
         ),
-        ExampleActionRequestConverter::default(),
+        ActionRequestConverter::default(),
         DefaultActiveActionRequestMap::default(),
         ActionReplyHandler::default(),
         action_router,
@@ -294,7 +292,7 @@ pub struct Pus8Wrapper<
         TmSender,
         TcInMemConverter,
         VerificationReporter,
-        ExampleActionRequestConverter,
+        ActionRequestConverter,
         ActionReplyHandler,
         DefaultActiveActionRequestMap,
         ActivePusActionRequestStd,
@@ -367,8 +365,8 @@ mod tests {
 
     use crate::{
         pus::tests::{
-            PusConverterTestbench, TargetedPusRequestTestbench, TARGET_ID, TEST_APID,
+            PusConverterTestbench, ReplyHandlerTestbench, TargetedPusRequestTestbench, TARGET_ID,
-            TEST_APID_TARGET_ID,
+            TEST_APID, TEST_APID_TARGET_ID,
         },
         requests::CompositeRequest,
     };
@@ -377,7 +375,7 @@ mod tests {
 
     impl
         TargetedPusRequestTestbench<
-            ExampleActionRequestConverter,
+            ActionRequestConverter,
             ActionReplyHandler,
             DefaultActiveActionRequestMap,
             ActivePusActionRequestStd,
@@ -414,7 +412,7 @@ mod tests {
                         verif_reporter.clone(),
                         EcssTcInVecConverter::default(),
                     ),
-                    ExampleActionRequestConverter::default(),
+                    ActionRequestConverter::default(),
                     DefaultActiveActionRequestMap::default(),
                     ActionReplyHandler::default(),
                     generic_req_router,
@@ -501,7 +499,7 @@ mod tests {
     }
 
     #[test]
-    fn test_basic_request() {
+    fn basic_request() {
         let mut testbench = TargetedPusRequestTestbench::new_for_action();
         // Create a basic action request and verify forwarding.
         let mut sp_header = SpHeader::tc_unseg(TEST_APID, 0, 0).unwrap();
@@ -545,7 +543,7 @@ mod tests {
     }
 
     #[test]
-    fn test_basic_request_routing_error() {
+    fn basic_request_routing_error() {
         let mut testbench = TargetedPusRequestTestbench::new_for_action();
         // Create a basic action request and verify forwarding.
         let mut sp_header = SpHeader::tc_unseg(TEST_APID, 0, 0).unwrap();
@@ -566,7 +564,7 @@ mod tests {
 
     #[test]
     fn converter_action_req_no_data() {
-        let mut testbench = PusConverterTestbench::new(ExampleActionRequestConverter::default());
+        let mut testbench = PusConverterTestbench::new(ActionRequestConverter::default());
         let mut sp_header = SpHeader::tc_unseg(TEST_APID, 0, 0).unwrap();
         let sec_header = PusTcSecondaryHeader::new_simple(8, 128);
         let action_id = 5_u32;
@@ -576,7 +574,7 @@ mod tests {
         app_data[4..8].copy_from_slice(&action_id.to_be_bytes());
         let pus8_packet = PusTcCreator::new(&mut sp_header, sec_header, &app_data, true);
         let token = testbench.add_tc(&pus8_packet);
-        let result = testbench.convert(token, &[]);
+        let result = testbench.convert(token, &[], TEST_APID, TEST_APID_TARGET_ID);
         assert!(result.is_ok());
         let (active_req, request) = result.unwrap();
         if let ActionRequestVariant::NoData = request.variant {
@@ -597,7 +595,7 @@ mod tests {
 
     #[test]
     fn converter_action_req_with_data() {
-        let mut testbench = PusConverterTestbench::new(ExampleActionRequestConverter::default());
+        let mut testbench = PusConverterTestbench::new(ActionRequestConverter::default());
         let mut sp_header = SpHeader::tc_unseg(TEST_APID, 0, 0).unwrap();
         let sec_header = PusTcSecondaryHeader::new_simple(8, 128);
         let action_id = 5_u32;
@@ -610,20 +608,12 @@ mod tests {
         }
         let pus8_packet = PusTcCreator::new(&mut sp_header, sec_header, &app_data, true);
         let token = testbench.add_tc(&pus8_packet);
-        let result = testbench.convert(token, &[]);
+        let result = testbench.convert(token, &[], TEST_APID, TEST_APID_TARGET_ID);
         assert!(result.is_ok());
         let (active_req, request) = result.unwrap();
         if let ActionRequestVariant::VecData(vec) = request.variant {
             assert_eq!(request.action_id, action_id);
             assert_eq!(active_req.action_id, action_id);
-            assert_eq!(
-                active_req.target_id(),
-                TargetAndApidId::new(TEST_APID, TEST_APID_TARGET_ID).raw()
-            );
-            assert_eq!(
-                active_req.token().request_id(),
-                testbench.request_id().unwrap()
-            );
             assert_eq!(vec, app_data[8..].to_vec());
         } else {
             panic!("unexpected action request variant");
@@ -631,5 +621,31 @@ mod tests {
     }
 
     #[test]
-    fn reply_handling_completion_success() {}
+    fn reply_handling_completion_success() {
+        let mut testbench = ReplyHandlerTestbench::new(ActionReplyHandler::default());
+        let action_id = 5_u32;
+        let (req_id, active_req) = testbench.add_tc(TEST_APID, TEST_APID_TARGET_ID, &[]);
+        let active_action_req =
+            ActivePusActionRequestStd::new_from_common_req(action_id, active_req);
+        let reply = ActionReplyPusWithActionId::new(action_id, ActionReplyPus::Completed);
+        let generic_reply = GenericMessage::new(req_id.into(), 0, reply);
+        let result = testbench.handle_reply(&generic_reply, &active_action_req, &[]);
+        assert!(result.is_ok());
+        assert!(result.unwrap());
+    }
+
+    #[test]
+    fn reply_handling_completion_failure() {
+        // TODO: Implement
+    }
+
+    #[test]
+    fn reply_handling_step_success() {
+        // TODO: Implement
+    }
+
+    #[test]
+    fn reply_handling_step_failure() {
+        // TODO: Implement
+    }
 }

satrs-example/src/pus/hk.rs

@@ -87,11 +87,11 @@ impl PusReplyHandler<ActivePusRequestStd, HkReply> for HkReplyHandler {
     }
 }
 
-pub struct ExampleHkRequestConverter {
+pub struct HkRequestConverter {
     timeout: Duration,
 }
 
-impl Default for ExampleHkRequestConverter {
+impl Default for HkRequestConverter {
     fn default() -> Self {
         Self {
             timeout: Duration::from_secs(60),
@@ -99,7 +99,7 @@ impl Default for ExampleHkRequestConverter {
     }
 }
 
-impl PusTcToRequestConverter<ActivePusRequestStd, HkRequest> for ExampleHkRequestConverter {
+impl PusTcToRequestConverter<ActivePusRequestStd, HkRequest> for HkRequestConverter {
     type Error = GenericConversionError;
 
     fn convert(
@@ -247,7 +247,7 @@ pub fn create_hk_service_static(
             verif_reporter.clone(),
             EcssTcInSharedStoreConverter::new(tc_pool, 2048),
         ),
-        ExampleHkRequestConverter::default(),
+        HkRequestConverter::default(),
         DefaultActiveRequestMap::default(),
         HkReplyHandler::default(),
         request_router,
@@ -289,7 +289,7 @@ pub fn create_hk_service_dynamic(
             verif_reporter.clone(),
             EcssTcInVecConverter::default(),
         ),
-        ExampleHkRequestConverter::default(),
+        HkRequestConverter::default(),
         DefaultActiveRequestMap::default(),
         HkReplyHandler::default(),
         request_router,
@@ -311,7 +311,7 @@ pub struct Pus3Wrapper<
         TmSender,
         TcInMemConverter,
         VerificationReporter,
-        ExampleHkRequestConverter,
+        HkRequestConverter,
         HkReplyHandler,
         DefaultActiveRequestMap<ActivePusRequestStd>,
         ActivePusRequestStd,
@@ -370,27 +370,23 @@ impl<
 mod tests {
     use satrs::{
         hk::HkRequest,
-        pus::{test_util::TEST_APID, ActiveRequestProvider},
+        pus::test_util::TEST_APID,
-        request::{GenericMessage, TargetAndApidId},
+        request::GenericMessage,
         spacepackets::{
-            ecss::{
+            ecss::{hk::Subservice, tc::PusTcCreator},
-                hk::Subservice,
-                tc::{PusTcCreator, PusTcReader},
-                WritablePusPacket,
-            },
             SpHeader,
         },
     };
 
-    use crate::pus::tests::{PusConverterTestbench, ReplyHandlerTestbench};
+    use crate::pus::tests::{PusConverterTestbench, ReplyHandlerTestbench, TEST_APID_TARGET_ID};
 
-    use super::{ExampleHkRequestConverter, HkReply, HkReplyHandler};
+    use super::{HkReply, HkReplyHandler, HkRequestConverter};
 
     #[test]
-    fn test_hk_converter_one_shot_req() {
+    fn hk_converter_one_shot_req() {
-        let mut hk_bench = PusConverterTestbench::new(ExampleHkRequestConverter::default());
+        let mut hk_bench = PusConverterTestbench::new(HkRequestConverter::default());
         let mut sp_header = SpHeader::tc_unseg(TEST_APID, 0, 0).unwrap();
-        let target_id = 2_u32;
+        let target_id = TEST_APID_TARGET_ID;
         let unique_id = 5_u32;
         let mut app_data: [u8; 8] = [0; 8];
         app_data[0..4].copy_from_slice(&target_id.to_be_bytes());
@@ -404,13 +400,9 @@ mod tests {
             true,
         );
         let accepted_token = hk_bench.add_tc(&hk_req);
-        let (active_req, req) = hk_bench
+        let (_active_req, req) = hk_bench
-            .convert(accepted_token, &[])
+            .convert(accepted_token, &[], TEST_APID, TEST_APID_TARGET_ID)
             .expect("conversion failed");
-        assert_eq!(
-            active_req.target_id(),
-            TargetAndApidId::new(TEST_APID, target_id).into()
-        );
         if let HkRequest::OneShot(id) = req {
             assert_eq!(id, unique_id);
         } else {
@@ -419,15 +411,65 @@ mod tests {
     }
 
     #[test]
-    fn test_hk_reply_handler() {
+    fn hk_converter_enable_periodic_generation() {
+        let mut hk_bench = PusConverterTestbench::new(HkRequestConverter::default());
+        let mut sp_header = SpHeader::tc_unseg(TEST_APID, 0, 0).unwrap();
+        let target_id = TEST_APID_TARGET_ID;
+        let unique_id = 5_u32;
+        let mut app_data: [u8; 8] = [0; 8];
+        app_data[0..4].copy_from_slice(&target_id.to_be_bytes());
+        app_data[4..8].copy_from_slice(&unique_id.to_be_bytes());
+        let mut generic_check = |tc: &PusTcCreator| {
+            let accepted_token = hk_bench.add_tc(tc);
+            let (_active_req, req) = hk_bench
+                .convert(accepted_token, &[], TEST_APID, TEST_APID_TARGET_ID)
+                .expect("conversion failed");
+            if let HkRequest::Enable(id) = req {
+                assert_eq!(id, unique_id);
+            } else {
+                panic!("unexpected HK request")
+            }
+        };
+        let tc0 = PusTcCreator::new_simple(
+            &mut sp_header,
+            3,
+            Subservice::TcEnableHkGeneration as u8,
+            Some(&app_data),
+            true,
+        );
+        generic_check(&tc0);
+        let tc1 = PusTcCreator::new_simple(
+            &mut sp_header,
+            3,
+            Subservice::TcEnableDiagGeneration as u8,
+            Some(&app_data),
+            true,
+        );
+        generic_check(&tc1);
+    }
+
+    #[test]
+    fn hk_conversion_disable_periodic_generation() {
+        // TODO: Implement
+    }
+
+    #[test]
+    fn hk_conversion_modify_interval() {
+        // TODO: Implement
+    }
+
+    #[test]
+    fn hk_reply_handler() {
         let mut reply_testbench = ReplyHandlerTestbench::new(HkReplyHandler::default());
         let sender_id = 2_u64;
-        let target_id = 3_u64;
+        let apid_target_id = 3_u32;
-        let (req_id, active_req) = reply_testbench.add_tc(TEST_APID, target_id, &[]);
+        let (req_id, active_req) = reply_testbench.add_tc(TEST_APID, apid_target_id, &[]);
         let reply = GenericMessage::new(req_id.into(), sender_id, HkReply::Ack);
         let result = reply_testbench.handle_reply(&reply, &active_req, &[]);
         assert!(result.is_ok());
+        assert!(result.unwrap());
+        reply_testbench.verif_reporter.check_completed(&req_id);
     }
 
-    // TODO: Add more tests.
+    // TODO: Add more tests for reply handler.
 }

satrs-example/src/pus/mod.rs

@@ -532,7 +532,7 @@ pub(crate) mod tests {
         pub fn add_tc(
             &mut self,
             apid: u16,
-            target_id: ComponentId,
+            apid_target: u32,
             time_stamp: &[u8],
         ) -> (verification::RequestId, ActivePusRequestStd) {
             let mut sp_header = SpHeader::tc_unseg(apid, 0, 0).unwrap();
@@ -553,7 +553,11 @@ pub(crate) mod tests {
                 .expect("start failed");
             (
                 started.request_id(),
-                ActivePusRequestStd::new(target_id, started, Duration::from_secs(30)),
+                ActivePusRequestStd::new(
+                    TargetAndApidId::new(apid, apid_target).raw(),
+                    started,
+                    Duration::from_secs(30),
+                ),
             )
         }
 
@@ -623,6 +627,8 @@ pub(crate) mod tests {
             &mut self,
             token: VerificationToken<TcStateAccepted>,
             time_stamp: &[u8],
+            expected_apid: u16,
+            expected_apid_target: u32,
         ) -> Result<(ActiveRequestInfo, Request), Converter::Error> {
             if self.current_packet.is_none() {
                 return Err(GenericConversionError::InvalidAppData(
@@ -631,8 +637,18 @@ pub(crate) mod tests {
             }
             let current_packet = self.current_packet.take().unwrap();
             let tc_reader = PusTcReader::new(&current_packet).unwrap();
-            self.converter
+            let (active_info, request) =
-                .convert(token, &tc_reader.0, time_stamp, &self.verif_reporter)
+                self.converter
+                    .convert(token, &tc_reader.0, time_stamp, &self.verif_reporter)?;
+            assert_eq!(
+                active_info.token().request_id(),
+                self.request_id().expect("no request id is set")
+            );
+            assert_eq!(
+                active_info.target_id(),
+                TargetAndApidId::new(expected_apid, expected_apid_target).raw()
+            );
+            Ok((active_info, request))
         }
     }
 

satrs-example/src/pus/mode.rs

@@ -130,9 +130,9 @@ impl PusReplyHandler<ActivePusRequestStd, ModeReply> for ModeReplyHandler {
 }
 
 #[derive(Default)]
-pub struct ExampleModeRequestConverter {}
+pub struct ModeRequestConverter {}
 
-impl PusTcToRequestConverter<ActivePusRequestStd, ModeRequest> for ExampleModeRequestConverter {
+impl PusTcToRequestConverter<ActivePusRequestStd, ModeRequest> for ModeRequestConverter {
     type Error = GenericConversionError;
 
     fn convert(
@@ -197,4 +197,33 @@ impl PusTcToRequestConverter<ActivePusRequestStd, ModeRequest> for ExampleModeRe
 }
 
 #[cfg(test)]
-mod tests {}
+mod tests {
+    use satrs::{
+        mode::ModeRequest,
+        pus::mode::Subservice,
+        spacepackets::{
+            ecss::tc::{PusTcCreator, PusTcSecondaryHeader},
+            SpHeader,
+        },
+    };
+
+    use crate::pus::tests::{PusConverterTestbench, TEST_APID, TEST_APID_TARGET_ID};
+
+    use super::ModeRequestConverter;
+
+    #[test]
+    fn mode_converter_set_mode_request() {
+        let mut testbench = PusConverterTestbench::new(ModeRequestConverter::default());
+        let mut sp_header = SpHeader::tc_unseg(TEST_APID, 0, 0).unwrap();
+        let sec_header = PusTcSecondaryHeader::new_simple(200, Subservice::TcReadMode as u8);
+        let mut app_data: [u8; 4] = [0; 4];
+        // Invalid ID, routing should fail.
+        app_data[0..4].copy_from_slice(&TEST_APID_TARGET_ID.to_be_bytes());
+        let tc = PusTcCreator::new(&mut sp_header, sec_header, &app_data, true);
+        let token = testbench.add_tc(&tc);
+        let (_active_req, req) = testbench
+            .convert(token, &[], TEST_APID, TEST_APID_TARGET_ID)
+            .expect("conversion has failed");
+        assert_eq!(req, ModeRequest::ReadMode);
+    }
+}

satrs/src/pus/action.rs

@@ -167,6 +167,10 @@ pub mod std_mod {
     }
 
     impl ActivePusActionRequestStd {
+        pub fn new_from_common_req(action_id: ActionId, common: ActivePusRequestStd) -> Self {
+            Self { action_id, common }
+        }
+
         pub fn new(
             action_id: ActionId,
             target_id: ComponentId,