From eaaf458226ad4e9f4d145504611e97e5326f9c1f Mon Sep 17 00:00:00 2001 From: Jimy Byerley Date: Fri, 7 Jul 2023 16:55:30 +0200 Subject: [PATCH] Config is now thread-safe --- examples/sdo_mapping.rs | 1 + examples/servodrive_run.rs | 6 +++--- src/mapping.rs | 42 +++++++++++++++++--------------------- 3 files changed, 23 insertions(+), 26 deletions(-) diff --git a/examples/sdo_mapping.rs b/examples/sdo_mapping.rs index a7220f4..2aa151f 100644 --- a/examples/sdo_mapping.rs +++ b/examples/sdo_mapping.rs @@ -37,6 +37,7 @@ async fn main() -> std::io::Result<()> { let error = pdo.push(Sdo::::complete(0x603f)); let position = pdo.push(Sdo::::complete(0x6064)); let torque = pdo.push(Sdo::::complete(0x6077)); + drop(slave); println!("done {:#?}", config); let allocator = mapping::Allocator::new(); diff --git a/examples/servodrive_run.rs b/examples/servodrive_run.rs index 088a180..51cd7b8 100644 --- a/examples/servodrive_run.rs +++ b/examples/servodrive_run.rs @@ -44,6 +44,7 @@ async fn main() -> std::io::Result<()> { let current_position = pdo.push(sdo::cia402::current::position); let _current_velocity = pdo.push(sdo::cia402::current::velocity); let _current_torque = pdo.push(sdo::cia402::current::torque); + drop(slave); println!("done {:#?}", config); let allocator = mapping::Allocator::new(); @@ -69,10 +70,9 @@ async fn main() -> std::io::Result<()> { async { let mut period = tokio::time::interval(Duration::from_millis(1)); loop { - let mut group = group.data().await; - period.tick().await; - group.exchange().await; + group.data().await.exchange().await; cycle.notify_waiters(); + period.tick().await; } }, async { diff --git a/src/mapping.rs b/src/mapping.rs index c01ff35..71c76f4 100644 --- a/src/mapping.rs +++ b/src/mapping.rs @@ -63,13 +63,13 @@ use crate::{ }; use core::{ fmt, - ops::Range, + ops::{Range, Deref}, cell::Ref, }; use std::{ cell::RefCell, collections::{HashMap, BTreeSet}, - sync::{Arc, Weak, Mutex}, + sync::{Arc, Weak, Mutex, MutexGuard, RwLock, RwLockWriteGuard}, }; use bilge::prelude::*; @@ -127,13 +127,13 @@ impl Allocator { } // update global config let mut slaves = HashMap::>::new(); - for (&k, slave) in mapping.config.slaves.borrow().iter() { + for (&k, slave) in mapping.config.slaves.lock().unwrap().iter() { slaves.insert(k, if let Some(value) = internal.slaves.get(&k).map(|v| v.upgrade()).flatten() // if config for slave already existing, we can use it, because we already checked it was perfectly the same in `self.compatible()` {value} else { - let new = Arc::new(slave.as_ref().clone()); + let new = Arc::new(slave.try_read().expect("a slave is still in mapping").clone()); internal.slaves.insert(k, Arc::downgrade(&new)); new }); @@ -175,10 +175,10 @@ impl AllocatorInternal { /// check that a given mapping is compatible with the already configured slaves in the allocator /// if true, a group can be initialized from the mapping fn compatible(&self, mapping: &Mapping) -> bool { - for (address, slave) in mapping.config.slaves.borrow().iter() { + for (address, slave) in mapping.config.slaves.lock().unwrap().iter() { if let Some(alter) = self.slaves.get(address) { if let Some(alter) = alter.upgrade() { - if slave.as_ref() != alter.as_ref() + if slave.try_read().expect("a slave is still in mapping").deref() != alter.as_ref() {return false} } } @@ -389,10 +389,10 @@ impl fmt::Debug for Group<'_> { /// struct holding configuration informations for multiple slaves, that can be shared between multiple mappings -#[derive(Clone, Default, Debug, Eq, PartialEq)] +#[derive(Default, Debug)] pub struct Config { // slave configs are boxed so that they won't move even while inserting in the hashmap - slaves: RefCell>>, + pub slaves: Mutex>>>, } /// configuration for one slave #[derive(Clone, Default, Debug, Eq, PartialEq)] @@ -423,12 +423,6 @@ pub struct ConfigFmmu { pub logical: u32, } -impl Config { - pub fn slaves(&self) -> Ref<'_, HashMap>> { - self.slaves.borrow() - } -} - /** Convenient struct to create a memory mapping for multiple slaves to the logical memory (responsible for realtime data exchanges). @@ -464,20 +458,22 @@ impl<'a> Mapping<'a> { /// /// data coming for different slaves can interlace, hence multiple slave mapping instances can exist at the same time pub fn slave(&self, address: u16) -> MappingSlave<'_> { - let mut slaves = self.config.slaves.borrow_mut(); + let mut slaves = self.config.slaves.lock().unwrap(); slaves .entry(address) - .or_insert_with(|| Box::new(ConfigSlave { + .or_insert_with(|| Box::new(RwLock::new(ConfigSlave { pdos: HashMap::new(), channels: HashMap::new(), fmmu: Vec::new(), - })); - let slave = slaves.get(&address).unwrap().as_ref(); + }))); + // uncontroled reference to self and to configuration + // this is safe since the slave config will not be removed from the hashmap and cannot be moved since it is heap allocated + // the returned instance holds an immutable reference to self so it cannot be freed + let slave = unsafe {core::mem::transmute::<_, &Box>>( + slaves.get(&address).unwrap() + )}; MappingSlave { - // uncontroled reference to self and to configuration - // this is safe since the config parts that will be accessed by this new slave shall be accessed only by it - // the returned instance holds an immutable reference to self so it cannot be freed - config: unsafe {&mut *(slave as *const _ as *mut _)}, + config: slave.try_write().expect("slave already in mapping"), mapping: self, buffer: SLAVE_PHYSICAL_MAPPABLE.start, additional: 0, @@ -493,7 +489,7 @@ impl<'a> Mapping<'a> { /// data coming from one's slave physical memory shall not interlace (this is a limitation due to this library, not ethercat) so any mapping methods in here are preventing multiple mapping instances pub struct MappingSlave<'a> { mapping: &'a Mapping<'a>, - config: &'a mut ConfigSlave, + config: RwLockWriteGuard<'a, ConfigSlave>, /// position in the physical memory mapping region buffer: u16, /// increment to add to `buffer` once the current mapped channel is done.