How can we share a bus between multiple drivers? about embedded-hal HOT 34 CLOSED

I think this is a very important issue and an a standardized and ergonomic solution is very much needed. Out of all the proposed solutions, I think @therealprof's proxy idea is the best. I wrote a little implementation to show how I would do it. You can find it here: i2c-proxy-demo

The concept consists of 3 components:

BusMutex

A trait which abstracts over different synchronization primitives. It has two methods, one to create a mutex and one to lock the mutex for the duration of a closure:

pub trait BusMutex<T> {
    fn create(v: T) -> Self;
    fn lock<R, F: FnOnce(&T) -> R>(&self, f: F) -> R;
}

This trait is implemented in the device crates (eg. cortex_m) that define the Mutex types.

BusManager

The bus manager own the actual bus peripheral and hands out proxy objects for the devices.
When creating the bus manager, you have to specify, which mutex type you want to use:

// Init i2c bus

let bus = proxy::BusManager::<cortex_m::interrupt::Mutex<_>, _>::new(i2c);

let device_a = DeviceA::new(bus.acquire()).unwrap();
let device_b = DeviceB::new(bus.acquire()).unwrap();

The bus manager should ideally be a part of embedded_hal. This is not an issue, because it is very generic:

pub struct BusManager<M: BusMutex<cell::RefCell<T>>, T>(M, ::core::marker::PhantomData<T>);

BusProxy

The proxy object which will be created by the bus manager. This object implements all bus traits (eg. i2c::Write) that the underlaying peripheral implements.
Should also be a part of embedded_hal.

Examples for those types/traits can be found in proxy.rs

This design has a few advantages:

• It solves the issue that a proxy would require knowledge of synchronization.
• It works with all the other solutions proposed here, so driver crates written without any workaround, crates using RefCells, and crates using the type state approach can all be used with this implementation.
• No additional work for either driver or application developers. The bus proxy handles synchronization transparently, so developers can focus on the logic of their code instead of how to manage access to the peripheral.

I feel like the type state approach lacks in the last point, both sides need to do a lot more work to do something, that is so fundamental to using i2c or other bus types.

The proxy approach does not require any additional code to be written, but it does have a runtime cost. I think that is ok, because with other approaches, the application developers would have to implement synchronization themselves which would end up looking similar. But this is definitely something, where the type state approach is better.

from embedded-hal.

Ok, I published a crate: shared-bus 🎉

For an example, I modified my demo to use the crate, take a look over here: i2c-proxy-demo
Alternatively, take a look at the docs: https://docs.rs/shared-bus/

For now, I only added a mutex implementation for std::sync::Mutex and cortex_m::interrupt::Mutex because those are the platforms I mainly use at the moment. But I am more than happy to add more implementations! Also, feedback would be very much appreciated.

from embedded-hal.

@japaric Just to float another idea: How about a generic proxy driver? The proxy would be initialised with some peripheral it would own, e.g. the initialised I2C bus and then you could use this proxy driver to request multiple proxy handles implementing the very same trait(s) which can be passed to a driver. Internally the proxy driver would ensure that only a single transfer is active at a time.

from embedded-hal.

@Rahix I just had some time to try your shared-bus. This is totally awesome and solves this issue nicely! The overhead on Cortex-M is negligible in my opinion with around 150 bytes for a shared I2C bus, which is a complete must have as I2C busses with only a single device are not that common.

I'd love to share your solution with the world, would you mind writing something like a blog article and add it to https://github.com/rust-embedded/awesome-embedded-rust via PR?

I just built a little high precision power meter using a Nucleo32-STM32F042, a SSD1306 OLED display and an TI INA260 power meter. If there's interest I could turn this into the first OSS Hardware + Software project using Rust. ;)

from embedded-hal.

You destruct the driver to get back the I2C handle and pass that to the other driver

Sure, but that's quite useless.

You use a RefCell

That is interesting. I should have a play with that.

You tweak the driver API to not own the I2C bus (I think this may be the best approach)

I tried but couldn't get it to compile without nasty tricks. This is not easy to implement on chips that have freely selectable pins, at least that's my experience. I also haven't quite figured out how to implement the non-blocking trains; maybe you should try non-STM32 MCUs some time to get a feeling for the different peripheral implementations out there. 😉

I'm not sure if it being Copy-able would be memory safe in every single scenario but it would certainly be error prone in the presence of preemption:

Yes, I agree. Copy-able traits in the presence of preemption are probably a bad idea. You really don't want an interrupt handler to take over the bus in a middle of a transfer.

from embedded-hal.

@Rahix I wanted to illustrate the approach more than a specific solution, namely that of decoupling the bus itself from operations on the bus, as well as experiment with the ergonomics of using move semantics and closures to provide exclusive access. For something like a shared SPI bus, I'm imagining (but haven't actually built) something like:

let gpioa = gpio::A::new(&mut clk_ctx.iop);
let mut dev1_cs = gpioa.PA0.into_output<...>(...); // device 1 chip select
let mut dev2_cs = gpioa.PA1.into_output<...>(...); // device 2 chip select
let mut spi_bus = spi::bus::new(clk_context, ...); // the bus itself

let mut device1 = device::new(device::Options{...});
let mut device2 = device::new(device::Options{...}); // note that the devices do not hold Tx, Rx

spi_bus.domain(&mut dev1_cs, |spi_ctx| {
    device1.issue(&mut spi_ctx.tx);
    device1.interrogate(&spi_ctx.rx);
}); // the spi bus is responsible for asserting/de-asserting the chip select pin

spi_bus.domain(&mut dev2_cs, |spi_ctx| {
    device2.issue(&mut spi_ctx.tx);
    device2.interrogate(&mut spi_ctx.rx);
});

It occurs to me that this approach may would technically be out of scope for embedded-hal specifically, since the traits here wouldn't need to change. This would just be a pattern used by HAL implementors.

from embedded-hal.

Yes, I believe it can.

The SpiDevice trait and the embedded-hal-bus now allow sharing SPI and I2C buses between multiple drivers.

from embedded-hal.

@therealprof awesome. I had this on my TODO list as well. My 2 cents why I think this will be the most flexible approach:

You destruct the driver to get back the I2C handle and pass that to the other driver

This is fine as long as the driver doesn't hold any state. For example: if you need the currently set measurement resolution to correctly interpret the sensor readings. You could probably design the driver to be able to attach/detach to a bus and manage the driver state independently. But this results in extra work for the driver implementer and the user.

You tweak the driver API to not own the I2C bus (I think this may be the best approach)

I first favored this method but then realized this would probably make hardware interface (I2C, SPI, ...) agnostic sensor traits impossible (I might have overseen sth. here, because of my not so extensive Rust knowledge). The trait methods would also need to pass the bus around and something like this would not work.

pub trait Thermometer {
    type Error;

    fn temp(&mut self) -> Result<f32, Self::Error>;
}

The RefCell approach would allow for minimal hardware interface management on the driver side and also allow bus independent trait implementation. I'm really looking forward to your PR and testing it.

from embedded-hal.

@japaric Well, the idea of the proxy is that only the proxy needs to be concerned with the locking of the multiplexed resources instead of each individual user of the traits so it can be generic and will only cause the overhead if it is actually used.

I'm toying around with an implementation of such a proxy for I2C but I'm not happy with the interface and not sure whether my Rust-Fu is already sufficient to make it work.

from embedded-hal.

@Rahix This looks fantastic. I definitely have to play with that. Just curious: Any reason why you didn't stick it in it's own library crate for easier reuse?

Note to self: pickup idea about embedded-hal-tools crate to offer a place for adapters like this. ;)

from embedded-hal.

@therealprof I wrote it as a POC, just to see if it is even possible. My original idea was to include it in embedded-hal in the end. This is, because each device-hal crate needs to implement the BusMutex trait. If we have an external crate, we would either need to ensure all device-hal crate maintainers know to implement it or use some feature-madness to add implementations. Both options are less than ideal ...

Before adding to embedded-hal we should however think about something else: As seen in the comments above, a bus is not the only peripheral that might need to be shared. It would be good if an implementation in embedded-hal could be generic enough to allow sharing arbitrary devices. In practice this means, the proxy needs to implement Deref for the type of the device. Unfortunately this isn't easily possible in the current implementation, because the mutex abstraction looks like this:

fn lock<R, F: FnOnce(&T) -> R>(&self, f: F) -> R;

Which means we can't return a reference from inside the mutex. I tried changing this abstraction, but rust's borrowing rules do not allow a different solution at this time (unless someone who has a much better understanding of this knows of one ... I either got it to work with std::sync::Mutex or cortex_m::interrupt::Mutex but never both)

But I see that I should make the proxy available as a standalone crate asap, I will do so when I get home from work today, hopefully. I have a few ideas for workarounds for the moment.

from embedded-hal.

How can we share a bus between multiple drivers?

There are a few ways:

• You destruct the driver to get back the I2C handle and pass that to the other driver

let mut driver_a = DriverA::new(i2c);
driver_a.stuff();
let i2c = driver_a.release();

let mut driver_b = DriverB::new(i2c);
driver_b.other_stuff();

• You use a RefCell

// these blanket implementations should be in embedded-hal
impl<'a, I> blocking::i2c::Write for &'a RefCell<I> where I: blocking::i2c::Write { .. }
// repeat for all the other traits

let shared_i2c = RefCell::new(i2c);

let mut driver_a = DriverA::new(&shared_i2c);
let mut driver_b = DriverB::new(&shared_i2c);

driver_a.stuff();
driver_b.other_stuff();

• You tweak the driver API to not own the I2C bus (I think this may be the best approach)

let mut driver_a = DriverA::new(&mut i2c);
let mut driver_b = DriverB::new(&mut i2c);

driver_a.stuff(&mut i2c);
driver_b.other_stuff(&mut i2c);

for the blocking APIs should also be safe

Should those bus types be made Copy?

I'm not sure if it being Copy-able would be memory safe in every single scenario but it would certainly be error prone in the presence of preemption:

app! {
    resources: {
        statci A: DriverA;
        statci B: DriverB;
    },

    tasks: {
        EXTI0: {
            path: exti0,
            priority: 1,
            resources: [A],
        },

        EXTI1: {
            path: exti1,
            priority: 2,
            resources: [B],
        },
    },
}

fn init(p: init::Peripherals) -> init::LateResources {
    // ..
    let i2c_alias: I2c = i2c; // due to `Copy`-ness

    init::LateResources { A: DriverA::new(i2c), B: DriverB::new(i2c_alias) }
}

fn exti0(r: &mut EXTI0::Resources) {
    r.A.stuff();
}

fn exti1(r: &mut EXTI0::Resources) {
    // can preempt `exti0` midway some operation on the I2C bus
    r.B.other_stuff();
}

from embedded-hal.

You use a RefCell

You tweak the driver API to not own the I2C bus (I think this may be the best approach)

How will this work with RTFM?

fn init(mut p: init::Peripherals, _r: init::Resources) -> init::LateResources {
    ...

    let mut i2c = I2c::i2c1(
        p.device.I2C1,
        (scl, sda),
        &mut afio.mapr,
        i2c::Mode::Fast { frequency: 400000, duty_cycle: i2c::DutyCycle::Ratio16to9 },
        clocks,
        &mut rcc.apb1,
    );

    let si5351 = SI5351::new(&mut i2c, false, 25_000_000).unwrap();

    let _ = ssd1306::init(&mut i2c);
    let _ = ssd1306::pos(&mut i2c, 0, 0);
    let _ = ssd1306::print_bytes(&mut i2c, b"Send ASCII over serial for some action");'

    init::LateResources {
        SI5351: si5351,
        I2C: i2c
    }
}

Since the value i2c is move'd when constructing init::LateResources, all the references to it will break. Same for the RefCell solution.

from embedded-hal.

@japaric I tried the RefCell approach and it works great (at least in my limited testing). I'll submit a PR for review later.

from embedded-hal.

And also let's not forget that it's quite common to need additional resources instead of just the bus itself. For I2C (and to a lesser extend) SPI there're often separate interrupt pin(s) and for SPI one often needs to worry about a chip select pin as well. The complexity of the solution to such an approach could explode rather quickly.

from embedded-hal.

I left a comment in #55 (comment) about the shortcomings of the "put the resource protection (e.g. RefCell) in the driver" approach.

You could probably design the driver to be able to attach/detach to a bus and manage the driver
state independently.

I think this might be the way to go. You design your driver as having two type states: one where it
owns the I2C bus and one where it doesn't. Something like this:

/* embedded-hal */

// imagine this is one of the blocking I2C traits
trait I2c {
    /* .. */
}

/* sensor1 */

// Only the sensor state
// The type parameter `I` is mainly to forbid the following scenario: you
// initialize the sensor using I2C1, then you perform some op on it using I2C2
// and then another op using I2C3.
pub struct State<I>
where
    I: hal::I2c,
{
    // ..
    i2c: PhantomData<I>,
}

pub fn init<I>(i2c: &mut I) -> State<I>
where
    I: hal::I2c,
{
    // ..
}

impl State<I> {
    pub fn take<I>(self, i2c: I) -> Owned<I>
    where
        I: I2c,
    {
        // ..
    }

    pub fn borrow<I>(&mut self, i2c: &mut I) -> RefMut<I>
    where
        I: I2c,
    {
        // ..
    }
}

struct Owned<I>
where
    I: hal::I2c,
{
    i2c: I,
    state: State,
}

impl Owned<I> {
    pub fn release<I>(self) -> (State, I)
    where
        I: I2c,
    {
        // ..
    }

    pub fn accel(&mut self) -> u16 {
        // defer to RefMut
        self.as_ref_mut(|s| s.accel())
    }

    fn as_ref_mut<R, F>(&mut self, f: F)
    where
        F: FnOnce(&mut RefMut<I>) -> R,
    {
        f(&mut RefMut {
            i2c: &mut self.i2c,
            state: &mut self.state,
        })
    }
}

pub struct RefMut<'a, I>
where
    I: hal::I2c,
{
    i2c: &'a mut I,
    state: &'a mut State,
}

impl RefMut<'a, I> {
    pub fn accel(&mut self) -> u16 {
        // ..
    }
}

/* app */
fn main() {
    let mut i2c: I2c1 = ..;

    // neither operation takes ownership of `i2c`
    let sensor1: sensor1::State<I2c1> = sensor1::init(&mut i2c);
    let sensor2 = sensor2::init(&mut i2c);

    // takes ownership of `i2c`
    let mut sensor1: sensor1::Owned<I2c1> = sensor1.take(i2c);
    let g = sensor1.accel();
    let (sensor1, mut i2c) = sensor1.release();

    // doesn't take ownership of `i2c`
    let m = sensor2.borrow(&mut i2c).mag();
    let ar = sensor2.borrow(&mut i2c).gyro();
}

The Owned variant is required to support async operations that involve Futures and/or
Generators: async code doesn't like (mutable) references (*) (unless they have 'static
lifetime) -- everything needs to be passed by value. RefMut is mostly a convenience to avoid
having to take - release on every I2C operation / handover. If you are going to deal with only
one I2C slave you can use the Owned variant and your code will look pretty much the same as it
looks today.

(*) Actually that's another reason why storing a reference to a RefCell / Mutex in the driver
is not as flexible as this approach.

@ilya-epifanov

How will this work with RTFM?

If you are going to do blocking operations in your tasks store I2c and State as resources and
use RefMut / borrow:

fn task1(r: EXTI0::Resources) {
    let g = r.SENSOR1.borrow(r.I2C1).accel();
}

fn task1(r: EXTI1::Resources) {
    let ar = r.SENSOR2.borrow(r.I2C1).gyro();
}

If you are going to use the DMA store Option<I2c> and Option<State> as resources and use
Owned / take:

fn task1(r: EXTI0::Resources) {
    let sensor1: sensor1::State<_> = r.SENSOR1.take().unwrap();
    let i2c = r.I2C.take().unwrap();

    let pending_transfer = sensor1.take(i2c).mag_dma();

    // send `pending_transfer` to some other task, etc.
}

@therealprof

And also let's not forget that it's quite common to need additional resources instead of just the
bus itself. For I2C (and to a lesser extend) SPI there're often separate interrupt pin(s) and for
SPI one often needs to worry about a chip select pin as well.

The NSS / NCS / interrupt pin can be permanently stored in the State struct; no need to move them
out -- it's not like you are going to share any of those pins between different sensors.

from embedded-hal.

@japaric I'll give your idea a spin.

The NSS / NCS / interrupt pin can be permanently stored in the State struct; no need to move them
out -- it's not like you are going to share any of those pins between different sensors.

I would not necessarily say that this is always the case. Pinsharing is not quite as uncommon as it seems and used for all kinds of stuff like measurements triggering, resets and sometimes they're also muxed (though in that case you'd probably need an alternative PIN driver anyway).

from embedded-hal.

@japaric This puts quite a few additional requirements on each individual driver without explicitly defining what they are. I would rather prefer a solution that requires the driver HAL implementation to make sure shared use is safe (by disabling interrupts if necessary) or the user to use a safe mechanism for sharing the bus, same as we already do when sharing resources internally.

from embedded-hal.

@therealprof That sounds like it would push all the synchronization concerns to the driver crate; meaning that the driver crate would have to be aware of rtfm::Resource, rtos1::Mutex, rtos2::Mutex, etc. and that it would need different proxy types -- one for each RTOS it supports. That still has the coupling problem I mentioned. Or did you mean something else? How would this proxy you are thinking of look like?

from embedded-hal.

I'm giving up. I sunk the better part of 2 days into it but I cannot get it to work due to the freakiness of Rust around mutable sharing. Things I've tried including encapsulating the RefCell solution so I can add locking; this one failed due to the inability to hand out proper I2C handles implementing the traits. Another thing was handing out objects initialised with closures to implement the traits which doesn't work due to not statically known size of all parameters.

It's really sad that I'm not to solve such a critical issue with all the drivers being worked on. If we have to change the interfacing and break all drivers to support such an important thing that'd be really a blow to embedded Rust.

from embedded-hal.

I took a stab at implementing the type states proposal from #35 (comment) in my simple sx1509 driver (wez/sx1509@a063f2c).

A notable difference is that I didn't include the delegation from Owned to Borrowed (which is labelled as RefMut in the original proposal, and that I renamed to avoid confusion with core::cell::RefMut). What I wanted to do there was use something like Deref and DerefMut to sugar it away, rather than fill the impl with boiler plate code to delegate every driver method. That didn't seem feasible to do, because I couldn't find a way to specify that deref should respect our desired lifetime requirements.
In the end I just omitted the delegation and left it to the user.

from embedded-hal.

Roughly following the example by @wez, in JoshMcguigan/tsl256x@8315f1f I updated a driver I've been working on to borrow the I2C bus, rather than taking exclusive ownership.

To keep it simple, rather than implementing two structs (owned and borrowed) to wrap my sensor type, I just modified each method on the sensor struct to take a &mut i2c. I believe that makes my sensor struct analogous to the Borrowed struct. As for the Owned struct, it should be trivial for users of the driver to implement that behavior on their own, or for it to be added to the driver in the future, as shown below:

struct Owned<I2C> {
    sensor: Tsl2561<I2C>,
    i2c: I2C,
}

let mut i2c = I2c::i2c1(p.I2C1, (scl, sda), 400.khz(), clocks, &mut rcc.apb1);

let mut sensor = Tsl2561::new(&mut i2c).unwrap();

let mut sensor_with_bus = Owned { sensor, i2c };

sensor_with_bus.sensor.power_on(&mut sensor_with_bus.i2c);

My goal was to come up with a solution which minimizes the complexity of the driver crates, so as to not discourage people from creating drivers, while also solving the bus sharing problem. Admittedly, my knowledge of how async plays into this is weak, so I may be missing something significant on that front. Any good resources for learning more about async drivers would be appreciated.

That said, I do think it is important that we try as a community to standardize on a solution here, so that driver users are well as driver developers get a consistent feeling across the various drivers. Any thoughts on this approach vs the approach taken by @wez vs the original suggestion by @japaric?

from embedded-hal.

I'm running into a similar issue (and have discussed it in mozilla#rust-embedded). In my current, specific case, multiple peripherals need to rely on a configurable clock, and the peripherals must be invalidated if the clock changes. What doesn't work but I'd like to make work would be something akin to:

struct HSI16Clock {
    div4: bool,
}
struct LSEClock;

impl HSI16Clock {
    fn new(div4: bool) -> Self { HSI16Clock{ div4 } }
}

enum SerialClock<'clk> {
    HSI16(&'clk HSI16Clock),
    LSE(&'clk LSEClock),
}

impl<'clk> SerialClock<'clk> {
    fn release(self) {}
}

struct Serial<'clk> {
    clk: &'clk SerialClock<'clk>,
}

impl<'clk> Serial<'clk> {
    fn destruct(self) {}
}

fn main() {
    let sc = HSI16Clock::new(false);
    let c = SerialClock::HSI16(&sc);
    let mut s1 = Serial{ clk: &c };
    let mut s2 = Serial{ clk: &c };

    // time to reconfigure slow down the clock!
    s1.destruct();
    s2.destruct();
    c.release();
    let sc = HSI16Clock::new(true);
}

This also needs a solution to making the clocks Singletons, which I haven't attempted yet.

from embedded-hal.

I spent some time scratching this itch, and I think I've come up with a decent solution. It is an implementation of the concepts discussed here. This seems like a reasonable place to request feedback.

The actual crate itself is still very rough, but you can see an (untested) implementation of the ideas in this repository. The idea is this: when you configure a peripheral, you must operate within the context of that peripheral's configuration. A code example is probably the best way to explain it:

fn main() -> ! {
    let p = cortex_m::Peripherals::take().unwrap();
    let d = stm32l0x1_hal::stm32l0x1::Peripherals::take().unwrap();
    let mut syst = p.SYST;
    let mut rcc = d.RCC.constrain();
    let mut pwr = d.PWR.constrain();
    let mut flash = d.FLASH.constrain();

    // Configure the Power peripheral to raise the VDD. The VDD range has implications for the
    // maximum clock speed one can run (higher voltage = higher MHz). As such, you want to make
    // sure this cannot change unless you are prepared to invalidate your clock settings.
    pwr.set_vcore_range(stm32l0x1_hal::power::VCoreRange::Range1)
        .unwrap();

    pwr.power_domain(&mut rcc, |rcc, mut pwr_ctx| {
        // Within the closure, we are guaranteed that VDD will not change, as to do so requires
        // mutating `pwr`, and Rust's move semantics prevent that!
        // The PowerContext `pwr_ctx` contains information we need to make
        // decisions based on the VDD and VCore levels available to us. Furthermore, a PowerContext
        // can only be obtained within this closure, and is required elsewhere.

        // Every time we loop, we will turn the LSE on and off. Use has_lse to track desired state.
        let mut has_lse = false;

        // Enable the system configuration clock so our changes will take effect.
        rcc.apb2.enr().modify(|_, w| w.syscfgen().set_bit());
        while !rcc.apb2.enr().read().syscfgen().bit_is_set() {}

        // Request that the HSI16 clock be turned on and divided by 4.
        rcc.hsi16.enable();
        rcc.hsi16.div4();
        
        // Use the prescaled 4 MHz HSI16 clock for SYSCLK (which drives SysTick)
        rcc.sysclk_src = stm32l0x1_hal::rcc::clocking::SysClkSource::HSI16;

        loop {

            if has_lse {
                rcc.lse = Some(stm32l0x1_hal::rcc::clocking::LowSpeedExternalOSC::new());
            } else {
                rcc.lse = None;
            }

            rcc.clock_domain(&mut flash, &mut pwr_ctx, |mut clk_ctx, _| {
                // Within this closure, similarly to `power_domain`, we are assured that the clock
                // configuration cannot change. You cannot turn a clock on or off because we've
                // already mutably borrowed `rcc`. The `clk_ctx` provides clock speed information
                // that would be useful to clocked peripheral busses.

                // Configure the SysTick frequency.
                syst.set_clock_source(SystClkSource::External);
                syst.set_reload(clk_ctx.sysclk().0 / 8); // 1s
                syst.clear_current();
                syst.enable_counter();
                syst.enable_interrupt();

                // Here we would construct I2C or Serial peripherals based on `clk_ctx`
                // let mut usart2 = Serial::usart2(d.USART2, (tx, rx), Bps(115200), clocking::USARTClkSource::PCLK, ...

                loop {
                    // main "application" loop to handle Serial tx/rx, etc.
                    asm::wfi();
                    break;
                }
            });

            has_lse = !has_lse;
        }
    });

    panic!("there is no spoon");
}

While I did a clean-sheet rewrite for this exploratory work, I think it wouldn't be too hard to refactor an existing reference crate (stm32f100?).

from embedded-hal.

@thenewwazoo: What you brought up looks very interesting, but I think it deserves a separate issue, because it does not really solve the original problem here: That the current implementation does not allow to share a bus between multiple devices. It is however a very good point that a bus is not the only device that might need to be shared.

from embedded-hal.

If there's interest I could turn this into the first OSS Hardware + Software project using Rust

Yes please! I could use one to be honest.

from embedded-hal.

@RandomInsano I'm having a hard time sourcing the INA260 at the moment and I don't really see any good alternatives at the moment.

from embedded-hal.

No worries. I’m assuming it’s a different package than what’s offered on DigiKey?
https://www.digikey.ca/products/en/integrated-circuits-ics/pmic-current-regulation-management/734?keywords=INA260

If nothing comes of it, no worries on my side. Producting a product takes a lot of effort even after a prototype is done.

from embedded-hal.

No, that's the one but I prefer different distributors due to shipping costs and neither of them has the INA260 in store at the moment.

from embedded-hal.

Heh. That’s the opposite for me, but I live about two hours from their distribution centre. 😃

from embedded-hal.

TL;DR: Could transactions solve shared-buss last problem?

I just read the shared-bus blog post and it's fantastic. It solves the shared bus problem well but with one issue:

The much bigger issue is that this approach does not synchronize bus accesses. Even worse than the first solution of not owning the bus at all, with this one, an application developer can't even implement it manually

I think this aligns well with what I've been researching for #94. I originally wanted messages to be composed of smaller parts but what if the idea were extended to multiple messages? Something I've been grappling with is whether the code overheard of this is worth the effort:

bus_proxy.transfer([
    [
        bus::write(&MESSAGE),
        bus::read(&data)
    ]
]);

/// Or if you're not into that whole brevity thing:
let mut message = [
    bus::write(&MESSAGE),
    bus::read(&data),
    bus::custom_thing(_, _, _),
];

let transaction = [
    message
];

bus_proxy.transfer(&transaction);

Bundling reads and writes into transactions allows the driver author to have a guarantee on the grouping of their messages. The Mutex within BusManager would guard the whole transaction.

The BusProxy struct could own the device address which solves a problem in my planned implementation where the device author would have had to create and manage the message factory. If that ownership was passed up one level, the driver author would only need to write the code above. The consumer of both device and driver then would only need to provide the address (i2c) or select line (spi) at proxy instantiation time. If the BusProxy were implemented for various bus types (i2c, spi, 1wire), there could be significant code sharing both at the BusManager level, but also device driver level if the proxy trait only exposed transfer and an enum of what bus type it implemented.

My next step is to research how to structure an RFC and outline this. My plan would break a large chunk of the ecosystem of the current device crates, so I'll need help finding holes in my plan.

from embedded-hal.

@RandomInsano: The statement you quoted is not a shared-bus issue, it is referring to the "RefCell-Approach".

You do however bring up an issue that I have not yet thought about which is the synchronization of multiple accesses to the same device. If I understand correctly, this is what your transactions are trying to solve?

The BusProxy struct could own the device address which solves a problem in my planned implementation where the device author would have had to create and manage the message factory.

Hmm, you are right that one proxy would usually only talk to one address. But the current I2C traits do not reflect this so it would need a breaking change.

My plan would break a large chunk of the ecosystem

I'd be very very careful about doing something like this. One of the main reasons for the design of shared-bus is that I do not want to break the existing APIs (see here). Are you sure this is really necessary?

from embedded-hal.

For those not following the other thread it's possible to avoid breaking the ecosystem. I muddied the conversation up a bit by cross-posting.

from embedded-hal.

Can this be closed now?

from embedded-hal.