Skip to content

Conversation

@wisp3rwind
Copy link
Contributor

@wisp3rwind wisp3rwind commented Sep 4, 2025

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 Migration Guide.
  • My changes are in accordance to the esp-rs developer guidelines

Extra:

Pull Request Details 📖

This adds buffer wrapping support to blocking/async rx/tx methods (previously, only blocking tx did support this).

Some preparatory refactoring and a few (IMO) convenient API additions are included. There's only one small change to existing API, which is included in the migration guide.

(Using this should also greatly help with esp-rs/esp-hal-community#32, if someone (not me) creates a PR to use async wrapping tx.)

Testing

HIL tests (incl. new ones)

Closes: #2723
Closes: #1779

@wisp3rwind wisp3rwind mentioned this pull request Sep 3, 2025
13 tasks
@wisp3rwind wisp3rwind force-pushed the rmt-wrap branch 8 times, most recently from 89a7387 to c140d1a Compare September 10, 2025 21:57
@wisp3rwind wisp3rwind force-pushed the rmt-wrap branch 3 times, most recently from b8e2993 to 1b8ae7f Compare September 25, 2025 09:37
@wisp3rwind wisp3rwind mentioned this pull request Oct 3, 2025
6 tasks
@wisp3rwind wisp3rwind force-pushed the rmt-wrap branch 3 times, most recently from 2e3849e to a381207 Compare October 7, 2025 17:34
- simplification and preparation for also supporting threshold
  interrupts
- the split into a generic async_interrupt_handler and
  chip_specific::handle_channel_interrupts didn't really simplify
  things: It avoided a little bit of repetition, at the cost of quite a
  bit of syntax, and was also a poor representation of the differences
  between chips (namely the fact that on esp32 and esp32s2, the ch_err
  interrupt does not differentiate rx/tx).
Preparatory work for async wrapping rx/tx. This has no effect for now,
since the corresponding interrupts are never enabled.
While not essential to parse received data, which will typically look
for an end marker to determine the actual end of the data, this still
appears useful.
instead of copying some garbage to the result buffer
- handle wrapping rx (and varying device support)
- fix/simplify logic around end markers
- check that the rx buffer is not overwritten beyond the actually
  received data
this can happen if a large buffer is supplied to Channel::receive, which
produces an InvalidDataLength Error and does not start rx, but still
returns RmtRxFuture (which will yield an error when polled for the first
time)
@wisp3rwind wisp3rwind marked this pull request as ready for review October 7, 2025 18:19
@wisp3rwind
Copy link
Contributor Author

This should finally be ready to go; what do you think?

Copy link
Contributor

@JurajSadel JurajSadel left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM, thank you!

@MabezDev MabezDev added this pull request to the merge queue Oct 8, 2025
Merged via the queue into esp-rs:main with commit 7eabbe4 Oct 8, 2025
29 checks passed
@wisp3rwind wisp3rwind mentioned this pull request Nov 13, 2025
6 tasks
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

[RMT] Allow reading of more data than fits into the peripheral's allocated RAM Async RMT Tx not working with >64 PulseCodes

3 participants