Skip to content
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

Using rppal with embedded_hal_async (async support) #134

Open
IhsenBouallegue opened this issue Dec 3, 2023 · 8 comments
Open

Using rppal with embedded_hal_async (async support) #134

IhsenBouallegue opened this issue Dec 3, 2023 · 8 comments

Comments

@IhsenBouallegue
Copy link

IhsenBouallegue commented Dec 3, 2023

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

@IhsenBouallegue IhsenBouallegue changed the title Using rppal with lora_phy Using rppal with lora_phy (embedded_hal_async support?) Dec 3, 2023
@golemparts
Copy link
Owner

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
Copy link
Author

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
Copy link
Owner

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 IhsenBouallegue changed the title Using rppal with lora_phy (embedded_hal_async support?) Using rppal with embedded_hal_async (async support) Dec 4, 2023
@IhsenBouallegue
Copy link
Author

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
Copy link
Author

IhsenBouallegue commented Jan 14, 2024

@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
Copy link
Owner

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
Copy link
Author

@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
Copy link
Owner

golemparts commented Jan 18, 2024

@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.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

2 participants