RMT: Revise Tx error handling and add more HIL tests #4617
Conversation
wisp3rwind
force-pushed
the
rmt-tx-error-hil
branch
from
c9b71dc to
c3b5e5d
Compare
|
/hil full |
JurajSadel
added
the
trusted-author
[HIL trust list]Trusted users for this PR (click to expand) |
|
Author @wisp3rwind was trusted for this PR via the |
|
Triggered full HIL run for #4617. Run: https://github.com/esp-rs/esp-hal/actions/runs/19943514564 Status update: ❌ HIL (full) run failed (conclusion: failure). |
There was a problem hiding this comment.
Copilot wasn't able to review any files in this pull request.
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
wisp3rwind
force-pushed
the
rmt-tx-error-hil
branch
from
c3b5e5d to
14a477b
Compare
to ensure that upcoming Iterator/Encoder-related refactoring dont't break anything. The EndMarkerMissing cases with different tx lengths seem redundant right now (since this is verified before even starting the transmission), but that will change when iterator input is supported and the presence of an end marker can only be verified when the iterator is exhausted.
wisp3rwind
force-pushed
the
rmt-tx-error-hil
branch
from
14a477b to
4b26b87
Compare
wisp3rwind
changed the title
|
/hil quick |
|
Triggered quick HIL run for #4617. Run: https://github.com/esp-rs/esp-hal/actions/runs/20067475027 Status update: ❌ HIL (quick) run failed (conclusion: failure). |
…read-only - shares a tiny bit more code between Blocking and Async tx - removes Slice-specific error handling in preparation for supporting iterator (-like) data types - makes RmtWriter.state read-only and only write it from the rmt::write module to make it easier to reason about As a side effect, continuous tx will now also report EndMarkerMissing errors. This makes sense: The loopcount interrupt only appears to trigger when hitting and end marker, and does not trigger when in loop tx mode with wrapping enabled and wrapping at the buffer end. In other words, in that case, loop tx seems to work at first, but does not trigger interrupts, so it should probably be prevented (it might make sense to allow LoopMode::Infinite without end marker, however).
cf. the commit message of the preceding commit
wisp3rwind
force-pushed
the
rmt-tx-error-hil
branch
from
54afe4d to
17d4de5
Compare
|
/hil quick |
|
Triggered quick HIL run for #4617. Run: https://github.com/esp-rs/esp-hal/actions/runs/20068076677 Status update: ❌ HIL (quick) run failed (conclusion: failure). |
|
/hil esp32 esp32c3 esp32c6 |
|
Triggered HIL run for #4617 (chips: esp32 esp32c3 esp32c6). Run: https://github.com/esp-rs/esp-hal/actions/runs/20068620841 Status update: ❌ HIL (per-chip) run failed (conclusion: failure). |
|
This should be ready now (HIL failures are due to the c6 runner acting up). Note that I extended the scope of this PR compared to the initial push, see the updated description and title above. |
There was a problem hiding this comment.
Pull request overview
Copilot reviewed 3 out of 4 changed files in this pull request and generated 4 comments.
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
There was a problem hiding this comment.
Pull request overview
Copilot reviewed 3 out of 4 changed files in this pull request and generated no new comments.
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
- remove (currently) unused getter - code style
3 participants
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 📖
Split out of #4604.
Test a little bit more thoroughly that tx methods return the expected errors for invalid data. Additionally, refactor the error handling code to be less specific for slices
[PulseCode]. This is in preparation for adding iterator/encoder data type support. Having the tests in place should help prevent regressions.There are some subtle behavioral changes due to this:
transmit_continuouslycan now report `Error::EndMarkerMissing´I think these changes are a) unavoidable when supporting iterator data types, and b) not really an issue, since all the same errors will still be detected.
See also the commit messages for details.
Testing
HIL tests.