RMT: Refactor state checks, support non-blocking poll on blocking transactions #3716
Conversation
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
wisp3rwind
force-pushed
the
rmt-get-state
branch
from
5f342a9 to
b4aa695
Compare
MabezDev
reviewed
Jul 1, 2025
wisp3rwind
force-pushed
the
rmt-get-state
branch
from
439308b to
0650e05
Compare
MabezDev
reviewed
Jul 18, 2025
Member
MabezDev
left a comment
MabezDev
left a comment
There was a problem hiding this comment.
Imo it can land with poll publicly documented. Please also add a changelog entry.
wisp3rwind
force-pushed
the
rmt-get-state
branch
from
0650e05 to
781946b
Compare
Contributor
Author
|
Rebased on main, removed |
wisp3rwind
force-pushed
the
rmt-get-state
branch
from
781946b to
67accfa
Compare
Unused right now; these methods allow to look at all flags without several volatile reads (in contrast to calling is_tx_done/is_rx_done, is_tx_threshold_set, is_error individually). `match`ing on their result leads to very idiomatic code, and also result in blocking and async code that is very similar. The #[inline] attribute might not be required, but better be sure: We really don't want the Option<Event> to actually be constructed, but rather that the compiler completely optimized it away in favor of direct bit tests in the calling function.
by merging two loops into one
by avoiding repeated register reads
This allows properly interleaving several such transactions (as the HIL loopback tests do, in fact) or other work. Regarding the implementation of the place_rmt_driver_in_ram feature for these methods, note that `#[ram]` implies `[inline(never)]`. Thus, this uses `#[inline(always)]` for `poll` and `wait` in that case, which are rather simple methods. If putting them in ram actually matters, the calling user code should probably be placed in ram as well, and then, forced inlining of both methods should have the desired effect. `poll_internal` in turn - contains more code and is called from `poll` and `wait` -> it seems sensible that it is not inlined to reduce code size. - includes the hot loop that copies to the hardware buffer -> should be forced to ram so `#[ram]` makes sense. Note that none of this was benchmarked.
Their use has entirely be replaced by get_tx_status and get_rx_status
wisp3rwind
force-pushed
the
rmt-get-state
branch
from
67accfa to
428a94e
Compare
6 tasks
bugadani
approved these changes
Jul 30, 2025
6 tasks
6 tasks
6 tasks
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
4 participants
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
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 📝
cargo xtask fmt-packagescommand to ensure that all changed code is formatted correctly.CHANGELOG.mdin the proper section.Extra:
Pull Request Details 📖
This refactors accesses to hardware state flags to avoid unnecessarily repeated volatile reads. Additionally, by using
matchstatements, the resulting code becomes more idiomatic and better exposes the similarities of blocking and async code. Finally, this PR add non-blockingpollmethods to blocking transactions, which are quite similar to their async equivalent. They enable proper interleaving of such transactions with other work. This is currently only relevant for tx due to the lack of wrapping rx support, but the latter will also be added as part of #3509.Except for the new methods, there should be no user-facing changes, hence no need for adding to the migration guide.
This should be easiest to review commit-by-commit while ignoring whitespace changes; see individual commit messages for more details on the rationale for some changes.
In the last commit, I removed the now unused low-level state getters. Does this seem reasonable to you, or should they be kept in case they'll be needed again? (FWIW, I'm not using them anywhere in #3509.)
Part of #3509.
Closes #3184
Closes #1749
(Not completely sure about both of these issues since they don't describe their use case in a lot of detail, but this does enable non-blocking operations.)
Testing
HIL tests on esp32c3, in addition to testing as part of #3509.