Thank you for your contribution!

We appreciate the time and effort you've put into this pull request.
To help us review it efficiently, please ensure you've gone through the following checklist:

Submission Checklist 📝

• I have updated existing examples or added new ones (if applicable).
• I have used cargo xtask fmt-packages command to ensure that all changed code is formatted correctly.
• My changes were added to the CHANGELOG.md in the proper section.
• I have added necessary changes to user code to the latest Migration Guide.
• My changes are in accordance to the esp-rs developer guidelines

Extra:

• I have read the CONTRIBUTING.md guide and followed its instructions.

Pull Request Details 📖

This changes the input data type for RMT Tx methods from &[PulseCode] to &mut impl Encoder where Encoder is conceptually similar to Iterator<Item = PulseCode>, but allows for more efficient code in many cases.

IDF has a similar encoder type: https://docs.espressif.com/projects/esp-idf/en/latest/esp32c3/api-reference/peripherals/rmt.html#rmt-rmt-encoder

General design

The Encoder trait differs from Iterator<Item = PulseCode> mainly in the following aspects:

• Writing data is driven by the encode method, which calls RmtWriter methods to push to the hardware, rather than RmtWriter pulling data from an iterator.
• The RmtWriter::write_many method helps write several codes to the hardware in a very tight loop.

The combination of both allows achieving very efficient inner loops when copying data to the hardware, without requiring unsafe code on the user side and without exposing any direct hardware access or any specifics about how much data is written to the user code: See for example the BytesEncoder implementation. I've not been able to achieve the same performance with just Iterators.

Specifically, the pattern from BytesEncoder is similar to what's required to send data to WS2812-style LEDs:
Fetch R, G, B bytes from RAM, assemble in the correct order into a u32, then shift out bits and write a PulseCode for each. That maps very cleanly to write_many, but leads to overhead with iterators.

I've been benchmarking this¹ using cycle counters, here are some results for a WS2812 LED stripe encoder/pulse code iterator which are the fastest I've been able to achieve:

// base case: not using Encoder, just write zeros to the hardware using raw pointers
Render benchmark (base): RMT BenchmarkResult:
	CPU clock: 160MHz
	Iterations: 39
	Codes written: 1441
	Encoding time: 126us
	Encoding time / code: 88ns ~ 14 cycles

// pre-compute PulseCodes and use CopyEncoder
Render benchmark (slice): RMT BenchmarkResult:
	CPU clock: 160MHz
	Iterations: 32
	Codes written: 1441
	Encoding time: 154us (9% of 1710us tx time)
	Encoding time / code: 107ns ~ 17 cycles

// custom impl of Iterator<Item = PulseCode> for an LED stripe encoder type
Render benchmark (iter): RMT BenchmarkResult:
	CPU clock: 160MHz
	Iterations: 20
	Codes written: 1441
	Encoding time: 249us (14% of 1710us tx time)
	Encoding time / code: 172ns ~ 27 cycles

// custom impl of Encoder for an LED stripe encoder type
Render benchmark (enc): RMT BenchmarkResult:
	CPU clock: 160MHz
	Iterations: 29
	Codes written: 1441
	Encoding time: 170us (9% of 1710us tx time)
	Encoding time / code: 118ns ~ 18 cycles

"Encoding time" is just the time to run the encoder_write function, not including any polling or interrupt/embassy dispatch overhead. Thus, the fact that it takes "only" ~10% of tx time is a bit misleading.

Note that the custom encoder version has a 40% lower cycle count compared to the iterator version, and is on par with with the CopyEncoder (which requires precomputing PulseCodes in a large buffer). Both encoder variants are quite close to the performance ceiling of the base case, which I presume is limited due to the APB speed.

In this case, the inner loop of the encoder compiles to optimal assembly, cf. the decompiled version²:

whereas the iterator version remains more convoluted.

API

The PR continues to use a single transmit() method for various data types, requiring explictly wrapping things in an Encoder:

let mut enc = CopyEncoder::new(&data);
channel.transmit(&mut enc)?;

I also considered an IntoEncoder trait with fn transmit(&mut self, data: impl IntoEncoder) with implementations provided for

• &[PulseCode],
• I where I: IntoIterator<Item = PulseCode>`,
• E where E: Encoder.

However, that immediately runs into issues with specialization due to the blanket impls.

Alternatively, one could consider different transmit method, i.e.

• fn transmit_slice(&mut self, data: &[PulseCode]) -> ...,
• fn transmit_iter(&mut self, data: impl IntoIterator<Item = PulseCode>) -> ...,
• fn transmit_enc(&mut self, data: impl Encoder) -> ....

The disadvantage is that this blows up the number of methods significantly. In particular, if/when methods are split into transmit(&mut self, ...) and transmit_owned(self, ...) similar to the SHA driver, as suggested by @Dominaezzz, this would lead to combinatorial explosion of the number of channel methods. Additionally, having such per-datatype methods isn't really much simpler than explictly creating encoders, in my opinion.

Questions

• I mentioned that I added some benchmarking code: This needs support in esp-hal for low-level hardware access. Would something like this in principle be in-scope for the project?
• Should BytesEncoder be part of esp-hal directly? It might make more sense to move it to an example, showcasing how to write an efficient Encoder`.

Testing

HIL tests, incl. new ones.

Closes #1768