jaguar3: gate PHY-status parsing on the RX-desc PHYST bit (DW0 bit 26)

[gilankpam]

Problem

The Jaguar3 RX loop parsed the drvinfo area as a PHY-status report whenever it was long enough:

if (!is_c2h && f.drvinfo_size >= 28)
  jaguar3::parse_phy_sts_jgr3(data + off + jaguar3::RXDESC_SIZE_8822C,
                              f.drvinfo_size, p.RxAtrib);

But RX_DRVINFO_SZ is a global register — 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. On all-but-one subframe of an A-MPDU the area therefore holds stale bytes from a previous frame, and the page nibble in physts[0] & 0x0f can alias a valid page number (0 = CCK, 1 = OFDM type1).

The parse then succeeded on garbage and folded it into the RF EMAs behind GetRxQuality(), GetActiveRxPaths() and the rx.path event — contaminated RSSI/SNR/EVM tails that track A-MPDU density rather than the link.

Change

• Decode DW0 bit 26 into Rx8822cFrame.physt and require it before parsing the report.
• parse_phy_sts_jgr3 now returns bool — whether it recognised the page layout (page 0 CCK, or page 1 OFDM type1) and actually filled the attrib's signal fields. Previously it returned void after silently ignoring an unknown page number, leaving the caller unable to tell a filled attrib from an untouched one.
• The internal EMA folds gate on that result (_rxq, _rxpaths, _cfo), so an unparsed page no longer contributes a zero-valued sample.
• tests/rx_physt_selftest.cpp — headless guard on the bit position and the plumbing (ctest: rx_physt_bit, built under DEVOURER_JAGUAR3), so a descriptor-layout regression fails the suite instead of quietly poisoning the averages.

Consistency with the other generations

rx_pkt_attrib::physt already exists and is already populated on two generations that decode the same bit — Jaguar3 was the one leaving it unset:

Generation	Site	Bit
Jaguar1	src/jaguar1/FrameParser.cpp:88	GET_RX_STATUS_DESC_PHY_STATUS_8812
RTL8733B	src/rtl8733b/FrameParser8733b.h:77	rx_bits(buf + 0x00, 26, 1)
Jaguar3	(this PR)	GET_RX_DESC_PHYST_8822C

Open question for reviewers

This PR sets RxAtrib.physt to the parse result (report written and page understood and fields filled), whereas Jaguar1 and RTL8733B set it to the raw descriptor bit. The parse-result semantics is what the internal EMA gate wants, but it means a caller reading RxAtrib.physt gets a slightly different meaning per generation.

The alternative is to assign p.RxAtrib.physt = f.physt unconditionally (matching the other two) and keep the parse-succeeded flag in a local for the gate. Happy to switch to that if you'd rather the field keep one cross-generation meaning — the on-air behaviour is identical either way.

Testing

• ctest: 54 passed, 1 skipped (la_csi_math, needs numpy), 0 failures — including the new rx_physt_bit.
• Clean Release build, no new warnings.
• Hardware: the contamination was found on an 8822EU RX session; the bit-level decode is what the selftest pins. Not re-validated on-air since the split from the working branch, hence draft.

Not included

Split out of the working branch deliberately — the same branch also carried an unrelated usb.rx_zerocopy default flip (intermittent zero-frame delivery on an xhci host). That is a separate concern and will come as its own PR if wanted.

🤖 Generated with Claude Code

https://claude.ai/code/session_019FN9qETmHFJr6ekfkViZv8

[qodo-free-for-open-source-projects]

PR Summary by Qodo

Gate Jaguar3 PHY-status parsing on RX descriptor PHYST

🐞 Bug fix 🧪 Tests 🕐 20-40 Minutes

AI Description

• Decode Jaguar3 RX-descriptor PHYST before reading per-frame PHY-status data.
• Exclude missing or unsupported PHY reports from RSSI, path, and CFO averages.
• Add a conditional self-test pinning DW0 bit 26 decoding.

Diagram

graph TD
  A["RX descriptor"] --> B["Frame parser"] --> C{"PHYST set?"}
  C -->|Yes| D["PHY parser"] --> E{"Page known?"}
  E -->|Yes| F["Signal attributes"] --> G["RF EMAs"]
  C -->|No| H["Skip metrics"]
  E -->|No| H

Loading

High-Level Assessment

The following are alternative approaches to this PR:

1. Preserve raw PHYST semantics

• ➕ Keeps rx_pkt_attrib::physt consistent across Jaguar1, Jaguar3, and RTL8733B.
• ➕ Retains the hardware-reported presence flag independently of parser support.
• ➕ Still protects EMAs by using the parser result as a local gate.
• ➖ Callers cannot use physt alone to determine whether signal fields were decoded.
• ➖ Requires maintaining a separate local parse-success variable.

2. Add explicit parse-status metadata

• ➕ Separates report presence from successful decoding without overloading physt.
• ➕ Can distinguish missing, malformed, and unsupported PHY pages.
• ➖ Expands the shared packet attribute API for a narrowly scoped fix.
• ➖ Requires broader consumer and compatibility review.

Recommendation: Preserve rx_pkt_attrib::physt as the raw descriptor bit and use a local parse-success boolean for EMA gating. This maintains cross-generation API semantics while retaining the PR's protection against stale or unsupported PHY reports; a separate status field is only warranted if consumers need detailed decode outcomes.

Files changed (4) +93 / -9

Bug fix (2) +25 / -9

FrameParserJaguar3.hDecode PHYST and report PHY parse success +15/-4

Decode PHYST and report PHY parse success

• Adds the RX descriptor PHYST flag to Rx8822cFrame and decodes it from DW0 bit 26. Changes parse_phy_sts_jgr3 to return true only for recognized page 0 or page 1 layouts that populate signal attributes.

src/jaguar3/FrameParserJaguar3.h

RtlJaguar3Device.cppGate PHY parsing and RF averages on valid reports +10/-5

Gate PHY parsing and RF averages on valid reports

• Requires PHYST before parsing Jaguar3 driver-info bytes and records whether the PHY page was understood. RSSI, active-path, and CFO averages now receive samples only from successfully parsed, CRC-valid reports.

src/jaguar3/RtlJaguar3Device.cpp

Tests (1) +58 / -0

rx_physt_selftest.cppVerify Jaguar3 DW0 PHYST bit decoding +58/-0

Verify Jaguar3 DW0 PHYST bit decoding

• Introduces a headless descriptor test covering both set and clear states of DW0 bit 26. The test verifies that parse_rx_8822c propagates the hardware flag into Rx8822cFrame.physt.

tests/rx_physt_selftest.cpp

Other (1) +10 / -0

CMakeLists.txtRegister the Jaguar3 PHYST descriptor self-test +10/-0

Register the Jaguar3 PHYST descriptor self-test

• Adds the RxPhystSelftest executable and rx_physt_bit CTest entry when DEVOURER_JAGUAR3 is enabled.

CMakeLists.txt

[qodo-free-for-open-source-projects]

Code Review by Qodo

🐞 Bugs (1) 📘 Rule violations (0) 📜 Skill insights (0)

1. PHYST semantics diverge by chipset 🐞 Bug ≡ Correctness

Description

Jaguar3 now reports RxAtrib.physt=false when the descriptor bit is set but the report is short or
uses an unsupported page, while Jaguar1 and RTL8733B expose the raw descriptor bit. Consumers of the
shared packet callback can therefore no longer use physt consistently to identify which A-MPDU
subframe carried PHY status.

Code

src/jaguar3/RtlJaguar3Device.cpp[R330-333]

+        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);

Evidence

rx_pkt_attrib belongs to the shared packet model, and its A-MPDU documentation associates PHY
status with the aggregate subframe carrying it. Jaguar1 and RTL8733B assign the raw descriptor bit
to this field and RTL8733B keeps parse success in a separate local, while the new Jaguar3 code
assigns the parser result; that parser explicitly returns false for short and unsupported-page
reports even when the descriptor bit was true.

src/RxPacket.h[8-11]
src/RxPacket.h[22-26]
src/RxPacket.h[72-79]
src/jaguar1/FrameParser.cpp[72-89]
src/rtl8733b/Rtl8733bDevice.cpp[353-380]
src/jaguar3/FrameParserJaguar3.h[278-317]
src/jaguar3/RtlJaguar3Device.cpp[330-340]

Agent prompt

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

## Issue description
Jaguar3 assigns PHY parse success to the shared `RxAtrib.physt` field, whereas other generations assign the raw RX-descriptor PHYST bit. Preserve the descriptor-bit meaning and use a separate local parse-success flag to gate RF EMAs.

## Issue Context
`RxAtrib.physt` is exposed through the generation-neutral `Packet` callback. `parse_phy_sts_jgr3` may return false even when the descriptor PHYST bit is set, including for unsupported pages, so these two facts must remain separate.

## Fix Focus Areas
- src/jaguar3/RtlJaguar3Device.cpp[297-340]
- src/jaguar3/FrameParserJaguar3.h[278-317]
- src/jaguar1/FrameParser.cpp[72-89]
- src/rtl8733b/Rtl8733bDevice.cpp[353-380]

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

---

Tip of the day

💡 Did you know, you can add REVIEW.md to your repo root and Qodo follows it on every PR

More tips ↗ | Customize Qodo ↗ | Qodo docs ↗

[qodo-free-for-open-source-projects]

1. Physt semantics diverge by chipset 🐞 Bug ≡ Correctness

Jaguar3 now reports RxAtrib.physt=false when the descriptor bit is set but the report is short or
uses an unsupported page, while Jaguar1 and RTL8733B expose the raw descriptor bit. Consumers of the
shared packet callback can therefore no longer use physt consistently to identify which A-MPDU
subframe carried PHY status.

Agent Prompt

## Issue description
Jaguar3 assigns PHY parse success to the shared `RxAtrib.physt` field, whereas other generations assign the raw RX-descriptor PHYST bit. Preserve the descriptor-bit meaning and use a separate local parse-success flag to gate RF EMAs.

## Issue Context
`RxAtrib.physt` is exposed through the generation-neutral `Packet` callback. `parse_phy_sts_jgr3` may return false even when the descriptor PHYST bit is set, including for unsupported pages, so these two facts must remain separate.

## Fix Focus Areas
- src/jaguar3/RtlJaguar3Device.cpp[297-340]
- src/jaguar3/FrameParserJaguar3.h[278-317]
- src/jaguar1/FrameParser.cpp[72-89]
- src/rtl8733b/Rtl8733bDevice.cpp[353-380]

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