smart-leds-rs / smart-leds-trait

A trait for implementing effects, modifiers and drivers for programmable leds
Apache License 2.0
2 stars 4 forks source link

Async trait #12

Open davoclavo opened 6 months ago

davoclavo commented 6 months ago

Hello! I was wondering if there is interest in adding a trait for async drivers.

I am currently using an Rmt async driver for my smart leds and couldn't find support for it anywhere, so I rolled my own, and wanted to contribute back.

I am happy to provide a PR to enable those traits (perhaps with a feature flag) something like the following, or perhaps via a generic type, as esp-hal does it

#[cfg(feature = "async")]
pub trait SmartLedsWriteAsync {
    type Error;
    type Color;
    async fn write<T, I>(&mut self, iterator: T) -> Result<(), Self::Error>
    where
        T: IntoIterator<Item = I>,
        I: Into<Self::Color>;
}
david-sawatzke commented 4 months ago

I wouldn't be against it, but, to be honest, I have very little experience with async rust. Feel free to open a PR.

Since I don't have any idea about async, let me ask a few questions:

chmanie commented 3 days ago

Embassy for example has two traits here for the shared_bus module: https://docs.embassy.dev/embassy-embedded-hal/git/default/shared_bus/index.html

They are separated by different modules: blocking and asynch. I don't think there's a reason to be concerned about naming collisions as most of the time you really just do one or the other. Plus there's always the option of renaming on import. At least that's how embassy and the embedded-hal-async are dealing with it.

I would suggest to add a sub-module here named asynch as well in which the SmartLedsWrite trait for async environments lives. If you want, we can also add a blocking module to make it more explicit and consistent.

I'll make a suggestion in a PR and you can let me know :)