DEVOURER_ACK_TIMEOUT_US: the hardware-ARQ range lever, every generation

[josephnef]

Prompted by the PixelPilot dev's question: yes, the hardware ACK window is adjustable — and it's the knob that caps hardware-ARQ distance (round trip eats ~6.7 µs/km).

Design: one default, every chip

DEVOURER_ACK_TIMEOUT_US (1..255 µs, clamped) with a library default of 128 µs programmed identically on every generation — the same knob value means the same range budget (~15 km round trip) no matter which die is plugged. The vendor defaults ranged 33..128 µs per chip (and per bandwidth), which made hardware-ARQ range silently die-dependent — the same per-chip-divergence trap the EDCCA silent no-op was. 128 is the vendor's own interop-blessed J1/J2 value and covers the slowest narrowband ACK in the tree (5 MHz vendor value = 117 µs), so the per-bandwidth special cases collapse into the one number; J3 gains ~12 km of range budget out of the box.

Register: REG_ACKTO 0x640 (11ac), R_AX_RSP_CHK_SIG 0xCC00 byte0 (Kestrel). The CTS window (0x641) is separate and untouched.

Measured, both directions

arm	ok	retries	write-offs/8 s (dead RA — the cost side)
33 µs (old J3 vendor default)	100%	~0	2719
128 µs (new uniform default)	100%	{0:895, 1:4}	2015
255 µs	100%	~0	1507
8 µs (below ACK flight — the proof arm)	0%	pinned {8:907} vs a live responder	—

The 8 µs arm proves the register gates the ARQ verdict; the write-off column is the honest price of long windows (every retry of a lost frame waits the full window) — sizing guidance at the field doc: ~6.7 µs × round-trip km + ~50 µs margin.

Validated live with no env: 0x640 reads 0x80 on the 8812CU and 8821AU, 0xCC00 byte0 0x80 on the 8832CU. The first cut of this rework also demonstrated why the clamps matter: a silently-failed default edit left the field 0, the <1 clamp wrote a 1 µs window, and the validation matrix caught 0% ok / retries pinned before it could ship.

Also fixes the stale retry_limit doc claim that the knob is inert on Kestrel (#375 wired it).

🤖 Generated with Claude Code

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

PR Summary by Qodo

Add DEVOURER_ACK_TIMEOUT_US to tune hardware ACK window across generations

✨ Enhancement 📝 Documentation 🕐 20-40 Minutes

AI Description

• Add DEVOURER_ACK_TIMEOUT_US to control the MAC ACK/CTS response window at bring-up.
• Wire the knob into Jaguar1/2/3 and Kestrel init paths via the correct per-gen register.
• Document the knob’s ARQ/range impact and correct Kestrel retry-limit documentation.

Diagram

graph TD
  A["Process env"] --> B["DeviceConfig.tx"] --> C["Device bring-up"] --> D[("ACK window register")]
  B --> E["Docs: scheduled MAC"]
  C --> C1["Jaguar1/2/3"] --> D1[("REG_ACKTO 0x640")]
  C --> C2["Kestrel"] --> D2[("RSP_CHK_SIG 0xCC00")]
  D --> D1
  D --> D2

Loading

High-Level Assessment

The following are alternative approaches to this PR:

1. Auto-tune ACK window from PHY mode + safety margin

• ➕ Avoids users needing to understand ACK duration differences across bandwidth/narrowband modes (especially J3 scaling).
• ➕ Can pick a conservative minimum that preserves ARQ while minimizing airtime spent waiting.
• ➖ Requires reliable derivation of worst-case ACK timing from current mode/rate tables; higher implementation and validation cost.
• ➖ May surprise power users who want fixed, reproducible behavior across setups.

2. Centralize per-generation register programming in a shared helper

• ➕ Removes repeated bring-up snippets across device classes and reduces drift.
• ➕ Makes it easier to extend the knob to future chip families or additional init phases.
• ➖ Small refactor across device layers for limited immediate benefit.
• ➖ Risks entangling otherwise independent init ordering constraints per chip.

Recommendation: The PR’s approach (a single explicit µs knob applied at bring-up, with per-generation register mapping) is the right minimal interface for a hardware ARQ gating parameter. Consider a follow-up to centralize the per-chip write logic (helper) and/or add an optional auto-tune mode for J3 narrowband users who may otherwise under-budget the slowest ACK duration.

Files changed (7) +48 / -8

Enhancement (6) +43 / -8

env_config.cppParse DEVOURER_ACK_TIMEOUT_US from environment +2/-0

Parse DEVOURER_ACK_TIMEOUT_US from environment

• Adds env var handling for DEVOURER_ACK_TIMEOUT_US and stores it into cfg.tx.ack_timeout_us. The value is clamped into the supported 1..255 range (matching the underlying 8-bit register semantics).

examples/common/env_config.cpp

DeviceConfig.hAdd tx.ack_timeout_us config field and update retry_limit docs +23/-8

Add tx.ack_timeout_us config field and update retry_limit docs

• Introduces DeviceConfig::tx::ack_timeout_us with detailed documentation on semantics, per-generation registers, and Jaguar3 override behavior. Updates retry_limit documentation to reflect Kestrel behavior (WD attempts field) and removes the stale claim that it is inert there (leaving only 8814A as called out inert).

src/DeviceConfig.h

RtlJaguarDevice.cppApply ACK timeout during Jaguar1 bring-up (REG_ACKTO) +6/-0

Apply ACK timeout during Jaguar1 bring-up (REG_ACKTO)

• Programs REG_ACKTO (0x640) with cfg.tx.ack_timeout_us when set (>0). The write is applied in both init entry points so the knob takes effect reliably during bring-up.

src/jaguar1/RtlJaguarDevice.cpp

RtlJaguar2Device.cppApply ACK timeout during Jaguar2 bring-up (REG_ACKTO) +4/-0

Apply ACK timeout during Jaguar2 bring-up (REG_ACKTO)

• Adds a bring-up step to write cfg.tx.ack_timeout_us into REG_ACKTO (0x640) when configured. Keeps the change near other early bring-up knobs like CCA mode.

src/jaguar2/RtlJaguar2Device.cpp

RtlJaguar3Device.cppOverride Jaguar3 per-bandwidth ACK timeout default +4/-0

Override Jaguar3 per-bandwidth ACK timeout default

• Writes cfg.tx.ack_timeout_us to REG_ACKTO (0x640) after halmac init, explicitly overriding the per-bandwidth default that init_wmac_cfg configures. Documents the override intent in-code to highlight that this replaces (not scales) the default behavior.

src/jaguar3/RtlJaguar3Device.cpp

RtlKestrelDevice.cppApply ACK timeout during Kestrel bring-up (R_AX_RSP_CHK_SIG) +4/-0

Apply ACK timeout during Kestrel bring-up (R_AX_RSP_CHK_SIG)

• Programs byte0 of R_AX_RSP_CHK_SIG (0xCC00) with cfg.tx.ack_timeout_us when set (>0). Aligns the knob with the same field used by the vendor narrowband scaling path.

src/kestrel/RtlKestrelDevice.cpp

Documentation (1) +5 / -0

scheduled-mac.mdDocument ACK timeout knob as an ARQ/range lever +5/-0

Document ACK timeout knob as an ARQ/range lever

• Adds a short note pointing readers to DEVOURER_ACK_TIMEOUT_US as the adjustable hardware ACK window. Clarifies its impact on hardware-ARQ range and references the detailed field documentation in DeviceConfig.

docs/scheduled-mac.md

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

Code Review by Qodo

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

1. ACK default unreachable ✓ Resolved 🐞 Bug ≡ Correctness

Description

examples/common/env_config.cpp clamps DEVOURER_ACK_TIMEOUT_US values <1 to 1, so setting the env var
to "0" (documented as “use per-chip default”) instead forces a 1µs timeout and overrides the chip
defaults. Because env_long treats the string "0" as present, this can silently break deployments
that set env vars to 0 to mean “disabled/default.”

Code

examples/common/env_config.cpp[R124-125]

+  if (env_long("DEVOURER_ACK_TIMEOUT_US", &v))
+    cfg.tx.ack_timeout_us = static_cast<int>(v < 1 ? 1 : (v > 255 ? 255 : v));

Evidence

DeviceConfig explicitly defines 0 as the sentinel default, and device bring-up only applies the
register write when the value is >0. But env_long returns true for "0", and the new env clamp
converts 0 into 1, forcing an override that should have been “no override.”

examples/common/env_config.cpp[31-39]
examples/common/env_config.cpp[124-127]
src/DeviceConfig.h[226-239]
src/jaguar1/RtlJaguarDevice.cpp[79-86]

Agent prompt

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

## Issue description
`DEVOURER_ACK_TIMEOUT_US` is documented as `0 = per-chip default`, but the env parser clamps any value `< 1` to `1`, making the sentinel value impossible to express via environment configuration.
### Issue Context
- `env_long()` returns true for the string "0" (it only checks presence/non-empty), so `DEVOURER_ACK_TIMEOUT_US=0` currently becomes `ack_timeout_us=1`.
- Device bring-up code only writes the register when `ack_timeout_us > 0`, so the intended sentinel is clearly `0`.
### Fix Focus Areas
- examples/common/env_config.cpp[124-125]
- src/DeviceConfig.h[226-239]
### Implementation notes
- Change the clamp to preserve 0 and clamp negatives safely, e.g. `v < 0 ? 0 : (v > 255 ? 255 : v)`.
- (Optional but safer) if the input string is non-numeric, treat it as “unset/default” (0) and emit a warning to stderr (similar to the AMPDU_MODE warning path).

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

---

2. ACK timeout wraps silently ✓ Resolved 🐞 Bug ☼ Reliability

Description

DeviceConfig::Tx::ack_timeout_us is an int and is cast directly to uint8_t when writing registers,
so out-of-range values set programmatically (or by future non-env config sources) will silently wrap
and program the wrong timeout. This can produce hard-to-debug ARQ failures because the configured
value and the hardware value diverge without any warning.

Code

src/jaguar3/RtlJaguar3Device.cpp[R883-884]

+  if (_cfg.tx.ack_timeout_us > 0)
+    _device.rtw_write8(0x0640, static_cast<uint8_t>(_cfg.tx.ack_timeout_us));

Evidence

The new config field is declared as an unconstrained int, but each hardware bring-up path casts it
to uint8_t for the register write. That conversion will wrap for values >255, producing a different
register value than requested with no diagnostics.

src/DeviceConfig.h[226-240]
src/jaguar3/RtlJaguar3Device.cpp[881-884]
src/jaguar2/RtlJaguar2Device.cpp[246-255]
src/kestrel/RtlKestrelDevice.cpp[257-263]

Agent prompt

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

## Issue description
The new `ack_timeout_us` config field is written using `static_cast<uint8_t>(int_value)`. If `ack_timeout_us` is ever set outside the env clamp path (e.g., direct struct construction, tests, future CLI/config file), values outside `[0,255]` will wrap modulo 256 and silently program an unintended timeout.
### Issue Context
- The field is public and typed as `int`, with no invariant enforcement.
- Multiple devices write it directly to HW as a byte.
### Fix Focus Areas
- src/DeviceConfig.h[226-239]
- src/jaguar1/RtlJaguarDevice.cpp[83-85]
- src/jaguar2/RtlJaguar2Device.cpp[252-254]
- src/jaguar3/RtlJaguar3Device.cpp[881-884]
- src/kestrel/RtlKestrelDevice.cpp[257-260]
### Implementation notes
- Clamp at the point of use (device bring-up) to `0..255` and only write when `>0`.
- Prefer logging a warning when clamping (similar to Kestrel’s retry_limit clamp) so misconfiguration is visible.
- Optionally change the type to `std::optional<uint8_t>` (unset = default) to make the invariant unrepresentable in an invalid form.

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

---

3. ACK knob docs overreach ✓ Resolved 🐞 Bug ⚙ Maintainability

Description

The new DEVOURER_ACK_TIMEOUT_US documentation describes an “ACK/CTS response window” and implies a
single universal field, but existing code shows related timeouts can be split (e.g., CTS2TO and CCK
ACK timeout are programmed separately on Jaguar3). This makes the knob’s documented semantics
misleading for users expecting it to affect all response-timeout paths.

Code

src/DeviceConfig.h[R226-234]

+    /* env: DEVOURER_ACK_TIMEOUT_US — hardware ACK/CTS response window in µs
+     * (1..255; 0 = per-chip default), the hardware-ARQ RANGE lever: the MAC
+     * writes off a frame (and retries) when no ACK is counted within this
+     * window, and round-trip propagation eats ~6.7 µs per km, so a long
+     * link needs the window opened. One 8-bit register everywhere:
+     * REG_ACKTO 0x640 on the 11ac generations (defaults: 0x80 J1/J2's MAC
+     * reset value; 0x21 J3 halmac, scaled to 0x3D/0x75 at 10/5 MHz
+     * narrowband — the override REPLACES the per-bandwidth default, so
+     * budget the slowest ACK duration in use), R_AX_RSP_CHK_SIG 0xCC00

Evidence

DeviceConfig.h claims the knob is an “ACK/CTS” window and implies a single field, but Jaguar3
bring-up code clearly programs REG_CTS2TO and REG_ACKTO_CCK separately from REG_ACKTO,
demonstrating that not all response-timeout behavior is covered by the single byte written by this
knob.

src/DeviceConfig.h[226-235]
src/jaguar3/HalmacJaguar3MacInit.cpp[766-785]
src/jaguar3/RtlJaguar3Device.cpp[881-884]

Agent prompt

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

## Issue description
The `DEVOURER_ACK_TIMEOUT_US` comment currently overstates the scope ("ACK/CTS") and implies a single universal register, but the codebase already distinguishes related timing registers (e.g., Jaguar3 programs `REG_CTS2TO` and `REG_ACKTO_CCK` separately from `REG_ACKTO`). The knob implementation only writes `REG_ACKTO` (or Kestrel’s `R_AX_RSP_CHK_SIG` byte0).
### Issue Context
This is primarily a contract/documentation correctness issue: users may assume CTS/CCK timing is also adjusted.
### Fix Focus Areas
- src/DeviceConfig.h[226-238]
- src/jaguar3/HalmacJaguar3MacInit.cpp[766-785]
### Implementation notes
- Option A (docs-only): rename wording to “ACK timeout (REG_ACKTO)” and explicitly state it does not touch `CTS2TO` / `ACKTO_CCK`.
- Option B (behavioral): if the intent truly includes CTS/CCK, add per-family writes for the additional registers where applicable, and document exactly which registers are set on each generation.

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

---

_{To customize comments, go to the Qodo configuration screen, or learn more in the docs.}