Skip to content

stream timing: join the CCX TX report to its frame on HalMAC (queue time, retries) - #483

Merged
josephnef merged 2 commits into
masterfrom
ccx-join
Oct 10, 2026
Merged

josephnef merged 2 commits into
masterfrom
ccx-join

Conversation

@josephnef

Copy link
Copy Markdown
Collaborator

What

Follow-up #476 from the stream-timing PR: the chip's own queue time, which no host stage can measure, joined to the per-frame record.

  • Two small library seams. IRtlRadio::NextTxReportTag() — the SW_DEFINE tag the next send_packet will carry (Jaguar2/3 with tx.report on; nullopt elsewhere). IRadio::SetTxReportSink() — each decoded CCX report handed to the caller beside the tx.report event; all four emit sites (J1/J2/J3) now go through one DeliverTxReport.
  • The join lives in the TX timing helper: tag read before every send into a 256-slot ring (TxReportJoin, pure, selftested), reports queued by the sink on the C2H thread and joined on the TX thread. Queue time (raw firmware units — the HalMAC unit is undocumented, Jaguar1's 256 µs is) and retries enter the stream.timing window (rpt_n, rpt_fail, q_p50_raw, q_max_raw, retries_max, rpt_unmatched, rpt_overflow) and a sampled per-frame ledger stream.txrpt with the report's host age. Nothing on the air changes; the join is off unless DEVOURER_TX_REPORT is set.
  • Harness: a report phase (streamtx on Jaguar3, whose coex thread drains C2H) and TX_REPORT=1 on the duplex phase (how a Jaguar2 transmitter, which needs an RX loop, is covered).

Measured (report requested on every frame, 20 s)

8812CU streamtx (~460 fps) 8812BU/T3U duplex (~680 fps)
steady state, reports joined per data frame 1.02 (every frame + the markers) 1.02
unmatched reports, steady state 0 0
report age, send → host p50 2.2 ms —
queue time p50 / max (raw) 1 / 41 1 / 47

The startup burst is where reports go missing, and the doc says so with the numbers: the stdin backlog aired at full rate makes the report latency outrun the tag ring (unmatched) and the 8812CU firmware drops reports in gaps of 7–11 (tx.report tag deltas), consistent with the emission ceiling in docs/scheduled-mac.md. From the first paced window on, every frame has its report. The join is exact where a report exists.

Headless: ctest 85/85 with the join-ring selftest added. The default matrix (join off) is unchanged: floor on the 8812CU PASS with rpt_join:0.

Closes #476.

🤖 Generated with Claude Code

…ime, retries)

The one stage the host cannot time is the chip's own queue. With
DEVOURER_TX_REPORT on, the HalMAC firmware's CCX report echoes the
descriptor's SW_DEFINE tag; two small library seams make that joinable:
IRtlRadio::NextTxReportTag() (the tag the next send_packet will carry,
Jaguar2/3) and IRadio::SetTxReportSink() (each decoded report handed to the
caller beside the tx.report event; every generation's emit site goes through
one DeliverTxReport). The TX timing helper reads the tag before every send
into a 256-slot ring (src/StreamTelemetry.h TxReportJoin, selftested),
joins the reports on the TX thread, and folds the on-chip queue time (raw
firmware units) and retry count into stream.timing (rpt_n, rpt_fail,
q_p50_raw, q_max_raw, retries_max, rpt_unmatched) plus a sampled per-frame
ledger, stream.txrpt, that also carries the report's host age. Nothing on
the air changes.

Measured (report per frame, 20 s): steady state joins every frame on the
8812CU (streamtx, ~460 fps) and the 8812BU/T3U (duplex, ~680 fps) -- 1.02
reports per data frame (the markers), 0 unmatched, report age p50 2.2 ms,
queue time p50 1 raw, max 41-607. The startup burst (the stdin backlog aired
at full rate) is where reports go missing: latency outruns the tag ring and
the 8812CU firmware drops reports in gaps of 7-11; the harness's `report`
phase judges the second half of the run and says so.

Closes #476.

Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com>
@qodo-free-for-open-source-projects

Copy link
Copy Markdown

PR Summary by Qodo

Join HalMAC CCX TX reports to stream timing frames

✨ Enhancement 🧪 Tests 📝 Documentation 🕐 40+ Minutes

Grey Divider

AI Description

• Correlates HalMAC TX reports with sent frames to expose chip queue time, retries, and report age.
• Adds window metrics and sampled per-frame events without changing transmitted frames.
• Tests the tag join and adds hardware harness coverage for Jaguar2 and Jaguar3.
Diagram

graph TD
  TX["TX timing helper"] --> Radio["HalMAC radio"] --> C2H["C2H decoder"] --> Delivery["Report delivery"] --> Queue["Report queue"] --> Join["Tag join"] --> Events["Timing events"]
  TX -->|"frame record"| Join
  Radio -->|"next tag"| TX
Loading
High-Level Assessment

The following are alternative approaches to this PR:

1. Correlate reports inside each radio backend
  • ➕ Could provide joined frame feedback to callers without a demo-owned tag ring.
  • ➖ Duplicates correlation across backends and couples driver code to stream-specific frame timing.

Recommendation: Keep the shared TX-helper join and small radio interfaces: they preserve generic report delivery while keeping stream-specific timing outside the drivers. Review the documented 8-bit tag wrap limitation when interpreting burst-period matches.

Files changed (14) +318 / -14

Enhancement (9) +203 / -11
stream_timing_tx.hJoin reports and emit TX timing telemetry +105/-1

Join reports and emit TX timing telemetry

• Records tags for data and marker sends, queues sink callbacks, and joins reports on the TX thread. Adds queue-time, retry, failure, unmatched, and overflow metrics plus sampled per-frame events.

examples/common/stream_timing_tx.h

IRadio.hExpose a thread-safe TX report sink +32/-0

Expose a thread-safe TX report sink

• Adds a configurable report callback and shared delivery method that retains the existing tx.report event. Copies the callback under a mutex before invoking it.

src/IRadio.h

IRtlRadio.hDefine the next-report-tag interface +8/-0

Define the next-report-tag interface

• Adds an optional tag query for the next TX descriptor, defaulting to no tag on unsupported devices.

src/IRtlRadio.h

StreamTelemetry.hAdd the 256-slot TX report join +40/-0

Add the 256-slot TX report join

• Introduces frame records and a tag-keyed join that consumes matches and counts unmatched reports. The helper is independent of radio and event handling.

src/StreamTelemetry.h

RtlJaguarDevice.cppRoute Jaguar1 reports through shared delivery +1/-1

Route Jaguar1 reports through shared delivery

• Sends decoded Jaguar1 reports through the new delivery method, preserving event emission and enabling sink callbacks without claiming tag support.

src/jaguar1/RtlJaguarDevice.cpp

RtlJaguar2Device.cppDeliver decoded Jaguar2 reports to the sink +3/-3

Deliver decoded Jaguar2 reports to the sink

• Routes HalMAC reports from the Jaguar2 RX loop through shared event-and-callback delivery.

src/jaguar2/RtlJaguar2Device.cpp

RtlJaguar2Device.hExpose Jaguar2's next descriptor tag +4/-0

Expose Jaguar2's next descriptor tag

• Returns the next low-byte TX report tag when reporting is enabled; otherwise returns no tag.

src/jaguar2/RtlJaguar2Device.h

RtlJaguar3Device.cppDeliver Jaguar3 reports from both C2H paths +6/-6

Deliver Jaguar3 reports from both C2H paths

• Routes decoded reports through shared delivery in both the RX loop and coex-runtime C2H drain.

src/jaguar3/RtlJaguar3Device.cpp

RtlJaguar3Device.hExpose Jaguar3's next descriptor tag +4/-0

Expose Jaguar3's next descriptor tag

• Returns the next low-byte TX report tag when reporting is enabled; otherwise returns no tag.

src/jaguar3/RtlJaguar3Device.h

Tests (2) +69 / -1
stream_telemetry_selftest.cppTest TX report tag matching +16/-0

Test TX report tag matching

• Covers reports before sends, successful matches, duplicate reports, and tag reuse after wrap.

tests/stream_telemetry_selftest.cpp

stream_timing_onair.shAdd on-air CCX join verdicts +53/-1

Add on-air CCX join verdicts

• Adds a report phase and optional duplex reporting. Checks steady-state joined-report coverage, unmatched reports, and queue overflow while retaining stream analysis.

tests/stream_timing_onair.sh

Documentation (3) +46 / -2
logging.mdDocument CCX timing fields and frame ledger +2/-1

Document CCX timing fields and frame ledger

• Defines the report-join fields on stream.timing and the sampled stream.txrpt event, including raw queue time and report age.

docs/logging.md

stream-timing.mdExplain CCX correlation and measured limitations +40/-0

Explain CCX correlation and measured limitations

• Describes tag-based joining, supported devices, C2H requirements, and measured steady-state coverage. Documents startup losses from ring wrap and firmware report gaps.

docs/stream-timing.md

README.mdDocument report-phase hardware coverage +4/-1

Document report-phase hardware coverage

• Explains the streamtx report phase and the duplex TX_REPORT option needed to exercise Jaguar2 with an RX loop.

tests/README.md

@qodo-free-for-open-source-projects

qodo-free-for-open-source-projects Bot commented Oct 10, 2026 •

Copy link
Copy Markdown

Code Review by Qodo

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

Grey Divider


Action required

1. Delayed reports can name the wrong frame 📎 Requirement gap ≡ Correctness
Description
TxReportJoin::sent() replaces a live slot when its eight-bit tag wraps, while match() checks
only whether that tag is live and cannot distinguish the old report from the replacement frame’s
report. If the old report arrives after 256 further sends but before the new report, its queue time,
retries, delivery state, and understated age_us are recorded against the newer frame in
stream.timing/stream.txrpt and count toward rpt_n; the newer frame’s report then becomes
unmatched.
Code

src/StreamTelemetry.h[R380-383]

+  void sent(uint8_t tag, const TxFrameRec &rec) {
+    _slot[tag] = rec;
+    _live[tag] = true;
+    ++_sent;
Evidence
sent() marks the tag live on every write, including when an unresolved record already occupies the
slot, and match() relies on that live flag without identifying the report’s send generation. The
descriptor carries only an eight-bit tag, so it provides no distinction between reports across
reuse. The self-test explicitly expects a wrapped tag to return the latest frame, whereas the struct
comment and docs/stream-timing.md describe an overwritten slot as unmatched.

Join CCX TX reports to per-frame timing records on Jaguar2/3
src/StreamTelemetry.h[380-390]
tests/stream_telemetry_selftest.cpp[175-177]
src/jaguar2/RtlJaguar2Device.cpp[1787-1794]
src/StreamTelemetry.h[365-391]
src/jaguar3/RtlJaguar3Device.cpp[2768-2771]
docs/stream-timing.md[219-224]
src/StreamTelemetry.h[365-390]
docs/stream-timing.md[219-225]

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

## Issue description
A delayed report can arrive after its eight-bit tag has been reused and be counted as joined to the newer frame. The overwritten record is not identified as lost, and the existing comment and documentation describe a different outcome.
## Fix Focus Areas
- src/StreamTelemetry.h[365-400]
- examples/common/stream_timing_tx.h[193-214]
- tests/stream_telemetry_selftest.cpp[165-179]
- docs/stream-timing.md[219-224]
## Recommended Fix
Track reuse of a tag whose slot is still live, making the overwrite visible in an overwritten counter, and avoid claiming an exact join when the report’s generation cannot be identified; count ambiguous reports as unmatched or in a separate misjoin counter. Add a test where an old report arrives after tag reuse but before the new report, then update the self-test expectation, struct comment, and documentation to reflect the implemented behavior.

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


2. Jaguar2 transmitters receive no reports ✓ Resolved
Description
docs/stream-timing.md presents DEVOURER_TX_WITH_RX=thread as a way to collect Jaguar2 reports,
but streamtx never starts the RX loop that decodes them. With the harness's default Jaguar2
transmitter, its new report phase enables reporting in that TX-only process and reaches the join
verdict with no collected reports.
Code

docs/stream-timing.md[R228-230]

+is a transmit-side instrument. C2H must flow for it — Jaguar3
+drains it on its coex thread, a Jaguar2 transmitter needs an RX loop
+(duplex, or `DEVOURER_TX_WITH_RX=thread`); Jaguar1 reports carry no tag, so
Evidence
Rule 3 requires an active RX loop for Jaguar2 C2H reports. The new report phase runs streamtx on the
default Jaguar2 device, but streamtx only initializes TX; its environment mapping changes
configuration without starting the RX worker that contains the report decoder.

Account for RX availability and device-specific report matching
docs/stream-timing.md[228-230]
tests/stream_timing_onair.sh[48-51]
tests/stream_timing_onair.sh[270-278]
examples/streamtx/main.cpp[260-266]
src/jaguar2/RtlJaguar2Device.cpp[675-713]

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

## Issue description
Jaguar2 delivers CCX reports through its RX loop, which streamtx does not start; the documented setting does not start it either.
## Fix Focus Areas
- docs/stream-timing.md[228-230]
- tests/stream_timing_onair.sh[270-278]
- examples/streamtx/main.cpp[260-266]
## Recommended Fix
Run and stop a Jaguar2 RX loop in TX-only reporting sessions, or restrict the report phase to supported transmitters and document duplex as the Jaguar2 path.

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


3. Hop-sync reports have no frame record ✓ Resolved
Description
StreamTimingTx records tags for its timing markers but has no corresponding recording path for
streamtx's direct hop-sync send. When slot hopping and TX reporting are both enabled, a hop-sync
report reaches drain_reports() without a frame record and increments the unmatched count.
Code

examples/common/stream_timing_tx.h[R247-251]

+    if (_join_on) {
+      if (auto tag = _rtl->NextTxReportTag()) {
+        devourer::stream_timing::TxFrameRec rec;
+        rec.frame = _frames; rec.send_ns = hn; rec.marker = true;
+        _join.sent(*tag, rec);
Evidence
Rule 1 includes marker sends in per-frame matching. The new recording path covers timing markers,
but streamtx sends hop-sync markers directly; the descriptor still advances its tag, and the new
matcher counts a report with no recorded slot as unmatched.

Join CCX TX reports to per-frame timing records on Jaguar2/3
examples/common/stream_timing_tx.h[247-253]
examples/streamtx/main.cpp[466-488]
src/StreamTelemetry.h[385-390]
src/jaguar3/RtlJaguar3Device.cpp[2768-2775]

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 join records timing-marker sends but omits the separate hop-sync sends made by streamtx.
## Fix Focus Areas
- examples/common/stream_timing_tx.h[247-253]
- examples/streamtx/main.cpp[466-488]
## Recommended Fix
Provide a way to record an external marker's tag immediately before sending and use it for streamtx's hop-sync frames. Check that their reports join as markers.

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



Remediation recommended

4. Queue overflow can race with logging ✓ Resolved
Description
The report sink increments _rpt_overflow under _rpt_mu, but maybe_marker reads it for
stream.timing without that lock or an atomic operation. If reports arrive while a full queue is
being logged, the C2H and transmit threads concurrently access the counter, producing a C++ data
race.
Code

examples/common/stream_timing_tx.h[292]

+        .f("rpt_overflow", (unsigned long long)_rpt_overflow);
Evidence
The callback's mutex protects its increment, but the event builder reads the same non-atomic member
outside that mutex while Jaguar3's coex thread can continue delivering reports.

examples/common/stream_timing_tx.h[100-104]
examples/common/stream_timing_tx.h[285-293]
src/jaguar3/RtlJaguar3Device.cpp[526-548]

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 C2H callback writes the overflow counter under a mutex while the transmit thread reads it without synchronization.
## Fix Focus Areas
- examples/common/stream_timing_tx.h[100-104]
- examples/common/stream_timing_tx.h[285-293]
## Recommended Fix
Read the counter under `_rpt_mu` when preparing the timing event, or make all accesses to it atomic.

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


5. Reported host age includes drain delay ✓ Resolved
Description
drain_reports computes age_us from the time the transmit thread drains its deque, not from when
the C2H callback received the report. When reports wait in that deque while a send or
transmit-thread scheduling is delayed, the ledger attributes that extra host-side wait to
send-to-report arrival time.
Code

examples/common/stream_timing_tx.h[R192-193]

+    const uint64_t now = now_ns();
+    for (const auto &r : q) {
Evidence
The callback enqueues only TxReport, while the timestamp is taken after the transmit thread swaps
the queue; the ledger and its documentation call the resulting field report arrival age.

examples/common/stream_timing_tx.h[100-104]
examples/common/stream_timing_tx.h[185-213]
docs/logging.md[192-192]

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 ledger calls transmit-thread drain time the report's host arrival time, so queueing in the callback deque inflates `age_us`.
## Fix Focus Areas
- examples/common/stream_timing_tx.h[100-104]
- examples/common/stream_timing_tx.h[185-213]
## Recommended Fix
Store a monotonic receive timestamp alongside each report in the sink callback, then calculate `age_us` from that timestamp and the frame's send timestamp.

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


6. Disabling reports fails the duplex test ✓ Resolved
Description
The duplex harness runs join_verdict whenever TX_REPORT is nonempty, including when it is 0.
With TX_REPORT=0, configuration disables report requests, yet the verdict still requires joined
reports and marks an otherwise valid duplex run as failed.
Code

tests/stream_timing_onair.sh[322]

+    if [ -n "$TX_REPORT" ]; then join_verdict "$OUT/tx_duplex.out" "duplex report" || fail "duplex: join"; fi
Evidence
The configuration parser accepts zero as the report divisor, while the harness passes that value
through and uses only a nonempty-string check before running a verdict that requires joined frames.

examples/common/env_config.cpp[162-163]
tests/stream_timing_onair.sh[261-266]
tests/stream_timing_onair.sh[316-322]

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

## Issue description
A nonempty `TX_REPORT=0` disables reports but still triggers the duplex report-join verdict.
## Fix Focus Areas
- tests/stream_timing_onair.sh[316-322]
## Recommended Fix
Run the duplex join verdict only when the requested report sampling divisor is greater than zero.

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



Informational

7. Report sink can run after the timing object is torn down ✓ Resolved
Description
StreamTimingTx::stop clears a sink that captures this, but DeliverTxReport may already have
copied the sink and can invoke it after releasing _tx_report_sink_mu. If ~StreamTimingTx calls
stop() while the RX worker or Jaguar3 coex thread delivers a C2H report, the callback can still
access _rpt_mu and _rpt_q after those members are destroyed.
Code

src/IRadio.h[R784-789]

+    std::function<void(const devourer::TxReport &)> sink;
+    {
+      std::lock_guard<std::mutex> lk(_tx_report_sink_mu);
+      sink = _tx_report_sink;
+    }
+    if (sink) sink(r);
Evidence
The sink lambda captures this and accesses _rpt_mu and _rpt_q. DeliverTxReport copies the
stored function under _tx_report_sink_mu but invokes the copy after unlocking, so clearing the
stored sink does not wait for a copied callback to finish. ~StreamTimingTx calls stop() before
its members are destroyed, while C2H reports can still be delivered by the RX worker or Jaguar3 coex
thread.

src/IRadio.h[781-790]
examples/common/stream_timing_tx.h[52-52]
examples/common/stream_timing_tx.h[100-113]
examples/common/stream_timing_tx.h[100-112]
src/IRadio.h[781-789]
src/jaguar3/RtlJaguar3Device.cpp[526-548]

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

## Issue description
Clearing the TX report sink does not wait for an invocation that already copied the callback, so destruction of `StreamTimingTx` can overlap with a callback accessing its members.
## Fix Focus Areas
- examples/common/stream_timing_tx.h[100-112]
- src/IRadio.h[672-675]
- src/IRadio.h[781-794]
## Recommended Fix
Make sink removal wait for in-flight invocations before the timing helper can be destroyed, such as by tracking active calls with a counter and condition variable. Ensure that once `SetTxReportSink({})` returns, no invocation can touch the old callback, without holding the sink mutex across arbitrary callback code. Alternatively, give the callback separately owned, thread-safe state that remains valid through in-flight invocations.

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


Grey Divider

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 ↗

Grey Divider

Qodo Logo

Comment thread src/StreamTelemetry.h
Comment thread docs/stream-timing.md Outdated
Comment thread examples/common/stream_timing_tx.h
Comment thread examples/common/stream_timing_tx.h Outdated
Comment thread examples/common/stream_timing_tx.h Outdated
Comment thread tests/stream_timing_onair.sh Outdated
Comment thread src/IRadio.h Outdated
…al, hop-sync tags

- TxReportJoin counts a send that reuses a still-live tag (rpt_overwritten):
  an eight-bit tag cannot name its generation, so such a window's joins are
  suspect; selftest covers the late-report-after-reuse case and says what
  happens. The harness verdict requires zero overwrites in steady state.
- IRadio::SetTxReportSink returns only once no earlier sink is still
  executing (in-flight count + condition variable), so a helper may destroy
  what its sink captured right after clearing it.
- The report queue stamps each report when the host decoded it; age_us is
  send -> that instant, not the TX thread's drain time. The overflow counter
  is atomic.
- streamtx's hop sync marker records its tag through the helper
  (note_external_send) so its report joins as a marker.
- The duplex join verdict runs only for a positive report divisor; the
  report phase is documented as Jaguar3's (streamtx runs no RX loop, so a
  Jaguar2 transmitter is covered by the duplex phase), and the verdict says
  so when no join window exists.

Measured after the changes: report phase 8812CU 1.02 joins/frame, 0
unmatched, 0 overwritten; duplex 8812BU/T3U ch36 1.02, 0, 0; hop + reports
on the 8812CU joins 1.30/frame steady (markers included) while ~15% of
sends get no report in bursts of 7-13 around retunes -- rpt_overwritten
counts that backlog, documented as such.

Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com>
@josephnef
josephnef merged commit 5be2b72 into master Oct 10, 2026
40 checks passed
@josephnef
josephnef deleted the ccx-join branch October 10, 2026 18:24
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.

Stream timing: join the CCX tx.report queue time to the per-frame record on HalMAC

1 participant