From 04b0076b8b63c94b76137deccced9a458e96c97c Mon Sep 17 00:00:00 2001 From: Ashin Walpola Date: Tue, 25 Aug 2026 15:13:23 +0200 Subject: [PATCH] Move the RX capture buffer off the WiFi driver's callback stack rx_item_t is ~800 bytes at RX_FRAME_MAX_LEN, and wifi_promisc_rx_cb declared one as a local. That callback runs on the WiFi driver's own task, already several frames deep in the driver's call chain, on a stack of roughly 3.5 KB (CONFIG_ESP_WIFI_TASK_STACK_SIZE, left at its default). Putting a fifth of that stack into a single local is a stack-overflow risk that only appears under real traffic - in front of an RSU rather than on the bench - and would present as a random panic rather than anything pointing at its cause. Both instances are now static: one in the callback, one in rx_forward_task. Safe because each is touched by exactly one task, so there is no re-entrancy to guard against; the same reasoning serial_link.c already uses for its static send buffers. xQueueSend copies the struct out before returning, so reusing the callback's buffer on the next frame is fine. Firmware-only, no protocol change, so it does not require a matching app install. Re-verified against live traffic after flashing: 1094 frames over 125 s with zero decode failures, USB errors, detaches, crashes or mutex timeouts. SPATEM capture rose from 3.20/s to 3.98/s against a theoretical maximum of 4.00/s, which is the direction relieving stack pressure would produce, though RF geometry moves between runs and this is not proof. Report updated with T9, the accepted 512-byte ceiling, and the decision to drop Phase B: the intersection use case is CAM-driven and needs none of it. --- 05-obu-bench-test-2026-08-25.md | 58 +++++++++++++++++++++++++++++---- obu-firmware/main/main.c | 26 +++++++++++---- 2 files changed, 71 insertions(+), 13 deletions(-) diff --git a/05-obu-bench-test-2026-08-25.md b/05-obu-bench-test-2026-08-25.md index b332b57..0ca9cfe 100644 --- a/05-obu-bench-test-2026-08-25.md +++ b/05-obu-bench-test-2026-08-25.md @@ -190,6 +190,37 @@ Nothing here is a firmware issue, and pseudonym rotation is the intended privacy transmitters. But any future logic that assumes a station ID identifies a physical unit over time will be wrong. +## T9 — Stack fix and re-verification + +`rx_item_t` was moved off both task stacks (`static` in the promiscuous callback and in +`rx_forward_task`), firmware reflashed, and the campaign re-run: + +| | before fix (305 s) | after fix (125 s) | +|---|---|---| +| Total | 9.40/s | 8.73/s | +| CAM | 4.08/s | 4.11/s | +| SPATEM | 3.20/s | **3.98/s** | +| DENM | 2.12/s | 0.64/s | +| Decode failures / IO errors / crashes | 0 | 0 | +| Mutex timeouts | — | 0 | + +SPATEM capture rose from ~80% to ~100% of the theoretical 4.00/s (2 Hz × two antennas). Not +attributable to the fix with confidence — RF geometry moves between runs — but it is the direction +stack pressure relief would produce, and worth re-checking on the next run. The DENM drop is the +CiT One's trigger being intermittent, not a receive problem. + +### Finding: no automatic reconnect after re-enumeration + +Reflashing resets the C5, which re-enumerates its USB device. The app did **not** recover: it went +to `Connection error - check the cable and native USB-C port, then try again` and stayed there until +Connect was tapped manually, followed by a fresh USB permission grant. + +This matters more for the intersection use case than SPATEM does. On a bike, a jostled cable that +re-enumerates leaves the link dead until the rider notices and taps a button — a silent loss of the +CAM stream the use case runs on. The permission grant is a genuine one-time consent and cannot be +automated, but retrying automatically when a matching device is already attached would cover the +common case. + ## Readiness ### Working @@ -200,15 +231,31 @@ will be wrong. - RSSI plausible and discriminating between transmitters (−48 to −65 dBm at bench distance) - No frame exceeded the serial payload cap under this traffic mix -### Blocking for real-world use +### Scope decision (2026-08-25): Phase B dropped + +Raising the payload cap was considered and **deliberately rejected**. The project goal is V2X +communication with at least one white-paper use case — incoming car at an intersection — working on +the ESP32. That use case is `IMA-B`/`IMA-S`, which `UseCaseDetectionEngine` drives entirely from CAM +kinematics; the engine contains **zero references to SPATEM or MAPEM**. Everything the goal needs +fits the current cap with margin: CAM 26–211 B, DENM 402 B, bench SPATEM 58 B, against a 498 B +budget. + +Phase B would buy only road-RSU SPATEM/MAPEM — the add-on, not the goal — while putting a measured, +zero-failure chain at risk. The one component of it that *reduces* risk, moving `rx_item_t` off the +WiFi callback stack, was done separately (T9). + +### Known ceiling, accepted 1. **Serial payload cap (512 B).** The bench RSU sends 58-byte SPATEMs, but the 2026-03-18 drive measured real road RSUs at 555 B median and 1243 B max — **roughly 70% would be dropped as oversize**. Raising `SERIAL_LINK_MAX_PAYLOAD` and `RX_FRAME_MAX_LEN` to ~1536 is required, and forces item 2. -2. **`rx_item_t` on the WiFi driver's callback stack** (`main.c`). At the current 800 B it is a - latent risk on a ~3.5 KB stack; at 1536 B it is a guaranteed overflow. Must be moved off the - stack as part of the same change. +2. ~~**`rx_item_t` on the WiFi driver's callback stack**~~ — **fixed 2026-08-25**, see T9. + +3. **DENM headroom is 96 B.** DENM matters to this project in a way SPATEM does not, and at 402 B + it is the closest message to the cap. A DENM carrying more optional containers than the CiT One's + HLN-SV currently sends would be silently dropped and counted as oversize. The `oversize` counter + on the CAM Pinger card is the thing to check if hazards ever stop appearing. ### Defect found and fixed during this test @@ -222,8 +269,7 @@ will be wrong. ### Untested here -- **Link recovery** — unplug/replug and USB permission re-grant were not exercised; needs physical - intervention. +- ~~**Link recovery**~~ — exercised by the reflash in T9: it does **not** auto-recover. See T9. - **Sustained load at road rates.** This bench ran at 9.4 frames/s. The drive data implies 24–32 frames/s with frames 3× larger, where the TX-mutex interaction (400 ms worst-case hold vs the 1 Hz heartbeat and the phone's 3-beat dead-link timeout) becomes the thing to watch. diff --git a/obu-firmware/main/main.c b/obu-firmware/main/main.c index 46a0f08..6b3d910 100644 --- a/obu-firmware/main/main.c +++ b/obu-firmware/main/main.c @@ -188,19 +188,31 @@ static void wifi_promisc_rx_cb(void *recv_buf, wifi_promiscuous_pkt_type_t type) return; } - rx_item_t item; - item.len = length > (int)sizeof(item.data) ? (int)sizeof(item.data) : length; - memcpy(item.data, packet->payload, (size_t)item.len); - item.rssi = packet->rx_ctrl.rssi; + // static, NOT a local: at RX_FRAME_MAX_LEN this struct is ~800 bytes, and this callback runs + // on the WiFi driver's own task - already several frames deep in the driver's call chain, on a + // stack of roughly 3.5 KB (CONFIG_ESP_WIFI_TASK_STACK_SIZE, left at its default). Putting + // ~23% of that stack in one local is a stack-overflow risk that only bites under real traffic, + // i.e. in front of an RSU rather than on the bench. + // + // Safe as a static because the promiscuous callback is only ever invoked from that one task, + // so there is no re-entrancy to guard against - the same reasoning serial_link.c uses for its + // static send buffers. rx_forward_task has its own separate copy below. + static rx_item_t s_cb_item; + s_cb_item.len = length > (int)sizeof(s_cb_item.data) ? (int)sizeof(s_cb_item.data) : length; + memcpy(s_cb_item.data, packet->payload, (size_t)s_cb_item.len); + s_cb_item.rssi = packet->rx_ctrl.rssi; - // 0 timeout: never block the WiFi driver's own task waiting for queue space. - xQueueSend(s_rx_queue, &item, 0); + // 0 timeout: never block the WiFi driver's own task waiting for queue space. xQueueSend copies + // the struct out before returning, so reusing s_cb_item on the next callback is fine. + xQueueSend(s_rx_queue, &s_cb_item, 0); } static void rx_forward_task(void *arg) { (void)arg; - rx_item_t item; + // Same reasoning as the callback: ~800 bytes is a fifth of this task's 4 KB stack. Only this + // task touches it, and it is fully overwritten by xQueueReceive before every use. + static rx_item_t item; while (1) { if (xQueueReceive(s_rx_queue, &item, portMAX_DELAY) != pdTRUE) {