How can I safely share an `rppal::i2c::I2c` on different threads?

I'm trying to instantiate an I2c object and share it amongst multiple threads. To start, I'm interested in being able to use the read() and write() methods which both require a mutable reference.

I only discovered I2c doesn't implement the Clone trait after writing this code:

use once_cell::sync::Lazy;
use rppal::i2c::{I2c,Error};
use std::sync::{Arc, Mutex};

const I2C_BUS: u8 = 1; // I2C bus number on Raspberry Pi

static GLOBAL_I2C: Lazy<Arc<Mutex<Option<I2c>>>> = Lazy::new(||
    {        Arc::new(Mutex::new(None))     }
);

fn initialize_i2c(bus: u8) -> Result<I2c, Error> {
    I2c::with_bus(bus)
}

pub fn init_and_or_get_i2c(errors: &mut TraceLog) -> Result<I2c, Error> {
    // Lock the mutex to access the I2c
    let binding = GLOBAL_I2C.clone();
    let mut guard = match binding.lock() {
        Ok(g) => g,
        Err(e) => {
            // Not really a Thread panic, only for the hpurpose of question
            return Err(Error::Io(std::io::Error::new(std::io::ErrorKind::Other, "Not really a thread panic.")))  
        }
    };

    // Check if the I2c has been initialized
    if guard.is_none() {
        // Lazily initialize the I2c
        match initialize_i2c(I2C_BUS) {
            Ok(initialized_i2c) => *guard = Some(initialized_i2c),
            Err(e) => {
                return Err(e)
            }
        }
    }

    // Return the I2c instance
    match guard.as_ref() {
        Some(i2c_instance) => Ok(i2c_instance.clone()),  // `I2c` does not implement `Clone`, so `&I2c` was cloned instead
        None => {
            // Not really a panic: only for the purposes of question
            Err(Error::Io(std::io::Error::new(std::io::ErrorKind::Other, "Not really a thread panic."))) 
        }
    }
}

My hope was to use the code above in this manner on the different threads:

match i2c::access_or_initialize_i2c() {
    Ok(i2c) => {
        i2c.set_slave_address(device_address);
        i2c.read(buf); // or i2c.write(buf); 
    }
    Err(e)  => { ... }

The I2C type intentionally doesn't implement Clone, because its whole purpose is to make sure exclusive access to the bus is held by one actor at a time.

If you want to make it globally available, the Mutex's lock has to be held as long as you're talking on the line. Make init_or_get_i2c return the MutexGuard, wrapped in a type of your own design

pub struct I2cLockGuard {
    guard: MutexGuard<'static, Option<I2c>>,
}
impl Deref for I2cLockGuard {
    type Target = I2c;
    #[inline]
    fn deref(&self) -> &I2c { self.guard.as_ref().expect("at construction, i2c is initialized") }
}
impl DerefMut for I2cLockGuard {
    #[inline]
    fn deref_mut(&mut self) -> &mut I2c { self.guard.as_mut().expect("at construction, i2c is initialized") }
}

pub fn init_and_or_get_i2c(errors: &mut TraceLog) -> Result<I2cLockGuard, Error> {
    // Lock the mutex to access the I2c
    let binding = GLOBAL_I2C.clone();
    let mut guard = match binding.lock() {
        Ok(g) => g,
        Err(e) => {
            // Not really a Thread panic, only for the hpurpose of question
            return Err(Error::Io(std::io::Error::new(std::io::ErrorKind::Other, "Not really a thread panic.")))  
        }
    };

    // Check if the I2c has been initialized
    if guard.is_none() {
        // Lazily initialize the I2c
        match initialize_i2c(I2C_BUS) {
            Ok(initialized_i2c) => *guard = Some(initialized_i2c),
            Err(e) => {
                return Err(e)
            }
        }
    }

    if guard.as_ref().is_none() {
        return Err(Error::Io(std::io::Error::new(std::io::ErrorKind::Other, "Not really a thread panic."))) ;
    }
    Ok(I2cLockGuard { guard })
}

Though I would prefer to use AtomicRefCell instead of Mutex, if you can guarantee that two threads won't ever try to write to the I2c port at once.

1 Like

Thanks for responding!

The I2C type intentionally doesn't implement Clone , because its whole purpose is to make sure exclusive access to the bus is held by one actor at a time.

And I get that, and when I got the error that about the Clone not being implemented I figured that omission was intentional.

the Mutex's lock has to be held as long as you're talking on the line

That, too, became apparent to me, and I wasn't confident that wrapping the Mutex in Arc would get the behaviour I needed. What I ended up doing was rather than exporting the I2c type (wrapped in a proprietary guard as you suggested) I passed in the action I wanted the I2c type to perform, that way the lock was contained in the module and didn't have to be exported. Something akin too:

fn perform_i2c_operation<T, F>(slave_address : u16, operation: F) -> Result<T, Error>
where
    F: FnOnce(&mut I2c) -> Result<T, Error>,
{
    let binding = GLOBAL_I2C.clone();
    let mut guard = match binding.lock() {
        Ok(g) => g,
        Err(e) => {
            return Err(Error::Io(std::io::Error::new(
                std::io::ErrorKind::Other,
                "Not really a thread panic.",
            )));
        }
    };

    *guard = if guard.is_none() {
        match initialize_i2c(I2C_BUS) {
            Ok(initialized_i2c) => Some(initialized_i2c),
            Err(e) => {
                return Err(e);
            }
        }
    } else {
        None
    };

    match &mut *guard {
        Some(i2c_instance) => match i2c_instance.set_slave_address(slave_address) {
            Ok(_) => operation(i2c_instance),
            Err(e) => Err(e),
        }
        None => {
            Err(Error::Io(std::io::Error::new(
                std::io::ErrorKind::Other,
                "Not really a thread panic.",
            )))
        }
    }
}

That, along with a helper functions like:

fn i2c_read_operation(i2c: &mut I2c, buf: &mut [u8]) -> Result<usize, Error> {
    i2c.read(buf)
}

pub fn i2c_read(slave_address : u16, buf: &mut [u8]) -> Result<usize, Error> {
    perform_i2c_operation(slave_address,|i2c| i2c_read_operation(i2c, buf))
}

and I think I've got some thread safe and well contained.

Though I would prefer to use AtomicRefCell instead of Mutex, if you can guarantee that two threads won't ever try to write to the I2c port at once.

I'm only a month into rust, so I would appreciate a little more explanation of that preference; I'm not able to guarantee that threads won't simultaneously want to access the I2C bus - which is why I employed the Mutex.

Let’s try to re-iterate the differences:

  • with Mutex, if multiple threads try to access it at the same time, then one thread blocks, waiting for the other to be done with the protected data. This is quite commonly what you want, which is why it’s also in the standard library.
  • then there’s RwLock, which is similar to Mutex in that if multiple threads try to access it at the same time, then one thread might block, waiting for the other to be done with the protected data. However it differentiates between read-only access and mutable access, and two threads can gain read-only access at the same time. This can have performance benefits over Mutex, if read-only access is common. On the other hand the type is a bit more complex, so if you’d only use it “like a Mutex” it might have some overhead.

Locking involves interacting with the OS on one hand, though as far as I’m aware, Rust’s Mutex/RwLock implementations currently have a fast-path using only atomics for the case where no blocking is necessary. Additionally, with locking, once you lock multiple things it’s always possible to run into dead-locks if you don’t make sure to follow strategies to avoid them.

  • with AtomicRefCell (which is a type defined not in the standard library but a crate), if multiple threads try to access it at the same time, similar to RwLock, multiple read-only accesses are fine, but otherwise it will panic which will usually crash the thread, or even your program. You’d only use it in case you know you cannot run into this error case. It avoids all the cost associated with the possibility of locking, when you don’t need that capability. I.e. no extra code-paths that could do it; you’ll know more easily if your program does end up unexpectedly trying to conflictingly access the protected data (which might be a bug)… According to its documentation, the benefits are slightly better performance than RwLock, and a bit more API flexibility around map methods of the guards (though I don’t think those are particularly commonly used.)
    Compared to Mutex, I’m not sure if there’s still any performance benefit; in any case, the more significant difference is that AtomicRefCell is like RwLock and unlike Mutex in allowing concurrent reads-only access.

Your case with I2c involves mutable access for most useful API as far as I can tell. (Like read or write want &mut self.) So the benefit of concurrent read-only access of AtomicRefCell is irrelevant.

Additionally, AtomicRefCell requires Sync for its contained type. This is a complication with I2c that doesn’t support Sync directly; Mutex on the other hand only requres a Send bound. It could be overcome by limiting your access to mutable access with something like SyncWrapper, but at that point it’s perhaps a bit tedious for little gains.

1 Like

I think you're looking for this crate: shared_bus - Rust

1 Like

This topic was automatically closed 90 days after the last reply. We invite you to open a new topic if you have further questions or comments.