diff --git a/docs/rx-spectrum-sensing.md b/docs/rx-spectrum-sensing.md index d019f887..b1bc4311 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,28 @@ 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. `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 +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..e92a825d 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,72 @@ 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** 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.** `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 +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..fec024c4 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,31 @@ 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 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"; 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/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/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)) 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);