diff --git a/CMakeLists.txt b/CMakeLists.txt index 017b28e..1aa0534 100644 --- a/CMakeLists.txt +++ b/CMakeLists.txt @@ -1124,6 +1124,18 @@ target_link_libraries(TxPktBankSelftest PRIVATE devourer) target_include_directories(TxPktBankSelftest PRIVATE src) add_test(NAME txpkt_bank_policy COMMAND TxPktBankSelftest) +# 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) + 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/RxPacket.h b/src/RxPacket.h index fc5ed40..f3d5e88 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 be9d145..fa28da9 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 65c3020..be75c9e 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 eca90ea..4aa0818 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 e71c5ca..5e207a7 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 1291c57..af767d3 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 9da2b48..e80e149 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,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. */ -inline void 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; + 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; + 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. */ @@ -303,7 +314,15 @@ 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 PhyStsFill::Full; } + /* 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 77c70b5..d33788c 100644 --- a/src/jaguar3/RtlJaguar3Device.cpp +++ b/src/jaguar3/RtlJaguar3Device.cpp @@ -318,20 +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. */ - if (!is_c2h && f.drvinfo_size >= 28) - jaguar3::parse_phy_sts_jgr3(data + off + jaguar3::RXDESC_SIZE_8822C, - f.drvinfo_size, p.RxAtrib); + /* 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) + 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) { + 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 new file mode 100644 index 0000000..049a057 --- /dev/null +++ b/tests/rx_physt_selftest.cpp @@ -0,0 +1,210 @@ +/* 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" + +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 c1d5520..f99db85 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;