golemparts / rppal

A Rust library that provides access to the Raspberry Pi's GPIO, I2C, PWM, SPI and UART peripherals.
MIT License
1.24k stars 97 forks source link

Using rppal with embedded_hal_async (async support) #134

Open IhsenBouallegue opened 11 months ago

IhsenBouallegue commented 11 months ago

I am trying to get lora_phy working with rppal. But I am getting the following errors image It seems like lora_phy is using embedded_hal_async next to embedded_hal. As far as I understood, only embedded_hal is implemented for rppal? is there a relatively simple way to add embedded_hal_async support, or is it pretty much a rewrite?

Here is also a reproduction repo. If you want to check my setup https://github.com/IhsenBouallegue/lora-phy-embedded-linux

golemparts commented 11 months ago

Hi @IhsenBouallegue. As you noted, embedded_hal_async isn't currently supported. From a cursory glance, supporting the async version should be relatively straightforward. However, since rppal doesn't support async natively, someone would have to write a wrapper to provide the needed functionality using the existing sync functions.

The best short-term solution would be to use a crate that supports embedded_hal_async on linux as I'm not sure when/if rppal will get support for embedded_hal_async. However, if anyone is up for adding support, I'd be happy to accept a PR.

IhsenBouallegue commented 11 months ago

Hey @golemparts thanks for the reply! I would also be happy to open a PR, but I am still lost on how to do this (and learning rust along the way). Would perhaps using something like this be possible? https://github.com/embassy-rs/embassy/blob/main/embassy-embedded-hal/src/adapter/blocking_async.rs I couldn't get it working yet but wanted to get more opinions and tips first.

golemparts commented 11 months ago

Thanks for the link. While that would technically work, it's not the approach I would go for, since it just turns the async implementation into a potentially blocking one, which could cause issues. rppal's native functionality can already be used in a (mostly) non-blocking way, although I haven't checked if that's possible for all peripherals and operations supported by embedded-hal-async. Some additional internal support might be needed.

Once embedded-hal 1.0.0 releases it'll be time for some clean-up anyway. If noone has had a chance to add support for async before then, I'll add it myself, since it does seem useful to have available for drivers that require it.

IhsenBouallegue commented 11 months ago

Awesome I will be looking forward to that and hopefully I can help out. I am writing my bachelor thesis in this more or less and I would to get to use the async functions!

IhsenBouallegue commented 10 months ago

@golemparts with the release of embedded-hal 1.0.0 is there a possibility to add proper async with embedded-hal-async now? I gave it a try myself. While I managed to make it "work" I am pretty sure this is an ugly workaround

impl embedded_hal_async::digital::Wait for InputPin {
    async fn wait_for_low(&mut self) -> core::result::Result<(), Self::Error> {
        let (sender, receiver) = oneshot::channel();
        let sender_mutex = Arc::new(std::sync::Mutex::new(Some(sender)));
        let _ = self.set_async_interrupt(super::Trigger::FallingEdge, move |_| {
            if let Some(sender) = sender_mutex.lock().unwrap().take() {
                sender.send(()).unwrap();
            }
        });
        Ok(receiver.await.unwrap())
    }
    ...

for spi I made a wrapper that fakes async but a proper implementation would benefit the library a lot. Sadly I couldn't manage that 😢

impl<T, E> embedded_hal_async::spi::SpiBus<u8> for BlockingAsync<T>
where
    E: embedded_hal::spi::Error + 'static,
    T: blocking::spi::Transfer<u8, Error = E> + blocking::spi::Write<u8, Error = E>,
{
    async fn flush(&mut self) -> Result<(), Self::Error> {
        Ok(())
    }

    async fn write(&mut self, data: &[u8]) -> Result<(), Self::Error> {
        self.wrapped.write(data)?;
        Ok(())
    }

    async fn read(&mut self, data: &mut [u8]) -> Result<(), Self::Error> {
        self.wrapped.transfer(data)?;
        Ok(())
    }

    async fn transfer(&mut self, read: &mut [u8], write: &[u8]) -> Result<(), Self::Error> {
        for i in 0..core::cmp::min(read.len(), write.len()) {
            read[i] = write[i].clone();
        }
        self.wrapped.transfer(read)?;
        Ok(())
    }

    async fn transfer_in_place(&mut self, data: &mut [u8]) -> Result<(), Self::Error> {
        self.wrapped.transfer(data)?;
        Ok(())
    }
}
golemparts commented 10 months ago

with the release of embedded-hal 1.0.0 is there a possibility to add proper async with embedded-hal-async now?

That's the plan, yes 😁 . I'm glad to see you figured out a workaround for yourself for now, assuming you're using that in your own projects. I haven't had a chance to dive into the async traits yet myself, other than a cursory glance when you first opened this issue. I'll be publishing the latest rppal with embedded-hal 1.0.0 support this week. I can't give you an exact schedule for when to expect async support, but it's on the list!

IhsenBouallegue commented 10 months ago

@golemparts Glad to see embedded-hal 1.0.0 coming to rppal! Awesome work 🥳 Sadly my workaround is just a workaround and things are starting to act weird now, and I suspect it's because of my implementation :'( I am using this as part of my thesis, so I am a bit constrained on time. I am up for implementing this in the library if I get some guidance. Let me know if I can be of any help! 😄

golemparts commented 10 months ago

@IhsenBouallegue unfortunately I'm pretty swamped with another project at the moment, so I wouldn't be able to offer much guidance. As you are still new to Rust and are constrained on time, I would recommend checking if there are any other embedded-hal-async implementations available. A generic linux implementation should generally work for the Pi. It just won't be as performant as rppal's Pi-specific implementation. Alternatively, perhaps another LoRa PHY crate offers similar functionality without the need for embedded-hal-async. Another option could be to implement the driver yourself using rppal or embedded-hal.

coolreader18 commented 2 weeks ago

Would it be reasonable to use something like tokio or async-io for this? It feels like a lot of unnecessary work to reimplement an event loop when async_io::Async::new(Interrupt::new()).poll_readable() does the same thing, ready-made.