From 6c3043a252b1fdee08d5e94bd92bb8fdeb38b817 Mon Sep 17 00:00:00 2001 From: Gilang Date: Thu, 3 Sep 2026 22:44:28 +0700 Subject: [PATCH 1/2] jaguar3: gate PHY-status parsing on the RX-desc PHYST bit (DW0 bit 26) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The jgr3 RX loop parsed the drvinfo area as a PHY-status report whenever it was long enough (>= 28 bytes). But drvinfo space is reserved on *every* frame — RX_DRVINFO_SZ is a global register — while the PHY writes a report only into the frames whose descriptor PHYST bit is set. On all-but-one subframe of an A-MPDU the area therefore holds stale bytes, and the page nibble in byte0 can alias a valid page number (0 = CCK, 1 = OFDM type1), so the parse silently folded garbage RSSI/SNR/EVM into the RF EMAs behind GetRxQuality() / GetActiveRxPaths() / the rx.path event. Decode DW0 bit 26 into Rx8822cFrame.physt and require it before parsing. This brings jaguar3 in line with jaguar1 (FrameParser.cpp:88) and rtl8733b (FrameParser8733b.h:77), which already decode the same bit into rx_pkt_attrib::physt — jaguar3 was the generation leaving it unset. parse_phy_sts_jgr3 now returns whether it recognised the page layout (page 0 CCK or page 1 OFDM type1) and actually filled the attrib's signal fields, rather than returning void after silently ignoring an unknown page number. The internal EMA folds (_rxq, _rxpaths, _cfo) gate on that result, so an unparsed page no longer contributes a zero-valued sample. tests/rx_physt_selftest.cpp is a headless guard on the bit position and the plumbing (ctest: rx_physt_bit, built with DEVOURER_JAGUAR3) so a descriptor-layout regression fails the suite instead of quietly poisoning the RF averages. Co-Authored-By: Claude Opus 5 (1M context) Claude-Session: https://claude.ai/code/session_019FN9qETmHFJr6ekfkViZv8 --- CMakeLists.txt | 10 ++++++ src/jaguar3/FrameParserJaguar3.h | 19 ++++++++--- src/jaguar3/RtlJaguar3Device.cpp | 15 ++++++--- tests/rx_physt_selftest.cpp | 58 ++++++++++++++++++++++++++++++++ 4 files changed, 93 insertions(+), 9 deletions(-) create mode 100644 tests/rx_physt_selftest.cpp diff --git a/CMakeLists.txt b/CMakeLists.txt index 017b28e4..884b23f0 100644 --- a/CMakeLists.txt +++ b/CMakeLists.txt @@ -1124,6 +1124,16 @@ target_link_libraries(TxPktBankSelftest PRIVATE devourer) target_include_directories(TxPktBankSelftest PRIVATE src) add_test(NAME txpkt_bank_policy COMMAND TxPktBankSelftest) +# Headless guard for the Jaguar3 RX-descriptor PHYST bit (DW0 bit 26): the +# "a PHY-status report was actually written for this frame" flag that keeps +# A-MPDU subframes' reserved-but-unwritten drvinfo bytes out of the RF EMAs. +if(DEVOURER_JAGUAR3) + add_executable(RxPhystSelftest tests/rx_physt_selftest.cpp) + target_link_libraries(RxPhystSelftest PRIVATE devourer) + target_include_directories(RxPhystSelftest PRIVATE src) + add_test(NAME rx_physt_bit COMMAND RxPhystSelftest) +endif() + # Headless guard for the USB TX-aggregation URB packing (src/TxAggPlan.h) — # block alignment, the never-a-bulk-multiple boundary shim, the OQT # descs-per-bulk guard, the frame/byte caps — plus the HalMAC descriptor agg diff --git a/src/jaguar3/FrameParserJaguar3.h b/src/jaguar3/FrameParserJaguar3.h index 9da2b488..2370227a 100644 --- a/src/jaguar3/FrameParserJaguar3.h +++ b/src/jaguar3/FrameParserJaguar3.h @@ -217,6 +217,11 @@ struct Rx8822cFrame { uint8_t shift; /* SHIFT_SZ */ uint32_t tsfl; /* hardware TSF-low at receive */ bool paggr; /* MPDU arrived inside an A-MPDU */ + bool physt; /* a PHY-status report was written for THIS frame; + * drvinfo space is reserved on every frame + * (RX_DRVINFO_SZ is global), so without this bit the + * area holds stale bytes — notably on all-but-one + * subframe of an A-MPDU */ uint8_t ppdu_cnt; /* 2-bit received-PPDU counter */ uint32_t next_offset; /* 8-byte-aligned offset of the next frame in an agg */ }; @@ -239,6 +244,7 @@ inline bool parse_rx_8822c(const uint8_t *buf, size_t buflen, out.rx_rate = static_cast(GET_RX_DESC_RX_RATE_8822C(buf)); out.tsfl = static_cast(GET_RX_DESC_TSFL_8822C(buf)); out.paggr = GET_RX_DESC_PAGGR_8822C(buf) != 0; + out.physt = GET_RX_DESC_PHYST_8822C(buf) != 0; out.ppdu_cnt = static_cast(GET_RX_DESC_PPDU_CNT_8822C(buf)); uint32_t frame_off = @@ -269,16 +275,19 @@ inline bool parse_rx_8822c(const uint8_t *buf, size_t buflen, * vendor's s(8,1) fields). The page type is taken from byte0 low nibble * (page_num) rather than guessed from the rate: 0 = CCK type0, else an OFDM * page; per-stream EVM/SNR are only present on the type1 OFDM page. - * Requires physts_len >= 28. */ -inline void parse_phy_sts_jgr3(const uint8_t *physts, uint16_t physts_len, + * Requires physts_len >= 28. Returns true iff physts pointed at a page + * layout this function actually understands (page 0 CCK or page 1 OFDM + * type1) and `a` was filled from it; false on a null/short buffer or any + * other page number (caller should not trust `a`'s signal fields then). */ +inline bool parse_phy_sts_jgr3(const uint8_t *physts, uint16_t physts_len, rx_pkt_attrib &a) { if (physts == nullptr || physts_len < 28) - return; + return false; const uint8_t page_num = physts[0] & 0x0f; if (page_num == 0) { /* type0 (CCK): DW0 = page_num(0), pwdb_a(1). Single path-A power. */ a.rssi[0] = physts[1]; - return; + return true; } /* OFDM header (valid for every jgr3 OFDM page): per-path pwdb[4] at bytes * 1..4, DW1 byte5 l_rxsc[3:0]/ht_rxsc[7:4], DW1 byte7 flags. */ @@ -303,7 +312,9 @@ inline void parse_phy_sts_jgr3(const uint8_t *physts, uint16_t physts_len, a.evm[i] = static_cast(physts[16 + i]); a.snr[i] = static_cast(physts[24 + i]); } + return true; } + return false; } } /* namespace jaguar3 */ diff --git a/src/jaguar3/RtlJaguar3Device.cpp b/src/jaguar3/RtlJaguar3Device.cpp index 77c70b51..cdbabad6 100644 --- a/src/jaguar3/RtlJaguar3Device.cpp +++ b/src/jaguar3/RtlJaguar3Device.cpp @@ -322,12 +322,17 @@ void RtlJaguar3Device::StartRxLoop(Action_ParsedRadioPacket packetProcessor) { * present (monitor_rx_cfg enables APP_PHYSTS + RX_DRVINFO_SZ=4, so the * 32-byte report is counted in drvinfo). Skips C2H reports and any * frame whose drvinfo is too short (e.g. CCK, which carries no OFDM - * report). The report sits immediately after the 24-byte descriptor. */ - if (!is_c2h && f.drvinfo_size >= 28) - jaguar3::parse_phy_sts_jgr3(data + off + jaguar3::RXDESC_SIZE_8822C, - f.drvinfo_size, p.RxAtrib); + * report). The report sits immediately after the 24-byte descriptor. + * f.physt (RX desc DW0 bit 26) says the PHY actually WROTE a report + * for this frame — the drvinfo space itself is reserved on every + * frame, so on A-MPDU subframes without the bit it holds stale bytes + * whose page nibble can alias 0/1 (contaminated RSSI/SNR tails). */ + if (!is_c2h && f.physt && f.drvinfo_size >= 28) + p.RxAtrib.physt = jaguar3::parse_phy_sts_jgr3( + data + off + jaguar3::RXDESC_SIZE_8822C, f.drvinfo_size, + p.RxAtrib); p.Data = std::span(const_cast(f.frame), f.frame_len); - if (!p.RxAtrib.crc_err) { + if (!p.RxAtrib.crc_err && p.RxAtrib.physt) { _rxq.add(p.RxAtrib.rssi[0], p.RxAtrib.snr[0], p.RxAtrib.evm[0]); _rxpaths.add(p.RxAtrib.rssi, p.RxAtrib.snr, p.RxAtrib.evm, 2); /* 8822C/8822E are 2T2R */ diff --git a/tests/rx_physt_selftest.cpp b/tests/rx_physt_selftest.cpp new file mode 100644 index 00000000..14fe4ce8 --- /dev/null +++ b/tests/rx_physt_selftest.cpp @@ -0,0 +1,58 @@ +/* Headless guard for the Jaguar3 RX-descriptor PHYST bit (DW0 bit 26, + * "the drvinfo area of THIS frame holds a written PHY-status report"). + * Inside an A-MPDU the drvinfo space is reserved on every subframe + * (RX_DRVINFO_SZ is a global register) but the PHY writes a report only + * where this bit is set — parsing the reserved bytes anyway reads stale + * garbage that can alias a valid page number, which contaminates the + * RSSI/SNR tails. A bit-position or plumbing regression here fails ctest + * instead of poisoning the RF EMAs. */ +#include +#include + +#include "jaguar3/FrameParserJaguar3.h" + +static int g_fail = 0; +#define CHECK(cond, ...) \ + do { \ + if (!(cond)) { \ + ++g_fail; \ + std::printf("FAIL: " __VA_ARGS__); \ + std::printf("\n"); \ + } \ + } while (0) + +/* 24-byte descriptor + 32-byte drvinfo + a 60-byte PSDU. */ +static constexpr uint32_t kDrvInfo = 32; +static constexpr uint32_t kFrameLen = 60; +static constexpr size_t kBufLen = + jaguar3::RXDESC_SIZE_8822C + kDrvInfo + kFrameLen; + +static void make_desc(uint8_t *buf, bool physt) { + std::memset(buf, 0, kBufLen); + /* DW0: PKT_LEN[13:0] = 60, DRV_INFO_SIZE[19:16] = 4 (units of 8 bytes), + * SHIFT[25:24] = 0, PHYST = bit 26. */ + buf[0] = kFrameLen; + buf[2] = kDrvInfo / 8; + if (physt) + buf[3] |= 0x04; +} + +static void test_physt_bit_decoded() { + uint8_t buf[kBufLen]; + jaguar3::Rx8822cFrame f; + + make_desc(buf, true); + CHECK(jaguar3::parse_rx_8822c(buf, kBufLen, f), "physt=1 desc must parse"); + CHECK(f.physt, "PHYST set in DW0 bit 26 -> Rx8822cFrame.physt true"); + + make_desc(buf, false); + CHECK(jaguar3::parse_rx_8822c(buf, kBufLen, f), "physt=0 desc must parse"); + CHECK(!f.physt, "PHYST clear -> Rx8822cFrame.physt false"); +} + +int main() { + test_physt_bit_decoded(); + if (g_fail == 0) + std::printf("rx_physt_selftest: all checks passed\n"); + return g_fail == 0 ? 0 : 1; +} From ca61f2dcbe1c184325b0bb3658a5532643fd478e Mon Sep 17 00:00:00 2001 From: Gilang Date: Wed, 9 Sep 2026 21:24:25 +0700 Subject: [PATCH 2/2] rx: gate PHY-status parsing on the descriptor PHYST bit on Jaguar1/2/3 MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Review follow-up to the Jaguar3 gate: the same defect is present on all three generations, and the shared `physt` field needed one meaning. REG_RX_DRVINFO_SZ is a GLOBAL register on every generation here — Jaguar1 _InitDriverInfoSize_8812A(4), Jaguar2 0x060F=4, Jaguar3 0x060F=4 — so the drvinfo space is reserved on every frame while the PHY writes a report only into frames whose RX-descriptor PHYST bit (DW0 bit 26) is set. Gating on drvinfo size alone decodes bytes left by an earlier frame, notably on all-but-one subframe of an A-MPDU. - Jaguar2 (RtlJaguar2Device.cpp): decode PHYST into Rx8822bFrame (the GET_RX_DESC_PHYST_8822B macro already existed, unused) and require it. This was the worst instance: the garbage cfo_tail fed _cfo.add(), which steers the closed-loop XtalCap crystal trim. - Jaguar1 (FrameParser.cpp): gate the phy-status memcpy on pattrib.physt, already decoded at the top of the same function and named in the comment as the vendor's gate; gate the EMA feed on it too. - Jaguar3: keep the gate, and assign the RAW descriptor bit to RxAtrib.physt rather than the parse outcome, so the shared field means the same thing on every generation that sets it. parse_phy_sts_jgr2/jgr3 now return PhyStsFill (None/Power/Full) instead of void/bool: which fields they filled, not whether they liked the buffer. A bool could not express the two real tiers, and the previous bool was wrong in both directions — it returned false for OFDM pages 2..6 after filling valid per-path RSSI from the shared common header (dropping a real measurement, and freezing GetRxQuality/GetActiveRxPaths with no diagnostic if the BB page selector ever left type1), and returned true for the CCK page, whose layout has no SNR/EVM/CFO at all. The CFO trackers now take only a Full fill. RxQualityAccumulator::add folded SNR unconditionally while its own doc comment promised it folded only when present, so CCK and non-type1 frames dragged snr_mean toward zero and pinned snr_min to 0 on every generation. SNR now carries its own sample count like EVM and the noise-floor term, surfaced as RxQuality::snr_valid. Also drop the stale comment claiming CCK frames carry no report and short drvinfo — neither is true, and parse_phy_sts_jgr3 decodes the CCK page. tests/rx_physt_selftest.cpp now covers both generations' descriptor-bit decode and the PhyStsFill tiers (short buffer, CCK page, OFDM type1, other OFDM page), so a regression in the return plumbing fails ctest instead of silently stopping all OFDM SNR/EVM accounting; rx_quality_selftest covers the mixed and CCK-only SNR windows. ctest 55/55 (la_csi_math skipped); JAGUAR1-only, JAGUAR2-only and JAGUAR3-only configs all build clean. Co-Authored-By: Claude Opus 5 (1M context) Claude-Session: https://claude.ai/code/session_01JiAS32956Z3cmTbnmcXYkT --- CMakeLists.txt | 10 +- src/RxPacket.h | 23 +++ src/RxQuality.h | 27 +++- src/jaguar1/FrameParser.cpp | 19 ++- src/jaguar1/RtlJaguarDevice.cpp | 8 +- src/jaguar2/FrameParserJaguar2.h | 20 ++- src/jaguar2/RtlJaguar2Device.cpp | 26 +++- src/jaguar3/FrameParserJaguar3.h | 28 ++-- src/jaguar3/RtlJaguar3Device.cpp | 36 +++-- tests/rx_physt_selftest.cpp | 242 +++++++++++++++++++++++++------ tests/rx_quality_selftest.cpp | 35 +++++ 11 files changed, 380 insertions(+), 94 deletions(-) diff --git a/CMakeLists.txt b/CMakeLists.txt index 884b23f0..1aa0534d 100644 --- a/CMakeLists.txt +++ b/CMakeLists.txt @@ -1124,10 +1124,12 @@ target_link_libraries(TxPktBankSelftest PRIVATE devourer) target_include_directories(TxPktBankSelftest PRIVATE src) add_test(NAME txpkt_bank_policy COMMAND TxPktBankSelftest) -# Headless guard for the Jaguar3 RX-descriptor PHYST bit (DW0 bit 26): the -# "a PHY-status report was actually written for this frame" flag that keeps -# A-MPDU subframes' reserved-but-unwritten drvinfo bytes out of the RF EMAs. -if(DEVOURER_JAGUAR3) +# Headless guard for the Jaguar2/Jaguar3 RX PHY-status gate: the descriptor +# PHYST bit (DW0 bit 26) that keeps A-MPDU subframes' reserved-but-unwritten +# drvinfo bytes out of the RF EMAs, plus the PhyStsFill tier each parser +# returns (which fields it actually filled). Both parsers are header-only and +# self-contained, so the test builds whenever either generation is selected. +if(DEVOURER_JAGUAR3 OR DEVOURER_JAGUAR2_8822B OR DEVOURER_JAGUAR2_8821C) add_executable(RxPhystSelftest tests/rx_physt_selftest.cpp) target_link_libraries(RxPhystSelftest PRIVATE devourer) target_include_directories(RxPhystSelftest PRIVATE src) diff --git a/src/RxPacket.h b/src/RxPacket.h index fc5ed408..f3d5e884 100644 --- a/src/RxPacket.h +++ b/src/RxPacket.h @@ -19,9 +19,32 @@ enum class RX_PACKET_TYPE C2H_PACKET }; +/* How much of an rx_pkt_attrib's signal block a PHY-status report actually + * filled. Reports are paged/typed per generation — a CCK page carries only + * path-A power, while the per-stream EVM/SNR and the CFO tail live on one OFDM + * page only — so a plain bool cannot tell a caller which of the fields below + * are a measurement and which are still zero. Feeding an unfilled field into a + * running average is not a null operation: it drags the mean toward zero. */ +enum class PhyStsFill : uint8_t +{ + None, /* nothing filled: no report, too short, or a layout not decoded */ + Power, /* per-path RSSI, plus ldpc/stbc/bw on an OFDM page */ + Full /* Power, plus per-stream EVM/SNR and the path-A CFO tail */ +}; + struct rx_pkt_attrib { uint16_t pkt_len; + /* RX-descriptor PHY-status bit: the PHY wrote a status report into THIS + * frame's drvinfo area. It is the RAW descriptor bit on every generation + * that decodes it (Jaguar1, Jaguar2, Jaguar3, RTL8733B) — deliberately NOT + * "the report parsed" and NOT "the signal fields below are valid", since a + * parser can still decline an unrecognised page (that is PhyStsFill's job, + * kept in a local at the parse site). The drvinfo area is reserved on every + * frame (RX_DRVINFO_SZ is a global register), so this bit is the only thing + * separating a written report from stale bytes left by an earlier frame — + * notably on all-but-one subframe of an A-MPDU. Never set on Kestrel, whose + * PHY status arrives as its own PPDU-status frame rather than in drvinfo. */ bool physt; uint8_t drvinfo_sz; uint8_t shift_sz; diff --git a/src/RxQuality.h b/src/RxQuality.h index be9d1450..fa28da9a 100644 --- a/src/RxQuality.h +++ b/src/RxQuality.h @@ -43,6 +43,10 @@ struct RxQuality { int rssi_max_dbm = 0; /* window peak — the strength signal (see LinkHealth) */ double snr_mean_db = 0.0; double snr_min_db = 0.0; + /* false when no frame in the window carried SNR (a CCK-only or non-type1 + * phy-status window); snr_mean_db / snr_min_db are 0 then, and are a mean + * over the reporting frames only otherwise — not over every decoded frame. */ + bool snr_valid = false; double evm_mean_db = 0.0; /* 0 when evm_valid is false */ bool evm_valid = false; @@ -84,6 +88,7 @@ struct RxQualitySnapshot { int rssi_max_raw = 0; int snr_mean_raw = 0; int snr_min_raw = 0; + bool snr_valid = false; /* false when no frame in the window carried SNR */ int evm_mean_raw = 0; bool evm_valid = false; double nf_mean_dbm = 0.0; @@ -99,7 +104,10 @@ class RxQualityAccumulator { /* Raw path-A values straight off rx_pkt_attrib. A frame with no phy-status * power (rssi_raw <= 0) is not a quality sample and is skipped. SNR/EVM are * folded only when present (raw != 0 — CCK / non-type1 phy-status leaves them - * 0), so a mixed stream doesn't bias those means toward zero. */ + * 0), so a mixed stream doesn't bias those means toward zero; snr_min_raw + * likewise ignores the absent ones rather than pinning itself to 0. Each + * carries its own sample count, so the window means are over the frames that + * actually reported the metric. */ void add(int rssi_raw, int snr_raw, int evm_raw) { if (rssi_raw <= 0) return; @@ -108,9 +116,12 @@ class RxQualityAccumulator { rssi_sum_ += rssi_raw; if (rssi_raw > rssi_max_) rssi_max_ = rssi_raw; - snr_sum_ += snr_raw; - if (snr_raw < snr_min_) - snr_min_ = snr_raw; + if (snr_raw != 0) { + snr_sum_ += snr_raw; + ++snr_n_; + if (snr_raw < snr_min_) + snr_min_ = snr_raw; + } if (evm_raw != 0) { evm_sum_ += evm_raw; ++evm_n_; @@ -130,8 +141,11 @@ class RxQualityAccumulator { if (n_) { s.rssi_mean_raw = rssi_sum_ / static_cast(n_); s.rssi_max_raw = rssi_max_; - s.snr_mean_raw = snr_sum_ / static_cast(n_); + } + if (snr_n_) { + s.snr_mean_raw = snr_sum_ / static_cast(snr_n_); s.snr_min_raw = snr_min_; + s.snr_valid = true; } if (evm_n_) { s.evm_mean_raw = evm_sum_ / static_cast(evm_n_); @@ -145,6 +159,7 @@ class RxQualityAccumulator { rssi_sum_ = 0; rssi_max_ = -128; snr_sum_ = 0; + snr_n_ = 0; snr_min_ = 127; evm_sum_ = 0; evm_n_ = 0; @@ -158,6 +173,7 @@ class RxQualityAccumulator { uint32_t n_ = 0; int32_t rssi_sum_ = 0, rssi_max_ = -128; int32_t snr_sum_ = 0, snr_min_ = 127; + uint32_t snr_n_ = 0; int32_t evm_sum_ = 0; uint32_t evm_n_ = 0; double nf_sum_ = 0.0; @@ -178,6 +194,7 @@ inline RxQuality build_rx_quality(const RxQualitySnapshot &s, const RxEnergy &e, q.rssi_max_dbm = s.rssi_max_raw - 110; q.snr_mean_db = s.snr_mean_raw / 2.0; q.snr_min_db = s.snr_min_raw / 2.0; + q.snr_valid = s.snr_valid; q.evm_valid = s.evm_valid; q.evm_mean_db = s.evm_mean_raw / 2.0; q.noise_floor_dbm = s.nf_mean_dbm; diff --git a/src/jaguar1/FrameParser.cpp b/src/jaguar1/FrameParser.cpp index 65c30208..be75c9e0 100644 --- a/src/jaguar1/FrameParser.cpp +++ b/src/jaguar1/FrameParser.cpp @@ -228,13 +228,18 @@ std::vector FrameParser::recvbuf2recvframe(std::span ptr) { struct _phy_status_rpt_8812 driver_data = {}; /* Only read the PHY-status report when the descriptor says one is - * present and it fits the remaining buffer. The kernel gates this - * on pattrib->physt (usb_ops_linux.c:179); drvinfo_sz >= the report - * size is the equivalent condition with the fields we carry — - * without it, frames with drvinfo_sz==0 had payload bytes decoded - * as RSSI/EVM/SNR, and a frame ending near the buffer tail - * over-read the transfer buffer. */ - if (pattrib.drvinfo_sz >= sizeof(driver_data) && + * present and it fits the remaining buffer. pattrib.physt (DW0 bit 26) + * is the per-frame fact — the same gate the kernel uses + * (usb_ops_linux.c:179 passes pbuf+RXDESC_OFFSET only when it is set). + * The size check alone is NOT equivalent: REG_RX_DRVINFO_SZ is a global + * register (_InitDriverInfoSize_8812A writes 4 = 32 bytes), so the + * drvinfo space is reserved on EVERY frame while the PHY writes a report + * only where the bit is set. On all-but-one subframe of an A-MPDU the + * area therefore holds bytes left by an earlier frame, and copying them + * decodes stale RSSI/SNR/EVM/CFO — including the per-chain values the + * 8814AU spatial-diversity work reads. The size check still guards the + * tail over-read it was added for. */ + if (pattrib.physt && pattrib.drvinfo_sz >= sizeof(driver_data) && pbuf.size() >= RXDESC_SIZE + sizeof(driver_data)) { memcpy(static_cast(&driver_data), pbuf.data() + RXDESC_SIZE, sizeof(driver_data)); diff --git a/src/jaguar1/RtlJaguarDevice.cpp b/src/jaguar1/RtlJaguarDevice.cpp index eca90ea9..4aa08181 100644 --- a/src/jaguar1/RtlJaguarDevice.cpp +++ b/src/jaguar1/RtlJaguarDevice.cpp @@ -1550,7 +1550,13 @@ void RtlJaguarDevice::StartRxLoop(Action_ParsedRadioPacket packetProcessor) { std::span{const_cast(data), (size_t)n})) { if (should_stop || g_devourer_should_stop) break; - if (!p.RxAtrib.crc_err) { + /* physt: the descriptor says the PHY wrote a status report for THIS + * frame. Without it FrameParser leaves the signal fields at 0 (the + * drvinfo space is reserved on every frame but written only where the + * bit is set), and folding those zeros would drag the running averages + * — the CFO tracker in particular, whose enable threshold a diluted + * average never crosses. */ + if (!p.RxAtrib.crc_err && p.RxAtrib.physt) { _rxq.add(p.RxAtrib.rssi[0], p.RxAtrib.snr[0], p.RxAtrib.evm[0]); _rxpaths.add(p.RxAtrib.rssi, p.RxAtrib.snr, p.RxAtrib.evm, _eepromManager->numTotalRfPath); diff --git a/src/jaguar2/FrameParserJaguar2.h b/src/jaguar2/FrameParserJaguar2.h index e71c5ca3..5e207a7e 100644 --- a/src/jaguar2/FrameParserJaguar2.h +++ b/src/jaguar2/FrameParserJaguar2.h @@ -207,6 +207,11 @@ struct Rx8822bFrame { uint8_t shift; uint32_t tsfl; /* hardware TSF-low at receive */ bool paggr; /* MPDU arrived inside an A-MPDU */ + bool physt; /* a PHY-status report was written for THIS frame; + * REG_RX_DRVINFO_SZ is global, so the drvinfo + * space is reserved on every frame and holds + * stale bytes without this bit — notably on + * all-but-one subframe of an A-MPDU */ uint8_t ppdu_cnt; /* 2-bit received-PPDU counter */ uint32_t next_offset; }; @@ -227,6 +232,7 @@ inline bool parse_rx_8822b(const uint8_t *buf, size_t buflen, out.rx_rate = static_cast(GET_RX_DESC_RX_RATE_8822B(buf)); out.tsfl = static_cast(GET_RX_DESC_TSFL_8822B(buf)); out.paggr = GET_RX_DESC_PAGGR_8822B(buf) != 0; + out.physt = GET_RX_DESC_PHYST_8822B(buf) != 0; out.ppdu_cnt = static_cast(GET_RX_DESC_PPDU_CNT_8822B(buf)); uint32_t frame_off = @@ -248,14 +254,19 @@ inline bool parse_rx_8822b(const uint8_t *buf, size_t buflen, * SNR rxsnr[i] and per-stream EVM rxevm[i] (both s(8,1), i.e. half-dB units, as * the vendor stores them). Values are the raw phy-status fields, matching the * Jaguar-1 FrameParser convention (rssi = per-path power byte, dBm = value-110). - * CCK (type0) reports a single path-A pwdb. Requires physts_len >= 28. */ -inline void parse_phy_sts_jgr2(const uint8_t *physts, uint16_t physts_len, - bool is_cck, rx_pkt_attrib &a) { + * CCK (type0) reports a single path-A pwdb. Requires physts_len >= 28. + * Returns which fields of `a` were filled (PhyStsFill): None on a null/short + * buffer, Power for the CCK type0 report (path-A power only — EVM/SNR and the + * CFO tail are NOT in that layout and stay 0), Full for type1. Callers must + * not fold a field the return value does not claim. */ +inline PhyStsFill parse_phy_sts_jgr2(const uint8_t *physts, uint16_t physts_len, + bool is_cck, rx_pkt_attrib &a) { if (physts == nullptr || physts_len < 28) - return; + return PhyStsFill::None; if (is_cck) { /* type0: DW0 = page_num(0), pwdb(1), ... */ a.rssi[0] = physts[1]; + return PhyStsFill::Power; } else { /* type1: DW0/1 pwdb[4] at bytes 1..4; DW4 rxevm[4] at bytes 16..19; * DW5 cfo_tail[4] at bytes 20..23; DW6 rxsnr[4] at bytes 24..27. */ @@ -276,6 +287,7 @@ inline void parse_phy_sts_jgr2(const uint8_t *physts, uint16_t physts_len, const uint8_t rxsc = (a.data_rate >= 4 && a.data_rate <= 11) ? l_rxsc : ht_rxsc; a.bw = rxsc >= 13 ? 2 : rxsc >= 9 ? 1 : 0; } + return PhyStsFill::Full; } } /* namespace jaguar2 */ diff --git a/src/jaguar2/RtlJaguar2Device.cpp b/src/jaguar2/RtlJaguar2Device.cpp index 1291c574..af767d34 100644 --- a/src/jaguar2/RtlJaguar2Device.cpp +++ b/src/jaguar2/RtlJaguar2Device.cpp @@ -618,17 +618,31 @@ void RtlJaguar2Device::StartRxLoop(Action_ParsedRadioPacket packetProcessor) { /* Per-frame RSSI/SNR/EVM from the jgr2 PHY-status (present when * APP_PHYSTS is on, i.e. drvinfo carries the 32-byte report). CCK rates * (DESC_RATE1M..11M = 0..3) use type0, everything else type1. C2H has no - * phy-status (drvinfo=0), so the size guard already skips it. */ - if (!is_c2h && f.drvinfo_size >= 28) - jaguar2::parse_phy_sts_jgr2(data + off + jaguar2::RXDESC_SIZE_8822B, - f.drvinfo_size, f.rx_rate <= 3, p.RxAtrib); + * phy-status (drvinfo=0), so the size guard already skips it. + * REG_RX_DRVINFO_SZ (0x060F) is a GLOBAL register, so the 32 drvinfo + * bytes are reserved on EVERY frame while the PHY writes a report only + * where the descriptor's PHYST bit (DW0 bit 26, f.physt) is set — + * on an A-MPDU's other subframes the area holds bytes left by an + * earlier frame, and parsing them anyway decodes garbage as + * rssi/snr/evm/cfo_tail. cfo_tail is the one that does damage: it + * steers the closed-loop XtalCap crystal trim below. */ + PhyStsFill phy = PhyStsFill::None; + if (!is_c2h && f.physt && f.drvinfo_size >= 28) + phy = jaguar2::parse_phy_sts_jgr2( + data + off + jaguar2::RXDESC_SIZE_8822B, f.drvinfo_size, + f.rx_rate <= 3, p.RxAtrib); + /* The RAW descriptor bit, matching the field's meaning on Jaguar1 / + * Jaguar3 / RTL8733B; `phy` says which fields are safe to fold. */ + p.RxAtrib.physt = f.physt; p.Data = std::span(const_cast(f.frame), f.frame_len); - if (!p.RxAtrib.crc_err) { + if (!p.RxAtrib.crc_err && phy != PhyStsFill::None) { _rxq.add(p.RxAtrib.rssi[0], p.RxAtrib.snr[0], p.RxAtrib.evm[0]); _rxpaths.add(p.RxAtrib.rssi, p.RxAtrib.snr, p.RxAtrib.evm, _variant == jaguar2::ChipVariant::C8821C ? 1 : 2); - if (_cfg.tuning.cfo_track) + /* cfo_tail lives only in the type1 layout; the 0 a CCK report leaves + * would pull the tracker's average below its enable threshold. */ + if (_cfg.tuning.cfo_track && phy == PhyStsFill::Full) _cfo.add(p.RxAtrib.cfo_tail); /* closed-loop CFO input (#217) */ } _packetProcessor(p); diff --git a/src/jaguar3/FrameParserJaguar3.h b/src/jaguar3/FrameParserJaguar3.h index 2370227a..e80e1493 100644 --- a/src/jaguar3/FrameParserJaguar3.h +++ b/src/jaguar3/FrameParserJaguar3.h @@ -275,19 +275,21 @@ inline bool parse_rx_8822c(const uint8_t *buf, size_t buflen, * vendor's s(8,1) fields). The page type is taken from byte0 low nibble * (page_num) rather than guessed from the rate: 0 = CCK type0, else an OFDM * page; per-stream EVM/SNR are only present on the type1 OFDM page. - * Requires physts_len >= 28. Returns true iff physts pointed at a page - * layout this function actually understands (page 0 CCK or page 1 OFDM - * type1) and `a` was filled from it; false on a null/short buffer or any - * other page number (caller should not trust `a`'s signal fields then). */ -inline bool parse_phy_sts_jgr3(const uint8_t *physts, uint16_t physts_len, - rx_pkt_attrib &a) { + * Requires physts_len >= 28. Returns which fields of `a` were filled + * (PhyStsFill): None on a null/short buffer, Full only on the type1 OFDM page + * that carries EVM/SNR/CFO, Power on the CCK page and on every other OFDM page + * — those share the common header, so their per-path power (and ldpc/stbc/bw) + * IS a measurement even though the type1-only fields are left at 0. Callers + * must not fold a field the return value does not claim. */ +inline PhyStsFill parse_phy_sts_jgr3(const uint8_t *physts, uint16_t physts_len, + rx_pkt_attrib &a) { if (physts == nullptr || physts_len < 28) - return false; + return PhyStsFill::None; const uint8_t page_num = physts[0] & 0x0f; if (page_num == 0) { /* type0 (CCK): DW0 = page_num(0), pwdb_a(1). Single path-A power. */ a.rssi[0] = physts[1]; - return true; + return PhyStsFill::Power; } /* OFDM header (valid for every jgr3 OFDM page): per-path pwdb[4] at bytes * 1..4, DW1 byte5 l_rxsc[3:0]/ht_rxsc[7:4], DW1 byte7 flags. */ @@ -312,9 +314,15 @@ inline bool parse_phy_sts_jgr3(const uint8_t *physts, uint16_t physts_len, a.evm[i] = static_cast(physts[16 + i]); a.snr[i] = static_cast(physts[24 + i]); } - return true; + return PhyStsFill::Full; } - return false; + /* Pages 2..6: the common header above was parsed and is valid, so this is a + * Power fill rather than a failure — reporting None here would throw away + * real per-path RSSI, and would silently freeze GetRxQuality/GetActiveRxPaths + * if the BB page selector ever left type1 (devourer's BB table pins + * 0x8C0[25:22]=1, but the vendor auto-switch and debug page helpers do not + * restore it). */ + return PhyStsFill::Power; } } /* namespace jaguar3 */ diff --git a/src/jaguar3/RtlJaguar3Device.cpp b/src/jaguar3/RtlJaguar3Device.cpp index cdbabad6..d33788c2 100644 --- a/src/jaguar3/RtlJaguar3Device.cpp +++ b/src/jaguar3/RtlJaguar3Device.cpp @@ -318,25 +318,37 @@ void RtlJaguar3Device::StartRxLoop(Action_ParsedRadioPacket packetProcessor) { devourer::emit_tx_report( _logger->events(), devourer::parse_ccx_halmac(f.frame, f.frame_len), "halmac"); - /* Decode the jgr3 PHY-status report (per-frame RSSI/SNR/EVM) when it is - * present (monitor_rx_cfg enables APP_PHYSTS + RX_DRVINFO_SZ=4, so the - * 32-byte report is counted in drvinfo). Skips C2H reports and any - * frame whose drvinfo is too short (e.g. CCK, which carries no OFDM - * report). The report sits immediately after the 24-byte descriptor. - * f.physt (RX desc DW0 bit 26) says the PHY actually WROTE a report - * for this frame — the drvinfo space itself is reserved on every - * frame, so on A-MPDU subframes without the bit it holds stale bytes - * whose page nibble can alias 0/1 (contaminated RSSI/SNR tails). */ + /* Decode the jgr3 PHY-status report (per-frame RSSI/SNR/EVM), which + * sits immediately after the 24-byte descriptor inside the drvinfo + * area (monitor_rx_cfg enables APP_PHYSTS + RX_DRVINFO_SZ=4). + * RX_DRVINFO_SZ is a GLOBAL register, so those 32 bytes are reserved + * on EVERY frame — CCK included, and the parser decodes the CCK page + * 0 report — while the PHY writes a report only where the descriptor's + * PHYST bit (DW0 bit 26, f.physt) is set. Parsing without that bit + * reads bytes left over from an earlier frame, notably on all-but-one + * subframe of an A-MPDU, and a stale page nibble can alias 0/1 so the + * parse "succeeds" on garbage (contaminated RSSI/SNR tails). C2H + * reports carry no phy-status. */ + PhyStsFill phy = PhyStsFill::None; if (!is_c2h && f.physt && f.drvinfo_size >= 28) - p.RxAtrib.physt = jaguar3::parse_phy_sts_jgr3( + phy = jaguar3::parse_phy_sts_jgr3( data + off + jaguar3::RXDESC_SIZE_8822C, f.drvinfo_size, p.RxAtrib); + /* The RAW descriptor bit, not the parse outcome — that is the meaning + * the shared field carries on Jaguar1 and the RTL8733B too, and what + * a caller needs to tell which A-MPDU subframe the report belonged to. + * `phy` is the local that says which fields are safe to fold. */ + p.RxAtrib.physt = f.physt; p.Data = std::span(const_cast(f.frame), f.frame_len); - if (!p.RxAtrib.crc_err && p.RxAtrib.physt) { + if (!p.RxAtrib.crc_err && phy != PhyStsFill::None) { _rxq.add(p.RxAtrib.rssi[0], p.RxAtrib.snr[0], p.RxAtrib.evm[0]); _rxpaths.add(p.RxAtrib.rssi, p.RxAtrib.snr, p.RxAtrib.evm, 2); /* 8822C/8822E are 2T2R */ - if (_cfg.tuning.cfo_track) + /* cfo_tail exists only on the type1 OFDM page. Feeding the 0 that a + * CCK or non-type1 report leaves would pull the tracker's running + * average below its enable threshold, so a real offset on a channel + * carrying CCK beacons/probes would go uncorrected. */ + if (_cfg.tuning.cfo_track && phy == PhyStsFill::Full) _cfo.add(p.RxAtrib.cfo_tail); /* closed-loop CFO input (#217) */ } /* TX-BF apply gate (DEVOURER_BF_TXBF): a VHT Compressed Beamforming diff --git a/tests/rx_physt_selftest.cpp b/tests/rx_physt_selftest.cpp index 14fe4ce8..049a0572 100644 --- a/tests/rx_physt_selftest.cpp +++ b/tests/rx_physt_selftest.cpp @@ -1,57 +1,209 @@ -/* Headless guard for the Jaguar3 RX-descriptor PHYST bit (DW0 bit 26, - * "the drvinfo area of THIS frame holds a written PHY-status report"). - * Inside an A-MPDU the drvinfo space is reserved on every subframe - * (RX_DRVINFO_SZ is a global register) but the PHY writes a report only - * where this bit is set — parsing the reserved bytes anyway reads stale - * garbage that can alias a valid page number, which contaminates the - * RSSI/SNR tails. A bit-position or plumbing regression here fails ctest - * instead of poisoning the RF EMAs. */ +/* Headless guard for the RX PHY-status gate on the Jaguar2 / Jaguar3 parsers. + * + * Two facts are pinned here, because the RF running averages behind + * GetRxQuality() / GetActiveRxPaths() / the CFO trim are fed from them and a + * regression in either is silent on air: + * + * 1. The RX-descriptor PHYST bit (DW0 bit 26) is decoded per frame. The + * drvinfo space is reserved on EVERY frame (REG_RX_DRVINFO_SZ is a global + * register) but the PHY writes a report only where this bit is set, so on + * all-but-one subframe of an A-MPDU the area holds bytes left by an earlier + * frame. A stale page nibble can alias a valid page number, so a parse + * "succeeds" on garbage and contaminates the RSSI/SNR/EVM/CFO tails. + * + * 2. parse_phy_sts_* reports WHICH fields it filled (PhyStsFill), not just + * whether it liked the buffer. A CCK page carries path-A power alone, and + * the per-stream EVM/SNR plus the CFO tail live on one OFDM page only — + * folding the untouched zeros drags the averages toward zero rather than + * leaving them alone. Losing the Full/Power distinction would stop all + * OFDM SNR/EVM accounting (or restart the zero-dilution) while ctest + * stayed green. + */ +#include #include #include +#include +#include "jaguar2/FrameParserJaguar2.h" #include "jaguar3/FrameParserJaguar3.h" -static int g_fail = 0; -#define CHECK(cond, ...) \ - do { \ - if (!(cond)) { \ - ++g_fail; \ - std::printf("FAIL: " __VA_ARGS__); \ - std::printf("\n"); \ - } \ - } while (0) - -/* 24-byte descriptor + 32-byte drvinfo + a 60-byte PSDU. */ -static constexpr uint32_t kDrvInfo = 32; -static constexpr uint32_t kFrameLen = 60; -static constexpr size_t kBufLen = - jaguar3::RXDESC_SIZE_8822C + kDrvInfo + kFrameLen; - -static void make_desc(uint8_t *buf, bool physt) { - std::memset(buf, 0, kBufLen); - /* DW0: PKT_LEN[13:0] = 60, DRV_INFO_SIZE[19:16] = 4 (units of 8 bytes), - * SHIFT[25:24] = 0, PHYST = bit 26. */ - buf[0] = kFrameLen; - buf[2] = kDrvInfo / 8; - if (physt) - buf[3] |= 0x04; -} - -static void test_physt_bit_decoded() { - uint8_t buf[kBufLen]; - jaguar3::Rx8822cFrame f; - - make_desc(buf, true); - CHECK(jaguar3::parse_rx_8822c(buf, kBufLen, f), "physt=1 desc must parse"); - CHECK(f.physt, "PHYST set in DW0 bit 26 -> Rx8822cFrame.physt true"); - - make_desc(buf, false); - CHECK(jaguar3::parse_rx_8822c(buf, kBufLen, f), "physt=0 desc must parse"); - CHECK(!f.physt, "PHYST clear -> Rx8822cFrame.physt false"); +namespace { +int g_fail = 0; + +void expect(const char *what, bool condition) { + if (condition) + return; + ++g_fail; + std::printf("FAIL: %s\n", what); +} + +void set_bits(uint8_t *p, unsigned bit, unsigned width, uint32_t value) { + uint32_t word = static_cast(p[0]) | + (static_cast(p[1]) << 8) | + (static_cast(p[2]) << 16) | + (static_cast(p[3]) << 24); + const uint32_t mask = width == 32 ? 0xffffffffu + : (((1u << width) - 1u) << bit); + word = (word & ~mask) | ((value << bit) & mask); + p[0] = static_cast(word); + p[1] = static_cast(word >> 8); + p[2] = static_cast(word >> 16); + p[3] = static_cast(word >> 24); +} + +/* 24-byte descriptor + 32-byte drvinfo + a 60-byte PSDU. drvinfo is carried in + * 8-byte units, so 4 units = the 32-byte phy-status block. */ +constexpr uint16_t kFrameLen = 60; +constexpr uint8_t kDrvInfoUnits = 4; + +std::vector desc(size_t desc_size, bool physt) { + std::vector b(desc_size + kDrvInfoUnits * 8u + kFrameLen, 0); + set_bits(b.data(), 0, 14, kFrameLen); + set_bits(b.data(), 16, 4, kDrvInfoUnits); + set_bits(b.data(), 24, 2, 0); /* SHIFT */ + set_bits(b.data(), 26, 1, physt ? 1 : 0); + return b; +} + +/* --- The descriptor bit is decoded, on both generations --- */ +void test_physt_bit_decoded() { + { + jaguar3::Rx8822cFrame f{}; + std::vector b = desc(jaguar3::RXDESC_SIZE_8822C, true); + expect("jgr3 physt=1 descriptor parses", + jaguar3::parse_rx_8822c(b.data(), b.size(), f)); + expect("jgr3 DW0 bit 26 set -> Rx8822cFrame.physt true", f.physt); + + b = desc(jaguar3::RXDESC_SIZE_8822C, false); + expect("jgr3 physt=0 descriptor parses", + jaguar3::parse_rx_8822c(b.data(), b.size(), f)); + expect("jgr3 DW0 bit 26 clear -> Rx8822cFrame.physt false", !f.physt); + } + { + jaguar2::Rx8822bFrame f{}; + std::vector b = desc(jaguar2::RXDESC_SIZE_8822B, true); + expect("jgr2 physt=1 descriptor parses", + jaguar2::parse_rx_8822b(b.data(), b.size(), f)); + expect("jgr2 DW0 bit 26 set -> Rx8822bFrame.physt true", f.physt); + + b = desc(jaguar2::RXDESC_SIZE_8822B, false); + expect("jgr2 physt=0 descriptor parses", + jaguar2::parse_rx_8822b(b.data(), b.size(), f)); + expect("jgr2 DW0 bit 26 clear -> Rx8822bFrame.physt false", !f.physt); + } +} + +/* A 32-byte jgr3 phy-status page: byte0 low nibble = page number, per-path + * pwdb at 1..4, flags at 7, rxevm at 16..19, cfo_tail at 20, rxsnr at 24..27. */ +std::vector jgr3_page(uint8_t page_num) { + std::vector p(32, 0); + p[0] = page_num; + for (int i = 0; i < 4; i++) + p[1 + i] = static_cast(90 + i); /* pwdb -> rssi */ + p[5] = 0x11; /* l_rxsc / ht_rxsc = 1 -> 20 MHz */ + p[7] = 0x20; /* bit5 = ldpc */ + for (int i = 0; i < 4; i++) { + p[16 + i] = static_cast(0xE0 + i); /* rxevm (signed) */ + p[24 + i] = static_cast(30 + i); /* rxsnr */ + } + p[20] = 7; /* path-A cfo_tail */ + return p; +} + +void test_jgr3_fill_tiers() { + /* Short / absent buffer fills nothing. */ + { + rx_pkt_attrib a{}; + std::vector p = jgr3_page(1); + expect("jgr3 null buffer -> None", + jaguar3::parse_phy_sts_jgr3(nullptr, 32, a) == PhyStsFill::None); + expect("jgr3 27-byte report -> None", + jaguar3::parse_phy_sts_jgr3(p.data(), 27, a) == PhyStsFill::None); + expect("jgr3 rejected report leaves the attrib untouched", + a.rssi[0] == 0 && a.snr[0] == 0 && a.cfo_tail == 0); + } + /* Page 0 (CCK): path-A power only. Claiming Full here would feed a zero SNR + * and a zero CFO tail into the trackers. */ + { + rx_pkt_attrib a{}; + std::vector p = jgr3_page(0); + p[1] = 88; + expect("jgr3 page 0 (CCK) -> Power", + jaguar3::parse_phy_sts_jgr3(p.data(), 32, a) == PhyStsFill::Power); + expect("jgr3 page 0 fills path-A rssi", a.rssi[0] == 88); + expect("jgr3 page 0 leaves snr/evm/cfo unset", + a.snr[0] == 0 && a.evm[0] == 0 && a.cfo_tail == 0); + } + /* Page 1 (OFDM type1): the only page carrying EVM/SNR/CFO. */ + { + rx_pkt_attrib a{}; + a.data_rate = 12; /* HT -> bandwidth read from ht_rxsc */ + std::vector p = jgr3_page(1); + expect("jgr3 page 1 (OFDM type1) -> Full", + jaguar3::parse_phy_sts_jgr3(p.data(), 32, a) == PhyStsFill::Full); + expect("jgr3 page 1 fills per-path rssi", + a.rssi[0] == 90 && a.rssi[3] == 93); + expect("jgr3 page 1 fills per-stream snr/evm", + a.snr[0] == 30 && a.snr[3] == 33 && + a.evm[0] == static_cast(0xE0)); + expect("jgr3 page 1 fills the path-A cfo tail", a.cfo_tail == 7); + expect("jgr3 page 1 fills ldpc/bw from the common header", + a.ldpc == 1 && a.stbc == 0 && a.bw == 0); + } + /* Pages 2..6 share the OFDM common header, so their per-path power IS a + * measurement — reporting None would throw it away — but the type1-only + * fields stay zero and must not be folded. */ + { + rx_pkt_attrib a{}; + std::vector p = jgr3_page(5); + expect("jgr3 page 5 (other OFDM page) -> Power", + jaguar3::parse_phy_sts_jgr3(p.data(), 32, a) == PhyStsFill::Power); + expect("jgr3 page 5 fills per-path rssi from the common header", + a.rssi[0] == 90 && a.rssi[3] == 93); + expect("jgr3 page 5 leaves snr/evm/cfo unset", + a.snr[0] == 0 && a.evm[0] == 0 && a.cfo_tail == 0); + } +} + +void test_jgr2_fill_tiers() { + { + rx_pkt_attrib a{}; + std::vector p = jgr3_page(1); /* same 32-byte field offsets */ + expect("jgr2 null buffer -> None", + jaguar2::parse_phy_sts_jgr2(nullptr, 32, false, a) == + PhyStsFill::None); + expect("jgr2 27-byte report -> None", + jaguar2::parse_phy_sts_jgr2(p.data(), 27, false, a) == + PhyStsFill::None); + } + { + rx_pkt_attrib a{}; + std::vector p = jgr3_page(0); + p[1] = 88; + expect("jgr2 CCK type0 -> Power", + jaguar2::parse_phy_sts_jgr2(p.data(), 32, true, a) == + PhyStsFill::Power); + expect("jgr2 CCK fills path-A rssi only", + a.rssi[0] == 88 && a.snr[0] == 0 && a.cfo_tail == 0); + } + { + rx_pkt_attrib a{}; + a.data_rate = 12; + std::vector p = jgr3_page(1); + expect("jgr2 OFDM type1 -> Full", + jaguar2::parse_phy_sts_jgr2(p.data(), 32, false, a) == + PhyStsFill::Full); + expect("jgr2 type1 fills rssi/snr/evm/cfo", + a.rssi[0] == 90 && a.snr[0] == 30 && + a.evm[0] == static_cast(0xE0) && a.cfo_tail == 7); + } } +} // namespace int main() { test_physt_bit_decoded(); + test_jgr3_fill_tiers(); + test_jgr2_fill_tiers(); if (g_fail == 0) std::printf("rx_physt_selftest: all checks passed\n"); return g_fail == 0 ? 0 : 1; diff --git a/tests/rx_quality_selftest.cpp b/tests/rx_quality_selftest.cpp index c1d55207..f99db857 100644 --- a/tests/rx_quality_selftest.cpp +++ b/tests/rx_quality_selftest.cpp @@ -65,6 +65,41 @@ int main() { check("q.noise_floor", approx(q.noise_floor_dbm, -55.0)); } + /* A window mixing OFDM frames (SNR present) with CCK / non-type1 ones (the + * phy-status page carries no SNR, so the field arrives as 0) must average + * the SNR over the reporting frames only. Folding the zeros would drag the + * mean down and pin snr_min to 0 — which reads as a collapsing link that + * tracks CCK traffic density rather than the channel. RSSI still counts + * every frame, since every page reports path-A power. */ + { + RxQualityAccumulator acc; + acc.add(60, 40, -50); /* OFDM: snr 40 */ + acc.add(70, 0, 0); /* CCK: no snr, no evm */ + acc.add(80, 20, -40); /* OFDM: snr 20 */ + RxQualitySnapshot s = acc.snapshot(); + check("mixed.frames", s.frames == 3); + check("mixed.rssi_mean_raw", s.rssi_mean_raw == 70); /* all three */ + check("mixed.snr_valid", s.snr_valid); + check("mixed.snr_mean_raw", s.snr_mean_raw == 30); /* (40+20)/2, not /3 */ + check("mixed.snr_min_raw", s.snr_min_raw == 20); /* not pinned to 0 */ + check("mixed.evm_mean_raw", s.evm_mean_raw == -45); + } + + /* A CCK-only window reports no SNR at all rather than a fabricated 0. */ + { + RxQualityAccumulator acc; + acc.add(75, 0, 0); + acc.add(85, 0, 0); + RxQualitySnapshot s = acc.snapshot(); + check("cck.frames", s.frames == 2); + check("cck.rssi_mean_raw", s.rssi_mean_raw == 80); + check("cck.snr_valid_false", !s.snr_valid); + check("cck.snr_mean_raw", s.snr_mean_raw == 0); + check("cck.evm_valid_false", !s.evm_valid); + RxQuality q = build_rx_quality(s, RxEnergy{}); + check("cck.q.snr_valid_false", !q.snr_valid); + } + /* snapshot() resets (delta semantics). */ { RxQualityAccumulator acc;