stream timing: five more per-frame bytes in addr1, a group DA every receiver delivers

[josephnef]

What

Follow-up #474 from the stream-timing PR. The per-frame field in addr3 had no room for a per-frame write time or queue time; both were windowed in the marker only.

• addr1 (the DA) now carries a FrameTimingExt on the stream's data frames: stdin read→send_packet for this frame, the previous frame's send_packet wall time (kestrel-air's T_WRITE), and the TX frame counter's low byte. The DA stays a group address (03:… = group + locally administered + version), so nothing ACKs it and every TX descriptor path still derives BMC from it. A broadcast DA decodes as "no extension". The marker, the hop sync marker and the Jaguar1 clock beacon keep ff.
• rxdemo adds tq_us, tw_us, ctr to rx.frame; the analyzer cross-checks the per-frame tw median against the marker's window p50 of the same quantity.

Measured first

tests/mcast_da_rx_check.sh (new, with tests/mcast_da_tx.cpp): an injector alternates the broadcast and the 03:… DA; each receiver family counts both by body tag, 15 s per cell.

receiver	broadcast	group DA	ratio
8822BU (Jaguar2)	1404	1399	1.00
8812CU (Jaguar3)	1374	1381	1.01
8821AU (Jaguar1)	1330	1317	0.99
8832CU (Kestrel)	1419	1423	1.00
MT7612U	1422	1428	1.00

The RTL8733B was not plugged and is unmeasured (said so in the doc).

With the extension live

Full matrix on the 8812CU → CF-924AC pair: floor, delay (20.08 ms on 9.8 %), hop, corrupt, svctx, svctx-live, duplex — all PASS. Floors on the 8821AU and 8832CU PASS. Per-frame tw median vs marker p50: 70/70 µs (8812CU), 20/20 µs (8821AU). Headless: selftest KAT for the extension (broadcast, unicast-looking and other-version DAs rejected), ctest 85/85.

Closes #474.

🤖 Generated with Claude Code

[qodo-free-for-open-source-projects]

PR Summary by Qodo

Add per-frame stream timing telemetry in the group destination address

✨ Enhancement 🧪 Tests 📝 Documentation 🕐 20-40 Minutes

AI Description

• Add per-frame queue time, previous write time, and frame counter without changing stream payloads.
• Decode the group-address extension in receiver events and compare its timing against periodic
 markers.
• Validate group-address reception across five receiver families and document the wire format.

Diagram

graph TD
  TX["TX demos"] --> ST["Timing stamper"] --> DA["Group DA"] --> RF["Radio link"] --> RX["RX events"] --> AN["Timing analyzer"]
  ST --> MK["Window marker"] --> RF

Loading

High-Level Assessment

The following are alternative approaches to this PR:

1. Keep timing only in periodic markers

• ➕ Preserves broadcast destination addresses and requires no per-frame wire-format change.
• ➖ Cannot attribute queue or write timing to individual frames.

2. Put timing in the stream payload

• ➕ Avoids relying on delivery of non-broadcast group addresses.
• ➖ Changes payload layout and potentially the MTU or FEC handling.

Recommendation: Use the group-DA extension: it supplies per-frame measurements without changing payloads, while the reception test addresses its principal compatibility risk. The RTL8733B remains unmeasured and should be validated before claiming support for it.

Files changed (13) +314 / -9

Enhancement (6) +67 / -7

stream_timing_tx.hStamp per-frame timing into addr1 +14/-3

Stamp per-frame timing into addr1

• Extends the shared timing stamper to optionally encode queue time, previous-frame write time, and the low frame-counter byte into addr1. Retains the previous send_packet duration for the next data frame.

examples/common/stream_timing_tx.h

main.cppEnable addr1 timing in duplex transmission +2/-1

Enable addr1 timing in duplex transmission

• Passes the data frame’s destination-address pointer to the shared timing stamper.

examples/duplex/main.cpp

main.cppDecode the group-DA extension in rx.frame +7/-0

Decode the group-DA extension in rx.frame

• Adds tq_us, tw_us, and ctr to receiver events when a telemetry data frame has a valid addr1 extension.

examples/rx/main.cpp

main.cppEnable addr1 timing in stream transmission +2/-1

Enable addr1 timing in stream transmission

• Passes the stream data frame’s destination-address pointer to the shared timing stamper.

examples/streamtx/main.cpp

main.cppEnable addr1 timing in service transmission +2/-1

Enable addr1 timing in service transmission

• Passes each service data frame’s destination-address pointer to the shared timing stamper.

examples/svctx/main.cpp

StreamTelemetry.hDefine the versioned FrameTimingExt codec +40/-1

Define the versioned FrameTimingExt codec

• Adds the six-byte addr1 codec with two 10-µs timing fields and a frame-counter byte. Decoding rejects broadcast, unicast-looking, and unsupported-version destination addresses.

src/StreamTelemetry.h

Tests (4) +212 / -0

mcast_da_rx_check.shCheck group-DA delivery across receiver families +84/-0

Check group-DA delivery across receiver families

• Runs the injector and receiver for each available hardware cell, counts frames by payload tag, and requires group-DA reception to reach at least 90% of broadcast reception.

tests/mcast_da_rx_check.sh

mcast_da_tx.cppInject alternating broadcast and group-DA frames +89/-0

Inject alternating broadcast and group-DA frames

• Adds a hardware test transmitter that alternates destination addresses on otherwise comparable tagged frames and reports successful sends.

tests/mcast_da_tx.cpp

stream_telemetry_selftest.cppTest extension encoding and address rejection +20/-0

Test extension encoding and address rejection

• Adds a known-answer round trip and verifies rejection of broadcast, unicast-looking, and unsupported-version addresses.

tests/stream_telemetry_selftest.cpp

stream_timing_analyze.pySummarize and cross-check per-frame timing +19/-0

Summarize and cross-check per-frame timing

• Reports extension coverage and per-frame queue and write-time percentiles. When markers and per-frame write times are present, checks coverage and compares the write-time median with marker statistics.

tests/stream_timing_analyze.py

Documentation (3) +35 / -2

logging.mdDocument new receiver timing fields +1/-1

Document new receiver timing fields

• Adds the addr1 extension’s tq_us, tw_us, and ctr fields to the rx.frame event reference.

docs/logging.md

stream-timing.mdSpecify the group-DA timing extension +30/-0

Specify the group-DA timing extension

• Documents the addr1 byte layout, its group-address requirement, and which frames retain a broadcast DA. Records reception measurements for five receiver families and identifies the unmeasured RTL8733B.

docs/stream-timing.md

README.mdDescribe the group-DA reception gate +4/-1

Describe the group-DA reception gate

• Adds the hardware reception check to the stream-timing test guidance.

tests/README.md

[qodo-free-for-open-source-projects]

Code Review by Qodo

🐞 Bugs (0) 📘 Rule violations (0) 📎 Requirement gaps (0) 🎨 UX issues (0) 🔗 Cross-repo conflicts (0) 📜 Skill insights (0)

1. The timing header duplicates the guide ✓ Resolved

Description

The new StreamTelemetry.h header comment repeats the group-address explanation already added to
docs/stream-timing.md instead of referring to the guide. The copies have already diverged on the
measured receivers: the guide includes MT7612U, but the header lists only four families, leaving a
later reader with conflicting accounts.

Code

src/StreamTelemetry.h[R7-10]

+ *     stream frame, and FrameTimingExt — five more in addr1 (the DA), which
+ *     stays a group address (bit 0 set) so nothing ACKs it and every TX
+ *     descriptor path still derives BMC from it; its first byte carries the
+ *     group + locally-administered bits and the version, so the broadcast

Evidence

Rule 3 prohibits duplicating header documentation. The added header comment repeats the guide's
group-address explanation, and their measurement lists differ.

CLAUDE.md: Keep Cross-Cutting Documentation Nonduplicative and Balanced: CLAUDE.md: Keep Cross-Cutting Documentation Nonduplicative and Balanced: CLAUDE.md: Keep Cross-Cutting Documentation Nonduplicative and Balanced: CLAUDE.md: Keep Cross-Cutting Documentation Nonduplicative and Balanced
src/StreamTelemetry.h[7-14]
docs/stream-timing.md[28-52]

Agent prompt

The issue below was found during a code review. Follow the provided context and guidance below and implement a solution

## Issue description
The new header narrative duplicates the stream timing guide and already omits one receiver listed there.
## Fix Focus Areas
- src/StreamTelemetry.h[7-14]
- docs/stream-timing.md[28-52]
## Recommended Fix
Keep the detailed explanation and measurements in the guide; replace the duplicated header narrative with a concise reference while retaining comments needed to understand the codec.

ⓘ Copy this prompt and use it to remediate the issue with your preferred AI generation tools

---

2. The on-air test skips USB teardown ✓ Resolved

Description

mcast_da_tx calls dev->Stop() but returns without destroying the radio before releasing the
claimed interface, closing the USB handle, and exiting the libusb context. The same cleanup is
absent when claiming, radio creation, or initialization fails, so those paths also leave
caller-owned USB resources unreleased until process exit.

Code

tests/mcast_da_tx.cpp[R86-88]

+  dev->Stop();
+  printf("{\"ev\":\"mcast.tx\",\"sent\":%u,\"ok_ff\":%u,\"ok_03\":%u}\n", n, ok_ff, ok_03);
+  return 0;

Evidence

Rule 7 requires ordered radio destruction and USB teardown. The new test opens and claims a handle,
but its success path stops the radio and returns without releasing the interface, closing the
handle, or exiting the context; its early returns have no such cleanup either.

CLAUDE.md: Preserve Device and Transport Lifecycle Ordering: CLAUDE.md: Preserve Device and Transport Lifecycle Ordering: CLAUDE.md: Preserve Device and Transport Lifecycle Ordering: CLAUDE.md: Preserve Device and Transport Lifecycle Ordering
tests/mcast_da_tx.cpp[57-72]
tests/mcast_da_tx.cpp[86-88]

Agent prompt

The issue below was found during a code review. Follow the provided context and guidance below and implement a solution

## Issue description
The on-air transmitter opens and claims a USB device but has no ordered cleanup on success or failure.
## Fix Focus Areas
- tests/mcast_da_tx.cpp[57-72]
- tests/mcast_da_tx.cpp[86-88]
## Recommended Fix
Use a cleanup path or scoped guards that stop and destroy the radio while the context exists, then release the claimed interface, close the handle, and exit the libusb context on every return path.

ⓘ Copy this prompt and use it to remediate the issue with your preferred AI generation tools

---

3. Valid captures can fail after warmup ✓ Resolved

Description

ext_tw_matches_marker compares the median of post-warmup frame values with the median of every
marker's window median. When write times change between startup and steady state, markers covering
discarded warmup frames can make that comparison fail even though the retained frames and their
corresponding markers agree.

Code

tests/stream_timing_analyze.py[R118-121]

+        mk = pct([m["tw_p50_us"] for m in markers], 0.5)
+        fr = pct(tw, 0.5)
+        # Same quantity from two carriers: agree within 20 us or 50%.
+        c = abs(fr - mk) <= max(20, 0.5 * max(fr, mk))

Evidence

Warmup filtering replaces tel with a reduced list, while markers remains unchanged. The new
comparison takes its frame median from the reduced list and its marker median from the complete
marker list.

tests/stream_timing_analyze.py[60-78]
tests/stream_timing_analyze.py[117-123]

Agent prompt

The issue below was found during a code review. Follow the provided context and guidance below and implement a solution

## Issue description
The new write-time check compares post-warmup frames against marker windows that include warmup.
## Fix Focus Areas
- tests/stream_timing_analyze.py[63-78]
- tests/stream_timing_analyze.py[117-123]
## Recommended Fix
Restrict the marker comparison to windows corresponding to retained frames, or compare matching frame and marker windows directly.

ⓘ Copy this prompt and use it to remediate the issue with your preferred AI generation tools

---

4. The receiver gate passes without a test ✓ Resolved

Description

mcast_da_rx_check.sh increments fails only for a cell that reaches the log parser, while
unavailable receivers, transmitters, and required firmware take continue paths. If every
configured cell is skipped, the script reports zero non-passing cells and exits successfully despite
measuring no receiver.

Code

tests/mcast_da_rx_check.sh[R82-84]

+done
+echo "== done: $fails non-PASS cell(s); logs in $OUT =="
+exit "$fails"

Evidence

Each availability check can skip its cell before parsing, and none changes fails. The final exit
status is solely fails, initialized to zero.

tests/mcast_da_rx_check.sh[40-52]
tests/mcast_da_rx_check.sh[61-84]

Agent prompt

The issue below was found during a code review. Follow the provided context and guidance below and implement a solution

## Issue description
The multicast-address gate returns success when every receiver cell is skipped.
## Fix Focus Areas
- tests/mcast_da_rx_check.sh[40-61]
- tests/mcast_da_rx_check.sh[82-84]
## Recommended Fix
Count cells that actually reach measurement and return a distinct skip or failure status when that count is zero; keep reporting unavailable cells as skipped.

ⓘ Copy this prompt and use it to remediate the issue with your preferred AI generation tools

---

5. Receiver gate tested a different DA than ships ✓ Resolved

Description

FrameTimingExt::encode writes 0x03 | (kExtVersion << 2) = 0x07 as addr1 byte 0, but the gate
(tests/mcast_da_tx.cpp kGroupDa) and every doc and comment (stream-timing.md, logging.md, the
StreamTelemetry.h header) describe and measure a 03:… DA. So the parity table in the PR was
measured on a version-0 address the code never sends, and anyone who greps captures for a 03 DA,
or reruns the gate expecting it to cover the shipped frames, gets the wrong answer.

Code

src/StreamTelemetry.h[R138-139]

+  void encode(uint8_t out[kSize]) const {
+    out[0] = static_cast<uint8_t>(0x03 | (kExtVersion << 2));

Evidence

The selftest KAT confirms that byte 0 is 0x07 (want[6] = {0x07, ...}). The injector sends
kGroupDa = {0x03, ...}, and the docs say the DA is 03:….

src/StreamTelemetry.h[128-139]
tests/stream_telemetry_selftest.cpp[54-54]
tests/mcast_da_tx.cpp[29-29]
docs/logging.md[91-91]
docs/stream-timing.md[28-33]

Agent prompt

The issue below was found during a code review. Follow the provided context and guidance below and implement a solution

## Issue description
The shipped addr1 extension starts with 0x07 (group + LA + version 1). The RX gate injects 03:57:42:75:05:d6, and the docs say `03:…`.
## Fix Focus Areas
- tests/mcast_da_tx.cpp[29-29]
- docs/stream-timing.md[28-33]
- docs/logging.md[91-91]
- src/StreamTelemetry.h[7-14]
- examples/common/stream_timing_tx.h[100-101]
## Recommended Fix
Set kGroupDa[0] in the injector to 0x07, or build it with FrameTimingExt::encode, so the gate measures the address that actually ships. Update the docs and comments to say `07:…`, or say "first byte 0x03 | version<<2".

ⓘ Copy this prompt and use it to remediate the issue with your preferred AI generation tools

---

View review recommended (1)

6. Analyzer cannot notice the extension vanishing ✓ Resolved

Description

stream_timing_analyze.py evaluates ext_present only inside if tw and markers, even though tw
is empty when no frame has a decoded addr1 extension. If a capture has enough ordinary telemetry
frames but no tw_us values, it skips both extension checks and can still pass; only partial
extension loss is caught.

Code

tests/stream_timing_analyze.py[R117-126]

+    if tw and markers:
+        mk = pct([m["tw_p50_us"] for m in markers], 0.5)
+        fr = pct(tw, 0.5)
+        # Same quantity from two carriers: agree within 20 us or 50%.
+        c = abs(fr - mk) <= max(20, 0.5 * max(fr, mk))
+        out["checks"]["ext_tw_matches_marker"] = c
+        ok &= c
+        c = len(tq) >= 0.9 * len(tel)
+        out["checks"]["ext_present"] = c
+        ok &= c

Evidence

tw is collected only from frames containing tw_us, which the receiver emits only after a
successful extension decode. With no decoded extensions, tw is empty, so the if tw and markers
block records neither ext_tw_matches_marker nor ext_present and neither check contributes to
ok; the minimum-data check still counts ordinary telemetry frames.

tests/stream_timing_analyze.py[77-78]
tests/stream_timing_analyze.py[117-126]
tests/stream_timing_analyze.py[63-78]
tests/stream_timing_analyze.py[108-126]
examples/rx/main.cpp[1398-1402]

Agent prompt

The issue below was found during a code review. Follow the provided context and guidance below and implement a solution

## Issue description
The analyzer skips extension coverage when no addr1 extension decodes, allowing a capture with enough ordinary telemetry frames but no extension data to pass.
## Fix Focus Areas
- tests/stream_timing_analyze.py[117-126]
## Recommended Fix
Evaluate `ext_present` whenever `tel` is non-empty, independently of `tw` and markers, using the existing threshold `len(tq) >= 0.9 * len(tel)` and recording its result in `ok`. Keep only the tw-versus-marker comparison conditional on `tw and markers`.

ⓘ Copy this prompt and use it to remediate the issue with your preferred AI generation tools

---

Tip of the day

💡 Did you know, you can copy the agent prompt from any finding and feed it to your IDE agent

More tips ↗ | Customize Qodo ↗ | Qodo docs ↗