Skip to content

feat(firmware): report the TX-path counters, so buffer questions are testable - #1818

Draft
clonea1 wants to merge 5 commits into
ruvnet:mainfrom
clonea1:contrib/tx-path-counters
Draft

feat(firmware): report the TX-path counters, so buffer questions are testable#1818
clonea1 wants to merge 5 commits into
ruvnet:mainfrom
clonea1:contrib/tx-path-counters

Conversation

@clonea1

@clonea1 clonea1 commented Sep 4, 2026

Copy link
Copy Markdown

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.


Rebased onto current main before opening: staged before today's seven merges, so it needed replaying to avoid reading as a revert of them. Clean rebase, no files deleted.

Joe and others added 3 commits September 4, 2026 15:02
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)
@clonea1

clonea1 commented Sep 4, 2026

Copy link
Copy Markdown
Author

Converting this to a draft — it is over-scoped and blocked, and I would rather say so than leave it sitting in the queue failing CI.

Why the fuzz job fails. The commit adds thermal.h, esp_system.h and esp_heap_caps.h to csi_collector.c along with node-health fields (heap, reset reason, thermal state). The fuzz target compiles the real csi_collector.c against test/stubs/, which has none of those headers, so it fails to build. Same root cause as #1805.

It also carries work that is not TX counters. As staged this branch bundles four separate things:

  1. the TX-path counters (send_fail, rate_skip, early_drop) — the actual subject
  2. the CSI frame wire v2/v3 change, which feat(wire): carry transmitter and transmission identity in the CSI frame #1805 already covers
  3. node-health fields in the sync packet
  4. the ADR-345 seqdiag probe

And it is blocked regardless. The counters live at sync-packet bytes [38..49], but this repository's sync packet is 32 bytes at proto v1. They cannot be carried until the packet is extended, which is #1807 (hardware-sync-packet). Scoping this branch today would produce a correct PR that still could not merge.

Plan: land #1807 first, then re-cut this against it carrying only the counters and their parser. I will un-draft it then.

My fault for opening it in this state — I screened the batch on whether each branch rebased cleanly and deleted nothing, which does not catch a commit that was scoped loosely when it was staged.

@clonea1
clonea1 marked this pull request as draft September 4, 2026 19:25
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>
The self-MAC read this branch adds to wifi_csi_callback() has no
declaration in test/stubs/, and the fuzz harness compiles the REAL
csi_collector.c against those stubs. Clang treats the implicit
declaration as an error, so `make all` exits 2 and the ADR-061 Layer 6
job fails before a single fuzz case runs -- the job was reporting a
build break, not a crash.

The stub returns a fixed non-zero MAC rather than ESP_FAIL so the
serializer takes the same branch it takes on a device; the all-zero
"not reported" path is the failure branch and is not what the fuzzer
is here to exercise.

Verified locally: all three fuzz targets and the four host tests
compile, link and run clean under ASan/UBSan.

Co-Authored-By: claude-flow <ruv@ruv.net>
Claude-Session: https://claude.ai/code/session_0191PwxLFAChNxRr5sGAVTxH
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.

1 participant