esp-rs / esp-hal

no_std Hardware Abstraction Layers for ESP32 microcontrollers
https://docs.esp-rs.org/esp-hal/
Apache License 2.0
660 stars 187 forks source link

Feature Request: iterator as input for RMT transmit #1768

Open tschundler opened 1 month ago

tschundler commented 1 month ago

Right now, the smart_leds support is limited significantly, needing to pre-allocate a buffer for RMT symbols. I'd like to send to >1,000 LEDs, but now that needs a buffer of >100kB

Building the RMT symbols is already done internally as an iterator, and the buffer is read with an iterator. There is time for ~2k instructions per byte transmitted, which should be plenty for translating symbols and keeping the RMT's ring buffer full.

Does that seem like a reasonable feature? transmit_iter or IntoIterator for the data?

    fn transmit<'a, D, R, T: Into<u32> + Copy + 'a>(
        self,
        data: D,
    ) -> SingleShotTxTransaction<'a, Self, R, T>
    where
        Self: Sized,
        D: IntoIterator<Item = &'a T, IntoIter = R> + 'a,
        R: Iterator<Item = &'a T> + 'a,

I'll likely try it out myself in the next week if I have time. Also I'm curious to see if I can drive multiple strips at once that way with embassy.

~Does anyone have opinions about when to refill the buffer? eg I wonder if I might get better results refilling in 16 byte chunks instead of once it is half depleted (24-32 bytes). That way in async or from the iterator if something eats a little extra time, there's more headroom. Does that seem reasonable?~ (Reading more into how RMT works, scratch the smaller threshold idea - it won't work, since there seems to be no way to know where the read head of the RMT device is. )

related: #1749 (RMT API changes), #1710 (async LEDs), #787 (original aync RMT pull), #1779 (async RMT only supports 64 pulse codes) my last experimenting with older/ version of the RMT driver: #355

tschundler commented 1 month ago

Playing a little, I have it working in blocking mode. I can generate some patterns with no buffers at all.

I did need to use a different signature. When iterating an array, Iterator<Item = &'a T> + 'a works, but when generating content on the fly, what is the reference to? It's gone when .next() finishes. So instead I need this signature for transmit:

    fn transmit<D, R, T: Into<u32> + Copy>(self, data: D) -> SingleShotTxTransaction<Self, R, T>
    where
        Self: Sized,
        D: IntoIterator<Item = T, IntoIter = R>,
        R: Iterator<Item = T>,

which is not backwards-compatible, since it can't take &[u32;n] as the data. So should I have transmit + transmit_iter (or better name?) where transmit calls transmit_iter(data.iter().cloned())

WIP: https://github.com/esp-rs/esp-hal/compare/main...tschundler:esp-hal:async-led-iterator

tschundler commented 1 month ago

An important question as I continue to work on this:

How important is API stability right now? I can change the transmit function or add transmit_iter, which means nothing breaks. I've added an underflow error to help diagnose misbehavior - should it be part of a separate error enum used by transmit_iter?

And for the SmartLEDsAdapter, change it, or add SmartLEDsUnbuffered?

tschundler commented 1 month ago

I should update in my own thread...

In the coming days I'll probably submit a pull requests related to RMT from iterator & SmartLEDs HAL via iterator. I am curious about any input before I submit a PR.

Dominaezzz commented 3 weeks ago

How important is API stability right now? I can change the transmit function or add transmit_iter, which means nothing breaks. I've added an underflow error to help diagnose misbehavior - should it be part of a separate error enum used by transmit_iter?

Feel free to break the API but be sure to provide a migration guide in the PR comment.