[snokvist]

What this is

After bring-up, RtlJaguar3Device always starts coex_runtime_loop. With no RX loop running, that thread keeps a synchronous 16 KiB bulk-IN read posted on the Wi-Fi bulk-IN endpoint to drain firmware C2H reports. Every ~2 s it also writes coex, TX-power-tracking and DIG/EDCCA registers.

A host that owns bulk-IN itself, or needs a fixed gain, has no way to opt out:

• Its packets race the thread's read, so roughly half of them land in the thread's buffer.
• Its IGI gets rewritten every ~2 s.

This adds an opt-out. The default is unchanged.

What changed

• DeviceConfig::tuning.j3_runtime_thread (default true), plus env DEVOURER_J3_RUNTIME=0 in examples/common/env_config.cpp.
• RtlJaguar3Device starts coex_runtime_loop only when the flag is set, and logs when it does not.
• Every existing join is already guarded by joinable(), so Stop() and the destructor need no change. The StartRxLoop phydm thread keys on the coex thread not running, which is the right behaviour for an RX session with the flag off.

Behaviour change: skip_dig now applies on Jaguar3 (4afa386). tuning.skip_dig / DEVOURER_SKIP_DIG used to be Jaguar2-only.

• On Jaguar3 it now makes the phydm watchdog skip its DIG step. That covers both callers of PhydmRuntimeJaguar3::tick: the runtime thread and the StartRxLoop worker.
• FA statistics, CCK-PD and EDCCA tracking still run.
• Anyone who already sets DEVOURER_SKIP_DIG on a J3 part will now keep their IGI instead of having DIG move it.
• One exception: the wide-TX IGI floor (wide_tx_igi_floor) still writes IGI. Use DEVOURER_WIDE_TX_IGI_FLOOR=0 as well for a fully fixed IGI.

Documentation

• DeviceConfig.h env: tags: what j3_runtime_thread = false costs. An RX session still runs DIG, via StartRxLoop's worker. A TX-only session gets no tx.report (nothing drains C2H), no pwr_track and no coex/FW heartbeats. Also what skip_dig covers on J2 vs J3, and the wide-TX floor exception.
• src/jaguar3/CLAUDE.md coex bullet and the Jaguar3 bullet in the root CLAUDE.md: name the opt-out and its cost.

Verification

Hardware: an 8812CU (8822C cut 5) on USB 2. A host program staged its own C2H packets (24-byte RX descriptor + PRBS payload) and read bulk-IN EP 0x84 with async transfers.

• Thread on (master): each run ended after exactly as many packets as the host had transfers posted (4, 8, 16, 32). usbmon showed every requested packet on the wire within 0.1–0.3 ms; the rest completed into the runtime thread's read.
• Thread off: 1600/1600 16 KiB packets host-driven, and 1024/1024 three times when the chip's CPU issued them (44.7 MB/s, the USB 2 limit).
• skip_dig on J3: 8812CU, ch36, rxdemo for 14 s per arm. With DEVOURER_J3_RUNTIME=0, the StartRxLoop worker still wrote IGI (Jaguar3 dig: IGI=0x1e); adding DEVOURER_SKIP_DIG=1 gave no DIG writes. Default unchanged.
• On-air TX (josephnef, review): 8812EU (8822E), ch36 MCS1/20, 10 000 frames to an 8822B. Delivery was 99.2–99.5 % with the thread on or off, and teardown was clean.
• ctest: 100 % passed after the merge with master (83, 2 skipped as on master).

Not covered

• Default behaviour is unchanged.
• With the flag off, the coex keepalive, TX-power tracking, tx.report and (outside StartRxLoop) DIG do not run. That is intended for the raw-transfer / fixed-gain use case and not suitable for a normal Wi-Fi session.
• Sustained TX longer than ~22 s with the flag off, and on-air TX on the 8822C, are untested.

🤖 Generated with Claude Code

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

PR Summary by Qodo

Make the Jaguar3 coex/PHYDM runtime thread optional

✨ Enhancement ⚙️ Configuration changes 🕐 10-20 Minutes

AI Description

• Add a default-on Jaguar3 runtime setting for hosts that own bulk-IN or require fixed gain.
• Allow examples to disable the thread with DEVOURER_J3_RUNTIME=0; log when it is skipped.
• Preserve default behavior while opting out of C2H draining and periodic coex/PHYDM maintenance.

Diagram

graph TD
  Env["Environment override"] --> Config["Device tuning"] --> Gate{"Runtime enabled?"} -->|yes| Runtime["Coex runtime"] --> Bulk["C2H bulk drain"]
  Runtime --> Watchdog["Coex PHYDM ticks"]
  Gate -->|no| Skip["Thread skipped"]

Loading

High-Level Assessment

The following are alternative approaches to this PR:

1. Disable only the C2H drain

• ➕ Retains periodic coex and PHYDM maintenance while a host owns bulk-IN.
• ➖ Does not provide fixed gain because PHYDM can still rewrite IGI.
• ➖ Requires separating responsibilities within the existing runtime loop.

Recommendation: Keep the single default-on switch for the stated raw-transfer and fixed-gain use cases: both need the thread's activity stopped. A separate drain switch would be useful only if hosts later need exclusive bulk-IN access while retaining periodic maintenance. With the runtime thread disabled, an RX session can still start its existing PHYDM worker.

Files changed (3) +13 / -1

Enhancement (2) +11 / -1

DeviceConfig.hAdd a default-on Jaguar3 runtime setting +7/-0

Add a default-on Jaguar3 runtime setting

• Adds j3_runtime_thread to DeviceConfig::Tuning and documents its effect on C2H draining, periodic maintenance, and fixed-gain use cases.

src/DeviceConfig.h

RtlJaguar3Device.cppConditionally start the coex/PHYDM runtime thread +4/-1

Conditionally start the coex/PHYDM runtime thread

• Starts coex_runtime_loop after bring-up only when j3_runtime_thread is enabled. Logs when the thread is skipped; existing joinable checks continue to handle shutdown.

src/jaguar3/RtlJaguar3Device.cpp

Other (1) +2 / -0

env_config.cppExpose the Jaguar3 runtime opt-out to examples +2/-0

Expose the Jaguar3 runtime opt-out to examples

• Reads DEVOURER_J3_RUNTIME into the tuning setting. Only an explicit value of "0" disables the default-on thread.

examples/common/env_config.cpp

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

Code Review by Qodo

🐞 Bugs (1) 📘 Rule violations (0) 📎 Requirement gaps (0) 🎨 UX issues (0) 🔗 Cross-repo conflicts (0) 📜 Skill insights (0)

1. Wide transmissions change fixed receive gain 🐞 Bug ≡ Correctness

Description

skip_dig prevents the watchdog from calling dig(), but it does not prevent the wide-transmit
gain floor from writing IGI. With the default floor of 0x35, the first 40 or 80 MHz frame raises a
lower host-selected IGI even when DEVOURER_SKIP_DIG is set; leaving that floor later can also
lower IGI.

Code

src/DeviceConfig.h[R377-379]

+    /* env: DEVOURER_SKIP_DIG — J2: no DIG at bring-up; J3: the phydm
+     * watchdog (runtime thread and StartRxLoop worker) skips its DIG step,
+     * so IGI stays where the host put it. */

Evidence

The new comment promises that skipping DIG keeps host-selected IGI, but the transmit path
independently applies a nonzero default floor, and the floor setter writes IGI when it is below that
floor or above the DIG window when the floor is removed.

src/DeviceConfig.h[376-380]
src/DeviceConfig.h[440-455]
src/jaguar3/RtlJaguar3Device.cpp[2779-2789]
src/jaguar3/PhydmRuntimeJaguar3.cpp[27-46]

Agent prompt

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

## Issue description
On Jaguar3, `skip_dig` disables watchdog DIG but the independent wide-transmit IGI floor still changes host-selected gain.
## Fix Focus Areas
- src/DeviceConfig.h[377-380]
- src/jaguar3/RtlJaguar3Device.cpp[2779-2789]
- src/jaguar3/PhydmRuntimeJaguar3.cpp[35-46]
## Recommended Fix
Decide and implement how fixed-gain mode interacts with the wide-transmit floor. If `skip_dig` promises fixed IGI, prevent floor arming and floor-release writes in that mode, and document the resulting wide-transmit tradeoff.

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

---

2. LA captures still change gain ✓ Resolved

Description

j3_runtime_thread gates only coex_runtime_loop; StartRxLoop starts a separate PHYDM thread
whenever the coex thread is absent. During an RX or LA capture with the flag off, that thread calls
tick() every two seconds, and its DIG step can rewrite the IGI value the host intended to keep
fixed.

Code

src/jaguar3/RtlJaguar3Device.cpp[R1180-1181]

+  if (_cfg.tuning.j3_runtime_thread)
+    _coex_thread = std::thread([this] { coex_runtime_loop(); });

Evidence

The new option explicitly names fixed-gain LA capture as a reason to turn the runtime thread off,
but the RX path starts another periodic caller of DIG under precisely that condition. The RX example
performs LA capture while its RX loop is live.

src/DeviceConfig.h[368-374]
src/jaguar3/RtlJaguar3Device.cpp[289-312]
src/jaguar3/PhydmRuntimeJaguar3.cpp[98-125]
src/jaguar3/PhydmRuntimeJaguar3.cpp[229-238]
examples/rx/main.cpp[2509-2540]

Agent prompt

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

## Issue description
Disabling `j3_runtime_thread` stops the coex thread, but `StartRxLoop` interprets its absence as a reason to start another PHYDM watchdog. That watchdog can change IGI during the fixed-gain LA capture advertised for this option.
## Fix Focus Areas
- src/jaguar3/RtlJaguar3Device.cpp[1180-1183]
- src/jaguar3/RtlJaguar3Device.cpp[289-312]
## Recommended Fix
Separate the decision to run the RX PHYDM watchdog from the coex thread's joinable state. Ensure the fixed-gain configuration suppresses DIG during RX sessions, while preserving the intended watchdog behavior for ordinary RX sessions.

ⓘ 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 type 'qodo, fix this' on a finding and the fix lands right on your PR

More tips ↗ | Customize Qodo ↗ | Qodo docs ↗

[snokvist]

/review

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

Code review by qodo was updated up to the latest commit 4afa386

[josephnef]

Code is fine and works on hardware. Two doc gaps to close before merge, plus two nits.

On-air check. An 8812EU (8822E) in a TX-only session, ch36 MCS1/20, 10 000 frames (~22 s of sustained 5 GHz TX), an 8822B receiving:

	rep 1	rep 2
default (thread on)	99.4 %	99.2 %
DEVOURER_J3_RUNTIME=0	99.5 %	99.4 %

With the flag off there are no coex ticks, the "not started" line is logged, and Stop/teardown is clean. Within 22 s on the 8822E I did not see the antenna go silent, which src/jaguar3/CLAUDE.md says happens without this thread. Longer runs and the 8822C were not tested.

Requested changes

1. Say in the title/body that skip_dig now applies on Jaguar3 (4afa386). It was Jaguar2-only before, so this is a behaviour change for anyone who sets DEVOURER_SKIP_DIG on a J3 part. The change is reasonable; it just isn't mentioned.
2. Document what a TX-only session loses with the flag off. Nothing drains C2H, so tx.report (CCX) is never emitted. pwr_track (thermal TX-power compensation for upper 5 GHz) and the coex/FW heartbeats also stop. Please put this in the j3_runtime_thread comment in DeviceConfig.h, and add the opt-out to the "Coex runtime thread" bullet in src/jaguar3/CLAUDE.md. That bullet currently says sustained 5 GHz TX needs the thread and that the thread is what feeds tx.report in TX-only sessions.

Nits

• env_config.cpp: use env_str("DEVOURER_J3_RUNTIME") like the neighbouring default-on knobs (DEVOURER_THERMAL_TRACK), not raw std::getenv.
• PhydmRuntimeJaguar3::tick(..., bool dig_track = true): both callers pass the argument, so drop the default.

[snokvist]

@josephnef thanks for the on-air run. All four points are addressed in 2282164 and 49a9fbf. I merged master in instead of rebasing, so your review still applies to the commits you saw.

Requested changes

1. skip_dig on Jaguar3: the title now says it. The description has a "Behaviour change" paragraph: J3 now skips the DIG step in both tick() callers, existing DEVOURER_SKIP_DIG users on J3 parts keep their IGI, and the wide-TX IGI floor still writes it (DEVOURER_WIDE_TX_IGI_FLOOR=0 for fully fixed IGI).
2. TX-only cost of the flag: the j3_runtime_thread comment in DeviceConfig.h now says a TX-only session with the thread off gets no tx.report (nothing drains C2H), no pwr_track and no coex/FW heartbeats. The coex bullet in src/jaguar3/CLAUDE.md names the opt-out and that cost. I added the same pointer to the Jaguar3 bullet in the root CLAUDE.md, which repeats the "needs the coex thread" line.

Nits

• DEVOURER_J3_RUNTIME is now read through env_str, with std::strcmp as for DEVOURER_THERMAL_TRACK.
• PhydmRuntimeJaguar3::tick has no default for dig_track.

Also fixed while there

• The skip_dig comment from 4afa386 said "J2: no DIG at bring-up". On Jaguar2 it actually stops StartRxLoop's DIG thread, so it now says that.
• The tick() doc no longer says the host's IGI "is kept", because the wide-TX floor can still move it.

Not claimed

• In your 22 s run the antenna didn't go silent with the thread off, so the new comments say what stops and point to src/jaguar3/CLAUDE.md for why sustained 5 GHz TX needs it, rather than asserting a silence I haven't measured.
• Longer TX runs and on-air TX on the 8822C are listed as untested in the description.

Build clean; ctest 83/83 (2 skipped, as on master).

[josephnef]

All four points are addressed in 2282164 / 49a9fbf. Approving.

• The title and description now disclose skip_dig on Jaguar3.
• What a TX-only session loses with the thread off is documented in DeviceConfig.h, src/jaguar3/CLAUDE.md and the root CLAUDE.md.
• env_str replaces the raw getenv, and the dig_track default is dropped.
• The new J2 skip_dig wording matches the code: StartRxLoop only starts the DIG thread under !skip_dig.

Re-checked on hardware at 49a9fbf, since the env parsing changed: 8812EU, TX-only, ch36 MCS1, 3000 frames.

DEVOURER_J3_RUNTIME	"not started" logged	coex ticks	submitted	exit
0	yes	0	3050	clean
1	no	5	3050	clean

The CLAUDE.md claim that sustained 5 GHz TX needs the thread still stands as written. I couldn't reproduce the silencing in 22 s on the 8822E. That is tracked in #499 and does not block this PR.