From 40120dc83f46e4e1127a64f0fa9246350ed96c37 Mon Sep 17 00:00:00 2001 From: Robin Mueller Date: Fri, 27 Jan 2023 00:04:56 +0100 Subject: [PATCH] various fixes and updates for satrs-core 1. Fix pools integration test to avoid occasional deadlocks 2. tmtc module: The `CcsdsPacketHandler` and `PusServiceProvider` do not require a send bound anymore. However, these traits now have an extension trait called `Sendable` which requires the trait + Send. A blanket implementation is provided. The helper structs like `PusDistributor` and `CcsdsDistributor` require the new Sendable trait version to allow more ergnomic usage with threads. --- satrs-core/src/tmtc/ccsds_distrib.rs | 19 ++++++++++++++----- satrs-core/src/tmtc/mod.rs | 9 +++++---- satrs-core/src/tmtc/pus_distrib.rs | 22 +++++++++++++++++----- satrs-core/tests/pools.rs | 3 ++- 4 files changed, 38 insertions(+), 15 deletions(-) diff --git a/satrs-core/src/tmtc/ccsds_distrib.rs b/satrs-core/src/tmtc/ccsds_distrib.rs index 55fd2ec..79b7b63 100644 --- a/satrs-core/src/tmtc/ccsds_distrib.rs +++ b/satrs-core/src/tmtc/ccsds_distrib.rs @@ -99,7 +99,7 @@ use spacepackets::{ByteConversionError, CcsdsPacket, SizeMissmatch, SpHeader}; /// This trait automatically implements the [downcast_rs::Downcast] to allow a more convenient API /// to cast trait objects back to their concrete type after the handler was passed to the /// distributor. -pub trait CcsdsPacketHandler: Downcast + Send { +pub trait CcsdsPacketHandler: Downcast { type Error; fn valid_apids(&self) -> &'static [u16]; @@ -114,12 +114,21 @@ pub trait CcsdsPacketHandler: Downcast + Send { downcast_rs::impl_downcast!(CcsdsPacketHandler assoc Error); +pub trait SendableCcsdsPacketHandler: CcsdsPacketHandler + Send {} + +impl SendableCcsdsPacketHandler for T {} + +downcast_rs::impl_downcast!(SendableCcsdsPacketHandler assoc Error); + /// The CCSDS distributor dispatches received CCSDS packets to a user provided packet handler. +/// +/// The passed APID handler is required to be [Send]able to allow more ergonomic usage with +/// threads. pub struct CcsdsDistributor { /// User provided APID handler stored as a generic trait object. /// It can be cast back to the original concrete type using the [Self::apid_handler_ref] or /// the [Self::apid_handler_mut] method. - pub apid_handler: Box>, + pub apid_handler: Box>, } #[derive(Debug, Copy, Clone, PartialEq, Eq)] @@ -155,20 +164,20 @@ impl ReceivesTcCore for CcsdsDistributor { } impl CcsdsDistributor { - pub fn new(apid_handler: Box>) -> Self { + pub fn new(apid_handler: Box>) -> Self { CcsdsDistributor { apid_handler } } /// This function can be used to retrieve a reference to the concrete instance of the APID /// handler after it was passed to the distributor. See the /// [module documentation][crate::tmtc::ccsds_distrib] for an fsrc-example. - pub fn apid_handler_ref>(&self) -> Option<&T> { + pub fn apid_handler_ref>(&self) -> Option<&T> { self.apid_handler.downcast_ref::() } /// This function can be used to retrieve a mutable reference to the concrete instance of the /// APID handler after it was passed to the distributor. - pub fn apid_handler_mut>(&mut self) -> Option<&mut T> { + pub fn apid_handler_mut>(&mut self) -> Option<&mut T> { self.apid_handler.downcast_mut::() } diff --git a/satrs-core/src/tmtc/mod.rs b/satrs-core/src/tmtc/mod.rs index 1ae4ac8..f410eea 100644 --- a/satrs-core/src/tmtc/mod.rs +++ b/satrs-core/src/tmtc/mod.rs @@ -63,19 +63,20 @@ impl AddressableId { /// This trait is implemented by both the [crate::tmtc::pus_distrib::PusDistributor] and the /// [crate::tmtc::ccsds_distrib::CcsdsDistributor] which allows to pass the respective packets in /// raw byte format into them. -pub trait ReceivesTcCore: Send { +pub trait ReceivesTcCore { type Error; fn pass_tc(&mut self, tc_raw: &[u8]) -> Result<(), Self::Error>; } -/// Extension trait of [ReceivesTcCore] which allows downcasting by implementing [Downcast] +/// Extension trait of [ReceivesTcCore] which allows downcasting by implementing [Downcast] and +/// is also sendable. #[cfg(feature = "alloc")] -pub trait ReceivesTc: ReceivesTcCore + Downcast {} +pub trait ReceivesTc: ReceivesTcCore + Downcast + Send {} /// Blanket implementation to automatically implement [ReceivesTc] when the [alloc] feature /// is enabled. #[cfg(feature = "alloc")] -impl ReceivesTc for T where T: ReceivesTcCore + 'static {} +impl ReceivesTc for T where T: ReceivesTcCore + Send + 'static {} #[cfg(feature = "alloc")] impl_downcast!(ReceivesTc assoc Error); diff --git a/satrs-core/src/tmtc/pus_distrib.rs b/satrs-core/src/tmtc/pus_distrib.rs index 6e457ac..9420b4c 100644 --- a/satrs-core/src/tmtc/pus_distrib.rs +++ b/satrs-core/src/tmtc/pus_distrib.rs @@ -67,7 +67,7 @@ use spacepackets::ecss::{PusError, PusPacket}; use spacepackets::tc::PusTc; use spacepackets::SpHeader; -pub trait PusServiceProvider: Downcast + Send { +pub trait PusServiceProvider: Downcast { type Error; fn handle_pus_tc_packet( &mut self, @@ -78,12 +78,22 @@ pub trait PusServiceProvider: Downcast + Send { } downcast_rs::impl_downcast!(PusServiceProvider assoc Error); +pub trait SendablePusServiceProvider: PusServiceProvider + Send {} + +impl SendablePusServiceProvider for T {} + +downcast_rs::impl_downcast!(SendablePusServiceProvider assoc Error); + +/// Generic distributor object which dispatches received packets to a user provided handler. +/// +/// This distributor expects the passed trait object to be [Send]able to allow more ergonomic +/// usage with threads. pub struct PusDistributor { - pub service_provider: Box>, + pub service_provider: Box>, } impl PusDistributor { - pub fn new(service_provider: Box>) -> Self { + pub fn new(service_provider: Box>) -> Self { PusDistributor { service_provider } } } @@ -122,11 +132,13 @@ impl ReceivesEcssPusTc for PusDistributor { } impl PusDistributor { - pub fn service_provider_ref>(&self) -> Option<&T> { + pub fn service_provider_ref>(&self) -> Option<&T> { self.service_provider.downcast_ref::() } - pub fn service_provider_mut>(&mut self) -> Option<&mut T> { + pub fn service_provider_mut>( + &mut self, + ) -> Option<&mut T> { self.service_provider.downcast_mut::() } } diff --git a/satrs-core/tests/pools.rs b/satrs-core/tests/pools.rs index c92570e..375d624 100644 --- a/satrs-core/tests/pools.rs +++ b/satrs-core/tests/pools.rs @@ -20,14 +20,15 @@ fn threaded_usage() { }); let jh1 = thread::spawn(move || { - let mut pool_access = shared_clone.write().unwrap(); let addr; { addr = rx.recv().expect("Receiving store address failed"); + let mut pool_access = shared_clone.write().unwrap(); let pg = PoolGuard::new(pool_access.deref_mut(), addr); let read_res = pg.read().expect("Reading failed"); assert_eq!(read_res, DUMMY_DATA); } + let pool_access = shared_clone.read().unwrap(); assert!(!pool_access.has_element_at(&addr).expect("Invalid address")); }); jh0.join().unwrap();