feat(firmware): count the edge pipeline's frames, so a dead one is visible - #1838
Draft
clonea1 wants to merge 5 commits into
Draft
feat(firmware): count the edge pipeline's frames, so a dead one is visible#1838clonea1 wants to merge 5 commits into
clonea1 wants to merge 5 commits into
Conversation
Wire v1 identifies neither which transmitter a frame came from nor which transmission it was. Both are needed before frames from different nodes can be related to each other. v2 appends the 802.11 addr2 (transmitter MAC) at bytes 20..25. A node in promiscuous MGMT+DATA mode with no filter produces CSI for every transmitter on the channel -- measured 2026-08-28 at ~75% non-AP in a normal home. Each frame is a valid measurement of a DIFFERENT link, and without the transmitter on the wire the sink interleaves them into one history with mixed geometry. v3 appends the 802.11 receive sequence number. v2 makes a frame attributable to a link; v3 makes it attributable to a transmission. The existing byte 12..15 carries a counter private to each node, so two nodes that captured the same packet off the air report unrelated numbers and the sink cannot tell it was one event observed twice. Arrival timestamps do not substitute -- the sink sees network jitter, not the moment of capture. This answers the blocker stated in ADR-138 (LinkGroup / ArrayCoordinator), which is Accepted-partial with integration glue pending because the mesh_aligned_us plumbing exists today only in the sensing server, not in a shared FrameMeta. Per-frame identity on the wire is that missing piece. Sequencing note: a receiver must accept v2 and v3 before a sender emits them, so the server-side parser should land first. Stacked on the thermal change: the ESP-NOW sync packet carries die temperature, thermal state and TX dBm in bytes 29..31, so this file depends on thermal.h.
The v1 ESP-NOW sync packet carries timing only, so a node's identity on the mesh is inferred rather than stated, and its condition is not reported at all. Two consequences: the server guesses which node a sync came from, and a node that is overheating, has rebooted, or is running low on heap looks identical to a healthy one until it stops responding entirely. v2 extends the packet to 38 bytes, adding the node's own MAC and a NodeHealth block: die temperature, thermal state, transmit power, minimum heap seen, and the reset reason. The reset reason is the useful part operationally. reset_reason_name() renders it, and rebooted_badly() distinguishes a panic or watchdog reset from a deliberate software restart -- a node that reboots quietly and rejoins looks healthy from the outside, and the distinction is what separates "somebody updated it" from "it is crashing". Version-gated so a v1 node keeps parsing exactly as before, and a padded v1 packet explicitly yields no MAC rather than reading whatever happened to sit in those bytes. Both cases are covered by tests. Co-Authored-By: claude-flow <ruv@ruv.net>
…testable
A node that cannot transmit says so only to a serial console it does not have.
`s_send_fail` -- the count of datagram sends that failed because lwIP could not
allocate a pbuf, which is the ENOMEM signature -- was reachable only as an
ESP_LOGW line, and only for the FIRST FIVE failures. On a board mounted to a
wall that is the same as not reporting it.
The consequence is not that the TX path is unmonitored; it is that any question
about transmit buffering is UNTESTABLE in a deployed fleet. An upstream change
raising CONFIG_ESP_WIFI_DYNAMIC_TX_BUFFER_NUM cannot be validated or refuted by
anyone who cannot see the counter it is meant to move.
Sync packet proto v3 appends three u32 counters, monotonic since boot:
[38..41] send_fail sends that failed to allocate a buffer
[42..45] rate_skip frames suppressed by the 20 ms send cap (50 Hz ceiling)
[46..49] early_drop callbacks discarded by the early rate gate
The two skip counters are here because send_fail alone cannot distinguish a
healthy radio from one whose rate limiters are working so hard that nothing
ever reaches the failing path. A zero send_fail beside a large rate_skip means
the cap is holding the line, not that there is headroom -- and that is exactly
the state this fleet is in.
Guarded on BOTH version and length when parsed. A padded v2 datagram is the
hazard: long enough to reach byte 49, containing nothing meaningful there.
Fabricated counters are worse than absent ones, because a non-zero send_fail
would be read as evidence of the very fault under investigation.
Backward compatible in both directions. An older sink ignores the tail; a v1 or
v2 node reports no counters and is read as such rather than as zeros.
Co-Authored-By: claude-flow <ruv@ruv.net>
(cherry picked from commit 9a20c27)
Two problems, both of which broke the fuzz job. The commit pulled in thermal.h, esp_system.h and esp_heap_caps.h along with node-health fields (minimum heap, reset reason, thermal state, transmit ceiling) written into the sync packet. None of that is TX-path counters. The fuzz target compiles the real csi_collector.c against test/stubs/, which has none of those headers, so it could not build. Health belongs with the thermal work; removed here. Separately the stub's wifi_csi_info_t had no rx_seq, which the serializer now reads. A stub that lacks a field the production code uses does not just fail to build -- it means the harness is no longer compiling the same thing the firmware does, which is the whole point of linking the real file. Added, mirroring IDF's esp_wifi_types_native.h. Firmware builds clean for esp32c6 on ESP-IDF v5.4. Co-Authored-By: claude-flow <ruv@ruv.net>
…sible
The subcarrier-grid guard in process_frame() rejects any frame wider than
EDGE_MAX_SUBCARRIERS and records nothing. That silence hid a real bug for
weeks: the grid was sized 128 for pre-HE parts while an HE-capable C6 delivers
256-bin HE20 frames, so every frame took that return and vitals, presence and
fall detection all produced nothing -- on nodes reporting full frame rate, good
RSSI, healthy heap and a normal DSP banner. Nothing observable distinguished it
from working.
Two counters, monotonic since boot, in the existing NodeHealth carrier:
frames_processed passed the guard and were processed
frames_rejected discarded by the guard (empty, or wider than the grid)
Separately each is ambiguous; together they separate the two silences:
processed == 0 && rejected == 0 nothing reaches the DSP task at all
processed == 0 && rejected > 0 frames arrive and every one is discarded,
i.e. the grid does not match the radio
The second row is the bug, and it now names itself from the server with no
cable -- which matters because these boards are on walls with no console, and
the counter that would have caught this was reachable only over serial.
Wire: sync packet 50 -> 58 bytes, proto v3 -> v4, edge counters at [50..57].
The parser was already versioned for this, so both directions stay compatible:
an old server requires only len >= its own size and ignores the tail, and a new
server admits the field only when proto_ver >= 4 AND len >= 58. Tests cover the
padded-v3 and truncated-v4 cases specifically, because either guard alone would
report a confidently wrong zero for a node whose pipeline was never measured --
worse than absent, exactly as with the MAC and TX counters before it.
/api/v1/mesh needs no change: NodeSyncSnapshot already derives Serialize over
NodeHealth, so `edge` appears beside `tx` automatically.
Verified: firmware builds for esp32c6 in espressif/idf:v5.4; 28 sync_packet
tests pass including five new ones; sensing-server checks clean. Not yet
observed on hardware -- the counters are the instrument for that, and the fleet
is mid-experiment.
Co-Authored-By: claude-flow <ruv@ruv.net>
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
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
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.
The subcarrier-grid guard in process_frame() rejects any frame wider than
EDGE_MAX_SUBCARRIERS and records nothing. That silence hid a real bug for
weeks: the grid was sized 128 for pre-HE parts while an HE-capable C6 delivers
256-bin HE20 frames, so every frame took that return and vitals, presence and
fall detection all produced nothing -- on nodes reporting full frame rate, good
RSSI, healthy heap and a normal DSP banner. Nothing observable distinguished it
from working.
Two counters, monotonic since boot, in the existing NodeHealth carrier:
frames_processed passed the guard and were processed
frames_rejected discarded by the guard (empty, or wider than the grid)
Separately each is ambiguous; together they separate the two silences:
processed == 0 && rejected == 0 nothing reaches the DSP task at all
processed == 0 && rejected > 0 frames arrive and every one is discarded,
i.e. the grid does not match the radio
The second row is the bug, and it now names itself from the server with no
cable -- which matters because these boards are on walls with no console, and
the counter that would have caught this was reachable only over serial.
Wire: sync packet 50 -> 58 bytes, proto v3 -> v4, edge counters at [50..57].
The parser was already versioned for this, so both directions stay compatible:
an old server requires only len >= its own size and ignores the tail, and a new
server admits the field only when proto_ver >= 4 AND len >= 58. Tests cover the
padded-v3 and truncated-v4 cases specifically, because either guard alone would
report a confidently wrong zero for a node whose pipeline was never measured --
worse than absent, exactly as with the MAC and TX counters before it.
/api/v1/mesh needs no change: NodeSyncSnapshot already derives Serialize over
NodeHealth, so
edgeappears besidetxautomatically.Verified: firmware builds for esp32c6 in espressif/idf:v5.4; 28 sync_packet
tests pass including five new ones; sensing-server checks clean. Not yet
observed on hardware -- the counters are the instrument for that, and the fleet
is mid-experiment.
Stacked on #1818
Four commits; only the last is new. The sync packet has to reach 50 bytes
before these counters can be appended at 50..57, so this depends on the wire
and TX-counter work in #1818 (which itself carries #1805 and #1807). Merge
those first and this reduces to one commit.
Firmware builds clean for esp32c6 on ESP-IDF v5.4; the 28 sync-packet parser
tests pass, including the padded-older-version and truncated-newer-version
guards -- either alone would report a confidently wrong zero for a node whose
pipeline was never measured, which is worse than reporting nothing.