Merge pull request 'optimize PUS stack code' (#180) from optimize-pus-stack-code into main

Reviewed-on: #180

satrs-example/src/pus/event.rs

@@ -68,12 +68,8 @@ impl<TmSender: EcssTmSender, TcInMemConverter: EcssTcInMemConverter> DirectPusSe
     const SERVICE_ID: u8 = PusServiceId::Event as u8;
 
     const SERVICE_STR: &'static str = "events";
-}
 
-impl<TmSender: EcssTmSender, TcInMemConverter: EcssTcInMemConverter>
-    EventServiceWrapper<TmSender, TcInMemConverter>
-{
-    pub fn poll_and_handle_next_tc(&mut self, time_stamp: &[u8]) -> HandlingStatus {
+    fn poll_and_handle_next_tc(&mut self, time_stamp: &[u8]) -> HandlingStatus {
         let error_handler = |partial_error: &PartialPusHandlingError| {
             log::warn!(
                 "PUS {}({}) partial error: {:?}",

satrs-example/src/pus/mod.rs

@@ -234,6 +234,15 @@ pub trait TargetedPusService {
     fn check_for_request_timeouts(&mut self);
 }
 
+/// Generic trait for services which handle packets directly. Kept minimal right now because
+/// of the difficulty to allow flexible user code for these services..
+pub trait DirectPusService {
+    const SERVICE_ID: u8;
+    const SERVICE_STR: &'static str;
+
+    fn poll_and_handle_next_tc(&mut self, timestamp: &[u8]) -> HandlingStatus;
+}
+
 /// This is a generic handler class for all PUS services where a PUS telecommand is converted
 /// to a targeted request.
 ///
@@ -524,11 +533,6 @@ pub fn generic_pus_request_timeout_handler(
     Ok(())
 }
 
-pub trait DirectPusService {
-    const SERVICE_ID: u8;
-    const SERVICE_STR: &'static str;
-}
-
 #[cfg(test)]
 pub(crate) mod tests {
     use std::time::Duration;

satrs-example/src/pus/scheduler.rs

@@ -84,36 +84,8 @@ impl<TmSender: EcssTmSender, TcInMemConverter: EcssTcInMemConverter> DirectPusSe
     const SERVICE_ID: u8 = PusServiceId::Verification as u8;
 
     const SERVICE_STR: &'static str = "verification";
-}
 
-impl<TmSender: EcssTmSender, TcInMemConverter: EcssTcInMemConverter>
-    SchedulingServiceWrapper<TmSender, TcInMemConverter>
-{
-    pub fn release_tcs(&mut self) {
-        let id = self.pus_11_handler.service_helper.id();
-        let releaser = |enabled: bool, info: &TcInfo, tc: &[u8]| -> bool {
-            self.tc_releaser.release(id, enabled, info, tc)
-        };
-
-        self.pus_11_handler
-            .scheduler_mut()
-            .update_time_from_now()
-            .unwrap();
-        let released_tcs = self
-            .pus_11_handler
-            .scheduler_mut()
-            .release_telecommands_with_buffer(
-                releaser,
-                &mut self.sched_tc_pool,
-                &mut self.releaser_buf,
-            )
-            .expect("releasing TCs failed");
-        if released_tcs > 0 {
-            info!("{released_tcs} TC(s) released from scheduler");
-        }
-    }
-
-    pub fn poll_and_handle_next_tc(&mut self, time_stamp: &[u8]) -> HandlingStatus {
+    fn poll_and_handle_next_tc(&mut self, time_stamp: &[u8]) -> HandlingStatus {
         let error_handler = |partial_error: &PartialPusHandlingError| {
             log::warn!(
                 "PUS {}({}) partial error: {:?}",
@@ -160,6 +132,34 @@ impl<TmSender: EcssTmSender, TcInMemConverter: EcssTcInMemConverter>
     }
 }
 
+impl<TmSender: EcssTmSender, TcInMemConverter: EcssTcInMemConverter>
+    SchedulingServiceWrapper<TmSender, TcInMemConverter>
+{
+    pub fn release_tcs(&mut self) {
+        let id = self.pus_11_handler.service_helper.id();
+        let releaser = |enabled: bool, info: &TcInfo, tc: &[u8]| -> bool {
+            self.tc_releaser.release(id, enabled, info, tc)
+        };
+
+        self.pus_11_handler
+            .scheduler_mut()
+            .update_time_from_now()
+            .unwrap();
+        let released_tcs = self
+            .pus_11_handler
+            .scheduler_mut()
+            .release_telecommands_with_buffer(
+                releaser,
+                &mut self.sched_tc_pool,
+                &mut self.releaser_buf,
+            )
+            .expect("releasing TCs failed");
+        if released_tcs > 0 {
+            info!("{released_tcs} TC(s) released from scheduler");
+        }
+    }
+}
+
 pub fn create_scheduler_service_static(
     tm_sender: PacketSenderWithSharedPool,
     tc_releaser: PacketSenderWithSharedPool,

satrs-example/src/pus/stack.rs

@@ -2,19 +2,17 @@ use crate::pus::mode::ModeServiceWrapper;
 use derive_new::new;
 use satrs::{
     pus::{EcssTcInMemConverter, EcssTmSender},
-    spacepackets::{
-        ecss::PusServiceId,
-        time::{cds, TimeWriter},
-    },
+    spacepackets::time::{cds, TimeWriter},
 };
-use satrs_example::config::CustomPusServiceId;
 
 use super::{
     action::ActionServiceWrapper, event::EventServiceWrapper, hk::HkServiceWrapper,
-    scheduler::SchedulingServiceWrapper, test::TestCustomServiceWrapper, HandlingStatus,
-    TargetedPusService,
+    scheduler::SchedulingServiceWrapper, test::TestCustomServiceWrapper, DirectPusService,
+    HandlingStatus, TargetedPusService,
 };
 
+// TODO: For better extensibility, we could create 2 vectors: One for direct PUS services and one
+// for targeted services..
 #[derive(new)]
 pub struct PusStack<TmSender: EcssTmSender, TcInMemConverter: EcssTcInMemConverter> {
     test_srv: TestCustomServiceWrapper<TmSender, TcInMemConverter>,
@@ -28,70 +26,56 @@ pub struct PusStack<TmSender: EcssTmSender, TcInMemConverter: EcssTcInMemConvert
 impl<TmSender: EcssTmSender, TcInMemConverter: EcssTcInMemConverter>
     PusStack<TmSender, TcInMemConverter>
 {
+    pub fn direct_service_checker<S: DirectPusService>(
+        service: &mut S,
+        timestamp: &[u8],
+        nothing_to_do: &mut bool,
+    ) {
+        let handling_status = service.poll_and_handle_next_tc(timestamp);
+        if handling_status == HandlingStatus::HandledOne {
+            *nothing_to_do = false;
+        }
+    }
+
+    pub fn targeted_service_checker<S: TargetedPusService>(
+        service: &mut S,
+        timestamp: &[u8],
+        nothing_to_do: &mut bool,
+    ) {
+        let request_handling = service.poll_and_handle_next_tc_default_handler(timestamp);
+        let reply_handling = service.poll_and_handle_next_reply_default_handler(timestamp);
+        if request_handling == HandlingStatus::HandledOne
+            || reply_handling == HandlingStatus::HandledOne
+        {
+            *nothing_to_do = false;
+        }
+    }
+
     pub fn periodic_operation(&mut self) {
         // Release all telecommands which reached their release time before calling the service
         // handlers.
         self.schedule_srv.release_tcs();
-        let time_stamp = cds::CdsTime::now_with_u16_days()
+        let timestamp = cds::CdsTime::now_with_u16_days()
             .expect("time stamp generation error")
             .to_vec()
             .unwrap();
         // Hot loop which will run continuously until all request and reply handling is done.
         loop {
             let mut nothing_to_do = true;
-            let mut is_srv_finished =
-                |_srv_id: u8,
-                 tc_handling_status: HandlingStatus,
-                 reply_handling_status: Option<HandlingStatus>| {
-                    if tc_handling_status == HandlingStatus::HandledOne
-                        || (reply_handling_status.is_some()
-                            && reply_handling_status.unwrap() == HandlingStatus::HandledOne)
-                    {
-                        nothing_to_do = false;
-                    }
-                };
-            is_srv_finished(
-                PusServiceId::Test as u8,
-                self.test_srv.poll_and_handle_next_tc(&time_stamp),
-                None,
+            Self::direct_service_checker(&mut self.test_srv, &timestamp, &mut nothing_to_do);
+            Self::direct_service_checker(&mut self.schedule_srv, &timestamp, &mut nothing_to_do);
+            Self::direct_service_checker(&mut self.event_srv, &timestamp, &mut nothing_to_do);
+            Self::targeted_service_checker(
+                &mut self.action_srv_wrapper,
+                &timestamp,
+                &mut nothing_to_do,
             );
-            is_srv_finished(
-                PusServiceId::Scheduling as u8,
-                self.schedule_srv.poll_and_handle_next_tc(&time_stamp),
-                None,
-            );
-            is_srv_finished(
-                PusServiceId::Event as u8,
-                self.event_srv.poll_and_handle_next_tc(&time_stamp),
-                None,
-            );
-            is_srv_finished(
-                PusServiceId::Action as u8,
-                self.action_srv_wrapper
-                    .poll_and_handle_next_tc_default_handler(&time_stamp),
-                Some(
-                    self.action_srv_wrapper
-                        .poll_and_handle_next_reply_default_handler(&time_stamp),
-                ),
-            );
-            is_srv_finished(
-                PusServiceId::Housekeeping as u8,
-                self.hk_srv_wrapper
-                    .poll_and_handle_next_tc_default_handler(&time_stamp),
-                Some(
-                    self.hk_srv_wrapper
-                        .poll_and_handle_next_reply_default_handler(&time_stamp),
-                ),
-            );
-            is_srv_finished(
-                CustomPusServiceId::Mode as u8,
-                self.mode_srv
-                    .poll_and_handle_next_tc_default_handler(&time_stamp),
-                Some(
-                    self.mode_srv
-                        .poll_and_handle_next_reply_default_handler(&time_stamp),
-                ),
+            Self::targeted_service_checker(
+                &mut self.hk_srv_wrapper,
+                &timestamp,
+                &mut nothing_to_do,
             );
+            Self::targeted_service_checker(&mut self.mode_srv, &timestamp, &mut nothing_to_do);
             if nothing_to_do {
                 // Timeout checking is only done once.
                 self.action_srv_wrapper.check_for_request_timeouts();

satrs-example/src/pus/test.rs

@@ -33,7 +33,7 @@ pub fn create_test_service_static(
     ));
     TestCustomServiceWrapper {
         handler: pus17_handler,
-        test_srv_event_sender: event_sender,
+        event_tx: event_sender,
     }
 }
 
@@ -51,7 +51,7 @@ pub fn create_test_service_dynamic(
     ));
     TestCustomServiceWrapper {
         handler: pus17_handler,
-        test_srv_event_sender: event_sender,
+        event_tx: event_sender,
     }
 }
 
@@ -59,7 +59,7 @@ pub struct TestCustomServiceWrapper<TmSender: EcssTmSender, TcInMemConverter: Ec
 {
     pub handler:
         PusService17TestHandler<MpscTcReceiver, TmSender, TcInMemConverter, VerificationReporter>,
-    pub test_srv_event_sender: mpsc::SyncSender<EventMessageU32>,
+    pub event_tx: mpsc::SyncSender<EventMessageU32>,
 }
 
 impl<TmSender: EcssTmSender, TcInMemConverter: EcssTcInMemConverter> DirectPusService
@@ -68,12 +68,8 @@ impl<TmSender: EcssTmSender, TcInMemConverter: EcssTcInMemConverter> DirectPusSe
     const SERVICE_ID: u8 = PusServiceId::Test as u8;
 
     const SERVICE_STR: &'static str = "test";
-}
 
-impl<TmSender: EcssTmSender, TcInMemConverter: EcssTcInMemConverter>
-    TestCustomServiceWrapper<TmSender, TcInMemConverter>
-{
-    pub fn poll_and_handle_next_tc(&mut self, timestamp: &[u8]) -> HandlingStatus {
+    fn poll_and_handle_next_tc(&mut self, timestamp: &[u8]) -> HandlingStatus {
         let error_handler = |partial_error: &PartialPusHandlingError| {
             log::warn!(
                 "PUS {}({}) partial error: {:?}",
@@ -120,7 +116,7 @@ impl<TmSender: EcssTmSender, TcInMemConverter: EcssTcInMemConverter>
                 .unwrap();
                 if subservice == 128 {
                     info!("generating test event");
-                    self.test_srv_event_sender
+                    self.event_tx
                         .send(EventMessage::new(PUS_TEST_SERVICE.id(), TEST_EVENT.into()))
                         .expect("Sending test event failed");
                     match self.handler.service_helper.verif_reporter().start_success(