RMT: Refactor state checks, support non-blocking poll on blocking transactions #3716
+199
−159
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
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
Their use has entirely be replaced by get_tx_status and get_rx_status
wisp3rwind
force-pushed
the
rmt-get-state
branch
from
439308b to
0650e05
Compare
MabezDev
reviewed
Jul 18, 2025
| // | ||
| // Returns whether transmission has ended (whether successfully or with an | ||
| // error). In that case, a subsequent call to `wait()` returns immediately. | ||
| #[doc(hidden)] |
There was a problem hiding this comment.
Rmt is unstable so wee can make this public, even if it's not ready.
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
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.