Skip to content

Add EspRadio::with_rx_queue_size to make the RX queue depth configurable#91

Open
snabb wants to merge 1 commit into
esp-rs:mainfrom
snabb:feat/esp-radio-rx-queue-setter
Open

Add EspRadio::with_rx_queue_size to make the RX queue depth configurable#91
snabb wants to merge 1 commit into
esp-rs:mainfrom
snabb:feat/esp-radio-rx-queue-setter

Conversation

@snabb

@snabb snabb commented Jun 28, 2026

Copy link
Copy Markdown
Contributor

EspRadio hardcodes the esp-radio receive-queue depth at 50 in update_driver_config(). The right value is workload-, stall-, and memory-dependent, so it can't be one constant — and esp-radio already exposes it as a runtime Config field (default 10); the wrapper just shadows it.

This adds a non-breaking builder, leaving the default at 50:

let radio = EspRadio::new(ieee).with_rx_queue_size(200);

Why it's needed: on Matter-over-Thread (ESP32-C6), fragmented commissioning bursts overflow 50 while the OpenThread consumer is briefly stalled (crypto) → esp-radio logs Receive queue full, frames drop, commissioning fails. Measured: rx=50 dropped ~54 frames in a bursty window; rx=200 → 0. Each slot is a ~130 B heap buffer, so the depth also bounds RX heap (200 ≈ 26 KB) — hence configurable rather than a higher default.

Default behavior is unchanged. HW-validated on esp32-c6 (the value propagates and commissioning is reliable at 200).

Companion passthrough (rs-matter-embassy): sysgrok/rs-matter-embassy#62. Background / discussion: #92.

Open to the API shape — happy to adjust if you'd prefer something else.

The esp-radio 802.15.4 driver buffers received frames in a software queue
(`rx_queue_size`, esp-radio default 10) that `EspRadio` hardcodes to 50. Under
bursty load where the OpenThread consumer briefly stalls — e.g. crypto during
Matter commissioning lets a fragmented exchange pile up — 50 overflows
(`Receive queue full`), dropping frames and breaking large fragmented exchanges;
a higher depth (e.g. 200) absorbs the burst.

Expose it as a builder setter, leaving the default at 50 so existing behavior is
unchanged. Each queued frame is a ~130-byte heap buffer (`RawReceived`) allocated
on demand, so the depth caps the worst-case RX heap (200 ≈ 26 KB).

Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
@ivmarkov

Copy link
Copy Markdown
Collaborator

@snabb Have you seen this:

sysgrok@01fad2b

Eager to test it with the Esp radio upon my return, but for the nrf-802154 it made all the difference, without needing to significantly increase the radio rx queue.

Might require uncommenting RX_WHEN_IDLE in the esp radio, but in any case I think your opus should think about this specific change.

It should really improve ALL drivers, as the meltdown-under-load that it fixes for the nrf-802154 I've seen under the esp driver as well.

@snabb

snabb commented Jun 28, 2026

Copy link
Copy Markdown
Contributor Author

@snabb Have you seen this:

sysgrok@01fad2b

Eager to test it with the Esp radio upon my return, but for the nrf-802154 it made all the difference, without needing to significantly increase the radio rx queue.

Might require uncommenting RX_WHEN_IDLE in the esp radio, but in any case I think your opus should think about this specific change.

It should really improve ALL drivers, as the meltdown-under-load that it fixes for the nrf-802154 I've seen under the esp driver as well.

No, I had not seen that. Yes it seems likely relevant. Interesting. I will run some more testing rounds with that to see if it reduces the need to increase the rx queue size (which would regardless still be nicer to have as a tunable). I will comment on that later (might take a day or two).

(I have managed to create Claude a skill that tells it how to flash and do repeated pair + unpair cycles with chip-tool over BLE to Home Assistant Thread + Matter controller while observing the serial logs and Matter controller logs. It is now actually quite pleasant to do long-lasting repeated A/B testing runs and collect statistics. Takes much of the effort and pain away. Suits a lazy person such as myself. :)

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.

2 participants