Repository navigation
jaguar2/examples: survive a failed register read under co-channel load - #511
Conversation
ClmWindow::arm set the new period before its register writes and the armed flag only after them. Each write is a read-modify-write, and a read that throws part-way over a window still armed (a not-elapsed one the caller re-arms) left that window armed with the new period, so a caller that caught the throw and read on could get a reading against a denominator its hardware never counted. Disarm before the writes. busy_window_selftest covers both failed accesses: a set_bb whose read throws mid-arm leaves no reading (fails without the fix: armed 1, valid 1), and a read that throws changes nothing, so the retry reads the window. The ArmChannelBusy contract says what each leaves, scoped to the Realtek USB backends: there a failed read throws, a failed write is not reported at all, and after a failed arm GetChannelBusy falls back to its sampled path, so the caller re-arms rather than reads. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
"rtw_read(0a2c), sizeof(T) = 4" could not say whether the device stalled the request, never answered it, or returned short - the first thing to know about a failure that comes and goes under load. The line now carries the rc and its libusb name. (read32_wide's fallback returns a sentinel rather than throwing and is left as it is.) What it showed on one 8812BU under a saturating co-channel 40 MHz injector: LIBUSB_ERROR_PIPE every time, in ~100 µs, on every address sampled (MAC, BB config, BB report, RF window). An immediate re-read from a throwaway probe build, not in this tree, succeeded 6/6 in one 90 s run. Recorded with its counterparts in src/jaguar2/CLAUDE.md. No retry is added: it would change every generation's transport, and one run does not validate it. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
rxdemo's DEVOURER_RX_BUSY_MS, _THERMAL_POLL_MS and _RX_ENERGY_MS threads call the device on their own cadence with no catch, so a register read that fails under co-channel load is std::terminate. The busy-window one aborted an 8812BU airtime witness (ArmChannelBusy's read-modify-write of CLM 0x990). txdemo's thermal sample sits on the TX loop with no catch around it either - by inspection, not reproduced. Each now catches, counts and skips the sample through one helper (examples/common/poll_skip.h): failures 1, 101, 201, ... warned, with the running count - the library DIG loop's shape. A failed busy read still emits its rx.busy event, with no reading and read_failed:1, because a busy-window consumer counts windows; a failed arm emits nothing and is retried. The thermal and energy series are timestamped samples, so a skipped one leaves a gap in t (a skipped rx.energy interval drops its frame counts too - logging.md says so). rx.busy is now in logging.md. A/B on one 8812BU (USB 3, ch 149 HT40+) next to an 8812EU 50 cm away injecting MCS1/40 at saturation, -10 dB offset, CLM busy 73-86% by run: with 5 ms busy windows and a 50 ms thermal poll, master aborted 2/2 within seconds (173 and 729 windows); this branch ran 60 s 3/3 (~10.3k windows each, 23-26 failed reads caught across DIG, busy and thermal). At the witness's own 1 s cadence (thermal 1 s) neither build aborted in 3 runs each, at 1-5 failed reads per 90 s on master and 1-6 on the branch: the fix changes survival, not the failure rate. Why the 1 s cadence did not abort in 3 runs is inferred, not counted: a 1 s poller issues few transfers next to DIG, so a failure seldom lands on its thread. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
The thermal-track guard warned on every failed tick with no count; the DIG loop next to it counts and warns on failures 1, 101, 201, ... Same shape here, so the two read alike in a log and neither floods it. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
PR Summary by QodoKeep Jaguar2 telemetry running after failed register reads
AI Description
Diagram
High-Level Assessment
Files changed (10)
|
Code Review by Qodo
1. Quality and energy failures share one warning
|
josephnef
left a comment
There was a problem hiding this comment.
Review: PR 511 (jaguar2/examples: survive a failed register read under co-channel load)
Verdict: approve once the Qodo thread is closed. The code is sound; the only red check is qodo-gate, which fails on the one unresolved (Optional) thread — the shared PollSkips counter between GetRxEnergy and GetRxQuality. Either give the quality read its own PollSkips("DEVOURER_RXQUALITY") (two lines) or dismiss it in the thread and resolve.
Verified in the tree
ClmWindow::armclearsarmed_before the read-modify-writes;read()returns no reading when unarmed andRtlJaguar2Device::GetChannelBusytakes the sampled branch on!_station.armed(), so theIRadiocontract text is accurate. The two new selftest cases pass (86/86 on the branch).- Every struct the catch paths leave default-constructed (
ChannelBusy,ThermalStatus,RxEnergy,RxQuality) has default member initialisers, so the post-catchvalid/rawchecks in rxdemo and txdemo are well-defined. - The energy thread's
continueafter a failedGetRxQualityskips theadapter.rxpathsemit and lands on the samenap, so the cadence holds. libusb_error_nameis in scope inUsbTransport.h; thertw_read(prefix the scripts grep is kept.- CI matrix green; build warnings on the branch are the same pre-existing vendored-header redefinitions as master.
Hardware (this bench)
The stressed cell from the PR body, 60 s each, 5 ms busy windows + 50 ms thermal, ch 149 HT40+, MCS1/40/LDPC at zero gap, -40 qdB:
| witness | injector | build | windows | failed reads | aborted |
|---|---|---|---|---|---|
8812BU 0bda:b812 (USB 2 hub) |
8812EU a81a |
master, PR | — | — | injector died (txdemo aborts at zero gap on this USB 2 hub); busy 0–1%, inconclusive |
8812BU 0bda:b812 (USB 2 hub) |
8812AU 8812 |
master | 10657 | 0 | no (busy 89%) |
8812BU 0bda:b812 (USB 2 hub) |
8812AU 8812 |
PR | 10634 | 0 | no (busy 80%) |
T3U 2357:012d (8812BU, USB 3) |
8812AU 8812 |
master | 5703 | 0 | no (busy 83%) |
So the LIBUSB_ERROR_PIPE stall did not occur here in 3 × 60 s at 80–89% CLM busy, on two 8812BU units, with a different injector. The PR's change is still the right shape (a poller must not terminate the process), and nothing regressed: event counts and busy readings match master at 5 ms windows. But the "23–26 failed reads per 60 s" rate is rig-specific — worth saying in src/jaguar2/CLAUDE.md that it was one host + an 8812EU injector, and that another bench (8812AU injector, same load) saw none in 3 min.
Nits
src/jaguar2/CLAUDE.md: "from a throwaway probe patch that is not in the tree" is history; keep the measured fact ("an immediate re-read succeeded 6/6") and drop the provenance clause.docs/logging.mdrx.busyrow is fine;read_failedas the discriminator from an unarmed window is a good call.
What
An RTL8812BU used as a passive airtime witness (
rxdemo,DEVOURER_RX_BUSY_MS=1000, 40 MHz) next to a saturating co-channel 8812EU logged intermittentrtw_read(…)errors, and once aborted withterminate called after throwing an instance of 'std::ios_base::failure'.Observed:
DEVOURER_RX_BUSY_MSpoller. ItsArmChannelBusyread-modify-writes CLM 0x990/0x994 with no catch around it. The rxdemo thermal and RX-energy pollers had no catch either.[E]lines.LIBUSB_ERROR_PIPEin ~100 µs. The device stalls the request: there were no timeouts and no reads slower than 5 ms.Not established:
usbctrl_vendorreq, rtw88rtw_usb_reg_sec) is not ported here, and was not tested as a cause.Change
Four commits, one per defect class:
ClmWindow::armnow disarms before its read-modify-writes. Before, a throw part-way through left an earlier window armed with the new period.busy_window_selftesthas two new cases, and the re-arm case fails without the fix. TheArmChannelBusycontract says what each failed access leaves behind, scoped to the Realtek USB backends.rtw_read(prefix is kept for the scripts that grep it. The measurement is recorded insrc/jaguar2/CLAUDE.mdwith its counterparts. No retry is added.examples/common/poll_skip.h, which has the DIG loop's rate-limited warning.rx.busy, withread_failed:1. A failed arm is retried and emits nothing.rx.busyis now indocs/logging.md.If you squash-merge, the commit messages carry the per-class detail. We are happy to split this into separate PRs instead if you prefer: commits 2 and 4 stand alone.
Measured
Rig: one 8812BU (
0bda:b812, USB 3) as the witness on ch 149 HT40+. An 8812EU 50 cm away injected MCS1/40/LDPC at saturation withDEVOURER_TX_PWR_OFFSET_QDB=-40(~1.8k frames/s). Both builds are upstreamrxdemo/txdemo; master isd0a9e10. Runs were interleaved master/branch, with the same injector binary throughout.The fix changes survival, not the failure rate. CLM busy was 73–86% by run.
Counterparts (master witness):
Verification:
ctestpasses, 83/83 with the default build and 86/86 withDEVOURER_MT7612U=ON DEVOURER_PCIE=ON, with no new warnings in touched files.Limits
ctrl_writereturns bool andset_bbdrops it). An arm can therefore succeed over an engine it did not fully program. That predates this PR; the contract now says so.Possible follow-ups (not in this PR)
PhydmWatchdog,_rxmask_thread,_qd_thread(8814, barertw_read32), and_therm_thread. Same defect class, one generation over.GetRxEnergyinDEVOURER_RX_SWEEP;SetStationIdentityreadbacks);chanmig/txpowerenergy and thermal reads.LIBUSB_ERROR_PIPEinUsbTransport::ctrl_read. It touches every generation, so it needs validating across them.Reproduce
Two adapters, each with no kernel driver bound.
TXis the injector,WITthe witness:🤖 Generated with Claude Code