sensing: a busy-airtime window the caller defines by snokvist · Pull Request #437 · OpenIPC/devourer

snokvist · 2026-09-20T08:16:16Z

Why

GetChannelBusy() reads whatever window the shared NHM read happened to arm:
~2 ms, once. For a consumer that ranks channels from a survey dwell, that is a
sample of the dwell, not a measurement of it — and on bursty traffic it is
bimodal rather than merely noisy.

Measured on an RTL8822BU against a 50 ms-on/450 ms-off interferer (true duty
~9%), one read per 300 ms dwell:

estimator	samples	result
sampled, ~2 ms	71	zero in 55 of them, 61-65% in the rest; mean 9.0, sd 21.9
MT7612U channel timers, 1 s	12	8.1-9.5% every sample, sd 0.5

Frames were decoded in nearly every one of those 71 windows, so the channel was
never free — the sample missed the burst 78% of the time. A ranker that reads
0% as "emptiest" does not lose information there, it gets a wrong answer, and
examples/chanscout takes exactly one such read per dwell.

CLM's period is its own field (the low half of the period dword) and reaches
65535 ticks of 4 us, so the hardware can integrate a whole dwell. The MediaTek
timers already integrate between reads. Neither was reachable through the
contract.

What

IRadio::ArmChannelBusy(window_us) — arm a window where you reset your
counters, read it with GetChannelBusy() when your dwell ends. It returns the
window actually armed, because the hardware clamps it. Nothing in-tree
calls it yet, so every existing consumer keeps the sampled path unchanged.

Realtek arms CLM alone (arm_clm_only/read_clm_only in
src/NhmReader.h): NHM's half of the shared period dword and its trigger bit
are not touched, because nhm_env is referenced to live IGI and a longer
histogram window would make it less stable, not more.
MediaTek resets its channel timers and interval mark through a new narrow
mt7612u_ch_time_arm(). Deliberately not mt7612u_link_stats_start(): that
also clears the MIB block and the link-stats interval, which belong to the
1 Hz telemetry caller, and arming per dwell through it would corrupt every
rate that tick reports.
devourer::ClmWindow (src/BusyWindow.h) owns the state machine with no
hardware access of its own, so every path is reachable from a selftest.

A window is only about the dwell if nothing else touched it. Three spoilers,
all measured, each yielding an INVALID reading that carries its reason
(ChannelBusy::spoil) instead of a plausible number:

spoiler	11AC map (Jaguar1/2)	JGR3 map (Jaguar3)
an NHM read mid-window	survives, reads +4 points high (74.8 vs 70.9)	destroyed — returns the 2 ms re-arm (326 of 62500 ticks where a clean read gave ~38000)
a retune mid-window	60-62% where the channel was 71%	44-47% where it was 61%
a read before it elapsed	the result register latches the PREVIOUS window and reads are non-destructive, so it is a stale number wearing a fresh timestamp	same

The first row is why the rule is enforced on every family rather than only
where it fails loudly: GetRxQuality() takes that NHM read, so a consumer
polling link quality inside its own survey dwell springs the trap without ever
touching the busy API. There is an on-air arm for exactly that call.

The arm, the read and the family's own NHM read are serialised on one CCX
mutex, taken under the family register lock where one exists (with_ccx()).
Without it, a telemetry thread's GetRxEnergy(true) could note "nothing armed"
and then re-arm the engine after a concurrent arm committed — a destroyed
window reading back valid, which is the failure this change exists to prevent.
with_ccx takes the locks rather than handing out accessors because the arm is
four register writes that must not interleave.

Own transmission is carried, not corrected. CLM counts receive-side
deferral only and a radio is deaf while its own PA is up, so a transmitting
sensor reads LOW; the MediaTek timers count own TX and read HIGH. Same loaded
channel, sensor silent vs transmitting:

sensor	silent	transmitting	frames sent in-window
Jaguar1 RTL8812AU	70.9%	24-26%	765-1227
Jaguar3 RTL8812CU	60.9%	18.4-18.6%	1503-3260

The two families are biased in opposite directions, so own_tx_in_window /
own_tx_frames are facts on the reading rather than a fudge inside it. This
also sharpens the TX-session section in docs/rx-spectrum-sensing.md: CLM is
alive there, but biased low.

Also measured, and reflected in caps: Jaguar1 CLM (70.6-70.9% under a load
a MediaTek adapter independently measured, 0.1-1.0% quiet) and the MT7612U
timers (64.0-64.3% / 0.0% quiet / 8.1-9.5% bursty). busy_airtime_measured is
true on both now. Measuring the MediaTek also turned up two defects in its
existing path: with RX down its idle timer still advances, so busy+idle was
non-zero and the ratio came out a valid 0% — the fabricated zero this
contract exists to prevent, and for a ranker 0% means "pick me" — and its
arming zeroed the interval mark, so the first read after an arm reported
window_us=0, a percentage with no denominator. Both fixed; a MediaTek retune
now spoils an armed window too.

Validation

Headless — tests/busy_window_selftest.cpp with mocked masked-write
semantics: the clamp, the CLM-only arm asserted as a sequence (no NHM
trigger edge, starting from the state a previous read_nhm leaves behind, since
a check on the final register image cannot see a 0→1→0 pulse), the
period-bounded ratio, every refusal including the stale-latch re-arm, spoil
precedence, the JGR3 map, a zero window, and own-TX accounting including a
counter that went backwards and one that jumped past the 32-bit field. Two
mutations confirm it bites: arming NHM alongside CLM fails the sequence check,
and ignoring the ready bit fails the stale and early cases. 69/69 ctest,
ASan+UBSan clean.

On air — tests/busy_window_probe.sh, ten arms through the public contract
only, gating on values rather than validity: the floor must be quiet, the load
must separate from it, each spoiler must refuse with its reason, a re-armed
window must not return the latched previous result, and under a bursty load the
armed spread must beat the sampled one. 8812AU sensor, MT7612U flooder:

load	estimator	mean	min	max	spread
bursty (50/450)	armed 240 ms	10%	0%	19%	19 points
bursty (50/450)	sampled ~2 ms	12%	0%	72%	72 points
steady	armed 240 ms	71%	71%	71%	0 points
steady	sampled ~2 ms	70%	69%	71%	2 points

One arm is the locking itself: a second thread hammering GetRxQuality() for
the whole window, where every reading must be refused rather than come back as
a short window wearing a valid flag. It is, 4/4 and 6/6.

Three results that cut against the feature

Under a steady load the sampled path is fine — spread 2-7 points against
the window's 0. The window earns its place on bursty interferers and on
single-dwell decisions; a consumer that can average many sweeps of a stable
channel gains little.
It does not fix the transmit bias, it only reports it. A scouting radio
that also carries an uplink still reads ~3x low on Realtek. The honest use
is to refuse such a sample, which is why the flag exists — but that is a
consumer policy this PR does not write.
A genuinely saturated channel is unmeasured. Two adapters could not
produce one: a flooder with no inter-frame gap holds this bench at ~71%, and
shortening the window to 1 ms only samples the gaps more finely (30 reads
spanned 67-80%, none at 100%). The rpt == period case is covered headlessly
only. The armed period is capped at 60000 ticks (240 ms) rather than the
hardware's 65535 so the ambiguity is unreachable instead of handled — the
count is bounded by the period (measured: arm 500 ticks, read 270 ms later,
get 304-364 on all three Realtek families), so a period below the field's
range cannot wrap into a small number that would read as a quiet channel.

Also worth setting expectations: the final code was re-verified on air on the
Jaguar1 and the MT7612U, which are the adapters on this bench today. The
Jaguar2 and Jaguar3 numbers quoted above come from the same register sequence
carried by an experimental build of it; the JGR3 truncation and the 11AC
inflation are why the refusal is unconditional rather than per-family, so a
re-run on those dies can only turn a refusal into a reading, never the reverse.

Deliberately not here

No scoring law is touched. ChannelScore, MigGate and the hopset
occupancy weights do not read the busy figure, and whether CLM belongs in
cell_occupancy needs its own re-validation of the migration matrix.
No in-tree caller. examples/chanscout still takes the sampled path, so
this is provably no behaviour change for every existing consumer; converting
it is a separate change with its own on-air comparison.
Kestrel and the RTL8733B return 0 from the arm and keep the sampled path.
(On the RTL8733B that is a placeholder, not a verdict — see below.)

Follow-ups

The RTL8733B has a working CLM engine, which GetAdapterCaps currently
reports as absent. Driven on the JGR3 map it read 62.4-63.3% under a ~63%
load, 1.6-2.1% off-channel under the same load, and 0.0% quiet, while
rx.energy comes back all-null despite 28k frames decoded. The vendor phydm
lists the 8733B as JGR3 (phydm_pre_define.h:432, phydm_ccx.c:333,1319).
Its FA/IGI registers are 8733B-specific and remain unmeasured, so the port is
its own PR.
The MediaTek energy leg. MT_ED_CCA_TIMER (0x1140) tracks load
(~63.5% against 64.1% busy) and is the energy_pct that sensing: a vendor-neutral channel-busy contract, and the survey dwell as a library primitive #432 left invalid.
The first read after arming is stale garbage and must be discarded.
Two MT7612U bugs found while measuring, both out of scope here:
IRadio::Init does not return on that backend (it drives RX inline, so a
single-threaded consumer cannot use it), and its MCU command times out
repeatedly under saturating RX.
Whether a longer window changes what chanscout should fold, now that the
per-dwell number is a measurement.

🤖 Generated with Claude Code

https://claude.ai/code/session_01SbJ75HT4fqof7e5e8NKL2j

Review round

Thirteen findings from the bot, plus seven from three further passes of my own. All in the new code; the ones that could hand a consumer a wrong number were on the MediaTek path, which had an armed-window flag but none of the state a reading needs.

No deadline — an early read returned a valid short window where the Realtek path refuses one. The requested length is recorded at arm.
A lock gap — the armed flag was cleared in one critical section and the timers read in another, so a retune landing between them found nothing to spoil and the remainder came back valid. One locked section now, with _rx_active re-checked inside it: the old check-then-lock let StopRxLoop finish in the gap while the C layer still answered, which is exactly the fabricated zero the gate exists to prevent.
own_tx_* was never populated there, although those timers count own transmission as busy.
An armed window survived an RX restart that re-zeroes the timers under it, and a zero-length request returned 0 — the "cannot arm" answer — while arming anyway.

The MediaTek decision moved out of the backend into a pure busy_from_ch_time_window() beside the conversion it qualifies, so those rules are covered headlessly now: every refusal, the elapsed boundary both ways, own-TX including a counter that went backwards, and an armed window over dead counters.

A self-deadlock of my own making: Jaguar1's FastRetune/FastSetBandwidth held the CCX mutex across the tune, and their declined fast paths call SetMonitorChannel(), which takes that same non-recursive mutex. One rule, two shapes now — where a family has a register lock that spans the change the note sits inside it and with_ccx's ordering does the rest; Jaguar1 has none, so the note is scoped and the single-control-thread contract carries it. Jaguar2 also kept a leftover pre-lock note that took the CCX mutex before _reg_mu, the inversion with_ccx is ordered against.

Two claims of mine were false. Worth stating plainly:

I described the disturbed-counter hazard as a 1 Hz telemetry poll stealing the shared timers. Nothing in Mt7612uRadio polls mt7612u_link_stats() — only a C-API caller that uses both does, which tools/bringup.cpp is. The guard stays, because such a caller really can take the counts, but it says so accurately, and it no longer breaks the public C signature: mt7612u_ch_time() keeps its four parameters, the flag has its own accessor, and both new symbols are in the api_link surface guard.
The saturation test claimed to cover the clamp while passing without it — just over the period, rounding lands on 100 either way. It overshoots far enough to kill the mutant now (an unclamped build reports 102).

Also: the neutral ArmChannelBusy contract said its return is the caller's denominator, which the MediaTek path contradicts by returning the request unchanged — the contract describes both shapes now and points at ChannelBusy::window_us as the one authoritative window. Spoil precedence agrees across families. The probe emits JSON Lines through the shared EventSink, adopts the libusb context, and its frame literal no longer leans on implicit zeros. The harness picks the family numerically rather than by the spelling of the VID, runs the load-free spoil arms on both families, and three of its assertions could pass vacuously — an empty extractor result, a stale arm whose records never appeared, and a spread comparison a handful of uniformly-wrong samples could satisfy. Each fails loudly now.

Still open and stated rather than fixed: the MediaTek loaded arms are skipped by the harness, because that adapter's bring-up does not complete on an already-saturated channel, so its loaded numbers come from a manual run with the sensor started first.

GetChannelBusy reads whatever window the shared NHM read happened to arm: ~2 ms, once. That is a sample, not a measurement of the caller's dwell, and on bursty traffic it is bimodal rather than merely noisy. Measured on an RTL8822BU against a 50 ms-on/450 ms-off interferer, one read per 300 ms read ZERO in 55 of 71 windows and ~63% in the rest, while 300-400 frames were decoded in nearly every one of them. A ranker that reads 0% as "emptiest" does not lose information there, it gets a wrong answer. CLM's period is its own field and reaches 65535 ticks of 4 us, so the hardware can integrate a whole dwell. ArmChannelBusy(window_us) on IRadio does that: arm where the counters are reset, read at the end. MediaTek's timers already integrate between reads, so there arming is resetting them and the mark. Both families then report busy airtime over the CALLER's window - not an identical measurement (one is hardware-timed and counts receive-side deferral, the other is the host-measured gap and counts own TX), which is what source/window_us/own_tx_in_window are for. tests/busy_window_probe.sh, 8812AU sensor, 6 samples on a ~9%-duty interferer: armed 240 ms window mean 10% spread 19 points; sampled 2 ms mean 24% spread 73 points. The sampled column is not just wider - it caught a burst twice in six reads and called a ~9% channel 24%. Under a steady load both agree (71%, spread 0 vs 6), which is why the bursty arm is the one that decides anything. A window only describes the dwell if nothing else touched it, and all three spoilers are measured rather than assumed: - An NHM read mid-window. The damage is register-map-dependent: the JGR3 map DESTROYS the window (326 of 62500 ticks where a clean read gave ~38000), the 11AC map survives it but reads 3-4 points HIGH. Enforced on both, because GetRxQuality() takes that NHM read - a consumer polling link quality inside its own survey dwell springs the trap without ever touching the busy API. The probe has an arm for exactly that call. - A retune mid-window: the counter runs across it and reports the blend of two channels (44-47% on a Jaguar3 where the channel was 61%). A Jaguar3 bandwidth change counts too, and so does a MediaTek retune. - A read before the window elapsed: the result register latches the PREVIOUS window and reads are non-destructive, so it is a stale number wearing a fresh timestamp. Each yields an INVALID reading carrying its reason (ChannelBusy::spoil), so a spoiled window is never a plausible number. The arm, the read and the family's own NHM read are serialised on one CCX mutex, taken under the family register lock where there is one (with_ccx). Without that, a telemetry thread's GetRxEnergy(true) could note "nothing armed", then re-arm the engine AFTER a concurrent arm committed - a destroyed window reading back valid, which is the failure this whole change exists to prevent. Bring-up forgets an armed window, and a no-op retune no longer spoils one. Own transmission is carried, not corrected: CLM counts receive-side deferral only and a radio is deaf while its own PA is up, so a transmitting sensor reads LOW (70.9% -> 24-26% on a Jaguar1, 60.9% -> 18.4-18.6% on a Jaguar3), while the MediaTek timers count own TX and read HIGH. The two families are biased in opposite directions, so own_tx_in_window/own_tx_frames are facts on the reading rather than a correction inside it. Also measured, and reflected in caps: Jaguar1 CLM (70.6-70.9% under a load a MediaTek independently measured, 0.1-1.0% quiet) and MT7612U channel timers (64.0-64.3% / 0.0% / 8.1-9.5% bursty). busy_airtime_measured is now true on both. Two MediaTek fixes came out of measuring it: with RX down its idle timer still advances, so busy+idle was non-zero and the ratio came out a plausible 0% - the fabricated zero this contract exists to prevent, now refused - and its arm stamps the interval mark instead of zeroing it, which had made the first read after an arm report window_us=0. Its arm also has its own narrow entry point rather than link_stats_start(), which would have reset the 1 Hz telemetry's MIB counters once per dwell. Validation: tests/busy_window_selftest.cpp with mocked masked-write semantics - the clamp, the CLM-only arm sequence asserted as a SEQUENCE (no NHM trigger edge, starting from the state a previous read_nhm leaves), the period-bounded ratio, every refusal including the stale-latch re-arm, spoil precedence, the JGR3 map, and own-TX accounting including a counter that went backwards. Two mutations confirm it bites: arming NHM alongside CLM fails the sequence check, ignoring the ready bit fails the stale and early cases. 67/67 ctest, ASan+UBSan clean. tests/busy_window_probe.sh drives ten on-air arms through the public contract only and gates on values, not just validity: the floor must be quiet, the load must separate from it, a spoiled window must refuse with its reason, a re-armed window must not return the latched previous result, and under a bursty load the armed spread must beat the sampled one. One arm is the locking itself - a second thread hammering GetRxQuality() for the whole window, where every reading must be refused rather than come back as a short window wearing a valid flag. PASS on an 8812AU sensor with an MT7612U flooder, steady and bursty; a MediaTek sensor runs the quiet arms and SKIPS the loaded ones, because its bring-up does not complete on an already-saturated channel. Not here: Kestrel and the RTL8733B return 0 from the arm and keep the sampled path; scoring laws are untouched; the saturation wrap near 100% is unmeasured (the bench flood tops out ~71%), which is why the period is capped at 240 ms rather than the hardware's 262. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01SbJ75HT4fqof7e5e8NKL2j

qodo-free-for-open-source-projects · 2026-09-20T08:17:06Z

PR Summary by Qodo

Add caller-defined busy-airtime measurement windows

✨ Enhancement 🐞 Bug fix 🧪 Tests 📝 Documentation 🕐 40+ Minutes

AI Description

• Adds caller-defined busy-airtime windows across Realtek and MediaTek radios.
• Invalidates measurements disrupted by NHM reads, retunes, or premature reads.
• Reports transmit bias and adds comprehensive headless and on-air validation.

Diagram

sequenceDiagram
    actor Caller
    participant API as IRadio API
    participant RTL as Realtek Window
    participant CLM as CCX CLM
    participant MT as MediaTek Timers
    participant Ops as Control Operations
    Caller->>API: Arm requested window
    alt Realtek backend
        API->>RTL: Arm state machine
        RTL->>CLM: Program CLM-only period
    else MediaTek backend
        API->>MT: Reset channel timers
    end
    opt NHM read or retune
        Ops->>RTL: Mark window spoiled
        Ops->>MT: Mark retune spoil
    end
    Caller->>API: Read busy airtime
    alt Clean completed window
        API-->>Caller: Busy percent and metadata
    else Spoiled or early window
        API-->>Caller: Invalid with reason
    end

High-Level Assessment

The following are alternative approaches to this PR:

1. Aggregate existing short samples

➕ Avoids a new radio API and backend state machine.
➕ Leaves shared CCX register behavior unchanged.
➖ Requires many sweeps before bursty occupancy converges.
➖ Single-dwell decisions remain bimodal and can falsely report zero busy airtime.
➖ Does not provide a measurement aligned to the caller's dwell.

2. Arm NHM and CLM together

➕ Reuses the existing shared CCX setup path.
➕ Could collect histogram and busy airtime over one interval.
➖ A long NHM window is unstable because its histogram references live IGI.
➖ Mid-window NHM users can truncate or inflate CLM results.
➖ Couples two sensors with different desirable window lengths.

3. Reuse MediaTek link-stats reset

➕ Uses an existing timer-reset entry point.
➕ Requires less new backend-specific code.
➖ Also clears MIB counters and the telemetry interval.
➖ Per-dwell arming would corrupt rates reported by the one-second telemetry caller.
➖ Creates unnecessary coupling between sensing and link statistics.

Recommendation: Keep the PR's explicit arm/read contract, isolated Realtek ClmWindow state machine, and narrow MediaTek channel-timer reset. It provides dwell-aligned measurements while preserving existing sampled consumers, protecting telemetry counters, and refusing results whenever hardware activity makes the requested window unreliable.

Files changed (25) +1910 / -36

Enhancement (15) +663 / -23

DeviceSession.hAllow probe sessions to release USB locks explicitly +7/-0

Allow probe sessions to release USB locks explicitly
• Adds release_lock so hardware probes using detached RX threads can drop their lock before exiting without destructors.
examples/common/DeviceSession.h

BusyWindow.hImplement the Realtek CLM window state machine +175/-0

Implement the Realtek CLM window state machine

• Introduces clamped CLM arming, one-shot reading, spoil tracking, period-bounded percentages, and saturated own-TX frame accounting without direct hardware dependencies.

src/BusyWindow.h

IRadio.hExpose caller-defined busy-airtime windows +31/-0

Expose caller-defined busy-airtime windows

• Adds the virtual ArmChannelBusy contract, including actual-window return semantics and a default unsupported implementation.

src/IRadio.h

IRtlRadio.hIntegrate serialized CLM windows into Realtek radios +85/-0

Integrate serialized CLM windows into Realtek radios

• Routes armed reads through ClmWindow while preserving the existing sampled fallback. Adds CCX locking, generation access hooks, spoil notifications, and lifecycle reset support.

src/IRtlRadio.h

NhmReader.hAdd CLM-only arm and read primitives +47/-0

Add CLM-only arm and read primitives
• Adds register helpers that program and read CLM independently without changing the NHM period or trigger.
src/NhmReader.h

RxSense.hExtend busy readings with spoil and TX metadata +45/-1

Extend busy readings with spoil and TX metadata

• Adds BusySpoil reasons plus own_tx_in_window and own_tx_frames fields to distinguish invalid windows and transmit-biased measurements.

src/RxSense.h

RtlJaguarDevice.cppProtect Jaguar1 windows across sensing and retunes +51/-5

Protect Jaguar1 windows across sensing and retunes
• Resets windows during initialization, serializes NHM reads, and spoils windows across channel or bandwidth changes. Marks Jaguar1 busy airtime as hardware-measured.
src/jaguar1/RtlJaguarDevice.cpp

RtlJaguarDevice.hProvide Jaguar1 CCX access under serialization +16/-0

Provide Jaguar1 CCX access under serialization
• Implements the generation-specific 11AC register map and register accessors used by the shared Realtek window logic.
src/jaguar1/RtlJaguarDevice.h

RtlJaguar2Device.cppProtect Jaguar2 windows across lifecycle operations +46/-2

Protect Jaguar2 windows across lifecycle operations
• Resets stale state during initialization and serializes NHM reads, retunes, and bandwidth changes with the CCX window.
src/jaguar2/RtlJaguar2Device.cpp

RtlJaguar2Device.hProvide locked Jaguar2 CCX access +15/-0

Provide locked Jaguar2 CCX access
• Implements shared window access using the 11AC map while enforcing register-lock-before-CCX-lock ordering.
src/jaguar2/RtlJaguar2Device.h

RtlJaguar3Device.cppProtect Jaguar3 windows from destructive operations +46/-3

Protect Jaguar3 windows from destructive operations
• Resets window state on initialization and marks windows spoiled during NHM reads, retunes, and bandwidth changes under ordered locks.
src/jaguar3/RtlJaguar3Device.cpp

RtlJaguar3Device.hProvide locked Jaguar3 CCX access +15/-0

Provide locked Jaguar3 CCX access
• Implements shared window access with the JGR3 register map under the family register lock and CCX mutex.
src/jaguar3/RtlJaguar3Device.h

Mt7612uRadio.cppImplement MediaTek armed busy windows +70/-12

Implement MediaTek armed busy windows

• Adds timer-based arming, retune spoil handling, and RX-active guards that prevent fabricated zero-busy readings. Updates adapter capabilities to reflect hardware validation.

src/mt7612u/Mt7612uRadio.cpp

Mt7612uRadio.hDeclare MediaTek busy-window state +6/-0

Declare MediaTek busy-window state
• Exposes the ArmChannelBusy override and adds mutex-guarded armed and spoiled state flags.
src/mt7612u/Mt7612uRadio.h

mt7612u.hDeclare the narrow channel-timer arm API +8/-0

Declare the narrow channel-timer arm API
• Adds mt7612u_ch_time_arm for resetting only busy-airtime counters without disturbing link telemetry.
src/mt7612u/include/mt7612u/mt7612u.h

Bug fix (1) +36 / -4

init.cppSeparate channel-timer arming from telemetry reset +36/-4

Separate channel-timer arming from telemetry reset
• Fixes interval marks to start at arming time and adds a channel-timer-only reset path that preserves MIB and link-stat intervals.
src/mt7612u/init.cpp

Tests (3) +1017 / -0

busy_window_probe.cppAdd a public-contract hardware probe +341/-0

Add a public-contract hardware probe

• Exercises sampled, armed, interrupted, retuned, early, stale, racing, and transmitting window scenarios while emitting machine-readable results.

tests/busy_window_probe.cpp

busy_window_probe.shAdd on-air busy-window acceptance checks +287/-0

Add on-air busy-window acceptance checks

• Orchestrates a sensor and flooder, validates load separation and spoil reasons, checks TX metadata, and compares bursty sampled versus armed spread.

tests/busy_window_probe.sh

busy_window_selftest.cppTest the CLM state machine headlessly +389/-0

Test the CLM state machine headlessly

• Mocks masked register writes to verify clamping, CLM-only sequencing, ratios, spoil precedence, stale-read refusal, TX accounting, and both register maps.

tests/busy_window_selftest.cpp

Documentation (5) +171 / -9

CLAUDE.mdDocument the explicit busy-window contract +12/-1

Document the explicit busy-window contract

• Explains how ArmChannelBusy replaces short incidental samples and why interrupted, retuned, or early windows are invalid. Clarifies that own transmission is reported rather than corrected.

CLAUDE.md

rx-spectrum-sensing.mdDocument armed-window behavior and measurements +115/-2

Document armed-window behavior and measurements
• Adds the public contract, hardware limits, spoil conditions, own-TX bias, generation coverage, and measured comparisons against sampled sensing.
docs/rx-spectrum-sensing.md

CLAUDE.mdRecord Jaguar1 CLM validation results +10/-1

Record Jaguar1 CLM validation results
• Documents measured busy-airtime capability and the 11AC-map overcount caused by an intervening NHM read.
src/jaguar1/CLAUDE.md

CLAUDE.mdDocument destructive JGR3 NHM interference +9/-0

Document destructive JGR3 NHM interference
• Records that an NHM read truncates an armed Jaguar3 CLM window to the short re-armed interval.
src/jaguar3/CLAUDE.md

CLAUDE.mdDocument measured MediaTek busy-timer behavior +25/-5

Document measured MediaTek busy-timer behavior
• Records hardware validation, the running-RX requirement, timer interval semantics, and remaining measurement limitations.
src/mt7612u/CLAUDE.md

Other (1) +23 / -0

CMakeLists.txtBuild and register busy-window validation targets +23/-0

Build and register busy-window validation targets
• Adds the hardware BusyWindowProbe executable and the headless BusyWindowSelftest CTest target with required includes and linkage.
CMakeLists.txt

qodo-free-for-open-source-projects · 2026-09-20T08:19:05Z

Code Review by Qodo

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

1. ~~Early reads look like completed windows~~ ✓ Resolved 📘 Rule violation ≡ Correctness

Description

Mt7612uRadio::GetChannelBusy() clears _busy_armed without checking whether the duration
requested by ArmChannelBusy() has elapsed. When the caller reads early and the timers have
accumulated any ticks, busy_from_ch_time() returns a valid short-window percentage with no
NotElapsed reason.

Code

src/mt7612u/Mt7612uRadio.cpp[870]
+    _busy_armed = false;

Evidence
Rule 18 requires premature armed-window reads to be invalid and carry their reason. The MediaTek
implementation records only armed and spoiled booleans, clears the armed state unconditionally, and
converts whatever interval has accumulated into a reading.
CLAUDE.md: Keep Channel-Busy Window Results Explicitly Invalid When Disturbed: CLAUDE.md: Keep Channel-Busy Window Results Explicitly Invalid When Disturbed: CLAUDE.md: Keep Channel-Busy Window Results Explicitly Invalid When Disturbed: CLAUDE.md: Keep Channel-Busy Window Results Explicitly Invalid When Disturbed: CLAUDE.md: Keep Channel-Busy Window Results Explicitly Invalid When Disturbed: CLAUDE.md: Keep Channel-Busy Window Results Explicitly Invalid When Disturbed: CLAUDE.md: Keep Channel-Busy Window Results Explicitly Invalid When Disturbed: CLAUDE.md: Keep Channel-Busy Window Results Explicitly Invalid When Disturbed: CLAUDE.md: Keep Channel-Busy Window Results Explicitly Invalid When Disturbed: CLAUDE.md: Keep Channel-Busy Window Results Explicitly Invalid When Disturbed: CLAUDE.md: Keep Channel-Busy Window Results Explicitly Invalid When Disturbed: CLAUDE.md: Keep Channel-Busy Window Results Explicitly Invalid When Disturbed: CLAUDE.md: Keep Channel-Busy Window Results Explicitly Invalid When Disturbed: CLAUDE.md: Keep Channel-Busy Window Results Explicitly Invalid When Disturbed: CLAUDE.md: Keep Channel-Busy Window Results Explicitly Invalid When Disturbed: CLAUDE.md: Keep Channel-Busy Window Results Explicitly Invalid When Disturbed
src/mt7612u/Mt7612uRadio.cpp[854-884]
src/mt7612u/Mt7612uRadio.cpp[897-907]
src/mt7612u/Mt7612uRadio.h[152-156]

Agent prompt

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

## Issue description
MediaTek armed-window reads do not retain a deadline, so an early read is returned as valid rather than invalid with `BusySpoil::NotElapsed`.
## Fix Focus Areas
- src/mt7612u/Mt7612uRadio.cpp[854-907]
- src/mt7612u/Mt7612uRadio.h[152-156]
## Recommended Fix
Store the requested window duration or deadline when arming. In `GetChannelBusy()`, compare the elapsed interval with that duration and return an invalid reading carrying `BusySpoil::NotElapsed` when the read occurs prematurely; add a headless test for this backend behavior.

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

2. ~~Retunes can pass as valid measurements~~ ✓ Resolved 📘 Rule violation ☼ Reliability

Description

Mt7612uRadio::GetChannelBusy() clears _busy_armed and releases _mu before reacquiring it to
read the channel timers. If SetMonitorChannel() runs between those critical sections, it resets
the timers but sees no armed window to spoil, allowing the subsequent read to describe a short
post-retune interval as valid.

Code

src/mt7612u/Mt7612uRadio.cpp[870]
+    _busy_armed = false;

Evidence
Rule 18 requires a retuned window to remain invalid with a reason. The new code clears the armed
marker in one locked block and reads the timers in a second, while the retune path only sets
_busy_spoiled when that marker is still true.
CLAUDE.md: Keep Channel-Busy Window Results Explicitly Invalid When Disturbed: CLAUDE.md: Keep Channel-Busy Window Results Explicitly Invalid When Disturbed: CLAUDE.md: Keep Channel-Busy Window Results Explicitly Invalid When Disturbed: CLAUDE.md: Keep Channel-Busy Window Results Explicitly Invalid When Disturbed: CLAUDE.md: Keep Channel-Busy Window Results Explicitly Invalid When Disturbed: CLAUDE.md: Keep Channel-Busy Window Results Explicitly Invalid When Disturbed: CLAUDE.md: Keep Channel-Busy Window Results Explicitly Invalid When Disturbed: CLAUDE.md: Keep Channel-Busy Window Results Explicitly Invalid When Disturbed: CLAUDE.md: Keep Channel-Busy Window Results Explicitly Invalid When Disturbed: CLAUDE.md: Keep Channel-Busy Window Results Explicitly Invalid When Disturbed: CLAUDE.md: Keep Channel-Busy Window Results Explicitly Invalid When Disturbed: CLAUDE.md: Keep Channel-Busy Window Results Explicitly Invalid When Disturbed: CLAUDE.md: Keep Channel-Busy Window Results Explicitly Invalid When Disturbed: CLAUDE.md: Keep Channel-Busy Window Results Explicitly Invalid When Disturbed: CLAUDE.md: Keep Channel-Busy Window Results Explicitly Invalid When Disturbed: CLAUDE.md: Keep Channel-Busy Window Results Explicitly Invalid When Disturbed
src/mt7612u/Mt7612uRadio.cpp[854-884]
src/mt7612u/Mt7612uRadio.cpp[562-592]

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 MediaTek busy-window state check and timer read use separate lock scopes, allowing a retune between them to escape invalidation.
## Fix Focus Areas
- src/mt7612u/Mt7612uRadio.cpp[854-884]
- src/mt7612u/Mt7612uRadio.cpp[562-592]
## Recommended Fix
Hold `_mu` continuously from the armed-window state check through `mt7612u_ch_time()`, and clear `_busy_armed` only after either returning the stored spoil reason or completing the timer read. Add a concurrency test that forces a retune against a read.

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

3. ~~Probe output breaks event parsers~~ ✓ Resolved 📘 Rule violation ◔ Observability

Description

emit() writes space-delimited BUSY records with printf() instead of JSON Lines beginning with
an ev field. Automated or concurrent consumers receive non-JSON machine events on stdout, and the
formatted output is not assembled and emitted through the configured atomic event stream.

Code

tests/busy_window_probe.cpp[R102-106]

+  std::printf("BUSY mode=%-9s i=%2d valid=%d busy=%u%% src=%-6s window_us=%u "
+              "armed_us=%u spoil=%-11s own_tx=%d tx_frames=%u sent=%u\n",
+              mode, i, b.valid ? 1 : 0, b.busy_pct, source_name(b.source),
+              b.window_us, armed_us, spoil_name(b.spoil),
+              b.own_tx_in_window ? 1 : 0, b.own_tx_frames, sent);

Evidence
Rule 6 requires machine events to be JSON Lines, begin with ev, use the configured event stream,
and be emitted atomically. The probe's documented machine-readable sample is a printf()-formatted
key-value line beginning with BUSY.
CLAUDE.md: Preserve the Structured Logging Contract: CLAUDE.md: Preserve the Structured Logging Contract: CLAUDE.md: Preserve the Structured Logging Contract: CLAUDE.md: Preserve the Structured Logging Contract: CLAUDE.md: Preserve the Structured Logging Contract: CLAUDE.md: Preserve the Structured Logging Contract: CLAUDE.md: Preserve the Structured Logging Contract: CLAUDE.md: Preserve the Structured Logging Contract: CLAUDE.md: Preserve the Structured Logging Contract: CLAUDE.md: Preserve the Structured Logging Contract: CLAUDE.md: Preserve the Structured Logging Contract: CLAUDE.md: Preserve the Structured Logging Contract: CLAUDE.md: Preserve the Structured Logging Contract: CLAUDE.md: Preserve the Structured Logging Contract: CLAUDE.md: Preserve the Structured Logging Contract: CLAUDE.md: Preserve the Structured Logging Contract
tests/busy_window_probe.cpp[98-108]
tests/busy_window_probe.cpp[1-8]

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 probe labels its sample output machine-readable but emits ad hoc key-value records rather than atomic JSON Lines through the structured event plane.
## Fix Focus Areas
- tests/busy_window_probe.cpp[98-108]
- tests/busy_window_probe.sh[96-156]
## Recommended Fix
Serialize each sample as one JSON object whose first field is `ev`, write the complete line with one atomic event-stream operation, and update the shell harness to parse the JSON fields. Keep human diagnostics on stderr.

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

View action required (6)

4. ~~Probe runs skip USB context teardown~~ ✓ Resolved 📘 Rule violation ☼ Reliability

Description

main() creates a DeviceSession after libusb_init() but never calls
session.adopt_context(ctx). Normal and early returns therefore let the session destroy the radio
and handle without calling libusb_exit() for the initialized context.

Code

tests/busy_window_probe.cpp[R170-177]

+  auto logger = std::make_shared<Logger>();
+  libusb_context *ctx = nullptr;
+  if (libusb_init(&ctx) < 0) {
+    std::fprintf(stderr, "libusb_init failed\n");
+    return 3;
+  }
+  devourer::DeviceSession session(logger);
+  libusb_device_handle *handle = libusb_open_device_with_vid_pid(ctx, vid, pid);

Evidence
Rule 9 requires the caller to call libusb_exit() after radio destruction, interface release, and
handle close. DeviceSession performs that sequence only for an adopted context, while the new
probe initializes ctx and never adopts it.
CLAUDE.md: Maintain Correct libusb Ownership and Teardown Order: CLAUDE.md: Maintain Correct libusb Ownership and Teardown Order: CLAUDE.md: Maintain Correct libusb Ownership and Teardown Order: CLAUDE.md: Maintain Correct libusb Ownership and Teardown Order: CLAUDE.md: Maintain Correct libusb Ownership and Teardown Order: CLAUDE.md: Maintain Correct libusb Ownership and Teardown Order: CLAUDE.md: Maintain Correct libusb Ownership and Teardown Order: CLAUDE.md: Maintain Correct libusb Ownership and Teardown Order: CLAUDE.md: Maintain Correct libusb Ownership and Teardown Order: CLAUDE.md: Maintain Correct libusb Ownership and Teardown Order: CLAUDE.md: Maintain Correct libusb Ownership and Teardown Order: CLAUDE.md: Maintain Correct libusb Ownership and Teardown Order: CLAUDE.md: Maintain Correct libusb Ownership and Teardown Order: CLAUDE.md: Maintain Correct libusb Ownership and Teardown Order: CLAUDE.md: Maintain Correct libusb Ownership and Teardown Order: CLAUDE.md: Maintain Correct libusb Ownership and Teardown Order
tests/busy_window_probe.cpp[170-181]
examples/common/DeviceSession.h[48-52]
examples/common/DeviceSession.h[79-99]

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 probe initializes a libusb context but does not transfer it to the RAII session, so its teardown omits `libusb_exit()`.
## Fix Focus Areas
- tests/busy_window_probe.cpp[170-181]
## Recommended Fix
Immediately call `session.adopt_context(ctx)` after constructing `DeviceSession`, before any later early return. Preserve the session's existing radio-first, interface-release, handle-close, context-exit teardown order.

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

5. ~~Backend sensing facts live at root~~ ✓ Resolved 📘 Rule violation ⚙ Maintainability

Description

The root CLAUDE.md addition records Realtek and MediaTek arming mechanisms, shared-engine
behavior, and family-specific transmission bias. Because the same change maintains
generation-specific CLAUDE.md files for these facts, later backend updates can leave the broad
root summary inconsistent with its narrower guidance.

Code

CLAUDE.md[R469-472]

+families implement it (Realtek arms CLM alone, MediaTek resets its channel
+timers), and a window that something else disturbed — an NHM read re-arming the
+shared engine, a retune, a read before it elapsed — comes back INVALID carrying
+its reason rather than a plausible number. Own transmission is reported, not

Evidence
Rule 1 requires chip-, generation-, and subsystem-specific facts to live in the narrowest applicable
nested guidance. The root addition names backend mechanisms and behavior while explicitly
acknowledging the corresponding nested generation documents.
CLAUDE.md: Place Documentation in the Narrowest Applicable CLAUDE.md: CLAUDE.md: Place Documentation in the Narrowest Applicable CLAUDE.md: CLAUDE.md: Place Documentation in the Narrowest Applicable CLAUDE.md: CLAUDE.md: Place Documentation in the Narrowest Applicable CLAUDE.md: CLAUDE.md: Place Documentation in the Narrowest Applicable CLAUDE.md: CLAUDE.md: Place Documentation in the Narrowest Applicable CLAUDE.md: CLAUDE.md: Place Documentation in the Narrowest Applicable CLAUDE.md: CLAUDE.md: Place Documentation in the Narrowest Applicable CLAUDE.md: CLAUDE.md: Place Documentation in the Narrowest Applicable CLAUDE.md: CLAUDE.md: Place Documentation in the Narrowest Applicable CLAUDE.md: CLAUDE.md: Place Documentation in the Narrowest Applicable CLAUDE.md: CLAUDE.md: Place Documentation in the Narrowest Applicable CLAUDE.md: CLAUDE.md: Place Documentation in the Narrowest Applicable CLAUDE.md: CLAUDE.md: Place Documentation in the Narrowest Applicable CLAUDE.md: CLAUDE.md: Place Documentation in the Narrowest Applicable CLAUDE.md: CLAUDE.md: Place Documentation in the Narrowest Applicable CLAUDE.md
CLAUDE.md[466-475]
src/jaguar1/CLAUDE.md[92-101]
src/mt7612u/CLAUDE.md[20-40]

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 root guidance now contains backend-specific busy-window implementation and bias details even though nested generation guidance exists.
## Fix Focus Areas
- CLAUDE.md[466-475]
## Recommended Fix
Keep only the cross-cutting existence and purpose of the busy-window API in the root file. Move or retain Realtek, MediaTek, register-map, and transmission-bias details in the applicable nested `CLAUDE.md` files and link to them.

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

6. ~~Busy-window contract is documented twice~~ ✓ Resolved 📘 Rule violation ⚙ Maintainability

Description

The root CLAUDE.md repeats the ArmChannelBusy() arm/read contract, disturbance rules, and
transmission caveat already documented on IRadio. A later contract change can update the
authoritative header while leaving this second detailed copy stale for repository readers.

Code

CLAUDE.md[R466-469]

+That last one is what `IRadio::ArmChannelBusy(window_us)` addresses: arm a
+window where you reset your counters, read it when your dwell ends, and the
+busy figure covers that window instead of a 2 ms slice of it. Both silicon
+families implement it (Realtek arms CLM alone, MediaTek resets its channel

Evidence
Rule 2 requires CLAUDE guidance to point to authoritative documented headers instead of copying API
contracts. The root addition repeats details that the same PR adds to the IRadio::ArmChannelBusy()
doc comment.
CLAUDE.md: Do Not Duplicate Header Documentation in CLAUDE.md: CLAUDE.md: Do Not Duplicate Header Documentation in CLAUDE.md: CLAUDE.md: Do Not Duplicate Header Documentation in CLAUDE.md: CLAUDE.md: Do Not Duplicate Header Documentation in CLAUDE.md: CLAUDE.md: Do Not Duplicate Header Documentation in CLAUDE.md: CLAUDE.md: Do Not Duplicate Header Documentation in CLAUDE.md: CLAUDE.md: Do Not Duplicate Header Documentation in CLAUDE.md: CLAUDE.md: Do Not Duplicate Header Documentation in CLAUDE.md: CLAUDE.md: Do Not Duplicate Header Documentation in CLAUDE.md: CLAUDE.md: Do Not Duplicate Header Documentation in CLAUDE.md: CLAUDE.md: Do Not Duplicate Header Documentation in CLAUDE.md: CLAUDE.md: Do Not Duplicate Header Documentation in CLAUDE.md: CLAUDE.md: Do Not Duplicate Header Documentation in CLAUDE.md: CLAUDE.md: Do Not Duplicate Header Documentation in CLAUDE.md: CLAUDE.md: Do Not Duplicate Header Documentation in CLAUDE.md: CLAUDE.md: Do Not Duplicate Header Documentation in CLAUDE.md
CLAUDE.md[466-475]
src/IRadio.h[596-625]

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 root guidance duplicates detailed busy-window API behavior already documented in the authoritative `IRadio` header.
## Fix Focus Areas
- CLAUDE.md[466-475]
## Recommended Fix
Replace the duplicated arm/read, clamping, spoil, and transmission contract with a concise reference to `src/IRadio.h` and the sensing document. Keep the detailed API contract solely in the header.

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

7. ~~MediaTek hides self-transmitted reads~~ ✓ Resolved 🐞 Bug ≡ Correctness

Description

Mt7612uRadio::ArmChannelBusy does not capture _tx_submitted, and GetChannelBusy returns
busy_from_ch_time without populating own_tx_in_window or own_tx_frames. When the adapter
transmits during an armed interval, its timers include that airtime but consumers receive an
apparently uncontaminated reading.

Code

src/mt7612u/Mt7612uRadio.cpp[R903-906]

+  if (mt7612u_ch_time_arm(_dev) != 0)
+    return 0;
+  _busy_armed = true;
+  _busy_spoiled = false;

Evidence
The new contract says MediaTek channel timers count own transmission and introduces fields to expose
it, but the MediaTek arm state stores no TX baseline and the conversion helper leaves both fields at
their defaults despite an existing _tx_submitted counter.
src/RxSense.h[194-208]
src/RxSense.h[247-265]
src/mt7612u/Mt7612uRadio.cpp[854-907]
src/mt7612u/Mt7612uRadio.h[152-166]

Agent prompt

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

## Issue description
MediaTek armed windows count the adapter's own transmissions as busy but never populate the newly introduced self-transmission metadata, so contaminated readings appear clean.
## Fix Focus Areas
- src/mt7612u/Mt7612uRadio.cpp[854-907]
- src/mt7612u/Mt7612uRadio.h[152-166]
- src/RxSense.h[194-208]
## Recommended Fix
Store the submitted-frame counter when a MediaTek window is armed. When consuming that armed window, calculate a saturating delta and set `own_tx_frames` and `own_tx_in_window` on the returned `ChannelBusy`, using the same reset and overflow safeguards as the Realtek implementation.

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

8. ~~Fallback channel changes can hang the radio~~ ✓ Resolved 🐞 Bug ☼ Reliability

Description

RtlJaguarDevice::FastRetune() and FastSetBandwidth() retain busy_window_mutex() while their
declined fast paths call SetMonitorChannel(), which tries to lock the same non-recursive
std::mutex again. A cross-band, unsupported-width, or uncached retune therefore blocks the calling
control thread before the full channel configuration can run.

Code

src/jaguar1/RtlJaguarDevice.cpp[R1961-1963]

+  std::lock_guard<std::mutex> ccx(busy_window_mutex());
+  busy_window_note_retune();
if (_radioManagement->fast_retune(channel, cache_rf)) {

Evidence
Both fast methods hold the newly added busy-window guard through the fast-path operation and
subsequent fallback call to SetMonitorChannel(). That fallback independently acquires the same
mutex, while the base class declares it as a non-recursive std::mutex, establishing that a
declined fast path attempts to reacquire a lock already held by the same thread.
src/jaguar1/RtlJaguarDevice.cpp[1955-1972]
src/jaguar1/RtlJaguarDevice.cpp[1974-1988]
src/jaguar1/RtlJaguarDevice.cpp[1923-1931]
src/IRtlRadio.h[254-260]
src/jaguar1/RtlJaguarDevice.cpp[1923-1937]
src/jaguar1/RtlJaguarDevice.cpp[1955-1988]

Agent prompt

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

## Issue description
`RtlJaguarDevice::FastRetune()` and `FastSetBandwidth()` retain the non-recursive busy-window mutex while falling back to `SetMonitorChannel()`, which attempts to acquire the same mutex again and deadlocks the calling thread.
## Fix Focus Areas
- src/jaguar1/RtlJaguarDevice.cpp[1955-1988]
- src/jaguar1/RtlJaguarDevice.cpp[1923-1931]
- src/IRtlRadio.h[254-260]
## Recommended Fix
Limit the existing busy-window lock's scope to the fast hardware operation. If that operation declines, release the lock before calling `SetMonitorChannel()`; that method will reacquire the mutex itself, mark the busy window as spoiled, and hold the lock across the full fallback retune.

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

9. ~~Concurrent retunes can deadlock busy-window arms~~ ✓ Resolved 🐞 Bug ☼ Reliability

Description

RtlJaguar2Device::FastRetune() first takes busy_window_mutex() for a preliminary spoil
notification and subsequently waits for _reg_mu, while ArmChannelBusy() reaches with_ccx()
which takes _reg_mu before the CCX mutex. If each operation obtains its first lock first, both
wait indefinitely for the other lock.

Code

src/jaguar2/RtlJaguar2Device.cpp[R941-944]

+  {
+    std::lock_guard<std::mutex> ccx(busy_window_mutex());
+    busy_window_note_retune();
+  }

Evidence
The new preliminary block acquires CCX before the register mutex. The shared arm/read access path
acquires the same locks in the reverse order, forming a direct two-thread lock cycle.
src/jaguar2/RtlJaguar2Device.cpp[937-952]
src/jaguar2/RtlJaguar2Device.h[213-224]
src/IRtlRadio.h[116-124]

Agent prompt

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

## Issue description
`FastRetune()` takes the CCX mutex before `_reg_mu` in its newly added preliminary spoil block, reversing the established `_reg_mu` then CCX ordering used by `with_ccx()`. An arm and retune on separate threads can therefore deadlock.
## Fix Focus Areas
- src/jaguar2/RtlJaguar2Device.cpp[937-952]
- src/jaguar2/RtlJaguar2Device.h[213-224]
## Recommended Fix
Remove the preliminary CCX-only spoil block. Keep the existing notification after acquiring `_reg_mu` and then the CCX mutex; it already marks an armed window before the hardware retune and preserves one consistent lock order.

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

10. ~~Restarted receivers reuse stale windows~~ ✓ Resolved 🐞 Bug ≡ Correctness

Description

Mt7612uRadio::ArmChannelBusy sets _busy_armed, but StopRxLoop and the following StartRxLoop
do not clear that state even though startup resets the hardware timers. If RX is stopped and
restarted between arm and read, GetChannelBusy consumes the post-restart interval as though it
began at the caller's original arm.

Code

src/mt7612u/Mt7612uRadio.cpp[R905-906]
+  _busy_armed = true;
+  _busy_spoiled = false;

Evidence

Arming persists only in the new software flags, while RX startup calls mt7612u_link_stats_start()
to establish a new hardware interval and teardown never resets those flags. GetChannelBusy
subsequently consumes the retained flag without detecting the lifecycle boundary.

src/mt7612u/Mt7612uRadio.cpp[416-422]
src/mt7612u/Mt7612uRadio.cpp[467-501]
src/mt7612u/Mt7612uRadio.cpp[854-907]

Agent prompt

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

## Issue description
MediaTek retains the software armed-window state while RX teardown and startup replace the corresponding hardware timer interval, allowing a later read to be attributed to an obsolete arm.
## Fix Focus Areas
- src/mt7612u/Mt7612uRadio.cpp[416-422]
- src/mt7612u/Mt7612uRadio.cpp[467-501]
- src/mt7612u/Mt7612uRadio.cpp[854-907]
## Recommended Fix
Clear or explicitly invalidate `_busy_armed` and `_busy_spoiled` under `_mu` whenever RX is torn down or its channel timers are reinitialized during startup. Ensure the first read after restart follows the ordinary sampled path rather than claiming the earlier explicit arm.

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

11. ~~A zero request secretly arms timers~~ ✓ Resolved 🐞 Bug ≡ Correctness

Description

Mt7612uRadio::ArmChannelBusy calls mt7612u_ch_time_arm and sets _busy_armed even when
window_us is zero, then returns zero to signal that nothing was armed. A caller following the
public fallback contract therefore makes its next sampled read consume the newly reset explicit
interval instead.

Code

src/mt7612u/Mt7612uRadio.cpp[R897-900]

+uint32_t Mt7612uRadio::ArmChannelBusy(uint32_t window_us) {
+  if (!_rx_active.load(std::memory_order_acquire))
+    return 0;
+  std::lock_guard<std::recursive_mutex> lock(_mu);

Evidence

The public method documents zero as meaning the backend did not arm a window, and the Realtek
implementation explicitly performs no writes for a zero request. MediaTek instead resets the timer
hardware, marks the window armed, and returns the unchanged zero argument.

src/IRadio.h[596-625]
src/BusyWindow.h[88-98]
src/mt7612u/Mt7612uRadio.cpp[897-907]

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 MediaTek backend returns the contract's no-arm sentinel for a zero request only after resetting and marking the channel timers as explicitly armed.
## Fix Focus Areas
- src/mt7612u/Mt7612uRadio.cpp[897-907]
- src/IRadio.h[596-625]
- src/BusyWindow.h[88-98]
## Recommended Fix
Return zero immediately when `window_us == 0`, before taking hardware action or changing busy-window state. Add a test asserting that a zero request neither resets the timers nor changes the following sampled-read behavior.

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

12. ~~Spoiled windows report the wrong reason~~ ✓ Resolved 🐞 Bug ◔ Observability

Description

ClmWindow::read checks the ready bit before the existing spoil_ value and overwrites that value
with NotElapsed when readiness is clear. If a caller reads early after an NHM interruption or
retune, the result loses the first recorded cause and reports only that the replacement or original
window has not elapsed.

Code

src/BusyWindow.h[R117-119]

+      spoil_ = BusySpoil::NotElapsed;
+      last_spoil_ = spoil_;
+      b.spoil = spoil_;

Evidence

The note methods deliberately retain the first spoil reason, and the self-test states that
first-reason precedence is required. The read path nevertheless assigns NotElapsed before
consulting the retained value, defeating that precedence whenever the ready bit is clear.

src/BusyWindow.h[112-125]
src/BusyWindow.h[149-159]
tests/busy_window_selftest.cpp[352-365]

Agent prompt

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

## Issue description
Realtek busy-window reads overwrite an already recorded interruption or retune with `NotElapsed` because readiness is evaluated before the stored spoil reason.
## Fix Focus Areas
- src/BusyWindow.h[112-125]
- src/BusyWindow.h[149-159]
- tests/busy_window_selftest.cpp[352-365]
## Recommended Fix
After reading the result register, return the existing non-`None` spoil reason before assigning `NotElapsed`. Extend the precedence tests to cover interrupted-plus-not-ready and retuned-plus-not-ready windows.

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

View review recommended (1)

13. ~~Telemetry can shorten a caller's busy window~~ ✓ Resolved 🐞 Bug ≡ Correctness

Description

ArmChannelBusy() resets the MediaTek channel timers through mt7612u_ch_time_arm(), but
mt7612u_link_stats() can read-and-clear those same timer registers before GetChannelBusy()
consumes them. Whenever link statistics are polled during an armed dwell, the later busy result
covers only the remainder while remaining valid and carrying the full arm-to-read interval.

Code

src/mt7612u/Mt7612uRadio.cpp[R903-907]

+  if (mt7612u_ch_time_arm(_dev) != 0)
+    return 0;
+  _busy_armed = true;
+  _busy_spoiled = false;
+  return window_us;

Evidence
The new arm deliberately clears and starts the channel timers. Both the channel-time reader used by
GetChannelBusy() and the separate link-statistics API read the same read-and-clear busy and idle
registers; the existing API documentation explicitly warns that using both splits counts.
src/mt7612u/Mt7612uRadio.cpp[897-907]
src/mt7612u/init.cpp[623-640]
src/mt7612u/init.cpp[645-671]
src/mt7612u/init.cpp[673-683]
src/mt7612u/include/mt7612u/mt7612u.h[433-446]

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 MediaTek busy-window arm shares read-and-clear channel timers with `mt7612u_link_stats()`. A link-statistics poll during the window consumes the counters without invalidating the busy window, so `GetChannelBusy()` reports an understated value for the entire elapsed interval.
## Fix Focus Areas
- src/mt7612u/Mt7612uRadio.cpp[897-907]
- src/mt7612u/init.cpp[623-640]
- src/mt7612u/init.cpp[673-683]
## Recommended Fix
Add explicit ownership or invalidation for channel timers while an IRadio busy window is armed. Ensure `mt7612u_link_stats()` cannot consume those timers during that interval, or record the interference so `GetChannelBusy()` returns an invalid spoiled reading rather than a partial measurement.

ⓘ 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 turn on the rule miner and Qodo learns your standards from review history

More tips ↗ | Customize Qodo ↗ | Qodo docs ↗

Qodo raised thirteen findings on #437 and three more review passes of my own found seven others. The ones that could hand a consumer a wrong number, all on the MediaTek path, which had an armed-window flag but none of the state a reading needs: - No deadline, so an early read returned a valid short window where the Realtek path refuses one. The requested length is recorded at arm and the C layer's interval mark measures against it. - The armed flag was cleared in one critical section and the timers read in another; a retune landing in the gap found nothing to spoil and the post-retune remainder came back valid. One locked section now, and _rx_active is re-checked inside it — the old check-then-lock let StopRxLoop finish in between, and the C layer answers regardless, which is the fabricated zero the gate exists to prevent. - own_tx_in_window/own_tx_frames were never populated there although these timers count own transmission as busy. A TX baseline is taken at arm. - An armed window survived an RX stop/start that re-zeroes the hardware timers under it, and a zero-length request returned 0 — the "cannot arm" answer — while arming anyway. The MediaTek decision moved out of the backend into a pure devourer::busy_from_ch_time_window() next to the conversion it qualifies, so those rules are now covered headlessly (the backend itself needs a radio): every refusal, the elapsed boundary both ways, own-TX including a counter that went backwards, and an armed window over dead counters. A self-deadlock of my own making: Jaguar1's FastRetune/FastSetBandwidth held busy_window_mutex() across the tune, and their declined fast paths call SetMonitorChannel(), which takes that same non-recursive mutex. One rule, two shapes now — where a family has a register lock that spans the change the note sits inside it and with_ccx's ordering does the rest; Jaguar1 has none, so the note is scoped and the single-control-thread contract carries it. Jaguar2's FastRetune also kept a leftover pre-lock note that took the CCX mutex BEFORE _reg_mu, the inversion with_ccx is ordered against. Deleted. Two claims of mine were simply false and are now either true or gone: - I described the disturbed-counter hazard as a 1 Hz telemetry poll stealing the shared timers. Nothing in Mt7612uRadio polls mt7612u_link_stats(); only a C-API caller that uses both does (tools/bringup.cpp). The guard stays, because such a caller really can take the counts, but it says so accurately — and it no longer breaks the public C signature: mt7612u_ch_time() keeps its four parameters and the flag has its own accessor, with both new symbols added to the api_link surface guard. - The saturation test claimed to cover the clamp while passing without it: at a tick count just over the period, rounding lands on 100 either way. It overshoots far enough to kill the mutant now (an unclamped build reports 102). Also: the neutral ArmChannelBusy contract said the return value is the caller's denominator, which the MediaTek path contradicts by returning the request unchanged — the contract now describes both shapes and points at ChannelBusy::window_us as the one authoritative window. Spoil precedence agrees across families (an interruption outranks "not elapsed", because the short interval is a symptom of it). The probe emits JSON Lines through the shared EventSink instead of a bespoke text format, adopts the libusb context, and its frame literal no longer relies on implicit zeros. The harness picks the family numerically rather than by the spelling of the VID, runs the load-free spoil arms on both families, and three of its assertions could pass vacuously — an empty extractor result, a stale arm whose records never appeared, and a spread comparison that a handful of unlucky 2 ms samples can satisfy by being uniformly wrong. Each fails loudly now. The root CLAUDE.md keeps a pointer rather than a second copy of a contract documented on IRadio. 69/69 ctest, ASan+UBSan clean on both selftests. On air: the Realtek harness passes steady and bursty on an 8812AU (armed mean 9% spread 19 against the sampled read's 75, which missed the load in 5 of 6 windows), and the MediaTek sensor now passes the quiet floor plus the retune and early-read refusals. The ready-bit assumption every Realtek refusal rests on is checked on air by the early and stale arms, 6/6 each, and that is recorded in docs/rx-spectrum-sensing.md rather than asserted. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01SbJ75HT4fqof7e5e8NKL2j

snokvist · 2026-09-20T08:47:47Z

Review round pushed as c749eb7. All thirteen findings addressed; three more passes of my own found seven others, and the PR description now carries a Review round section with the detail.

The four that could hand a consumer a wrong number were all on the MediaTek path, which had an armed-window flag but none of the state a reading needs — no deadline, a lock gap between clearing the flag and reading the timers, no own-TX reporting, and an armed window surviving an RX restart. That decision now lives in a pure busy_from_ch_time_window() next to the conversion it qualifies, so every one of those rules is covered headlessly rather than only by hand on hardware.

Two things worth calling out as corrections rather than fixes:

Finding 13's premise was mine and it was wrong. I had described the disturbed-counter hazard as a 1 Hz telemetry poll stealing the shared timers. Nothing in Mt7612uRadio polls mt7612u_link_stats() — only a C-API caller that uses both does, which tools/bringup.cpp is. The guard stays because such a caller genuinely can take the counts, but it says so accurately now, and it no longer breaks the public C signature: mt7612u_ch_time() keeps its four parameters, the flag has its own accessor, and both new symbols are in the api_link surface guard.
A self-deadlock I introduced while fixing an earlier review comment: Jaguar1's FastRetune/FastSetBandwidth held the CCX mutex across the tune, and their declined fast paths re-enter SetMonitorChannel() on the same non-recursive mutex.

Findings 5 and 6 (doc placement, contract duplicated) are taken as written — the root file keeps a pointer, and the contract lives once on the IRadio declaration.

69/69 ctest, ASan+UBSan clean. On air the Realtek harness passes steady and bursty on an 8812AU, and the MediaTek sensor now passes the quiet floor plus the retune and early-read refusals. Its loaded arms are still skipped by the harness — that adapter's bring-up does not complete on an already-saturated channel, so those numbers come from a manual run with the sensor started first, and the script says so rather than passing quietly.

The probe's no-destructor exit used POSIX _exit through <unistd.h>, which MSVC has no header for (C1083 in the windows-latest job). std::_Exit from <cstdlib> has the same semantics and is what examples/sense already uses. Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com>

josephnef

Review: on-air on the final code, plus a harness trap and two contract gaps

On air, this bench, head c749eb7 — tests/busy_window_probe.sh with a T3U (8822BU) txdemo flooder on ch165, so all three sensors saw the same load. These are the dies the PR says its final code was not re-verified on.

sensor	harness	armed 240 ms spread	sampled spread
Jaguar3 RTL8812CU	PASS	63 pt (harness trap, below)	2 pt
Jaguar2 RTL8822BU	PASS	1 pt	4 pt
Jaguar1 RTL8821AU	PASS	0 pt	4 pt

Every spoil arm refused with its reason on all three (interrupted / retuned / not-elapsed), the stale re-arm never returned the latched value, the cross-thread GetRxQuality() race arm was refused 5/5, and txsess stayed valid with own_tx set. The 8821AU is a die the PR did not measure; it behaves like the 8812AU. Build + 69/69 ctest locally.

The Jaguar3 spread is a harness trap, not the sensor

The first armed windows after bring-up read a valid 0% on a 64% channel (samples 1–2: 0, 30, then 63/63/63), and the bursty Jaguar3 run failed load separates from floor: 3 < 5 for the same reason. Chased through five scripted arms:

not chip idle time (15 s idle before InitWrite → clean from the first 50 ms window);
not the CLM-only arm sequence (a priming read_nhm looked like it fixed it, but so did simply running second);
reproducible 5/5 as "first sensor bring-up within ~5 s of the flooder launch" (seven 50 ms windows at 0, then 59–66) and 0/5 with the flooder left running;
sensor first, flooder second: the flooder's first bulk_send lands at window #37, the sensor sees load only from window #99–100 — the 8822BU flooder does not air at level for ~4 s after its first submit.

start_flood sleeps 4 s, the Jaguar3 brings up in ~0.6 s, so its first arm lands exactly on that edge; the MT7612U flooder on the author's bench starts instantly, so this never showed there. Gate start_flood on a sampled read that shows load rather than a fixed sleep. Why the 8822BU takes ~4 s to radiate at level after its first submit is its own question (thermal/pwr-track first tick? DIG/EDCCA onset?) and worth its own issue.

Blocking

MSVC red — tests/busy_window_probe.cpp included <unistd.h> for _exit (C1083). Pushed the fix: std::_Exit, the tree's precedent in examples/sense.
A not-elapsed read consumes the window. ClmWindow::read clears armed_ before the ready check, so the natural retry falls through to the sampled path and comes back valid=true, window_us=2000 — and on the JGR3 map that read_nhm re-arm destroys the hardware window still counting. IRadio.h says an early read "comes back INVALID", never that the window is gone. Keep it armed on NotElapsed (the MediaTek path has the same consume-on-any-verdict shape: "consumed, whatever the verdict").

Should fix

No bring-up gate on the Realtek arm. with_ccx writes BB registers and returns true unconditionally; every family already has _brought_up (the CCA gates check it). An arm before Init reaches busy_window_reset() vanishes silently.
Jaguar1 retune note is scoped before the tune with the lock released. send_packet runs FastRetune on the TX thread for a radiotap CHANNEL hop, so an arm landing between the note and the tune reads back a valid two-channel blend. Cheapest close: note after the tune as well.
MT7612U StopRxLoop clears _rx_active after the unlocked ring teardown (up to ~2 s), so GetChannelBusy's under-lock recheck passes with MAC RX already off → the valid ~0% the gate exists to prevent. Clear it (or a quiesced flag) in the same _mu scope as the quiesce.
mt7612u_link_stats_start's new comment says "its own disturbance flag is cleared below"; nothing clears it there (only ch_time_arm and the accessor do).
busy_from_ch_time_window: an armed, elapsed, undisturbed window over 0/0 timers returns valid=false, spoil=None — the "no sensor" shape the header says an armed window must never produce; report Interrupted like the failed-read branch does.

Docs (current-state rule)

docs/rx-spectrum-sensing.md:331 still says "Jaguar1 is unmeasured" ten lines after "Jaguar1 is measured now"; the TX-session section is corrected by an annotation 150 lines later instead of being rewritten.
src/rtl8733b/Rtl8733bDevice.cpp:1044 still claims "no CCX/NHM register set" — the PR body's own 8733B measurement refutes that; "not ported" is the true statement.
"now measured" / "is now true" phrasing in RtlJaguarDevice.cpp, Mt7612uRadio.cpp, src/mt7612u/CLAUDE.md — git is the changelog.
Low: own_tx_* keys on TxStats.submitted, so Jaguar1's queued async URBs (≤256) can air inside a window unflagged.

…iew found Review round on the busy window, with the on-air re-run on the final code (Jaguar3 8812CU, Jaguar2 8822BU, Jaguar1 8821AU; steady and bursty). - A read before the window elapsed no longer consumes it. Consuming it sent the natural retry down the sampled path, which re-arms the engine for 2 ms and reports that as a valid reading, and on the JGR3 map destroys the window still counting. On the MediaTek the refusal is decided from the host-side arm time BEFORE the timers are read, because those are read-and-clear and a read that reached them would take the counts. - with_ccx lends nothing before bring-up on all three Realtek families. - Jaguar1 notes the retune on both sides of the tune: it has no register lock spanning it, and send_packet's radiotap CHANNEL hop reaches FastRetune from the TX thread, so an arm between the note and the tune read back a valid two-channel blend. - MT7612U GetChannelBusy/ArmChannelBusy also refuse on _rx_stop: StopRxLoop clears _rx_active only after the unlocked ring teardown, and in between the MAC answered with a plausible 0%. - mt7612u_link_stats_start clears the disturbed flag its comment claimed it cleared; an armed, elapsed MediaTek window over dead timers reports a lost window rather than "no sensor". - The harness waits for the flooder to air at level before the loaded arms: a Jaguar2 txdemo does not radiate at level for ~4 s after its first submit (sensor-first measurement), and the fixed 4 s sleep put the Jaguar3's first arm on that edge, where it read a VALID 0%. - Docs and caps comments read as current state: Jaguar1 measured on both dies, the RTL8733B has a CLM engine that is not ported rather than absent. Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com>

josephnef · 2026-09-20T10:51:19Z

Pushed f61fcd4 addressing the review above (plus f7df523 for MSVC), and re-ran the harness on it with the gated flooder:

sensor	harness	armed 240 ms spread	sampled spread
Jaguar3 RTL8812CU, steady	PASS	1 pt	2 pt
Jaguar2 RTL8822BU, steady	PASS	1 pt	4 pt
Jaguar1 RTL8821AU, steady	PASS	1 pt	2 pt
Jaguar3 RTL8812CU, bursty 50/450	PASS (was FAIL)	armed mean 15%, sampled missed the burst 4/5

69/69 ctest. The 8822BU flooder onset that caused the earlier zeros is filed as #438. Remaining from the review and deliberately not changed here: own_tx_* keyed on submitted rather than completed frames (low, Jaguar1 async path only).

josephnef

Review items addressed in f61fcd4; on-air re-run PASS on Jaguar1/2/3 (steady + bursty), 69/69 ctest, CI green.

OpenIPC#437's caps note recorded that this die has a working CLM engine devourer reports as absent. It does. Ported and measured. The register block is in this tree: hal/hal8733b_tables.c:792 onward, the shipped 8733B phy_reg init table, programs 0x1e40, 0x1e44, 0x1e48, 0x1e5c and 0x1e60 at bring-up — the CCX block at the JGR3 addresses. The vendor agrees: at the pinned reference/rtl8733bu-20230626, hal/phydm/phydm_pre_define.h:513 lists ODM_RTL8733B in PHYDM_IC_SUPPORT_IFS_CLM and :523-525 define PHYDM_IC_JGR3_SERIES_SUPPORT when RTL8733B_SUPPORT is set. That init table is also the only other writer of those registers — nothing in src/rtl8733b/ touches them at runtime — so the masked read-modify-writes cannot corrupt another subsystem, and arm_clm_only never touches 0x1e5c where the table leaves a non-zero value. with_ccx() lends the engine nhm_regs_jgr3() under _reg_mu then the CCX mutex, the way Jaguar2/3 do. No new machinery. BUSY AIRTIME WITHOUT A PHYDM COUNTER BLOCK. arm_clm_only/read_clm_only take no IGI argument — busy airtime is a hardware tick count, not a histogram referenced to the receiver's own floor the way NHM's thresholds are — so CLM ports cleanly to a backend that implements no GetRxEnergy. This is the first backend where the two caps split: busy_airtime_ok true, rx_energy_ok false. The sampled path still reports NO READING here; the armed window is the only way a number comes out. AdapterCaps.h's busy_airtime_ok definition is amended to say so — it promised that GetChannelBusy answers, which on this die is true only after ArmChannelBusy. An out-of-tree consumer that sampled on the strength of that flag must now handle "no reading" from a backend whose flag is true. KNOWN GAP, pre-existing, with one affected consumer. src/sensing/SenseWindow.h picks its source by whether the IRtlRadio* is non-null rather than by rx_energy_ok, so on this backend it takes the phydm branch, gets the all-invalid base GetRxEnergy and never calls GetChannelBusy(). That is the wrong discriminator AdapterCaps.h opens by warning about. examples/chanscout hits it: main.cpp:323 builds an IRtlRadio* by dynamic_cast, which succeeds on this die, and hands it to a DwellExecutor at :342 — so a scout here reports neither CLM nor NHM. (An earlier draft of this message and of the PR body claimed nothing constructs SenseWindow outside tests. That was false; chanscout does.) Gating that branch on the capability is necessary but NOT sufficient: an unarmed GetChannelBusy() falls back to busy_from_rx_energy(GetRxEnergy(true)) and yields nothing here, and nothing in src/sensing/ ever calls ArmChannelBusy — the only callers in the tree are tests/busy_window_probe.cpp. A complete fix is the gate plus an arm, and arming needs an observation window that no layer between ScanPlanConfig::dwell_ms and SenseWindow carries. It would also change behaviour on four other backends, where an arm interacts with the NHM read that spoils the same engine — which is what tests/dwell_exec_selftest.cpp exists to pin. That belongs in src/sensing/ with its own tests, not in a backend port. Documented in docs/rx-spectrum-sensing.md and src/rtl8733b/CLAUDE.md. MEASURED, tests/busy_window_probe.sh, RTL8733BU sensor against an MT7612U flooder on ch165: quiet 0.0% steady load 69%, spread 0 over 6 windows bursty 50/450 ms mean 10%, true duty ~9% retune mid-window refused, spoil=retuned read before elapsed refused, spoil=not-elapsed re-arm then read never returns the previous latched value sensor transmitting valid, own_tx flagged A Jaguar3 8812CU on that same flooder read 69% too, so the two device paths onto the JGR3 map agree on one load. busy_airtime_measured is set on that evidence, per the house rule. The Interrupted spoiler is unreachable on this die by construction: with no NHM read there is nothing that can re-arm the shared engine mid-window. The other two fire normally. The CCA gate cannot bias the reading either — SetCcaMode throws on this backend, so a CCA-disabled session is unreachable rather than merely unlikely. LOCKING AND LIFECYCLE. Both retune notes are scoped. FastRetune calls SetMonitorChannel on its declined path while already holding _reg_mu — recursive, so that part is fine — and the CCX mutex is NOT recursive, so holding it across the tune would self-deadlock there. One note per site suffices because _reg_mu spans the whole tune and with_ccx takes _reg_mu first, so a concurrent arm cannot interleave: the Jaguar2/3 situation, not Jaguar1's. The Init/InitWrite resets sit INSIDE _reg_mu for the same reason. with_ccx reads _phy_ready under _reg_mu rather than before it, which the Jaguar2/3 overrides do not: it is a plain bool written under that lock, so the earlier read was a data race and a TOCTOU against a concurrent Stop(). Stop() forgets the window too, which is not obvious: a stop is not a retune, but clearing _phy_ready does not protect an armed window either, because SetMonitorChannel and FastRetune both call bring_up_to_phy(), which sets that flag true again. Without the reset, a window armed before a stop revives on the brought-up chip and the retune note hands back a Retuned spoil earned by a session that no longer exists. Jaguar1/2/3 share this hole and have it worse — none of their Stop() implementations resets the window, and none clears the flag their with_ccx gates on — noted in the chip guide so the asymmetry is not read as an 8733B quirk. HARNESS. busy_window_probe.sh gated its NHM and sampled arms on the sensor's USB VID, which calls this chip Realtek and then runs arms it cannot pass. It now gates on rx_energy_ok from the probe's own caps record — the capability, not the identity, which is the discriminator AdapterCaps.h argues for. A caps value it cannot parse is a loud failure, not a silent skip: defaulting to "no GetRxEnergy" would switch off the sampled arm, all three NHM arms and the bursty comparison while still printing PASS. Each skip branch now states how many arms it bypasses (the NHM branch skips three, not one; the MediaTek exit seven, not one) and the bursty comparison is counted as a check rather than an arm, so the verdict's number means "arms that did not run". Also fixed: expect_all printed "ok" unconditionally, so a failing arm reported its FAILs and then said ok underneath them. Verified: ctest 69/69 (67 run, 2 MediaTek generator tests skipped), ASan/UBSan clean on both selftests, and five on-air runs — RTL8733BU steady (PASS, 4 arms skipped) and bursty (PASS, 4 arms + 1 check), Jaguar3 8812CU PASS with every arm running and zero skips (so the gate discriminates rather than disabling), MT7612U sensor against an 8812CU flooder PASS on the quiet arms (7 arms skipped), and a mutation renaming the caps field fails loudly where it would previously have printed PASS. The Stop() reset is compile- and reasoning-verified rather than re-measured: its triggering sequence, a Stop followed by a retune with no re-Init, is out of contract and there is no mock seam for the device class. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01SbJ75HT4fqof7e5e8NKL2j

…acts (#439) Follow-up to #437. **No production code** — two regression tests and three documentation facts that PR overtook. ## The two rules it pins #437's busy window has two asymmetries that decide whether a spoiled reading refuses or lies. Both are currently held by comments only: I mutated this tree and the suite passes with either one inverted. **1. `ClmWindow::read` consumes a SPOILED window but keeps a NOT-ELAPSED one.** Deleting the `armed_ = false` from the spoil branch — the tidy "consume once, at the top" simplification — passes every existing check in `busy_window_selftest.cpp`. Consuming is what hands the caller back to the sampled path. `IRtlRadio::GetChannelBusy` takes the armed branch on `armed()`, and `spoil_` is sticky (only `arm()` clears it), so a spoiled window left armed strands every later call there returning invalid-Retuned — the sampled fallback unreachable for every unrelated consumer until something re-arms or resets. Not a wrong number: a sensor that stops answering. **2. `busy_from_ch_time_window` reports a lost window only when one was ARMED.** Dropping the `w.armed &&` passes every existing check in `channel_busy_selftest.cpp`, while telling every sampled caller its window was lost when it never armed one. "No sensor here" and "your window died" are the two facts the `spoil` field exists to separate. A third, smaller gap closes with them: the existing retune block asserts on `w.last_spoil()`, so a branch reporting every spoil as `Interrupted` passes the whole suite. What a consumer acts on is `ChannelBusy::spoil`. Each mutation is caught by exactly one new assertion: | mutation | fails | |---|---| | never consume a spoiled window | `spoiled: window consumed` | | hardcode the reason to `Interrupted` | `spoiled: with its reason` (got 1 want 2) | | drop `w.armed &&` | `and no window to have lost` | I deleted a fourth assertion I had written — it could not fail while the line above it passed, because `read()` returns at the `!armed_` guard before touching the mock. ## The three stale facts - **`AdapterCaps.h`** names Jaguar1 and the MT7612U as the false-as-unmeasured cases for `busy_airtime_measured`. #437 measured both and flipped them — all four backends that set `busy_airtime_ok` now set `measured` too. It also cited `tests/ccx_clm_probe.sh`, which separates a *carrier* from a quiet channel on the sampled read; the armed window's harness is `tests/busy_window_probe.sh`. This is the worst place in the tree to be stale, since it is where the flags are defined. - **`docs/rx-spectrum-sensing.md:130`** still labels the MT7612U busy-airtime leg "unvalidated", which the same page contradicts two sections later and the backend contradicts with `busy_airtime_measured = true`. ## Verification 69/69 ctest, ASan/UBSan clean on both selftests. As a **no-change control** — this commit touches no production code, so the run re-verifies #437's — `tests/busy_window_probe.sh` PASS on an 8812AU sensor against an MT7612U flooder on ch165: | arm | armed 240 ms | sampled | |---|---|---| | steady | 71%, spread 0 | spread 8 | | bursty 50/450 | mean 9% (true duty ~9%), spread 19 | missed the load 5/6, spread 70 | 🤖 Generated with [Claude Code](https://claude.com/claude-code) https://claude.ai/code/session_01SbJ75HT4fqof7e5e8NKL2j Co-authored-by: Claude Opus 5 (1M context) <noreply@anthropic.com>

The RTL8733B has a working CCX CLM engine. #437's caps note recorded that and left the port as a follow-up; this is it. ## Where the register block comes from In this tree: `hal/hal8733b_tables.c:792` onward — the shipped 8733B phy_reg init table — programs `0x1e40`, `0x1e44`, `0x1e48`, `0x1e5c` and `0x1e60` at bring-up. That is the CCX block at the JGR3 addresses, on this die, checkable without leaving the repo. The vendor agrees: at the pinned `reference/rtl8733bu-20230626`, `hal/phydm/phydm_pre_define.h:513` lists `ODM_RTL8733B` in `PHYDM_IC_SUPPORT_IFS_CLM`, and `:523-525` define `PHYDM_IC_JGR3_SERIES_SUPPORT` when `RTL8733B_SUPPORT` is set. That init table is also the **only other writer** of those registers — nothing in `src/rtl8733b/` touches them at runtime — so the masked read-modify-writes cannot corrupt another subsystem's state, and `arm_clm_only` never touches `0x1e5c`, where the table leaves a non-zero value. `with_ccx()` lends the engine `nhm_regs_jgr3()` under `_reg_mu` then the CCX mutex, the way Jaguar2/3 do. No new machinery. ## Busy airtime without a phydm counter block `arm_clm_only`/`read_clm_only` take no IGI argument — busy airtime is a hardware tick count, not a histogram referenced to the receiver's own noise floor the way NHM's thresholds are. So CLM ports cleanly to a backend that implements no `GetRxEnergy`. **This is the first backend where the two caps split**: `busy_airtime_ok` true, `rx_energy_ok` false. The sampled path still reports *no reading* here; an armed window is the only way a number comes out. `AdapterCaps.h`'s `busy_airtime_ok` definition is amended accordingly — it promised that `GetChannelBusy` answers, which on this die is true only after `ArmChannelBusy`. ## Measured `tests/busy_window_probe.sh`, RTL8733BU sensor against an MT7612U flooder on ch165: | arm | result | |---|---| | quiet | 0.0% | | steady load | **69%**, spread 0 over 6 windows | | bursty 50/450 ms | mean 9-10%, true duty ~9% | | retune mid-window | refused, `spoil=retuned` | | read before elapsed | refused, `spoil=not-elapsed` | | re-arm then read | never returns the previous latched value | | sensor transmitting | valid, `own_tx` flagged | A Jaguar3 8812CU on the same flooder read **69%** too, so the two device paths onto the JGR3 map agree on one load. `busy_airtime_measured` is set on that evidence, per the house rule. The `Interrupted` spoiler is unreachable on this die by construction: with no NHM read, nothing can re-arm the shared engine mid-window. The CCA gate cannot bias the reading either — `SetCcaMode` throws on this backend, so a CCA-disabled session is unreachable rather than merely unlikely. ## Known gap, pre-existing, with one affected consumer `src/sensing/SenseWindow.h` picks its source by whether the `IRtlRadio*` is non-null rather than by `rx_energy_ok`, so on this backend it takes the phydm branch, gets the all-invalid base `GetRxEnergy`, and never calls `GetChannelBusy()`. That is the same wrong discriminator `AdapterCaps.h` opens by warning about. **`examples/chanscout` hits it.** `main.cpp:323` builds an `IRtlRadio*` by `dynamic_cast`, which succeeds on this die, and hands it to a `DwellExecutor` at `:342` — so a scout on an RTL8733B reports neither CLM nor NHM. *(An earlier revision of this description claimed nothing constructs `SenseWindow` outside tests. That was false, and it was the premise this section rested on. Corrected.)* Gating that branch on the capability is **necessary but not sufficient**: an unarmed `GetChannelBusy()` falls back to `busy_from_rx_energy(GetRxEnergy(true))` and yields nothing here, and nothing in `src/sensing/` ever calls `ArmChannelBusy` — the only callers in the tree are `tests/busy_window_probe.cpp`. A complete fix is the gate **plus an arm**, and arming needs an observation window that no layer between `ScanPlanConfig::dwell_ms` and `SenseWindow` carries. It would also change behaviour on four other backends, where an arm interacts with the NHM read that spoils the same engine — which is what `tests/dwell_exec_selftest.cpp` exists to pin. So this is a real gap with a real consumer, and I do not think it belongs in a backend port: the fix is a `src/sensing/` design change needing its own tests and its own review. The defect is pre-existing — on master the same die already produces the same nothing — and leaving `busy_airtime_ok` false to mask it would be a lie about the silicon in a header whose whole point is that the flag describes hardware. Documented in `docs/rx-spectrum-sensing.md` and `src/rtl8733b/CLAUDE.md`. Say the word if you would rather have it here. **One thing for out-of-tree consumers:** `busy_airtime_ok`'s definition changes meaning slightly — it now says the backend *has* an engine and explicitly does not promise an unarmed `GetChannelBusy()` answers. Anything that sampled on the strength of that flag must handle "no reading" from a backend whose flag is true. ## Locking Both retune notes are scoped. `FastRetune` calls `SetMonitorChannel` on its declined path while already holding `_reg_mu` — recursive, so that part is fine — and the CCX mutex is **not** recursive, so holding it across the tune would self-deadlock there. One note per site suffices because `_reg_mu` spans the whole tune and `with_ccx` takes `_reg_mu` first, so a concurrent arm cannot interleave: the Jaguar2/3 situation, not Jaguar1's. The `Init`/`InitWrite` window resets sit **inside** `_reg_mu` for the same reason — ahead of it, an arm landing between the released CCX lock and `_reg_mu` would survive the bring-up the reset exists to forget. `with_ccx` reads `_phy_ready` under `_reg_mu` rather than before it, which the Jaguar2/3 overrides do not: it is a plain bool written under that lock, so the earlier ordering was a data race and a TOCTOU against a concurrent `Stop()`. ## Harness `busy_window_probe.sh` gated its NHM and sampled arms on the sensor's USB VID, which calls this chip Realtek and then runs three arms it cannot pass. It now gates on `rx_energy_ok` from the probe's own caps record — the capability, not the identity, which is the discriminator `AdapterCaps.h` argues for. A caps value it cannot parse is a **loud failure, not a silent skip**: defaulting to "no GetRxEnergy" would switch off the sampled arm, all three NHM arms and the bursty comparison while still printing PASS. The verdict line now carries the skip count for the same reason. Also fixed: `expect_all` printed "ok" unconditionally, so a failing arm reported its FAILs and then said ok underneath them. ## Verification ctest 69/69 (67 run, 2 MediaTek generator tests skipped), ASan/UBSan clean on both selftests, and five on-air runs: | run | result | |---|---| | RTL8733BU steady | PASS — 69%, spread 0, 4 arms skipped | | RTL8733BU bursty 50/450 | PASS — mean 10% vs ~9% true duty, 4 arms + 1 check skipped | | Jaguar3 8812CU | PASS — every arm ran, **zero skips**, 69% | | MT7612U sensor vs 8812CU flooder | PASS — quiet arms, 7 arms skipped | | caps-field rename mutation | **fails loudly** where it previously printed PASS | The `Stop()` window reset is compile- and reasoning-verified rather than re-measured: its triggering sequence — a `Stop` followed by a retune with no re-`Init` — is out of contract, and there is no mock seam for the device class. Lock order and re-entrancy for it were checked against every `busy_window_mutex()` site in the tree. Note: #439 edits the adjacent `busy_airtime_measured` paragraph in the same `AdapterCaps.h` comment block. Whichever lands second needs a trivial rebase. 🤖 Generated with [Claude Code](https://claude.com/claude-code) https://claude.ai/code/session_01SbJ75HT4fqof7e5e8NKL2j --------- Co-authored-by: Claude Opus 5 (1M context) <noreply@anthropic.com> Co-authored-by: Joseph <162703152+josephnef@users.noreply.github.com>

qodo-free-for-open-source-projects Bot reviewed Sep 20, 2026

View reviewed changes

josephnef requested changes Sep 20, 2026

View reviewed changes

josephnef mentioned this pull request Sep 20, 2026

Jaguar2 8822BU TX does not air at level for ~4 s after its first submitted frame #438

Open

josephnef approved these changes Sep 20, 2026

View reviewed changes

josephnef merged commit af13106 into master Sep 20, 2026
24 checks passed

josephnef deleted the feat/ccx-busy-window branch September 20, 2026 11:01

This was referenced Sep 20, 2026

sensing: pin the busy window's two asymmetries, and three stale doc facts #439

Merged

sensing: port the CCX CLM busy window to the RTL8733B #440

Merged

Navigation Menu

Uh oh!

sensing: a busy-airtime window the caller defines - #437

sensing: a busy-airtime window the caller defines#437
josephnef merged 4 commits into
masterfrom
feat/ccx-busy-window

snokvist commented Sep 20, 2026 •

edited

Loading

Uh oh!

qodo-free-for-open-source-projects Bot commented Sep 20, 2026

Uh oh!

qodo-free-for-open-source-projects Bot commented Sep 20, 2026 •

edited

Loading

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

snokvist commented Sep 20, 2026

Uh oh!

josephnef left a comment

Uh oh!

josephnef commented Sep 20, 2026

Uh oh!

josephnef left a comment

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

2 participants

Uh oh!

Conversation

snokvist commented Sep 20, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Why

What

Validation

Three results that cut against the feature

Deliberately not here

Follow-ups

Review round

Uh oh!

qodo-free-for-open-source-projects Bot commented Sep 20, 2026

PR Summary by Qodo

Uh oh!

qodo-free-for-open-source-projects Bot commented Sep 20, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Code Review by Qodo

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

snokvist commented Sep 20, 2026

Uh oh!

josephnef left a comment

Choose a reason for hiding this comment

Review: on-air on the final code, plus a harness trap and two contract gaps

The Jaguar3 spread is a harness trap, not the sensor

Blocking

Should fix

Docs (current-state rule)

Uh oh!

josephnef commented Sep 20, 2026

Uh oh!

josephnef left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

2 participants

snokvist commented Sep 20, 2026 •

edited

Loading

qodo-free-for-open-source-projects Bot commented Sep 20, 2026 •

edited

Loading