Skip to content

Is it possible to use this library with RTIC? #4

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Closed
capnfabs opened this issue Jun 18, 2019 · 24 comments · Fixed by #17
Closed

Is it possible to use this library with RTIC? #4

capnfabs opened this issue Jun 18, 2019 · 24 comments · Fixed by #17

Comments

@capnfabs
Copy link

Hi there,

I'm a visitor from rust-embedded/embedded-hal#35 :) I'm currently trying to use shared-bus in an RTFM project and am trying to late-initialise peripherals. I can't quite get it to work when using shared-bus, however, because the initial BusManager either goes out of scope at the end of the init method, or gets moved into init::LateResources, and therefore I can't figure out how to annotate the lifetime of the BusProxy. Here's a small-ish example:

//! examples/init.rs

#![deny(unsafe_code)]
#![no_main]
#![no_std]

extern crate panic_semihosting;

use stm32f0::stm32f0x0::I2C1;

use stm32f0xx_hal::{
    i2c::I2c,
    prelude::*,
    gpio,
    gpio::{
        Alternate,
        gpiob,
    },
};

use rtfm::app;

use shared_bus::BusManager;
use ssd1306::Builder;
use ssd1306::prelude::*;
use embedded_graphics::prelude::*;

type I2C1Bus = I2c<I2C1, gpiob::PB8<Alternate<gpio::AF1>>, gpiob::PB9<Alternate<gpio::AF1>>>;

#[app(device = stm32f0::stm32f0x0)]
const APP: () = {
    static mut I2C_BUS: BusManager<
        cortex_m::interrupt::Mutex<core::cell::RefCell<I2C1Bus>>, I2C1Bus> = ();
    static mut DISPLAY: ssd1306::mode::graphics::GraphicsMode<
        ssd1306::interface::i2c::I2cInterface<
            shared_bus::proxy::BusProxy<
                '_,  // this lifetime parameter is the one I can't figure out.
                cortex_m::interrupt::Mutex<core::cell::RefCell<I2C1Bus>>,
                I2C1Bus>>> = ();

    #[init]
    fn init() -> init::LateResources {
        let dp = device;

        let mut flash = dp.FLASH;
        let mut rcc = dp.RCC.configure().freeze(&mut flash);
        let gpiob = dp.GPIOB.split(&mut rcc);

        // Configure i2C pins
        let (scl, sda) = cortex_m::interrupt::free(|token| {
            (
                gpiob.pb8.into_alternate_af1(token),
                gpiob.pb9.into_alternate_af1(token),
            )
        });
        let i2c = I2c::i2c1(dp.I2C1, (scl, sda), 400.khz(), &mut rcc);
        let bus = BusManager::<cortex_m::interrupt::Mutex<_>, _>::new(i2c);

        let mut disp: GraphicsMode<_> = Builder::new().connect_i2c(bus.acquire()).into();

        init::LateResources {
            I2C_BUS: bus,
            DISPLAY: disp,
        }
    }
};

My questions are:

  • is it possible to initialise i2c devices using SharedBus in the #[init] method of an RTFM application?
  • if not, is this something that you'd be open to design ideas / pull requests for?

Also, I can't figure out if this is more an issue on the RTFM side or on the shared-bus side - I suspect that it's a limitation intrinsic to the way resources are initialised with RTFM. I'm still new to rust; please let me know if this seems like something I should be raising over there instead.

Thanks in advance.

@Rahix
Copy link
Owner

Rahix commented Jun 19, 2019

Hey!

Thanks for your interest! To be quite honest, I have never actually used RTFM so far, so the following might not be 100% correct ...

First of all, something unrelated to your issue but which is still very relevant: The cortex_m::interrupt::Mutex is a very bad idea when using RTFM, because it masks all interrupts and thus also disables higher priority tasks. Instead, RTFM should implement its own Mutex type which properly handles priorities. I made some effort over in rust-embedded/embedded-hal#132 to standardize the Mutex interface but this is still in the works (mostly blocked on me not having the time to continue right now ...). Once that is done, RTFM could (AFAIK) implement this trait and allow shared-bus to correctly handle priorities.

Now onto your actual issue: From your code I unfortunately think this is not trivially possible right now. The problem is that rust does not allow you to store a value and a reference to that value in the same struct (See this StackOverflow post).

is it possible to initialise i2c devices using SharedBus in the #[init] method of an RTFM application?

As explained in the link above, this can never work like you tried because you move the LateResources which would invalidate the references to BusManager. All hope is not lost, however (Unless RTFM does some magic that would disallow it ...): You can keep the BusManager at a stable address outside the RTFM resources by using a global. This is sound from a safety perspective because you never mutate the bus manager. You will however need unsafe to initialize the manager as there is no equivalent to once_cell for no_std right now.

In code, this would roughly look like this (untested):

// Note that I still used the cortex-m mutex here
// which does bad things to your realtime system
static mut I2C_BUS: Option<shared_bus::CortexMBusManager<...> = None;

fn init() -> init::LateResources {
    let i2c_bus = {
        let i2c = I2c::i2c1(dp.I2C1, (scl, sda), 400.khz(), &mut rcc);
        let bus = shared_bus::CortexMBusManager::<_>::new(i2c);

        unsafe {
            I2C_BUS = Some(bus);
            // This reference is now &'static
            &I2C_BUS.as_ref().unwrap()
        }
    };

    // Initialize all your drivers now
    let mut disp: GraphicsMode<_> = Builder::new().connect_i2c(bus.acquire()).into();

    init::LateResources {
        DISPLAY: disp,
    }
}

if not, is this something that you'd be open to design ideas / pull requests for?

Definitely! The solution above is not pretty ... I think the only real solution would be a once_cell equivalent for no_std, but if you have any other ideas, I would love to hear them!

@capnfabs
Copy link
Author

Hi, thanks so much for the comprehensive writeup!

I tried what you suggested in this example, and it works excellently - thank you!

The cortex_m::interrupt::Mutex is a very bad idea when using RTFM, because it masks all interrupts and thus also disables higher priority tasks

Yep, I'm aware of this :) Thanks for making the effort to standardise the Mutex trait! I also don't have much experience with RTFM, but I'll have a play around with this in the coming days / weeks schedule-permitting and see if I can answer your question with regards to handling priorities correctly.

Thanks again for the help.

@capnfabs
Copy link
Author

I guess I'll leave this issue open because it's still not entirely clear if this works with RTFM, due to the fact that it's unclear whether there's a good mutex solution available for RTFM.

@mpasternacki
Copy link

I'm having the same problem here. The issue, as I understand it, is that RTFM implements its own locks, with ceiling analysis etc, to ensure safe access – and shared-bus would want to have its own locks. I'm not sure it's possible without using cortex_m::interrupt::Mutex, disabling interrupts completely, and throwing RTFM preemption / priorities / real-time promises under the bus (pun not intended).

The "official" way in RTFM would be to use a shared resource with the item being locked; problem is that multiple devices are sharing a bus that needs to be locked safely. My initial idea was to ditch shared-bus and just pass the original bus in context, and making it a required parameter in calls to the individual devices' APIs. But then I can't use any existing drivers – crates with drivers for I²C devices consume whole bus for one device. I need either shared-bus, or my own drivers.

For my project, I figured I can have my cake and eat it (I think – if there's any obvious error in my approach, I'd appreciate the feedback). I have a struct that holds BusManager and all I²C devices I'm using (and performs a bit of lifetime trickery to keep the compiler happy). This struct is a single shared resource, RTFM manages access / locking to it, so every task that got it can safely access the bus (if it's a lower priority task, it has to use RTFM's lock on the whole bus+devices struct). This way, no locking is needed in the shared-bus (it's only used to be able to have more than one device) – I included a no-op implementation of BusMutex.

My current code is here: https://gist.github.com/mpasternacki/93c73d2f94630fbc356e28436e7f70ac

@mpasternacki
Copy link

Now I wonder if I even need shared-bus with this mechanism – wouldn't it be the same (and still safe) if I just unsafely cloned the controller in the "bus+devices" struct constructor?

@Rahix
Copy link
Owner

Rahix commented Nov 24, 2019

(and performs a bit of lifetime trickery to keep the compiler happy)

Hm, that looks like a very hacky solution ... It also would not work for any bus peripheral which is not zero-sized.

if I just unsafely cloned the controller in the "bus+devices" struct constructor?

This would lead to issues if the bus driver has some internal state (which would then be duplicated).

@mpasternacki
Copy link

(and performs a bit of lifetime trickery to keep the compiler happy)

Hm, that looks like a very hacky solution ...

It's hacky and it's a big tradeoff (the whole bus is locked per task, so it might not be useful for all solutions), but it seems to cooperate with RTFM rather than work against it or need to reimplement much of RTFM's logic.

But yeah, this might be more of a workaround than solution. Still, this issue is open since June, so I figured it might be useful to write it down.

It also would not work for any bus peripheral which is not zero-sized.

Why wouldn't it? The transmute calls only manually modify lifetimes, is there a copy operation I'm missing? Is it about Send for Bus<T> trait?

The only part I see that's zero-sized-dependent is the helper method at the bottom to unsafely get the raw device (pub unsafe fn lpi2c1 etc), but this is to work around another issue altogether and is clearly unsafe. This part shouldn't be in the example snippet, I've removed it.

if I just unsafely cloned the controller in the "bus+devices" struct constructor?

This would lead to issues if the bus driver has some internal state (which would then be duplicated).

True, I'd need to use some kind of cell here to actually share the instance.

Thanks for the feedback – and for the library! It's really useful, as each device driver seems to want to consume whole bus driver.

@ryan-summers
Copy link
Contributor

ryan-summers commented Jul 3, 2020

Hey guys, I know this may be a little bit late, but I was also having this issue. What I discovered is that there is a safe API to do this without modifying shared-bus.

When RTIC (previously known as RTFM) finalizes resources in init(), it then moves them to 'static lifetimes. Essentially, this means that after init(), you can freely acquire references to the manager (e.g. you are free to use manager.acquire()).

An implementation employing this essentially stores the BusManager as an RTFM resource. Any devices that require the bus cannot be constructed until after the init() function completes, so they should instead be wrapped in an Option() and be set to None during init().

Once you first enter idle, the BusManager is now static and we are free to hand out references, so we can replace the Options that we made earlier with the actual devices.

type MutexInner = core::cell::RefCell<I2c>;
type Mutex = cortex_m::interrupt::Mutex<MutexInner>;
type BusManager = shared_bus::CortexMBusManager<MutexInner, I2c>;
type BusProxy = shared_bus::proxy::BusProxy<'static, Mutex, I2c>;

//...
struct Resources {
    manager: BusManager,
    device: Option<MyI2cDevice<BusProxy>>,
}

fn init() -> init::LateResources {
    let i2c = ();
    let manager = BusManager(i2c);

    init::LateResources {
        manager,
        device: None,
    }
}

fn idle(c: idle::Context) -> ! {
    // Finally, construct our I2C device using the manager reference.
    c.resources.device.replace(MyI2cDevice::new(c.resources.manager.acquire()));
}

Of course, as @Rahix noted earlier, the mutex used by shared-bus will block all interrupts, which conflicts with RTFM. A workaround for this would be to implement a "dummy" mutex for shared-bus (where we assume that RTFM will handle all resource requests, so it is safe to ignore resource conflicts at this level).

Edit: Upon analysis later, I realized that the above statement about a dummy mutex is only valid if all devices on the shared-bus are stored as a single RTIC resource. Otherwise, you may have contention with the shared underlying resource.

@BlinkyStitt
Copy link

Any suggestions on how to implement a "dummy" mutex?

@ryan-summers
Copy link
Contributor

ryan-summers commented Jul 19, 2020

@wysenynja https://github.com/ryan-summers/shared-bus-rtic/blob/b33e05009dd92639197bc63174fb51ff5ed1078e/src/lib.rs#L59-L86 I originally used a dummy mutex in shared-bus-rtic, but was encountering issues with sharing the bus across multiple RTIC tasks (due to it not implementing Sync) and have since switched away from this method.

That dummy mutex does panic if you attempt to use the bus at the same time in two different contexts (e.g. a device on the bus pre-empts another device on the bus) due to an atomic bool check.

The underlying problem was that the reference that shared-bus gives out is inherently thread unsafe I believe (because a reference can't be shared between threads or something along those lines - I don't recall exactly).

@Rahix Rahix changed the title Is it possible to use this library with RTFM? Is it possible to use this library with RTIC? Aug 16, 2020
@ryan-summers
Copy link
Contributor

@Rahix I believe it is now safe to close this issue after the release of 0.2.0 - I'm successfully using this crate as-is with RTIC. I'll look into pulling shared-bus-rtic from crates.io now that shared-bus supports this use case.

@Rahix
Copy link
Owner

Rahix commented Aug 31, 2020

That's cool! Can you share some code? Should we add/change something to make it even easier? Also, would it make sense to add an example and documentation for this usecase?

@ryan-summers
Copy link
Contributor

That's cool! Can you share some code? Should we add/change something to make it even easier? Also, would it make sense to add an example and documentation for this usecase?

Right now, my use-case is unfortunately closed source (although the intent is to open-source it in the near future). However, I'm able to contribute the Mutex implementation back to shared-bus for future users of shared-bus and RTIC - I was just trying to get a PR together :)

@dbrgn
Copy link

dbrgn commented Oct 22, 2020

For reference, the PR by @ryan-summers is at #17 🙂 🎉

@antonok-edm
Copy link

I just gave @ryan-summers's approach a try for sharing SPI in RTIC. Looks like it's still not possible because acquire_spi is only defined for BusManager<NullMutex<T>>. Is that limitation still relevant? If so, perhaps it's still worth keeping this issue open.

@ryan-summers
Copy link
Contributor

I just gave @ryan-summers's approach a try for sharing SPI in RTIC. Looks like it's still not possible because acquire_spi is only defined for BusManager<NullMutex<T>>. Is that limitation still relevant? If so, perhaps it's still worth keeping this issue open.

This is by-design. You cannot safely share a SPI bus + CS pins using this library alone, as the embedded-hal does not support CS within the FullDuplex SPI trait. As such, there's always a possible race condition between asserting CSn and using the SPI bus. You can use shared-bus-rtic for SPI currently if you observe the restrictions outlined in the usage documentation (notably, all shared bus resources must lock the bus before asserting CSn, which can be done inherently by RTIC).

@Rahix Rahix linked a pull request Apr 20, 2021 that will close this issue
@Rahix
Copy link
Owner

Rahix commented Apr 20, 2021

With #17 merged and 0.2.2 released, I think this issue is finally one step closer to being solved :)

Still missing:

  • Currently only works with cortex-m
  • No support for ARMv6

@mbrieske
Copy link

mbrieske commented Jan 7, 2022

Hi, I have been playing around with the shared-bus library on RTIC and I am a bit confused by all the examples I find - would it be too much to ask if anyone would put together a minimal example on how to initialize the shared bus to use some I2C device in a RTIC task (on cortex-m)?
The examples all seem to refer to older versions of this library, or at least your comments suggest that since the examples have been published some pull requests were merged that changed things.

Thanks!

@dbrgn
Copy link

dbrgn commented Jan 7, 2022

Does https://github.com/dbrgn/chicken-door/blob/main/firmware/src/main.rs help? It's still on RTIC 0.5, but the code should be quite easy to adapt if you're already on 1.0.

@mbrieske
Copy link

mbrieske commented Jan 7, 2022

Exactly what I was looking for, thanks a lot! :)

Edit: This example is using shared-bus-rtic instead of shared-bus. Is this still the preferred lib? @ryan-summers comment #4 (comment) lead me to believe that his library was deprecated and shared-bus would work with RTIC now.

@Rahix
Copy link
Owner

Rahix commented Jan 7, 2022

@mbrieske, try https://github.com/ryan-summers/shared-bus-example I think.

@dbrgn
Copy link

dbrgn commented Jan 8, 2022

@mbrieske good point, I missed that. I should probably upgrade to plain shared-bus 🙂

@ryan-summers
Copy link
Contributor

ryan-summers commented Jan 8, 2022

shared-bus-rtic's only current use is to share a SPI bus for RTIC applications, since shared-bus can't quite do that yet due to some soundness issues around CS ownership

@nullobject
Copy link

This issue should be closed, with recent versions of shared-bus it is possible to use it with RTIC.

@Rahix Rahix closed this as completed Nov 23, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging a pull request may close this issue.

9 participants