From 675fec28312f86e1ff9530a8634c44f8fdbc6c69 Mon Sep 17 00:00:00 2001 From: snokvist Date: Sun, 20 Sep 2026 15:00:39 +0200 Subject: [PATCH 1/2] sensing: port the CCX CLM busy window to the RTL8733B MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit 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) Claude-Session: https://claude.ai/code/session_01SbJ75HT4fqof7e5e8NKL2j --- docs/rx-spectrum-sensing.md | 55 +++++++++++---- src/AdapterCaps.h | 24 +++++-- src/rtl8733b/CLAUDE.md | 100 +++++++++++++++++++++++++- src/rtl8733b/Rtl8733bDevice.cpp | 89 +++++++++++++++++++++-- src/rtl8733b/Rtl8733bDevice.h | 52 ++++++++++++++ tests/busy_window_probe.sh | 120 ++++++++++++++++++++++++++++---- 6 files changed, 397 insertions(+), 43 deletions(-) diff --git a/docs/rx-spectrum-sensing.md b/docs/rx-spectrum-sensing.md index d019f887..de005626 100644 --- a/docs/rx-spectrum-sensing.md +++ b/docs/rx-spectrum-sensing.md @@ -127,13 +127,15 @@ hardware it runs on: |---|---|---| | returns | `ChannelBusy` — busy airtime + energy-above-floor | `RxEnergy` — the phydm counter set | | available on | any backend with a hardware busy-airtime counter | Realtek only | -| today | Jaguar1/2/3 (CCX CLM), MT7612U (MAC channel timers) | Jaguar1/2/3, Kestrel (floor only) | -| not available | Kestrel, RTL8733B — both report *no reading*, never zero | RTL8733B, MT7612U | +| today | Jaguar1/2/3 and RTL8733B (CCX CLM), MT7612U (MAC channel timers) | Jaguar1/2/3, Kestrel (floor only) | +| not available | Kestrel — reports *no reading*, never zero | RTL8733B, MT7612U | Advertised statically by `AdapterCaps::busy_airtime_ok` / `busy_airtime_measured` / `rx_energy_ok`. **Do not use a successful `dynamic_cast` as the discriminator** — it was never correct: the -RTL8733B derives from `IRtlRadio` and implements no energy reader at all. +RTL8733B derives from `IRtlRadio` and implements no energy reader at all — +while nonetheless answering `GetChannelBusy()` through CLM, so the two flags +have to be read separately rather than inferred from each other. `ChannelBusy` carries its own `source` (`Clm` or `ChTime`) because the two facilities define busy differently: the MediaTek timers count TX+RX+NAV+EIFS, @@ -360,6 +362,7 @@ The facilities differ by generation but all three read the same fields: | Jaguar1 (8812/8821/8814) | yes | classic AC — FA 0xF48/0xA5C, CCA 0xF08, IGI 0xC50; NHM 0x994/0x990/0x998/0xfa8/0xfb4 | | Jaguar2 (8822BU/8821CU) | yes | classic AC (FA/CCA sampled by the DIG thread; same NHM map) | | Jaguar3 (8822CU/8822EU) | yes | newer BB — CCA 0x2c08, CCK-FA 0x1a5c, OFDM-FA 0x2d0x, IGI 0x1d70; NHM 0x1e60/0x1e40/0x1e44/0x2d40/0x2d4c | +| RTL8733B (8731BU/8733BU) | **CLM only** | the JGR3 CCX map (ctrl 0x1e60, period 0x1e40, result 0x2d88). No phydm FA/CCA block and no NHM here — CLM needs no IGI reference, so it ports alone | ## The armed busy window (`ArmChannelBusy`) @@ -414,11 +417,11 @@ quick-connect decision has. Three things spoil one, all measured, and each makes the reading come back invalid with a reason (`ChannelBusy::spoil`) rather than plausible: -| spoiler | Jaguar1 (11AC) | Jaguar3 (JGR3) | -|---|---|---| -| 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 on the 250 ms window used for that measurement) | -| a retune mid-window | 60-62% where the channel was 71% | 44-47% where it was 61% | -| reading before it elapsed | the result register latches the PREVIOUS window; reads are non-destructive, so an early read is a stale number wearing a fresh timestamp | +| spoiler | Jaguar1 (11AC) | Jaguar3 (JGR3) | RTL8733B (JGR3) | +|---|---|---|---| +| 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 on the 250 ms window used for that measurement) | unreachable — no NHM reader on this die, so nothing can re-arm the shared engine | +| a retune mid-window | 60-62% where the channel was 71% | 44-47% where it was 61% | refused with `spoil=retuned` | +| reading before it elapsed | the result register latches the PREVIOUS window; reads are non-destructive, so an early read is a stale number wearing a fresh timestamp | as the other two: refused with `spoil=not-elapsed` | The third row rests on one silicon behaviour nothing else here depends on: **triggering CLM clears the ready bit**, so a window that has not finished @@ -471,13 +474,35 @@ where a result at or above the period reports 100%. ### Coverage -`ArmChannelBusy` works on Jaguar1, Jaguar2 and Jaguar3 (CLM) and on the -MT7612U (channel timers). Under one flooder on one channel the three Realtek -families and the MediaTek independently measured the same load at 61-71% — the -spread is antenna and receiver gain, not a units disagreement. Kestrel and the -RTL8733B return 0 from the arm (the CCX engine is not wired up on either — the -RTL8733B has a working one, see the caps comment in its device source), and -their callers keep the sampled path. +`ArmChannelBusy` works on Jaguar1, Jaguar2, Jaguar3 and the RTL8733B (CLM) +and on the MT7612U (channel timers). Under one flooder on one channel the +Realtek families and the MediaTek independently measured the same load at +61-71% — the spread is antenna and receiver gain, not a units disagreement. +The RTL8733B and a Jaguar3 8812CU read the same load at 69% apiece on the same +flooder, across the two different device paths onto the same JGR3 map. Kestrel +returns 0 from the arm — its CCX engine is not wired up — and its callers keep +the sampled path. + +The RTL8733B is the one backend where an armed window is the ONLY way to get a +number: it implements no `GetRxEnergy`, so its sampled path reports no reading +by design. One consequence to know before building on it — `src/sensing/` +cannot reach that reading yet. `SenseWindow::read` chooses its source by +whether the `IRtlRadio*` is non-null rather than by `rx_energy_ok`, so on a +backend that derives from `IRtlRadio` and implements no energy reader it takes +the phydm branch and never calls `GetChannelBusy()`. That is the same wrong +discriminator `AdapterCaps.h` warns about, and it predates this port — the +RTL8733B is simply the first backend that makes it observable — and +`examples/chanscout` does hit it, through a `dynamic_cast` that +succeeds on this die. Gating the branch on the capability is necessary but not +sufficient: nothing in `src/sensing/` arms, and an unarmed read yields nothing +here, so the complete fix is the gate plus an arm with an observation window +the sensing layer does not yet carry. Until then a caller on this die should +use `IRadio::ArmChannelBusy`/`GetChannelBusy` directly. + +`tests/busy_window_probe.sh` skips its sampled and NHM arms for that reason, +gating on `rx_energy_ok` from the probe's own caps record rather +than on the sensor's USB VID — a Realtek chip with CCX and no phydm counters +is exactly what a VID test gets wrong. Both harness runs above used an MT7612U flooder. With a Jaguar2 (8822BU) `txdemo` flooder the same harness read a **valid 0%** for the first ~600 ms of diff --git a/src/AdapterCaps.h b/src/AdapterCaps.h index 772dd20d..9ac81f84 100644 --- a/src/AdapterCaps.h +++ b/src/AdapterCaps.h @@ -275,16 +275,30 @@ struct AdapterCaps { * RTL8733B derives from IRtlRadio and implements no GetRxEnergy at all, so * the cast reports a sensor that returns nothing. * - * busy_airtime_ok: IRadio::GetChannelBusy returns a real busy-airtime - * reading — the Realtek CCX CLM engine (Jaguar1/2/3) or the MediaTek MAC - * channel timers. FALSE on Kestrel (its NHM rides the halbb glue, not - * NhmReader, so it has no CLM) and on the RTL8733B. + * The two flags below are also independent of each other, and the RTL8733B + * is where that stops being theoretical: it has a working CCX CLM engine + * and no phydm FA/CCA block, so busy_airtime_ok is true while rx_energy_ok + * is false. Do not read either from the other. + * + * busy_airtime_ok: the backend HAS a hardware busy-airtime engine that + * IRadio::GetChannelBusy can report — the Realtek CCX CLM engine + * (Jaguar1/2/3 and the RTL8733B, which the vendor phydm puts on the JGR3 + * map) or the MediaTek MAC channel timers. FALSE on Kestrel, whose NHM + * rides the halbb glue rather than NhmReader, so it has no CLM. + * + * It does NOT promise that an unarmed GetChannelBusy() answers. On the + * RTL8733B it does not: that backend feeds its sampled path from + * GetRxEnergy, which it does not implement, so an unarmed call reports no + * reading and IRadio::ArmChannelBusy is the only way to get a number out of + * it. A consumer that wants a reading from an arbitrary backend should arm + * rather than sample; one that samples must handle "no reading" from a + * backend whose flag is true. * * busy_airtime_measured: that reading has been separated from a quiet * channel ON AIR for this family, not merely implemented. The harness is * tests/busy_window_probe.sh, which pits an armed window against the quiet * floor under a known load, and against each way a window can be spoiled. - * True today on all four backends that set busy_airtime_ok. The flag stays + * True today on all five backends that set busy_airtime_ok. The flag stays * because the two facts are independent: a port can land the engine before * anyone has run it on air, and false-as-unmeasured is the house rule for * that state. diff --git a/src/rtl8733b/CLAUDE.md b/src/rtl8733b/CLAUDE.md index a34509bb..951cb485 100644 --- a/src/rtl8733b/CLAUDE.md +++ b/src/rtl8733b/CLAUDE.md @@ -240,7 +240,8 @@ untouched) and fall back to the full path. ## Not ported -`ReadTsf`/beacons, A-MPDU, CCX / `tx.report` per-frame TX outcomes, +`ReadTsf`/beacons, A-MPDU, CCX `tx.report` per-frame TX outcomes (the +TX-report engine — CCX **CLM** busy airtime IS ported, see below), `FastSetBandwidth`, the flat-index / per-rate-diff TX-power knobs (`SetTxPowerIndexOverride`, `SetTxPowerRateDiffs`, `ReApplyTxPower` — only the relative `SetTxPowerOffsetQdb` is ported), `rx.path` per-chain telemetry, @@ -369,7 +370,10 @@ The retry-12 shortfall from the ideal 13 may be passive-monitor loss or genuinely fewer airings, so the ratio is reported as an observation, not an exact hardware count. -**CCX / `tx.report` is NOT ported, and its root cause is unresolved.** The +**CCX `tx.report` is NOT ported, and its root cause is unresolved.** This is +the per-frame TX-outcome engine, a different thing from the CCX **CLM** +busy-airtime measurement documented under "Frame-free sensing" below, which +is ported and measured. The descriptor and receive-side investigation narrows the problem but does not prove a firmware defect: SPE_RPT is dword2[19] and SW_DEFINE dword6[11:0] via the generic halmac @@ -421,3 +425,95 @@ Headless coverage: `tests/rtl8733b_{efuse,phy_table,rx_parse,tx_desc}_selftest.c in `ctest`. Hardware: `tests/rtl8733b_lifecycle_soak.sh` (bounded warm lifecycle, explicitly not a true VBUS cycle) and `examples/rtl8733bprobe` (staged identity → power/EFUSE → firmware → MAC/PHY → TSSI audit). + +## Frame-free sensing: CLM busy airtime, and nothing else + +`IRadio::ArmChannelBusy()` / `GetChannelBusy()` work on this die. The vendor +phydm puts the 8733B on JGR3 — 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. The engine +answers on that register map through +the shared `devourer::ClmWindow` — `with_ccx()` in `Rtl8733bDevice.h` lends it +`nhm_regs_jgr3()` under `_reg_mu` then the CCX mutex, exactly as Jaguar2/3 do. + +The flag relationship and the armed-only behaviour are the contract, and it +lives at `src/AdapterCaps.h` on `busy_airtime_ok` / `rx_energy_ok` — the one +place it can be kept true. What is specific to this die: + +- **Why CLM ports without a phydm block at all.** `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. That is what makes it separable from FA/CCA here. +- There is no NHM read on this die, so the `Interrupted` spoiler is + unreachable by construction: nothing can re-arm the shared engine + mid-window. `Retuned` and `NotElapsed` both fire normally. + +Measured with `tests/busy_window_probe.sh` (RTL8733BU sensor, MT7612U flooder, +ch165): **0.0% quiet, 69% under a steady load, 10% against a 50/450 ms burst +whose true duty is ~9%**, spread 0 on the steady arm. A Jaguar3 8812CU read +that same flooder at 69% as well, so the two device paths onto the JGR3 map +agree on one load. The retune and premature-read spoilers each refused with +their reason, and a re-armed window never returned the previous latched value. + +**`src/sensing/` cannot reach this reading, and that is a pre-existing bug +this die is the first to expose.** `SenseWindow::read` branches on whether the +`IRtlRadio*` is non-null, not on whether the backend has phydm counters: + +```cpp +if (rtl_) { r.energy = rtl_->GetRxEnergy(...); r.busy = busy_from_rx_energy(r.energy); } +else if (radio_) { r.busy = radio_->GetChannelBusy(); } +``` + +The RTL8733B derives from `IRtlRadio`, so it takes the first branch, gets the +all-invalid base `GetRxEnergy`, and `GetChannelBusy()` is never called. That +is exactly the "a successful `dynamic_cast` is not a correct +discriminator" failure `AdapterCaps.h` opens with, living in the sensing +layer. It is not new — it would mis-handle any backend with busy airtime and +no phydm block — but before this port no such backend existed, so nothing +exposed it. **`examples/chanscout` is affected**: it builds a +`DwellExecutor` with `dynamic_cast(dev)` +(`examples/chanscout/main.cpp:323,342`), which succeeds on this die, so a +scout here reports neither CLM nor NHM. + +Gating the branch on `rx_energy_ok` is necessary but NOT sufficient: an +unarmed `GetChannelBusy()` on this backend falls back to +`busy_from_rx_energy(GetRxEnergy(true))` and yields nothing, 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 capability gate PLUS an +arm, and arming needs an observation window that no layer between +`ScanPlanConfig::dwell_ms` and `SenseWindow` carries at all — `DwellExecConfig` +stamps a `settle_ms` but has no field for the dwell the window would be sized +to. That is a design decision for `src/sensing/`, with its own tests, not a +rider on a backend port. +**Until then, a caller on this die must reach +`IRadio::ArmChannelBusy`/`GetChannelBusy` directly.** + +**The CCA gate cannot bias the reading here.** CLM counts CCA-busy, so a +session with CCA disabled would under-report — but `SetCcaMode` throws "CCA +disable is not implemented by this backend" on this die, so that state is +unreachable rather than merely unlikely. + +**`Stop()` forgets the window, and that is not obvious.** A stop is not a +retune, so nothing would spoil an armed window — but clearing `_phy_ready` +does not protect it either, because `SetMonitorChannel` and `FastRetune` both +call `bring_up_to_phy()`, which sets that flag true again. Without a reset in +`Stop()`, a window armed before a stop comes back to life on the revived chip +and the retune note hands the caller a `Retuned` spoil earned by a hardware +session that no longer exists: invalid either way, but the reason would be a +lie. The reset is scoped under the `_reg_mu` `Stop()` already holds. + +Jaguar1/2/3 share this hole and have it worse — none of their `Stop()` +implementations resets the window either, and unlike this backend none of them +clears the flag their `with_ccx` gates on, so a `GetChannelBusy()` straight +after `Stop()` reads CCX registers on a deinitialised chip with no retune +needed. Not fixed here; noted so the asymmetry is not mistaken for an 8733B +quirk. + +Retune notes live in `SetMonitorChannel` and `FastRetune`, both **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 — holding it across the tune would self-deadlock on that path. One +note per site suffices because `_reg_mu` is held across the whole tune and +`with_ccx` takes `_reg_mu` first, so a concurrent arm cannot interleave; that +is the Jaguar2/3 situation, not Jaguar1's. diff --git a/src/rtl8733b/Rtl8733bDevice.cpp b/src/rtl8733b/Rtl8733bDevice.cpp index 0c4b0943..d19013f9 100644 --- a/src/rtl8733b/Rtl8733bDevice.cpp +++ b/src/rtl8733b/Rtl8733bDevice.cpp @@ -174,6 +174,20 @@ void Rtl8733bDevice::Init(Action_ParsedRadioPacket packetProcessor, try { { std::lock_guard lock(_reg_mu); + /* Bring-up forgets any armed busy window: the BB is reprogrammed and + * retuned below, so a window armed against the previous state describes + * a chip that no longer exists. + * + * INSIDE _reg_mu — which is held across the whole bring-up — and scoped + * within it so the CCX lock is never held across a call that re-enters + * this class. Ahead of _reg_mu the reset would be close to pointless: + * with_ccx takes _reg_mu first, so an arm landing between the released + * CCX lock and _reg_mu would survive the very bring-up the reset exists + * to forget. Same argument as SetMonitorChannel's note below. */ + { + std::lock_guard ccx(busy_window_mutex()); + busy_window_reset(); + } bring_up_to_phy(); if (!_phy.set_channel(channel)) throw std::runtime_error("RTL8733B channel configuration failed"); @@ -221,6 +235,11 @@ void Rtl8733bDevice::Init(Action_ParsedRadioPacket packetProcessor, void Rtl8733bDevice::InitWrite(SelectedChannel channel) { try { std::lock_guard lock(_reg_mu); + /* Same as Init, and inside _reg_mu for the same reason. */ + { + std::lock_guard ccx(busy_window_mutex()); + busy_window_reset(); + } bring_up_to_phy(); if (!_phy.set_channel(channel)) throw std::runtime_error("RTL8733B channel configuration failed"); @@ -468,6 +487,23 @@ void Rtl8733bDevice::StartRxLoop( void Rtl8733bDevice::SetMonitorChannel(SelectedChannel channel) { std::lock_guard lock(_reg_mu); + /* A window armed before this retune would integrate across the channel + * change and report the blend of two channels as one channel's occupancy. + * + * Inside _reg_mu, which is held across the whole tune below, and which + * with_ccx takes BEFORE the CCX lock — so a concurrent arm cannot land + * between this note and the tune. That is the Jaguar2/3 situation, not + * Jaguar1's, and it is why one note suffices here rather than a pair + * bracketing the tune. + * + * Scoped, and never held across the tune: FastRetune below calls this + * function on its declined path while already holding _reg_mu (which is + * recursive, so that part is fine), and the CCX mutex is NOT recursive — + * holding it across the tune would self-deadlock on that path. */ + { + std::lock_guard ccx(busy_window_mutex()); + busy_window_note_retune(); + } bring_up_to_phy(); const bool was_tx_ready = _tx_ready; _tx_ready = false; @@ -493,6 +529,14 @@ void Rtl8733bDevice::FastRetune(uint8_t channel, bool cache_rf) { std::lock_guard lock(_reg_mu); if (_phy_ready && channel == _channel.Channel) return; + /* Noted AFTER the same-channel early return above: that path tunes nothing, + * so spoiling a window there would refuse a measurement that was never + * disturbed. Scoped for the reason SetMonitorChannel spells out — the + * declined path below calls it, and the CCX mutex is not recursive. */ + { + std::lock_guard ccx(busy_window_mutex()); + busy_window_note_retune(); + } SelectedChannel target = _channel; target.Channel = channel; if (_phy_ready && @@ -994,6 +1038,21 @@ void Rtl8733bDevice::SetCcaMode(bool disabled) { void Rtl8733bDevice::Stop() { std::lock_guard lock(_reg_mu); + /* The armed window dies with the session. Clearing _phy_ready below is not + * enough on its own: SetMonitorChannel and FastRetune both call + * bring_up_to_phy(), which sets it true again, so a window armed before a + * Stop would come back to life on the revived chip — and the retune note + * would hand the caller a Retuned spoil earned by a hardware session that + * no longer exists. Invalid either way, but the reason would be a lie. + * + * Scoped, and taken under _reg_mu (already held) in the house order. Never + * held across the teardown I/O below: nothing Stop() calls re-enters this + * class, but the CCX mutex is non-recursive and this is not the place to + * start depending on that. */ + { + std::lock_guard ccx(busy_window_mutex()); + busy_window_reset(); + } _rx_stop = true; _device.quiesce_tx(); _tx_ready = false; @@ -1041,13 +1100,29 @@ devourer::AdapterCaps Rtl8733bDevice::GetAdapterCaps() { caps.marketing_names = "RTL8731BU/RTL8733BU"; caps.chip_id = rtl8733b::kChipId; caps.generation = devourer::ChipGeneration::Rtl8733b; - /* No frame-free sensing ported: the phydm FA/CCA block and the CCX engine - * are not wired up here, and GetRxEnergy is not overridden — so both report - * false rather than letting the IRtlRadio cast imply a sensor. The die does - * have a working CLM engine on the JGR3 map (62-63% under a ~63% load, - * 0.0% quiet, one unit); its FA/IGI registers are unmeasured, so the port is - * its own change. */ - caps.busy_airtime_ok = false; + /* Busy airtime yes, phydm counters no — and this die is the first backend + * where those two split. + * + * CCX CLM is ported (see with_ccx), on the JGR3 map the vendor puts this + * die on (phydm_pre_define.h:513 and :523-525 at the pinned + * reference/rtl8733bu-20230626). It needs no IGI reference — + * arm_clm_only/read_clm_only take none, because busy airtime is a tick + * count rather than a histogram referenced to the receiver's own floor — so + * it works here even though the phydm FA/CCA block is not wired up and + * GetRxEnergy is not overridden. The sampled path consequently still + * reports NO READING; the armed window (IRadio::ArmChannelBusy) is how a + * caller gets a number out of this backend. + * + * rx_energy_ok stays false, which is the half of the old caps note that was + * always right: the IRtlRadio cast would imply an FA/CCA/IGI reader that + * does not exist here. */ + caps.busy_airtime_ok = true; + /* Separated on air through tests/busy_window_probe.sh on an RTL8733BU + * against an MT7612U flooder on ch165: 0% quiet, 69% under a steady load, + * and 10% against a 50/450 ms burst whose true duty is ~9%. A Jaguar3 + * 8812CU on the same flooder read 69% too, so the two register paths agree + * on one load. Every spoiler refused with its reason. */ + caps.busy_airtime_measured = true; caps.rx_energy_ok = false; caps.variant = "cut-selected"; caps.transport = _device.is_usb() ? "usb" : "unknown"; diff --git a/src/rtl8733b/Rtl8733bDevice.h b/src/rtl8733b/Rtl8733bDevice.h index 49c553c3..84a3d8a5 100644 --- a/src/rtl8733b/Rtl8733bDevice.h +++ b/src/rtl8733b/Rtl8733bDevice.h @@ -94,6 +94,58 @@ class Rtl8733bDevice : public IRtlRadio { devourer::FwBootStatus GetFwBootStatus() override; private: + /* This generation's CCX map and register access, under its locks — see + * IRtlRadio::with_ccx. Private: the base class calls it, nobody else. + * + * The JGR3 map, and the proof 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, on this die. 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 these registers: + * nothing in src/rtl8733b/ touches 0x1e40-0x1e60 or 0x2d88 at runtime, + * so the masked read-modify-writes below cannot corrupt another + * subsystem's state. arm_clm_only never touches 0x1e5c, where the table + * leaves a non-zero value. + * + * CLM is reachable here even though GetRxEnergy is not overridden: + * arm_clm_only() and read_clm_only() take no IGI argument, because busy + * airtime is a tick count and needs no receiver-noise reference the way + * NHM's IGI-referenced thresholds do. So this backend answers + * GetChannelBusy through an armed window while its sampled path still + * reports no reading, and rx_energy_ok stays false. */ + bool with_ccx(const CcxFn &fn) override { + std::lock_guard reg(_reg_mu); + /* Nothing to lend before the BB is programmed: a window armed against it + * would be forgotten by Init/InitWrite's reset. _phy_ready is this + * family's _brought_up, and it goes true the moment _phy.initialize() + * succeeds rather than at the end of bring_up_to_phy() — which is the + * right point for THIS question, because everything after it is MAC-level + * (the ACK window, the ACK responder) and does not bear on whether a BB + * register is safe to touch. Cleared by Stop(). + * + * Read UNDER _reg_mu, unlike the Jaguar2/3 overrides this is modelled on. + * It is a plain bool written under that lock by bring_up_to_phy() and + * Stop(), so checking it before taking the lock is both a data race and a + * TOCTOU: the check passes, the lock then blocks behind a concurrent + * Stop(), and the register access below proceeds against a card that has + * just been powered down. Costs nothing to do it in the right order. */ + if (!_phy_ready) + return false; + std::lock_guard ccx(busy_window_mutex()); + const Read32 rd = [this](uint16_t a) { + return _device.rtw_read(a); + }; + const SetBb wr = [this](uint16_t a, uint32_t m, uint32_t v) { + _device.phy_set_bb_reg(a, m, v); + }; + fn(devourer::nhm_regs_jgr3(), rd, wr); + return true; + } + void bring_up_to_phy(); bool configure_tx_power(SelectedChannel channel); /* Fill one [txdesc][frame] block at `out`. `agg_num` is the USB TX diff --git a/tests/busy_window_probe.sh b/tests/busy_window_probe.sh index 09e9c443..9b7ead1f 100755 --- a/tests/busy_window_probe.sh +++ b/tests/busy_window_probe.sh @@ -14,12 +14,22 @@ # window armed, under load -- the feature. # sampled unarmed, same load -- the spread comparison. # interrupt NHM read mid-window -- must be INVALID, spoil=interrupted. +# quality GetRxQuality() mid-window -- the same trap by the route a +# consumer actually takes. INVALID, spoil=interrupted. +# race GetRxQuality() hammered from a second thread -- must never +# yield a SHORT window wearing a valid flag. # retune retune mid-window -- must be INVALID, spoil=retuned. # early read before elapsed -- must be INVALID, spoil=not-elapsed # (both families: Realtek from the CCX ready bit, MediaTek from # the window length recorded at arm). +# stale re-arm, then read early -- must not return the latched result +# of the PREVIOUS window. # txsess sensor transmits -- must stay VALID and be flagged own_tx. # +# Nine arms. The skip counts below are stated against this list, so if you +# add one, add it here too or the verdict's "N arm(s) skipped" stops being +# auditable. +# # sudo tests/busy_window_probe.sh # SENSOR_PID=0xc812 FLOOD_PID=0x8812 CHANNEL=100 sudo tests/busy_window_probe.sh # DUTY_ON=50 DUTY_OFF=450 sudo tests/busy_window_probe.sh # bursty arm @@ -55,6 +65,7 @@ mkdir -p "$OUT" rx_flag=""; [ "$SENSOR_RX" = "1" ] && rx_flag="--rx" fails=0 skips=0 +checks_skipped=0 stop_flood() { pkill -x txdemo 2>/dev/null; sleep 1; } trap 'stop_flood' EXIT INT TERM @@ -163,12 +174,29 @@ run_arm() { # mode, label | tee "$OUT/$mode.log" | grep -E '"ev":"busy\.' } +# How many arms a skip branch bypasses, stated at the branch rather than +# assumed to be one. The verdict line reports this, so an undercount is a +# claim that more of the suite ran than did. +skip_arms() { skips=$((skips + $1)); } + +caps_field() { # file, field -> that field from the probe's busy.caps record + python3 -c " +import json,sys +for line in open(sys.argv[1]): + line = line.strip() + if not line.startswith('{'): continue + try: r = json.loads(line) + except ValueError: continue + if r.get('ev') == 'busy.caps': print(r.get(sys.argv[2])); break +" "$1" "$2" +} + # A spoiled arm must produce NO valid readings; a working arm must produce # only valid ones. Both directions matter: a probe that passes whatever the # hardware does is not a test. expect_all() { # mode, valid(0|1), [spoil] local mode="$1" want_valid="$2" want_spoil="${3:-}" - local n bad + local n bad before="$fails" # `grep -c` prints 0 and EXITS 1 when it matches nothing, so a `|| echo 0` # here appended a second line and made this guard dead: an empty log then # reported "ok: 0 samples". The whole point of the guard is the empty case. @@ -198,7 +226,11 @@ expect_all() { # mode, valid(0|1), [spoil] echo "FAIL $mode: $bad/$n samples not spoil=$want_spoil" fails=$((fails + 1)); } fi - echo " ok: $n samples valid=$want_valid ${want_spoil:+spoil=$want_spoil}" + # Only when nothing above failed. This line used to print unconditionally, + # so a failing arm reported its FAILs and then said "ok" underneath them. + [ "$fails" -eq "$before" ] && + echo " ok: $n samples valid=$want_valid ${want_spoil:+spoil=$want_spoil}" + return 0 } echo "== sensor $SENSOR_VID:$SENSOR_PID, flooder $FLOOD_VID:$FLOOD_PID, ch$CHANNEL" @@ -220,6 +252,34 @@ quiet_mean="$(stat_of quiet mean)" # with the sensor parked on the wrong channel. assert_le "quiet floor is quiet" "$quiet_mean" 5 +# Which of the arms below apply is a CAPABILITY question, not a chip-identity +# one. The NHM arms spoil a window by driving GetRxQuality -> GetRxEnergy, and +# the sampled arm reads the path GetRxEnergy feeds. A backend that implements +# no GetRxEnergy has nothing to interrupt the window WITH, and its sampled +# path reports no reading by design — the RTL8733B is exactly that: Realtek, +# CCX CLM working, no phydm FA/CCA block. A VID test calls it Realtek and runs +# arms it cannot pass, which is the same wrong discriminator AdapterCaps.h +# documents ("a successful dynamic_cast is not a correct +# discriminator and never was"). Ask the caps instead. +nhm_sensor=0 +rx_energy_cap="$(caps_field "$OUT/quiet.log" rx_energy_ok)" +case "$rx_energy_cap" in + True) nhm_sensor=1 ;; + False) nhm_sensor=0 ;; + *) + # Anything else means no caps record, or the field was renamed. Quietly + # treating that as "no GetRxEnergy" would skip the sampled arm, all three + # NHM arms AND the bursty comparison, and the run would still print PASS + # — a gate that switches its own tests off the moment it stops working. + # The arms it guards are the ones that catch a spoiled window being + # reported as data, so failing loudly is the only safe default. + echo "FAIL caps: rx_energy_ok not readable from the probe (got '$rx_energy_cap')" + echo " — refusing to decide which arms apply from a caps record this" + echo " harness cannot parse." + fails=$((fails + 1)) + ;; +esac + # A MediaTek sensor cannot be brought up on an already-saturated channel: its # firmware MCU times out and the arm never becomes available (measured). The # loaded arms therefore need the flooder started AFTER the sensor is up, which @@ -240,9 +300,13 @@ if [ "$realtek_sensor" != "1" ]; then echo " build/BusyWindowProbe --vid $SENSOR_VID --pid $SENSOR_PID \\" echo " --channel $CHANNEL --mode window --reps 45 --rx &" echo " # then start the flooder once it prints its first sample" - skips=$((skips + 1)) + # window, sampled, interrupt, quality, race, stale, txsess — everything + # below this exit. Four of them would be skipped by their own branches on + # this backend anyway, but "did not run" is what the verdict claims, so + # they are counted here. + skip_arms 7 echo - [ "$fails" -eq 0 ] && echo "busy_window_probe: PASS (quiet arms only, $skips skipped)" \ + [ "$fails" -eq 0 ] && echo "busy_window_probe: PASS (quiet arms only, $skips arm(s) skipped)" \ || echo "busy_window_probe: $fails FAILURE(S)" exit $((fails ? 1 : 0)) fi @@ -268,13 +332,20 @@ else assert_ge "load separates from floor" "$((window_mean - quiet_mean))" "$sep_min" fi -f0="$(flood_frames)" -run_arm sampled "shipped sampled read, same load" -expect_all sampled 1 -assert_flood_alive "$f0" "sampled" +if [ "$nhm_sensor" = "1" ]; then + f0="$(flood_frames)" + run_arm sampled "shipped sampled read, same load" + expect_all sampled 1 + assert_flood_alive "$f0" "sampled" +else + echo "== sampled arm skipped: this backend implements no GetRxEnergy, so" + echo " the sampled path reports NO READING by design — the armed window" + echo " is the only way a number comes out of it." + skip_arms 1 +fi f0="$(flood_frames)" -if [ "$realtek_sensor" = "1" ]; then +if [ "$nhm_sensor" = "1" ]; then run_arm interrupt "NHM read mid-window" expect_all interrupt 0 interrupted run_arm quality "GetRxQuality() mid-window — the trap a consumer springs" @@ -286,7 +357,10 @@ if [ "$realtek_sensor" = "1" ]; then run_arm race "GetRxQuality() hammered from another thread" expect_all race 0 interrupted else - echo "== NHM arms skipped (non-Realtek sensor has no NHM engine)" + echo "== NHM arms skipped: no GetRxEnergy on this backend, so there is no" + echo " NHM read that could re-arm the shared engine mid-window." + # interrupt, quality, race — three arms, not one. + skip_arms 3 fi if [ "$realtek_sensor" = "1" ]; then @@ -333,15 +407,22 @@ stop_flood echo echo "== spread: the point of the window" -for m in window sampled; do +arms="window"; [ "$nhm_sensor" = "1" ] && arms="window sampled" +for m in $arms; do printf " %-8s n=%s mean=%s%% min=%s%% max=%s%% spread=%s pts\n" \ "$m" "$(busy_values $m | wc -l)" "$(stat_of $m mean)" "$(stat_of $m min)" \ "$(stat_of $m max)" "$(stat_of $m spread)" done # The claim this feature exists for, gated rather than printed. Under a STEADY # load both are tight, so this only means something with DUTY_ON set — hence -# the guard rather than an unconditional assert. -if [ "$DUTY_ON" != "0" ]; then +# the guard rather than an unconditional assert. It also needs a sampled arm +# to compare against, which a backend without GetRxEnergy does not have. +if [ "$DUTY_ON" != "0" ] && [ "$nhm_sensor" != "1" ]; then + echo " (armed-vs-sampled comparison needs a sampled path; this backend has" + echo " none, so the bursty claim is not testable on it)" + # A check, not an arm: counted separately so "arm(s) skipped" stays true. + checks_skipped=$((checks_skipped + 1)) +elif [ "$DUTY_ON" != "0" ]; then # Comparing spreads alone is fragile at small REPS: on a ~10% duty channel a # handful of 2 ms reads can ALL miss the burst, which looks like a spread of # zero while being entirely wrong. The claim is that the sampled estimator is @@ -369,5 +450,16 @@ if [ "$DUTY_ON" != "0" ]; then fi echo -[ "$fails" -eq 0 ] && echo "busy_window_probe: PASS" || echo "busy_window_probe: $fails FAILURE(S)" +# The skip count belongs in the verdict line, not only in the per-arm chatter +# above: a backend without GetRxEnergy legitimately skips four arms, and a +# reader scraping the last line for PASS would otherwise see the same word for +# "every arm ran" and "two thirds of them did not". +if [ "$fails" -ne 0 ]; then + echo "busy_window_probe: $fails FAILURE(S)" +elif [ "$skips" -ne 0 ] || [ "$checks_skipped" -ne 0 ]; then + echo "busy_window_probe: PASS ($skips arm(s), $checks_skipped check(s) skipped"\ + "— see above for why)" +else + echo "busy_window_probe: PASS" +fi exit $((fails ? 1 : 0)) From f3e05e8586bf54bb57428c357b5ad75adc581c7c Mon Sep 17 00:00:00 2001 From: Joseph <162703152+josephnef@users.noreply.github.com> Date: Sun, 20 Sep 2026 18:13:33 +0300 Subject: [PATCH 2/2] rtl8733b: state the bursty spread, home the sensing gap in src/sensing The 50/450 ms burst figure now carries its 19-point spread: a 240 ms window inside a 500 ms burst period misses whole bursts, so only the mean is a measurement. The SenseWindow discriminator gap is a sensing-layer bug and now lives in src/sensing/CLAUDE.md, with the 8733B subtree and the sensing article pointing at it. The channel_busy selftest comment no longer calls the RTL8733B a die without a CCX engine. Co-Authored-By: Claude Fable 5.1 --- docs/rx-spectrum-sensing.md | 19 +++++--------- src/rtl8733b/CLAUDE.md | 45 ++++++++------------------------- src/rtl8733b/Rtl8733bDevice.cpp | 8 +++--- src/sensing/CLAUDE.md | 22 ++++++++++++++++ tests/channel_busy_selftest.cpp | 6 +++-- 5 files changed, 48 insertions(+), 52 deletions(-) diff --git a/docs/rx-spectrum-sensing.md b/docs/rx-spectrum-sensing.md index de005626..b1bc4311 100644 --- a/docs/rx-spectrum-sensing.md +++ b/docs/rx-spectrum-sensing.md @@ -485,19 +485,12 @@ the sampled path. The RTL8733B is the one backend where an armed window is the ONLY way to get a number: it implements no `GetRxEnergy`, so its sampled path reports no reading -by design. One consequence to know before building on it — `src/sensing/` -cannot reach that reading yet. `SenseWindow::read` chooses its source by -whether the `IRtlRadio*` is non-null rather than by `rx_energy_ok`, so on a -backend that derives from `IRtlRadio` and implements no energy reader it takes -the phydm branch and never calls `GetChannelBusy()`. That is the same wrong -discriminator `AdapterCaps.h` warns about, and it predates this port — the -RTL8733B is simply the first backend that makes it observable — and -`examples/chanscout` does hit it, through a `dynamic_cast` that -succeeds on this die. Gating the branch on the capability is necessary but not -sufficient: nothing in `src/sensing/` arms, and an unarmed read yields nothing -here, so the complete fix is the gate plus an arm with an observation window -the sensing layer does not yet carry. Until then a caller on this die should -use `IRadio::ArmChannelBusy`/`GetChannelBusy` directly. +by design. `src/sensing/` cannot reach that reading yet — `SenseWindow` picks +its source by a non-null `IRtlRadio*` rather than by `rx_energy_ok`, the same +wrong discriminator `AdapterCaps.h` warns about, and `examples/chanscout` hits +it on this die. The gap and what a fix needs are in `src/sensing/CLAUDE.md`; +until then a caller on this die uses `IRadio::ArmChannelBusy`/`GetChannelBusy` +directly. `tests/busy_window_probe.sh` skips its sampled and NHM arms for that reason, gating on `rx_energy_ok` from the probe's own caps record rather diff --git a/src/rtl8733b/CLAUDE.md b/src/rtl8733b/CLAUDE.md index 951cb485..e92a825d 100644 --- a/src/rtl8733b/CLAUDE.md +++ b/src/rtl8733b/CLAUDE.md @@ -450,44 +450,21 @@ place it can be kept true. What is specific to this die: mid-window. `Retuned` and `NotElapsed` both fire normally. Measured with `tests/busy_window_probe.sh` (RTL8733BU sensor, MT7612U flooder, -ch165): **0.0% quiet, 69% under a steady load, 10% against a 50/450 ms burst -whose true duty is ~9%**, spread 0 on the steady arm. A Jaguar3 8812CU read +ch165): **0.0% quiet, 69% under a steady load** with spread 0, and against a +50/450 ms burst (true duty ~9%) a mean of 6-10% over five windows with a +**19-point spread** — a 240 ms window inside a 500 ms burst period misses +whole bursts, so single windows read 0-19% and only the mean is a measurement. A Jaguar3 8812CU read that same flooder at 69% as well, so the two device paths onto the JGR3 map agree on one load. The retune and premature-read spoilers each refused with their reason, and a re-armed window never returned the previous latched value. -**`src/sensing/` cannot reach this reading, and that is a pre-existing bug -this die is the first to expose.** `SenseWindow::read` branches on whether the -`IRtlRadio*` is non-null, not on whether the backend has phydm counters: - -```cpp -if (rtl_) { r.energy = rtl_->GetRxEnergy(...); r.busy = busy_from_rx_energy(r.energy); } -else if (radio_) { r.busy = radio_->GetChannelBusy(); } -``` - -The RTL8733B derives from `IRtlRadio`, so it takes the first branch, gets the -all-invalid base `GetRxEnergy`, and `GetChannelBusy()` is never called. That -is exactly the "a successful `dynamic_cast` is not a correct -discriminator" failure `AdapterCaps.h` opens with, living in the sensing -layer. It is not new — it would mis-handle any backend with busy airtime and -no phydm block — but before this port no such backend existed, so nothing -exposed it. **`examples/chanscout` is affected**: it builds a -`DwellExecutor` with `dynamic_cast(dev)` -(`examples/chanscout/main.cpp:323,342`), which succeeds on this die, so a -scout here reports neither CLM nor NHM. - -Gating the branch on `rx_energy_ok` is necessary but NOT sufficient: an -unarmed `GetChannelBusy()` on this backend falls back to -`busy_from_rx_energy(GetRxEnergy(true))` and yields nothing, 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 capability gate PLUS an -arm, and arming needs an observation window that no layer between -`ScanPlanConfig::dwell_ms` and `SenseWindow` carries at all — `DwellExecConfig` -stamps a `settle_ms` but has no field for the dwell the window would be sized -to. That is a design decision for `src/sensing/`, with its own tests, not a -rider on a backend port. -**Until then, a caller on this die must reach -`IRadio::ArmChannelBusy`/`GetChannelBusy` directly.** +**`src/sensing/` cannot reach this reading.** `SenseWindow` picks its source +by whether the `IRtlRadio*` is non-null rather than by `rx_energy_ok`, so on +this die it takes the phydm branch and never arms or reads CLM — +`examples/chanscout` on an RTL8733B reports neither. That is a sensing-layer +bug this die is merely the first to expose; it is described, with what a fix +needs, in `src/sensing/CLAUDE.md`. Until it lands, a caller on this die +reaches `IRadio::ArmChannelBusy`/`GetChannelBusy` directly. **The CCA gate cannot bias the reading here.** CLM counts CCA-busy, so a session with CCA disabled would under-report — but `SetCcaMode` throws "CCA diff --git a/src/rtl8733b/Rtl8733bDevice.cpp b/src/rtl8733b/Rtl8733bDevice.cpp index d19013f9..fec024c4 100644 --- a/src/rtl8733b/Rtl8733bDevice.cpp +++ b/src/rtl8733b/Rtl8733bDevice.cpp @@ -1119,9 +1119,11 @@ devourer::AdapterCaps Rtl8733bDevice::GetAdapterCaps() { caps.busy_airtime_ok = true; /* Separated on air through tests/busy_window_probe.sh on an RTL8733BU * against an MT7612U flooder on ch165: 0% quiet, 69% under a steady load, - * and 10% against a 50/450 ms burst whose true duty is ~9%. A Jaguar3 - * 8812CU on the same flooder read 69% too, so the two register paths agree - * on one load. Every spoiler refused with its reason. */ + * and a 50/450 ms burst (true duty ~9%) at a mean of 6-10% over five + * windows with a 19-point spread — a 240 ms window inside a 500 ms burst + * period misses whole bursts, so only the mean means anything there. A + * Jaguar3 8812CU on the same flooder read 69% too, so the two register + * paths agree on one load. Every spoiler refused with its reason. */ caps.busy_airtime_measured = true; caps.rx_energy_ok = false; caps.variant = "cut-selected"; diff --git a/src/sensing/CLAUDE.md b/src/sensing/CLAUDE.md index 3a50b36f..0dc097db 100644 --- a/src/sensing/CLAUDE.md +++ b/src/sensing/CLAUDE.md @@ -76,3 +76,25 @@ must be field-comparable against a pre-change capture. The width-restoration rule specifically needs `DEVOURER_SCOUT_FULLWIDTH_MS > 0` against a 40/80 MHz candidate, because a headless test can only prove the call sequence, never that the chip actually came back to 20 MHz. + +## Known gap: a backend with busy airtime and no phydm counters + +`SenseWindow::read` chooses its source by whether the `IRtlRadio*` is +non-null, not by `AdapterCaps::rx_energy_ok`. The RTL8733B derives from +`IRtlRadio`, implements no `GetRxEnergy`, and answers `GetChannelBusy()` only +through an armed CLM window — so on that die the window takes the phydm branch, +gets the all-invalid base energy read, and never calls `GetChannelBusy()`. +`examples/chanscout` constructs exactly that shape (a `dynamic_cast` that +succeeds on the die) and reports neither CLM nor NHM there. Every other backend +is unaffected: on the Jaguar families both flags are true, and on the MT7612U +the cast fails and the neutral branch runs. + +Gating the branch on the capability is necessary but not sufficient. An unarmed +`GetChannelBusy()` on that backend falls back to the energy read and yields +nothing, and nothing in this subtree calls `ArmChannelBusy` — the arm needs an +observation window sized to the dwell, which no layer between +`ScanPlanConfig::dwell_ms` and `SenseWindow` carries (`DwellExecConfig` stamps a +settle but no dwell). Arming also changes behaviour on the Jaguar families, +where the NHM read spoils the same engine, which is what `dwell_executor` +pins. So the fix is a capability gate plus a dwell-sized arm, with its own +selftest arm — a design change here, not a rider on a backend port. diff --git a/tests/channel_busy_selftest.cpp b/tests/channel_busy_selftest.cpp index bf42cfee..0866c8a0 100644 --- a/tests/channel_busy_selftest.cpp +++ b/tests/channel_busy_selftest.cpp @@ -29,8 +29,10 @@ static void check(const char *what, long got, long want) { int main() { /* --- RxEnergy -> ChannelBusy --- */ { - /* No CLM at all: a with_nhm=false read, or a generation with no CCX - * engine (Kestrel, RTL8733B). Not a quiet channel. */ + /* No CLM in the read: a with_nhm=false read, a generation with no CCX + * engine (Kestrel), or one that implements no GetRxEnergy at all (the + * RTL8733B, whose CLM answers only through an armed window). Not a quiet + * channel. */ RxEnergy e; const ChannelBusy b = busy_from_rx_energy(e); check("no-clm invalid", b.valid, 0);