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

Feature Request: iterator as input for RMT transmit #1768

Open
tschundler opened this issue Jul 7, 2024 · 4 comments
Open

Feature Request: iterator as input for RMT transmit #1768

tschundler opened this issue Jul 7, 2024 · 4 comments
Labels
peripheral:rmt RMT peripheral

Comments

@tschundler
Copy link

tschundler commented Jul 7, 2024

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 tschundler changed the title Feature Request: iterator as input for RMT Feature Request: iterator as input for RMT transmit Jul 7, 2024
@jessebraham jessebraham added the peripheral:rmt RMT peripheral label Jul 8, 2024
@tschundler
Copy link
Author

tschundler commented Jul 9, 2024

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: main...tschundler:esp-hal:async-led-iterator

tschundler added a commit to tschundler/esp-hal that referenced this issue Jul 9, 2024
tschundler added a commit to tschundler/esp-hal that referenced this issue Jul 9, 2024
@tschundler
Copy link
Author

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

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

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.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
peripheral:rmt RMT peripheral
Projects
Status: Todo
Development

No branches or pull requests

3 participants