Skip to content

Prefer reporting an error instead of triggering an assert#5865

Open
bjoernQ wants to merge 6 commits into
esp-rs:mainfrom
bjoernQ:esp32-fix-rmt-rx
Open

Prefer reporting an error instead of triggering an assert#5865
bjoernQ wants to merge 6 commits into
esp-rs:mainfrom
bjoernQ:esp32-fix-rmt-rx

Conversation

@bjoernQ

@bjoernQ bjoernQ commented Jul 9, 2026

Copy link
Copy Markdown
Contributor

Fixes #4697

On ESP32 (and most probably ESP32-S2) there are situations when the end-interrupt got triggered with an prematurely (while it should be an error)

Reproducing with the (modified) embassy_rmt_rx example:

Don't connect GPIO4 -> GPIO5, run the example, wait a bit, connect GPIO4+GPIO5 (or even just touch the pins) .... the assert triggered.

Now this condition is reported as an error.

I was considering adding an HIL test without success - I can put more effort into that if we think it's really needed

@bjoernQ bjoernQ added the skip-changelog No changelog modification needed label Jul 9, 2026

@MabezDev MabezDev left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

I'd like a HIL test if we can make one 🥺

Comment thread esp-hal/src/rmt/reader.rs
raw: DynChannelAccess<Rx>,
final_: bool,
) {
) -> bool {

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Needs some docs explaining what the return value means imo

@bjoernQ

bjoernQ commented Jul 10, 2026

Copy link
Copy Markdown
Contributor Author

I'd like a HIL test if we can make one 🥺

Will try as soon as I get probe-rs to talk to EspProg again (on my machine)

@bjoernQ bjoernQ force-pushed the esp32-fix-rmt-rx branch from ceb5a0b to 3548e67 Compare July 10, 2026 14:18
Comment thread hil-test/src/bin/rmt.rs Outdated
Comment on lines +1263 to +1265
for _ in 0..1000 {
tx.toggle();
}

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

I think this test could use a bit of explanation - why do we toggle a GPIO at full speed in an async block?

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

ok - that looks definitely sus - hope the added comments help a little bit. it's generally an odd issue

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Should we try and blast a stable-ish SPI clock into the RMT input instead of a GPIO signal of undeterminate frequency?

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

we can try

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

skip-changelog No changelog modification needed

Projects

None yet

Development

Successfully merging this pull request may close these issues.

debug_assert() in rmt/reader.rs on ESP32

3 participants