epd-waveshare icon indicating copy to clipboard operation
epd-waveshare copied to clipboard

Add async support

Open vhdirk opened this issue 2 years ago • 7 comments

Hi,

I'm adding support for async runtimes this driver. My project uses embassy so this is quite the necessity for me. It doesn't seem all that difficult from the get go; just a lot of work. Mainly, the idea is to use the SpiDevice type from embedded-hal-async and make a bunch of methods async.

Before I start with the grunt of the work, I'd like some opinions on code structuring. The options that I currently see are:

keep the code structure mostly as is, replacing sync versions with async where needed

This would look something like the following:

#[cfg(not(feature="async"))]
impl<SPI, BUSY, DC, RST, DELAY> Epd1in54<SPI, BUSY, DC, RST, DELAY>
where
    SPI: embedded_hal::spi::SpiDevice,
    BUSY: InputPin,
    DC: OutputPin,
    RST: OutputPin,
    DELAY: DelayUs,
{
}
#[cfg(feature="async")]
impl<SPI, BUSY, DC, RST, DELAY> Epd1in54<SPI, BUSY, DC, RST, DELAY>
where
    SPI: embedded_hal_async::spi::SpiDevice,
    BUSY: InputPin,
    DC: OutputPin,
    RST: OutputPin,
    DELAY: DelayUs,
{
}

The drawback here is that sync and async cannot co-exist. I don't believe that too big of an issue since it would be unlikely to need both versions at the same time. However; I'm not sure how cargo test --all-features would behave.

For each module, create a nested async module

Inspired by this: https://github.com/esp-rs/esp-wifi/blob/a20545ca8f8463192cb84aeba573d8e68e144cc9/esp-wifi/src/wifi/mod.rs#L1535

I would still have to duplicate all common traits, too. The drawback here is that there will be even more code duplication than option 1 since ever type that is generic over SpiDevice would need an async version.

Fork this repo and keep it wholly separated

The pinnacle of code duplication, all for the purpose of clarity.

Make everything async and provide a blocking wrapper

This is how https://docs.rs/reqwest/latest/reqwest/ does it


Unfortunately, any approach that I can think of will involve a fair amount of code duplication.
Do you see any more options? Which would have your preference?

vhdirk avatar Nov 06 '23 07:11 vhdirk

Thanks for your work! Do you know if there is already an async blocking wrapper for an embedded driver? I think I would prefer option 4 or option 1, if 4 is more difficult or different than I would expect. 2 and 3 sound like too much duplication and therefore later code divergence to me. What is your favorite?

caemor avatar Jan 11 '24 23:01 caemor

I think option 1 would be best, even though 4 makes way more sense from a developer point of view. Thing is that async has more stack space requirements than sync. IIRC, that was mentioned to me by https://github.com/Dirbaio. Therefore, I think 4 will come with its own issues which not a lot of users may expect.

That being said, my project https://github.com/vhdirk/papertrain is pretty much done, so I won't spend much time on this driver anymore. Feel free to take whatever you need from my async fork and papertrain, though.

vhdirk avatar Jan 12 '24 06:01 vhdirk

Thanks :+1:

caemor avatar Jan 12 '24 07:01 caemor

In case this is still being worked on, another option would be to use a crate like maybe-async to avoid duplicate the whole api.

avsaase avatar May 29 '24 17:05 avsaase

Yes, I've picked this up again last week. I've been rebasing on master, but I have yet to complete and push it. It might take me a while, I'm quite busy at the moment.

vhdirk avatar May 29 '24 18:05 vhdirk