Config is now thread-safe

Author: jimy-byerley

examples/sdo_mapping.rs

@@ -37,6 +37,7 @@ async fn main() -> std::io::Result<()> {
                 let error = pdo.push(Sdo::<u16>::complete(0x603f));
                 let position = pdo.push(Sdo::<i32>::complete(0x6064));
                 let torque = pdo.push(Sdo::<i16>::complete(0x6077));
+    drop(slave);
     println!("done {:#?}", config);
 
     let allocator = mapping::Allocator::new();

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 {

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::<u16, Arc<ConfigSlave>>::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<HashMap<u16, Box<ConfigSlave>>>,
+    pub slaves: Mutex<HashMap<u16, Box<RwLock<ConfigSlave>>>>,
 }
 /// 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<u16, Box<ConfigSlave>>> {
-        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<RwLock<_>>>( 
+                        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.