caemor / epd-waveshare

Drivers for various EPDs from Waveshare
ISC License
207 stars 128 forks source link

Add async support #175

Open vhdirk opened 8 months ago

vhdirk commented 8 months ago

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?

caemor commented 6 months ago

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?

vhdirk commented 6 months ago

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.

caemor commented 6 months ago

Thanks :+1:

avsaase commented 1 month ago

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.

vhdirk commented 1 month ago

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.