From 7bbbab56d578de3d67e78dd282e1b0ff712c7af4 Mon Sep 17 00:00:00 2001
From: Robin Mueller <robin.mueller.m@gmail.com>
Date: Wed, 28 Feb 2024 17:20:51 +0100
Subject: [PATCH] more layering improvements

---
 satrs/src/mode.rs        | 342 ++++++++++++++++++++++++---------------
 satrs/src/request.rs     |  29 +++-
 satrs/tests/mode_tree.rs |  33 ++--
 3 files changed, 252 insertions(+), 152 deletions(-)

diff --git a/satrs/src/mode.rs b/satrs/src/mode.rs
index 17f2393..72a316d 100644
--- a/satrs/src/mode.rs
+++ b/satrs/src/mode.rs
@@ -3,6 +3,12 @@ use core::mem::size_of;
 use serde::{Deserialize, Serialize};
 use spacepackets::ByteConversionError;
 
+#[cfg(feature = "alloc")]
+pub use alloc_mod::*;
+
+#[cfg(feature = "std")]
+pub use std_mod::*;
+
 use crate::{
     queue::GenericTargetedMessagingError,
     request::{GenericMessage, MessageReceiver, MessageReceiverWithId, RequestId},
@@ -201,11 +207,115 @@ pub trait ModeRequestHandler: ModeProvider {
 #[cfg_attr(doc_cfg, doc(cfg(feature = "alloc")))]
 pub mod alloc_mod {
     use crate::request::{
-        MessageSender, MessageSenderAndReceiver, RequestAndReplySenderAndReceiver,
+        MessageSender, MessageSenderAndReceiver, MessageSenderMap, MessageSenderMapWithId,
+        RequestAndReplySenderAndReceiver,
     };
 
     use super::*;
 
+    impl<S: MessageSender<ModeRequest>> MessageSenderMap<ModeRequest, S> {
+        pub fn send_mode_request(
+            &self,
+            request_id: RequestId,
+            local_id: ChannelId,
+            target_id: ChannelId,
+            request: ModeRequest,
+        ) -> Result<(), GenericTargetedMessagingError> {
+            self.send_message(request_id, local_id, target_id, request)
+        }
+
+        pub fn add_request_target(&mut self, target_id: ChannelId, request_sender: S) {
+            self.add_message_target(target_id, request_sender)
+        }
+    }
+
+    impl<S: MessageSender<ModeReply>> MessageSenderMap<ModeReply, S> {
+        pub fn send_mode_reply(
+            &self,
+            request_id: RequestId,
+            local_id: ChannelId,
+            target_id: ChannelId,
+            request: ModeReply,
+        ) -> Result<(), GenericTargetedMessagingError> {
+            self.send_message(request_id, local_id, target_id, request)
+        }
+
+        pub fn add_reply_target(&mut self, target_id: ChannelId, request_sender: S) {
+            self.add_message_target(target_id, request_sender)
+        }
+    }
+
+    impl<S: MessageSender<ModeReply>> ModeReplySender for MessageSenderMapWithId<ModeReply, S> {
+        fn send_mode_reply(
+            &self,
+            request_id: RequestId,
+            target_channel_id: ChannelId,
+            reply: ModeReply,
+        ) -> Result<(), GenericTargetedMessagingError> {
+            self.send_message(request_id, target_channel_id, reply)
+        }
+
+        fn local_channel_id(&self) -> ChannelId {
+            self.local_channel_id
+        }
+    }
+
+    impl<S: MessageSender<ModeRequest>> ModeRequestSender for MessageSenderMapWithId<ModeRequest, S> {
+        fn local_channel_id(&self) -> ChannelId {
+            self.local_channel_id
+        }
+
+        fn send_mode_request(
+            &self,
+            request_id: RequestId,
+            target_id: ChannelId,
+            request: ModeRequest,
+        ) -> Result<(), GenericTargetedMessagingError> {
+            self.send_message(request_id, target_id, request)
+        }
+    }
+
+    impl<FROM, S: MessageSender<ModeReply>, R: MessageReceiver<FROM>> ModeReplySender
+        for MessageSenderAndReceiver<ModeReply, FROM, S, R>
+    {
+        fn local_channel_id(&self) -> ChannelId {
+            self.local_channel_id_generic()
+        }
+
+        fn send_mode_reply(
+            &self,
+            request_id: RequestId,
+            target_id: ChannelId,
+            request: ModeReply,
+        ) -> Result<(), GenericTargetedMessagingError> {
+            self.message_sender_map.send_mode_reply(
+                request_id,
+                self.local_channel_id(),
+                target_id,
+                request,
+            )
+        }
+    }
+
+    impl<TO, S: MessageSender<TO>, R: MessageReceiver<ModeReply>> ModeReplyReceiver
+        for MessageSenderAndReceiver<TO, ModeReply, S, R>
+    {
+        fn try_recv_mode_reply(
+            &self,
+        ) -> Result<Option<GenericMessage<ModeReply>>, GenericTargetedMessagingError> {
+            self.message_receiver.try_recv_message()
+        }
+    }
+    impl<TO, S: MessageSender<TO>, R: MessageReceiver<ModeRequest>> ModeRequestReceiver
+        for MessageSenderAndReceiver<TO, ModeRequest, S, R>
+    {
+        fn try_recv_mode_request(
+            &self,
+        ) -> Result<Option<GenericMessage<ModeRequest>>, GenericTargetedMessagingError> {
+            self.message_receiver.try_recv_message()
+        }
+    }
+
     impl<FROM, S: MessageSender<ModeRequest>, R: MessageReceiver<FROM>> ModeRequestSender
         for MessageSenderAndReceiver<ModeRequest, FROM, S, R>
     {
@@ -323,8 +433,7 @@ pub mod alloc_mod {
         fn try_recv_mode_reply(
             &self,
         ) -> Result<Option<GenericMessage<ModeReply>>, GenericTargetedMessagingError> {
-            self.reply_receiver
-                .try_recv_message(self.local_channel_id_generic())
+            self.reply_receiver.try_recv_message()
         }
     }
 
@@ -340,10 +449,57 @@ pub mod alloc_mod {
         fn try_recv_mode_request(
             &self,
         ) -> Result<Option<GenericMessage<ModeRequest>>, GenericTargetedMessagingError> {
-            self.request_receiver
-                .try_recv_message(self.local_channel_id_generic())
+            self.request_receiver.try_recv_message()
         }
     }
+
+    /// Helper type definition for a mode handler which can handle mode requests.
+    pub type ModeRequestHandlerInterface<S, R> =
+        MessageSenderAndReceiver<ModeReply, ModeRequest, S, R>;
+
+    impl<S: MessageSender<ModeReply>, R: MessageReceiver<ModeRequest>>
+        ModeRequestHandlerInterface<S, R>
+    {
+        pub fn try_recv_mode_request(
+            &self,
+        ) -> Result<Option<GenericMessage<ModeRequest>>, GenericTargetedMessagingError> {
+            self.try_recv_message()
+        }
+
+        pub fn send_mode_reply(
+            &self,
+            request_id: RequestId,
+            target_id: ChannelId,
+            reply: ModeReply,
+        ) -> Result<(), GenericTargetedMessagingError> {
+            self.send_message(request_id, target_id, reply)
+        }
+    }
+
+    /// Helper type defintion for a mode handler object which can send mode requests.
+    pub type ModeRequestorInterface<S, R> = MessageSenderAndReceiver<ModeRequest, ModeReply, S, R>;
+
+    impl<S: MessageSender<ModeRequest>, R: MessageReceiver<ModeReply>> ModeRequestorInterface<S, R> {
+        pub fn try_recv_mode_reply(
+            &self,
+        ) -> Result<Option<GenericMessage<ModeReply>>, GenericTargetedMessagingError> {
+            self.try_recv_message()
+        }
+
+        pub fn send_mode_request(
+            &self,
+            request_id: RequestId,
+            target_id: ChannelId,
+            reply: ModeRequest,
+        ) -> Result<(), GenericTargetedMessagingError> {
+            self.send_message(request_id, target_id, reply)
+        }
+    }
+
+    /// Helper type defintion for a mode handler object which can both send mode requests and
+    /// process mode requests.
+    pub type ModeInterface<S0, R0, S1, R1> =
+        RequestAndReplySenderAndReceiver<ModeRequest, ModeReply, S0, R0, S1, R1>;
 }
 
 #[cfg(feature = "std")]
@@ -351,148 +507,33 @@ pub mod alloc_mod {
 pub mod std_mod {
     use std::sync::mpsc;
 
-    use crate::request::{
-        MessageSender, MessageSenderAndReceiver, MessageSenderMap, MessageSenderMapWithId,
-        RequestAndReplySenderAndReceiver,
-    };
-
     use super::*;
 
-    impl<S: MessageSender<ModeRequest>> MessageSenderMap<ModeRequest, S> {
-        pub fn send_mode_request(
-            &self,
-            request_id: RequestId,
-            local_id: ChannelId,
-            target_id: ChannelId,
-            request: ModeRequest,
-        ) -> Result<(), GenericTargetedMessagingError> {
-            self.send_message(request_id, local_id, target_id, request)
-        }
-
-        pub fn add_request_target(&mut self, target_id: ChannelId, request_sender: S) {
-            self.add_message_target(target_id, request_sender)
-        }
-    }
-
-    impl<S: MessageSender<ModeReply>> MessageSenderMap<ModeReply, S> {
-        pub fn send_mode_reply(
-            &self,
-            request_id: RequestId,
-            local_id: ChannelId,
-            target_id: ChannelId,
-            request: ModeReply,
-        ) -> Result<(), GenericTargetedMessagingError> {
-            self.send_message(request_id, local_id, target_id, request)
-        }
-
-        pub fn add_reply_target(&mut self, target_id: ChannelId, request_sender: S) {
-            self.add_message_target(target_id, request_sender)
-        }
-    }
-
-    impl<S: MessageSender<ModeReply>> ModeReplySender for MessageSenderMapWithId<ModeReply, S> {
-        fn send_mode_reply(
-            &self,
-            request_id: RequestId,
-            target_channel_id: ChannelId,
-            reply: ModeReply,
-        ) -> Result<(), GenericTargetedMessagingError> {
-            self.send_message(request_id, target_channel_id, reply)
-        }
-
-        fn local_channel_id(&self) -> ChannelId {
-            self.local_channel_id
-        }
-    }
-
-    impl<S: MessageSender<ModeRequest>> ModeRequestSender for MessageSenderMapWithId<ModeRequest, S> {
-        fn local_channel_id(&self) -> ChannelId {
-            self.local_channel_id
-        }
-
-        fn send_mode_request(
-            &self,
-            request_id: RequestId,
-            target_id: ChannelId,
-            request: ModeRequest,
-        ) -> Result<(), GenericTargetedMessagingError> {
-            self.send_message(request_id, target_id, request)
-        }
-    }
-
-    impl<FROM, S: MessageSender<ModeReply>, R: MessageReceiver<FROM>> ModeReplySender
-        for MessageSenderAndReceiver<ModeReply, FROM, S, R>
-    {
-        fn local_channel_id(&self) -> ChannelId {
-            self.local_channel_id_generic()
-        }
-
-        fn send_mode_reply(
-            &self,
-            request_id: RequestId,
-            target_id: ChannelId,
-            request: ModeReply,
-        ) -> Result<(), GenericTargetedMessagingError> {
-            self.message_sender_map.send_mode_reply(
-                request_id,
-                self.local_channel_id(),
-                target_id,
-                request,
-            )
-        }
-    }
-
-    impl<TO, S: MessageSender<TO>, R: MessageReceiver<ModeReply>> ModeReplyReceiver
-        for MessageSenderAndReceiver<TO, ModeReply, S, R>
-    {
-        fn try_recv_mode_reply(
-            &self,
-        ) -> Result<Option<GenericMessage<ModeReply>>, GenericTargetedMessagingError> {
-            self.message_receiver
-                .try_recv_message(self.local_channel_id_generic())
-        }
-    }
-    impl<TO, S: MessageSender<TO>, R: MessageReceiver<ModeRequest>> ModeRequestReceiver
-        for MessageSenderAndReceiver<TO, ModeRequest, S, R>
-    {
-        fn try_recv_mode_request(
-            &self,
-        ) -> Result<Option<GenericMessage<ModeRequest>>, GenericTargetedMessagingError> {
-            self.message_receiver
-                .try_recv_message(self.local_channel_id_generic())
-        }
-    }
-
-    pub type ModeRequestHandlerConnector<S, R> =
-        MessageSenderAndReceiver<ModeReply, ModeRequest, S, R>;
-    pub type MpscModeRequestHandlerConnector = ModeRequestHandlerConnector<
+    pub type ModeRequestHandlerMpsc = ModeRequestHandlerInterface<
         mpsc::Sender<GenericMessage<ModeReply>>,
         mpsc::Receiver<GenericMessage<ModeRequest>>,
     >;
-    pub type MpscBoundedModeRequestHandlerConnector = ModeRequestHandlerConnector<
+    pub type ModeRequestHandlerMpscBounded = ModeRequestHandlerInterface<
         mpsc::SyncSender<GenericMessage<ModeReply>>,
         mpsc::Receiver<GenericMessage<ModeRequest>>,
     >;
 
-    pub type ModeRequestorConnector<S, R> = MessageSenderAndReceiver<ModeRequest, ModeReply, S, R>;
-    pub type MpscModeRequestorConnector = ModeRequestorConnector<
+    pub type ModeRequestorMpsc = ModeRequestorInterface<
         mpsc::Sender<GenericMessage<ModeRequest>>,
         mpsc::Receiver<GenericMessage<ModeReply>>,
     >;
-    pub type MpscBoundedModeRequestorConnector = ModeRequestorConnector<
+    pub type ModeRequestorBoundedMpsc = ModeRequestorInterface<
         mpsc::SyncSender<GenericMessage<ModeRequest>>,
         mpsc::Receiver<GenericMessage<ModeReply>>,
     >;
 
-    pub type ModeConnector<S0, R0, S1, R1> =
-        RequestAndReplySenderAndReceiver<ModeRequest, ModeReply, S0, R0, S1, R1>;
-    pub type MpscModeConnector = ModeConnector<
+    pub type ModeRequestorAndHandlerMpsc = ModeInterface<
         mpsc::Sender<GenericMessage<ModeRequest>>,
         mpsc::Receiver<GenericMessage<ModeReply>>,
         mpsc::Sender<GenericMessage<ModeReply>>,
         mpsc::Receiver<GenericMessage<ModeRequest>>,
     >;
-    pub type MpscBoundedModeConnector = ModeConnector<
+    pub type ModeRequestorAndHandlerMpscBounded = ModeInterface<
         mpsc::SyncSender<GenericMessage<ModeRequest>>,
         mpsc::Receiver<GenericMessage<ModeReply>>,
         mpsc::SyncSender<GenericMessage<ModeReply>>,
@@ -502,7 +543,54 @@ pub mod std_mod {
 
 #[cfg(test)]
 mod tests {
+    use std::sync::mpsc;
+
+    use crate::{
+        mode::{ModeAndSubmode, ModeReply},
+        request::GenericMessage,
+    };
+
+    use super::{ModeRequest, ModeRequestorMpsc};
+
+    const TEST_CHANNEL_ID_0: u32 = 5;
+    const TEST_CHANNEL_ID_1: u32 = 6;
+    const TEST_CHANNEL_ID_2: u32 = 7;
 
     #[test]
-    fn test_request_sender() {}
+    fn test_simple_mode_requestor() {
+        let (reply_sender, reply_receiver) = mpsc::channel();
+        let (request_sender, request_receiver) = mpsc::channel();
+        let mut mode_requestor = ModeRequestorMpsc::new(TEST_CHANNEL_ID_0, reply_receiver);
+        mode_requestor.add_message_target(TEST_CHANNEL_ID_1, request_sender);
+
+        // Send a request and verify it arrives at the receiver.
+        let request_id = 2;
+        let sent_request = ModeRequest::ReadMode;
+        mode_requestor
+            .send_mode_request(request_id, TEST_CHANNEL_ID_1, sent_request)
+            .expect("send failed");
+        let request = request_receiver.recv().expect("recv failed");
+        assert_eq!(request.request_id, 2);
+        assert_eq!(request.sender_id, TEST_CHANNEL_ID_0);
+        assert_eq!(request.message, sent_request);
+
+        // Send a reply and verify it arrives at the requestor.
+        let mode_reply = ModeReply::ModeReply(ModeAndSubmode::new(1, 5));
+        reply_sender
+            .send(GenericMessage::new(
+                request_id,
+                TEST_CHANNEL_ID_1,
+                mode_reply,
+            ))
+            .expect("send failed");
+        let reply = mode_requestor.try_recv_mode_reply().expect("recv failed");
+        assert!(reply.is_some());
+        let reply = reply.unwrap();
+        assert_eq!(reply.sender_id, TEST_CHANNEL_ID_1);
+        assert_eq!(reply.request_id, 2);
+        assert_eq!(reply.message, mode_reply);
+    }
+
+    #[test]
+    fn test_simple_mode_request_handler() {}
 }
diff --git a/satrs/src/request.rs b/satrs/src/request.rs
index 4d09cec..b0aada7 100644
--- a/satrs/src/request.rs
+++ b/satrs/src/request.rs
@@ -165,7 +165,6 @@ impl<MSG, R: MessageReceiver<MSG>> From<R> for MessageWithSenderIdReceiver<MSG,
 impl<MSG, R: MessageReceiver<MSG>> MessageWithSenderIdReceiver<MSG, R> {
     pub fn try_recv_message(
         &self,
-        _local_id: ChannelId,
     ) -> Result<Option<GenericMessage<MSG>>, GenericTargetedMessagingError> {
         self.0.try_recv()
     }
@@ -303,6 +302,28 @@ pub mod alloc_mod {
         pub fn local_channel_id_generic(&self) -> ChannelId {
             self.local_channel_id
         }
+
+        /// Try to send a message, which can be a reply or a request, depending on the generics.
+        pub fn send_message(
+            &self,
+            request_id: RequestId,
+            target_channel_id: ChannelId,
+            message: TO,
+        ) -> Result<(), GenericTargetedMessagingError> {
+            self.message_sender_map.send_message(
+                request_id,
+                self.local_channel_id_generic(),
+                target_channel_id,
+                message,
+            )
+        }
+
+        /// Try to receive a message, which can be a reply or a request, depending on the generics.
+        pub fn try_recv_message(
+            &self,
+        ) -> Result<Option<GenericMessage<FROM>>, GenericTargetedMessagingError> {
+            self.message_receiver.try_recv_message()
+        }
     }
 
     pub struct RequestAndReplySenderAndReceiver<
@@ -377,8 +398,8 @@ pub mod std_mod {
         }
     }
 
-    pub type MpscSenderMap<MSG> = MessageReceiverWithId<MSG, mpsc::Sender<MSG>>;
-    pub type MpscBoundedSenderMap<MSG> = MessageReceiverWithId<MSG, mpsc::SyncSender<MSG>>;
+    pub type MessageSenderMapMpsc<MSG> = MessageReceiverWithId<MSG, mpsc::Sender<MSG>>;
+    pub type MessageSenderMapBoundedMpsc<MSG> = MessageReceiverWithId<MSG, mpsc::SyncSender<MSG>>;
 
     impl<MSG> MessageReceiver<MSG> for mpsc::Receiver<GenericMessage<MSG>> {
         fn try_recv(&self) -> Result<Option<GenericMessage<MSG>>, GenericTargetedMessagingError> {
@@ -394,5 +415,5 @@ pub mod std_mod {
         }
     }
 
-    pub type MpscMessageReceiverWithId<MSG> = MessageReceiverWithId<MSG, mpsc::Receiver<MSG>>;
+    pub type MessageReceiverWithIdMpsc<MSG> = MessageReceiverWithId<MSG, mpsc::Receiver<MSG>>;
 }
diff --git a/satrs/tests/mode_tree.rs b/satrs/tests/mode_tree.rs
index 88f1e4f..c9b111e 100644
--- a/satrs/tests/mode_tree.rs
+++ b/satrs/tests/mode_tree.rs
@@ -1,13 +1,10 @@
 use core::cell::Cell;
 use std::{println, sync::mpsc};
 
-use satrs::mode::std_mod::{
-    MpscBoundedModeConnector, MpscBoundedModeRequestHandlerConnector,
-    MpscBoundedModeRequestorConnector,
-};
 use satrs::mode::{
     ModeError, ModeProvider, ModeReplyReceiver, ModeReplySender, ModeRequestHandler,
-    ModeRequestReceiver, ModeRequestSender,
+    ModeRequestHandlerMpscBounded, ModeRequestReceiver, ModeRequestSender,
+    ModeRequestorAndHandlerMpscBounded, ModeRequestorBoundedMpsc,
 };
 use satrs::request::RequestId;
 use satrs::{
@@ -27,7 +24,7 @@ pub enum TestChannelId {
 
 struct PusModeService {
     pub request_id_counter: Cell<u32>,
-    pub mode_node: MpscBoundedModeRequestorConnector,
+    pub mode_node: ModeRequestorBoundedMpsc,
 }
 
 impl PusModeService {
@@ -46,7 +43,7 @@ impl PusModeService {
 
 struct TestDevice {
     pub name: String,
-    pub mode_node: MpscBoundedModeRequestHandlerConnector,
+    pub mode_node: ModeRequestHandlerMpscBounded,
     pub mode_and_submode: ModeAndSubmode,
     pub mode_requestor_info: Option<(RequestId, ChannelId)>,
 }
@@ -120,7 +117,7 @@ impl ModeRequestHandler for TestDevice {
 }
 
 struct TestAssembly {
-    pub mode_node: MpscBoundedModeConnector,
+    pub mode_node: ModeRequestorAndHandlerMpscBounded,
     pub mode_requestor_info: Option<(RequestId, ChannelId)>,
     pub mode_and_submode: ModeAndSubmode,
     pub target_mode_and_submode: Option<ModeAndSubmode>,
@@ -246,26 +243,20 @@ fn main() {
     let (reply_sender_to_pus, reply_receiver_pus) = mpsc::sync_channel(10);
 
     // Mode requestors and handlers.
-    let mut mode_node_assy = MpscBoundedModeConnector::new(
+    let mut mode_node_assy = ModeRequestorAndHandlerMpscBounded::new(
         TestChannelId::Assembly as u32,
         request_receiver_assy,
         reply_receiver_assy,
     );
     // Mode requestors only.
-    let mut mode_node_pus = MpscBoundedModeRequestorConnector::new(
-        TestChannelId::PusModeService as u32,
-        reply_receiver_pus,
-    );
+    let mut mode_node_pus =
+        ModeRequestorBoundedMpsc::new(TestChannelId::PusModeService as u32, reply_receiver_pus);
 
     // Request handlers only.
-    let mut mode_node_dev1 = MpscBoundedModeRequestHandlerConnector::new(
-        TestChannelId::Device1 as u32,
-        request_receiver_dev1,
-    );
-    let mut mode_node_dev2 = MpscBoundedModeRequestHandlerConnector::new(
-        TestChannelId::Device2 as u32,
-        request_receiver_dev2,
-    );
+    let mut mode_node_dev1 =
+        ModeRequestHandlerMpscBounded::new(TestChannelId::Device1 as u32, request_receiver_dev1);
+    let mut mode_node_dev2 =
+        ModeRequestHandlerMpscBounded::new(TestChannelId::Device2 as u32, request_receiver_dev2);
 
     // Set up mode request senders first.
     mode_node_pus.add_message_target(TestChannelId::Assembly as u32, request_sender_to_assy);