rtl8733b: port the hardware-ARQ surface — ACK responder, retry limit, honoured ACK window by snokvist · Pull Request #406 · OpenIPC/devourer

snokvist · 2026-08-30T18:09:01Z

Closes the four items in snokvist#2, filed by a consumer scoping a ground→craft return path on the 8733BU. Three are now measured working on hardware; the fourth is root-caused and refused out loud rather than faked.

What changed

1. tx.ack_timeout_us was silently ignored. init_wmac() hardcoded REG_ACKTO to 0x21 (33 µs) and the config field was read nowhere in src/rtl8733b/. So the one library default every generation programs identically was, on this die alone, pinned to the bottom of the 33..128 µs spread that default exists to abolish. Now applied at bring-up with the same register and 1..255 clamp the Jaguars use, and read back before it is claimed:

`DEVOURER_ACK_TIMEOUT_US`	`<unset>`	128	33	200
REG_ACKTO reads	128	128	33	200

The vendor write stays in init_wmac() — that MAC plane is shared verbatim with rtl8733bprobe, which wants the vendor recipe and carries no DeviceConfig.

2. SetAckResponder ported and measured → ack_responder_ok = true. The src/AckResponder.h recipe applies unchanged, and that is not an assumption: the vendor tree's own port-0 descriptor names these three registers (net_type REG_CR_8733B + 2 = 0x0102, MACID 0x0610, BSSID 0x0618). MAC bring-up leaves net_type at No Link because init_mac writes only REG_CR's low half — exactly why a monitor radio here never ACKed.

phase (8733B as responder, 8812AU soliciting)	ack_rate	mean retries	frames
armed	1.00	0.00	1736/1736
re-armed on a different MAC	1.00	0.00	1733/1733
disarmed (control)	0.00	12.0 (pinned at limit)	1714

The arm refuses a group MAC, is read back before being reported, and teardown disarms it — _mac.stop() clears only REG_CR's low half, so a session ending with teardown_power_down off would otherwise leave an unowned radio auto-ACKing. Verified: after such a session the peer reads ack_rate 0.00.

3. tx.retry_limit drives real autonomous retransmission → tx_retry_limit_ok = true. This could not be measured the way the Jaguars were — that A/B reads the TX side's own CCX reports, and this die has none. New tests/rtl8733b_retry_limit_onair.sh judges from the air: unicast to an unowned RA so no ACK ever returns, a passive monitor counting airings per submitted frame. It takes a dose-response rather than an on/off pair, because one pair could be ambient and a straight line through three levels cannot:

retry_limit	0	3	12	0	12
airings/frame (expected 1+N)	0.93	3.93	12.27	0.93	12.27

4. CCX / tx.report is NOT ported — the cause is the firmware. Everything under this backend's control was verified correct on air: descriptor SPE_RPT and SW_DEFINE at the vendor's own bit positions, C2H decoded at the vendor's dword2[28], bulk-IN as the vendor's delivery path (its USB interrupt handler is an empty stub), and the fw-offload C2H format this firmware speaks per its own dispatch. With an RX loop live and 3160 frames received, the chip returned zero C2H packets in any format — with and without net_type armed, peer both ACKing and silent.

The plumbing is therefore not shipped: descriptors stay byte-identical and the knob warns at bring-up. A knob that looks granted while producing nothing is the exact failure the issue was filed about. The outstanding lead (halmac H2C queue + MEDIA_STATUS_RPT to register the descriptor MACID — this backend has no H2C transport at all) is recorded so the next attempt starts where this one stopped.

Counterparts

Stated because the numbers above are uniformly favourable: one physical unit and one peer generation (f72b RTL8731BU vs RTL8812AU), no second responder die, no vendor-driver A/B, and the second b733 sample has not run these cells. The retry figure is witnessed by a monitor that can only lose airings, never invent them, so it is a floor — its ~0.07/frame shortfall is the rx.txhit sampling quantization the harness documents.

Verification

Builds clean and ctest green on default / 8733B-only / 8733B-off / ASan+UBSan (54/50/49/54 tests).
Lifecycle soak 6/6 HEALTHY; ASan teardown clean on hardware.
Monitor RX unaffected by arming the responder (201/197/181/210 frames across alternating arms, no trend).
Added the 8733B to tests/teardown_gen_sanity.sh, whose cell list predated the chip.
docs/scheduled-mac.md's cross-generation ARQ matrix updated — the 8733B is a new combination there: closed-loop ARQ without a CCX report.

🤖 Generated with Claude Code

https://claude.ai/code/session_01VjcZRPjNtsy2mxfDkAWNpd

… honoured ACK window Closes the four items in #2, filed by a consumer scoping a ground->craft return path on this die. Three are now measured working; the fourth is root-caused and refused out loud instead of being faked. - **`tx.ack_timeout_us` was silently ignored.** `init_wmac()` hardcoded REG_ACKTO to 0x21 (33 us) and the config field was never read anywhere in `src/rtl8733b/` — so the one library default every generation is supposed to program identically was, on this die alone, pinned to the bottom of the 33..128 us spread that default exists to abolish. It is now applied at bring-up with the same register and 1..255 clamp the Jaguars use, and read back before it is claimed: `<unset>/128/33/200 -> 128/128/33/200`. The vendor write stays in `init_wmac()` because that MAC plane is shared verbatim with `rtl8733bprobe`, which wants the vendor recipe and carries no DeviceConfig. - **`SetAckResponder` is ported and measured** — `ack_responder_ok = true`. The `src/AckResponder.h` recipe applies unchanged, which is not an assumption: the vendor tree's own port-0 descriptor names these three registers (net_type `REG_CR_8733B + 2` = 0x0102, MACID 0x0610, BSSID 0x0618). MAC bring-up leaves net_type at No Link because `init_mac` writes only REG_CR's low half, which is exactly why a monitor radio here never ACKed. Measured against an RTL8812AU soliciting TX: armed 1736/1736 frames ACKed at retries_mean 0.00; re-armed on a different MAC, 1736/1736 again; disarmed, 0.00 with retries pinned at 12. The arm refuses a group MAC and is read back before being reported, and teardown disarms it — `_mac.stop()` clears only REG_CR's low half, so a session ending with `teardown_power_down` off would otherwise leave an unowned radio auto-ACKing (verified: after such a session the peer reads ack_rate 0.00). - **`tx.retry_limit` drives real autonomous retransmission** — `tx_retry_limit_ok = true`. It could not be measured the way the Jaguars were: that A/B reads the TX side's own CCX reports and this die has none. New `tests/rtl8733b_retry_limit_onair.sh` judges from the air instead — unicast to an unowned RA so no ACK ever returns, a passive monitor counting airings per submitted frame — and takes a dose-response rather than an on/off pair, because one pair could be ambient and a straight line through three levels cannot: 0/3/12 -> 0.93/3.93/12.27 airings per frame against an expected 1 + N, repeatable over a 0/3/12/0/12 ladder. - **CCX / `tx.report` is NOT ported, and the cause is the firmware.** Everything under this backend's control was verified correct on air — descriptor SPE_RPT and SW_DEFINE at the vendor's own bit positions, C2H decoded at the vendor's dword2[28], bulk-IN as the vendor's delivery path, and the fw-offload C2H format this firmware speaks per its own dispatch — and with an RX loop live and 3160 frames received the chip returned zero C2H packets in any format, with and without net_type armed and with the peer both ACKing and silent. The plumbing is therefore NOT shipped: descriptors stay byte-identical and the knob warns at bring-up, because a knob that looks granted while producing nothing is the exact failure this issue was filed about. The outstanding lead (halmac H2C queue + MEDIA_STATUS_RPT to register the descriptor MACID) is recorded so the next attempt starts where this one stopped. Counterparts, stated because the numbers above are uniformly favourable: one physical unit and one peer generation, no second responder die, no vendor-driver A/B. The retry figure is witnessed by a monitor that can only lose airings, never invent them, so it is a floor; its ~0.07/frame shortfall is the sampling quantization the harness documents. Verification: builds clean on default / 8733B-only / 8733B-off / ASan+UBSan with ctest green in each; lifecycle soak 6/6 HEALTHY; monitor RX unaffected by arming the responder; ASan teardown clean on hardware, and the 8733B added to `tests/teardown_gen_sanity.sh`, whose cell list predated the chip. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01VjcZRPjNtsy2mxfDkAWNpd

qodo-free-for-open-source-projects · 2026-08-30T18:10:05Z

PR Summary by Qodo

Port hardware ARQ controls to RTL8733B

🐞 Bug fix ✨ Enhancement 🧪 Tests 📝 Documentation 🕐 40+ Minutes

AI Description

• Honor and verify configured ACK timeouts during RTL8733B bring-up.
• Add safe, read-back-verified ACK responder arming and teardown disarming.
• Validate autonomous retries on air while explicitly refusing unavailable CCX reports.

Diagram

graph TD
  CFG["Device Config"] --> DEV["8733B Device"] --> MAC["HALMAC Layer"] --> REG["MAC Registers"]
  DEV --> ACK["ACK Responder"] --> REG
  DEV --> TX["TX Descriptor"] --> AIR["On-air Behavior"]
  TEST["Hardware Harness"] --> AIR

High-Level Assessment

The following are alternative approaches to this PR:

1. Implement H2C and CCX support now

➕ Could provide per-frame delivery reports on RTL8733B.
➕ Would align the backend with Jaguar TX-report behavior.
➖ Requires a new H2C transport and firmware MACID registration path.
➖ Firmware behavior remains uncertain despite correct descriptor and C2H plumbing.
➖ Substantially increases scope and hardware validation risk.

Recommendation: Ship the measured register-level ARQ controls and explicitly refuse TX reports, as this avoids presenting an ineffective feature as supported. Pursue H2C queue transport and MEDIA_STATUS_RPT separately before enabling CCX, with dedicated firmware-level validation.

Files changed (11) +463 / -10

Enhancement (2) +164 / -0

Rtl8733bDevice.cppImplement RTL8733B hardware ARQ lifecycle controls +155/-0

Implement RTL8733B hardware ARQ lifecycle controls

• Applies the configured ACK timeout, ports the shared ACK responder with unicast validation and register readback, and disarms it during teardown. It advertises measured responder and retry capabilities while warning that firmware-backed TX reports remain unavailable.

src/rtl8733b/Rtl8733bDevice.cpp

Rtl8733bDevice.hDeclare responder APIs and armed-state tracking +9/-0

Declare responder APIs and armed-state tracking
• Overrides ACK responder controls and tracks whether teardown must clear the hardware responder state.
src/rtl8733b/Rtl8733bDevice.h

Bug fix (2) +20 / -0

Halmac8733bMac.cppAdd verified ACK timeout register programming +15/-0

Add verified ACK timeout register programming
• Adds a MAC-layer setter that writes and reads back REG_ACKTO while preserving the vendor initialization recipe for probe users.
src/rtl8733b/Halmac8733bMac.cpp

Halmac8733bMac.hExpose RTL8733B ACK timeout control +5/-0

Expose RTL8733B ACK timeout control
• Declares the register-plane API for setting and verifying the non-CCK ACK timeout.
src/rtl8733b/Halmac8733bMac.h

Tests (2) +125 / -0

rtl8733b_retry_limit_onair.shAdd on-air retry-limit dose-response harness +123/-0

Add on-air retry-limit dose-response harness

• Measures autonomous retransmissions across repeated retry-limit levels using an unowned unicast address and passive witness. It rejects incomplete runs and evaluates measured airings against expected attempts.

tests/rtl8733b_retry_limit_onair.sh

teardown_gen_sanity.shCover RTL8733B variants in teardown sanity tests +2/-0

Cover RTL8733B variants in teardown sanity tests
• Adds standalone and combo RTL8733B USB identifiers to the cross-generation ASan initialization and teardown matrix.
tests/teardown_gen_sanity.sh

Documentation (5) +154 / -10

CLAUDE.mdUpdate top-level RTL8733B feature boundaries +3/-2

Update top-level RTL8733B feature boundaries

• Reclassifies hardware ACK and retry limits as measured RTL8733B capabilities while retaining CCX/TX reports among unsupported features.

CLAUDE.md

rtl8733b.mdDocument RTL8733B hardware ARQ support and limits +45/-3

Document RTL8733B hardware ARQ support and limits

• Adds measured ACK timeout, responder, and retry-limit results. Records the firmware-rooted CCX gap, validation constraints, and application-level timeout consequence.

docs/rtl8733b.md

scheduled-mac.mdAdd RTL8733B to cross-generation ARQ guidance +13/-2

Add RTL8733B to cross-generation ARQ guidance
• Documents RTL8733B closed-loop ARQ and responder results while explaining why its missing CCX reports exclude it from TX-report metrics.
docs/scheduled-mac.md

AdapterCaps.hRecord RTL8733B ARQ capability evidence +7/-2

Record RTL8733B ARQ capability evidence
• Extends capability comments with measured ACK responder and on-air retry-limit evidence for RTL8733B.
src/AdapterCaps.h

CLAUDE.mdCapture RTL8733B ARQ implementation and bench findings +86/-1

Capture RTL8733B ARQ implementation and bench findings
• Documents register choices, lifecycle safeguards, hardware measurements, validation limitations, and the unresolved firmware dependency for CCX reports.
src/rtl8733b/CLAUDE.md

qodo-free-for-open-source-projects · 2026-08-30T18:11:05Z

Code Review by Qodo

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

1. ~~Root contains RTL8733B specifics~~ ✓ Resolved 📘 Rule violation ⚙ Maintainability

Description

The root CLAUDE.md now records RTL8733B-specific CCX, ACK-responder, and retry-limit status even
though src/rtl8733b/CLAUDE.md is the applicable nested guidance file. This places subtree-specific
facts at the wrong documentation scope.

Code

CLAUDE.md[R62-65]

+  Everything the backend has not ported (TSF/beacons, A-MPDU, CCX/`tx.report`,
the flat-index and per-rate TX-power knobs) falls through to `IRtlDevice`'s
-  not-ported defaults rather than being faked, so read the base class before
+  not-ported defaults rather than being faked (hardware ACK/BlockAck IS ported
+  and measured, as is `tx.retry_limit` — `src/rtl8733b/CLAUDE.md`), so read the base class before

Evidence
Compliance rule 1 requires generation- and chip-specific facts to live in the deepest applicable
CLAUDE.md. The root guidance itself identifies src/rtl8733b/CLAUDE.md as the location for
per-chip mechanisms, while the changed root lines add this backend's exact unsupported and measured
ARQ feature status.
CLAUDE.md: Place Documentation in the Narrowest Applicable CLAUDE.md: CLAUDE.md: Place Documentation in the Narrowest Applicable CLAUDE.md: CLAUDE.md: Place Documentation in the Narrowest Applicable CLAUDE.md: CLAUDE.md: Place Documentation in the Narrowest Applicable CLAUDE.md: CLAUDE.md: Place Documentation in the Narrowest Applicable CLAUDE.md: CLAUDE.md: Place Documentation in the Narrowest Applicable CLAUDE.md: CLAUDE.md: Place Documentation in the Narrowest Applicable CLAUDE.md: CLAUDE.md: Place Documentation in the Narrowest Applicable CLAUDE.md
CLAUDE.md[3-9]
CLAUDE.md[62-65]
src/rtl8733b/CLAUDE.md[279-283]

Agent prompt

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

## Issue description
Remove the newly added RTL8733B-specific feature-status facts from the root `CLAUDE.md` and keep those details in the backend's nested guidance.
## Issue Context
The root file is reserved for cross-cutting facts, while the RTL8733B ACK, retry, and CCX details are already documented in the applicable nested file.
## Fix Focus Areas
- CLAUDE.md[62-65]
- src/rtl8733b/CLAUDE.md[279-362]

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

2. ~~ARQ docs duplicate headers~~ ✓ Resolved 📘 Rule violation ⚙ Maintainability

Description

The new ARQ prose repeats ack_timeout_us range/default/register behavior and capability
measurements already maintained in DeviceConfig.h and AdapterCaps.h. Multiple copies of these
authoritative field and capability descriptions can drift independently.

Code

src/rtl8733b/CLAUDE.md[R294-297]

+`DeviceConfig::tx::ack_timeout_us` **is** honoured, and was not until that
+issue: `bring_up_to_phy` overwrites REG_ACKTO (0x0640) from the config with the
+same 1..255 clamp the Jaguars use, right after MAC bring-up wrote the vendor
+0x21 — 33 us, the bottom of the 33..128 per-chip spread that field's one

Evidence
Compliance rule 2 makes the listed headers authoritative. The new nested documentation repeats the
same 1..255 clamp, 128 default rationale, register, and measured capability evidence documented
by DeviceConfig.h and AdapterCaps.h, and the new public documentation table repeats those field
and capability descriptions again.
CLAUDE.md: Do Not Duplicate Header Documentation: CLAUDE.md: Do Not Duplicate Header Documentation: CLAUDE.md: Do Not Duplicate Header Documentation: CLAUDE.md: Do Not Duplicate Header Documentation: CLAUDE.md: Do Not Duplicate Header Documentation: CLAUDE.md: Do Not Duplicate Header Documentation: CLAUDE.md: Do Not Duplicate Header Documentation: CLAUDE.md: Do Not Duplicate Header Documentation
src/rtl8733b/CLAUDE.md[294-301]
docs/rtl8733b.md[378-383]
src/DeviceConfig.h[226-247]
src/AdapterCaps.h[169-195]

Agent prompt

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

## Issue description
Replace duplicated configuration-field and capability descriptions in the new ARQ documentation with references to the authoritative header doc-comments.
## Issue Context
`src/DeviceConfig.h` owns the `ack_timeout_us` contract and `src/AdapterCaps.h` owns the capability-flag contract. Keep backend-specific implementation or bench notes only where they do not restate those contracts.
## Fix Focus Areas
- docs/rtl8733b.md[378-383]
- src/rtl8733b/CLAUDE.md[294-335]
- src/DeviceConfig.h[226-247]
- src/AdapterCaps.h[169-195]

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

3. ~~Measured claim omits counterpart~~ ✓ Resolved 📘 Rule violation ≡ Correctness

Description

The root summary newly claims the RTL8733B hardware ACK and retry-limit behavior is measured without
stating the one-unit/one-peer limitation or an adverse/control result in that context. A link to the
nested document does not put the required counterpart alongside the favorable claim.

Code

CLAUDE.md[R64-65]

+  not-ported defaults rather than being faked (hardware ACK/BlockAck IS ported
+  and measured, as is `tx.retry_limit` — `src/rtl8733b/CLAUDE.md`), so read the base class before

Evidence
Compliance rule 3 requires a favorable measurement and its adverse result, control, limitation, or
counterexample in the same context. The changed root text only says both features are ported and
measured, whereas the caveats and disarmed/control evidence appear solely in the nested file.
CLAUDE.md: Report Favorable Measurements With Adversarial Counterparts: CLAUDE.md: Report Favorable Measurements With Adversarial Counterparts: CLAUDE.md: Report Favorable Measurements With Adversarial Counterparts: CLAUDE.md: Report Favorable Measurements With Adversarial Counterparts: CLAUDE.md: Report Favorable Measurements With Adversarial Counterparts: CLAUDE.md: Report Favorable Measurements With Adversarial Counterparts: CLAUDE.md: Report Favorable Measurements With Adversarial Counterparts: CLAUDE.md: Report Favorable Measurements With Adversarial Counterparts
CLAUDE.md[64-65]
src/rtl8733b/CLAUDE.md[285-292]
src/rtl8733b/CLAUDE.md[319-335]

Agent prompt

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

## Issue description
Do not leave the favorable measured-ARQ claim in the root summary without its adversarial counterpart or validation limitation.
## Issue Context
The detailed RTL8733B guidance records the relevant disarmed control and the one-unit/one-peer limitation. Prefer removing this chip-specific claim from the root; if retained elsewhere, place the counterpart in the same context.
## Fix Focus Areas
- CLAUDE.md[64-65]
- src/rtl8733b/CLAUDE.md[285-292]
- src/rtl8733b/CLAUDE.md[319-335]

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

View action required (1)

4. ~~Partial arm escapes teardown~~ ✓ Resolved 🐞 Bug ☼ Reliability

Description

SetAckResponder() records _ack_armed only after the multi-operation ack::enable() call
returns, so an exception after the net-type write reaches hardware causes Init()/InitWrite()
teardown to skip disarming it. With power-down disabled, this can leave the adapter auto-ACKing
after the failed session because _mac.stop() does not clear the surviving net-type field.

Code

src/rtl8733b/Rtl8733bDevice.cpp[834]
+  devourer::ack::enable(_device, mac.data());

Evidence
The enable helper performs multiple separately failing register accesses before software records the
armed state. Initialization delegates exception cleanup to Stop(), whose new guard skips the
disable whenever that state was not recorded, while this PR's own teardown logic documents that the
hardware net-type survives _mac.stop().
src/AckResponder.h[43-53]
src/rtl8733b/Rtl8733bDevice.cpp[154-168]
src/rtl8733b/Rtl8733bDevice.cpp[172-201]
src/rtl8733b/Rtl8733bDevice.cpp[834-842]
src/rtl8733b/Rtl8733bDevice.cpp[927-947]

Agent prompt

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

## Issue description
Prevent a partially completed ACK-responder enable from escaping teardown. The current state flag is set only after all register operations return, although an earlier write—including the final net-type write—may already have reached hardware when a later USB operation reports failure.
## Issue Context
`ack::enable()` performs several independent register operations, while both initialization paths catch bring-up exceptions and call `Stop()`. `Stop()` only disables the responder when `_ack_armed` is true.
## Fix Focus Areas
- src/rtl8733b/Rtl8733bDevice.cpp[834-842]
- src/rtl8733b/Rtl8733bDevice.cpp[939-946]
- src/AckResponder.h[43-53]

ⓘ 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 ↗

… duplication - **Partial arm could escape teardown** (Qodo OpenIPC#4, real bug). `_ack_armed` was set after `ack::enable()` returned. That call is five register writes ending with the net_type field that actually arms the engine, and any of them can throw on a stalling device — including after the arming write reached hardware. The exception unwinds into Init()/InitWrite()'s catch, which calls Stop(); with the flag still false that teardown skipped the disarm, leaving an unowned radio auto-ACKing when teardown_power_down is off. Latch before the first write instead: a redundant disarm costs one USB round-trip, a missed one costs an unowned transmitter. - **Root CLAUDE.md carried subtree facts** (Qodo #1, OpenIPC#3). The root file is cross-cutting only, and the added parenthetical also stated a favourable measured result with its counterpart a link away rather than in the same breath. Dropped — the not-ported list is updated, and the ARQ narrative stays in src/rtl8733b/CLAUDE.md where its counterparts already sit. - **ARQ prose restated the ack_timeout_us contract** (Qodo #2). Range, clamp, default and register are doc-commented at the DeviceConfig declaration; the subtree file now points there and keeps only what is specific to this backend (the vendor 0x21 that init_wmac still writes and why, the readback, the measured values). Re-verified on hardware after the change: responder 1349/1349 ACKed at retries_mean 0.00, retarget 1348/1349, disarmed control 0.00 with retries pinned at 12; teardown disarm still holds with power-down disabled. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01VjcZRPjNtsy2mxfDkAWNpd

Pass 198 shipped both hybrid knobs off in every deploy/*.json, deliberately: arming is operator-owned. This arms the RESPONDER half only. Split from the original two-file change on purpose. The two halves are independently deployable by design (§3.0: receivers accept both frame shapes), and only one of them carries risk: - The craft half is measured HARMLESS. Arm C (responder on, ground not soliciting) was 829/901 = 92.01% against arm A's 820/900 = 91.11%, downlink 3 permille in both. A responder only answers when asked; it cannot originate traffic and cannot storm. It is pure upside the moment any ground solicits. - The ground half (policy.return.unicast) is what costs airtime, and it is held back. It has no posture for a craft that is PRESENT but cannot ACK (issue #261): the SA latch stays fresh off the craft's own video, the storm guard never fires, and the ground pays retry_limit+1 copies of every return indefinitely — measured 14 permille against a 4 permille baseline. That is the .181 8733BU today and every craft mid-rolling-upgrade, and it matters most for the usual ground use-case, which is latching whichever craft the operator picks rather than a fixed pair. Device-verified on this craft (8812EU / Jaguar3, merged f7f3a36): with a ground soliciting, tx_report_fails fell from 2796/2796 to 48/2790 (1.7%) and report delivery reached 99.89%. Numbers and geometry in docs/findings.md (2026-08-30 entries). .181 deliberately does NOT get this: SetAckResponder is unported on its 8733BU, so arming there is a logged degrade, not a responder. OpenIPC/devourer#406 ports it; revisit once vendored. Live-toggleable without a restart: POST /api/v1/air/ack_responder. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_015G1r7W361tynRf3aZqGPEC

Pass 198 shipped both hybrid knobs off in every deploy/*.json, deliberately: arming is operator-owned. This arms the RESPONDER half only. Split from the original two-file change on purpose. The two halves are independently deployable by design (§3.0: receivers accept both frame shapes), and only one of them carries risk: - The craft half is measured HARMLESS. Arm C (responder on, ground not soliciting) was 829/901 = 92.01% against arm A's 820/900 = 91.11%, downlink 3 permille in both. A responder only answers when asked; it cannot originate traffic and cannot storm. It is pure upside the moment any ground solicits. - The ground half (policy.return.unicast) is what costs airtime, and it is held back. It has no posture for a craft that is PRESENT but cannot ACK (issue #261): the SA latch stays fresh off the craft's own video, the storm guard never fires, and the ground pays retry_limit+1 copies of every return indefinitely — measured 14 permille against a 4 permille baseline. That is the .181 8733BU today and every craft mid-rolling-upgrade, and it matters most for the usual ground use-case, which is latching whichever craft the operator picks rather than a fixed pair. Device-verified on this craft (8812EU / Jaguar3, merged f7f3a36): with a ground soliciting, tx_report_fails fell from 2796/2796 to 48/2790 (1.7%) and report delivery reached 99.89%. Numbers and geometry in docs/findings.md (2026-08-30 entries). .181 deliberately does NOT get this: SetAckResponder is unported on its 8733BU, so arming there is a logged degrade, not a responder. OpenIPC/devourer#406 ports it; revisit once vendored. Live-toggleable without a restart: POST /api/v1/air/ack_responder. Claude-Session: https://claude.ai/code/session_015G1r7W361tynRf3aZqGPEC Co-authored-by: Claude Opus 5 (1M context) <noreply@anthropic.com>

@5540

… parks, retune reverts (Pass 199) (#262) * deploy: arm the §3.0 responder half on craft .232 Pass 198 shipped both hybrid knobs off in every deploy/*.json, deliberately: arming is operator-owned. This arms the RESPONDER half only. Split from the original two-file change on purpose. The two halves are independently deployable by design (§3.0: receivers accept both frame shapes), and only one of them carries risk: - The craft half is measured HARMLESS. Arm C (responder on, ground not soliciting) was 829/901 = 92.01% against arm A's 820/900 = 91.11%, downlink 3 permille in both. A responder only answers when asked; it cannot originate traffic and cannot storm. It is pure upside the moment any ground solicits. - The ground half (policy.return.unicast) is what costs airtime, and it is held back. It has no posture for a craft that is PRESENT but cannot ACK (issue #261): the SA latch stays fresh off the craft's own video, the storm guard never fires, and the ground pays retry_limit+1 copies of every return indefinitely — measured 14 permille against a 4 permille baseline. That is the .181 8733BU today and every craft mid-rolling-upgrade, and it matters most for the usual ground use-case, which is latching whichever craft the operator picks rather than a fixed pair. Device-verified on this craft (8812EU / Jaguar3, merged f7f3a36): with a ground soliciting, tx_report_fails fell from 2796/2796 to 48/2790 (1.7%) and report delivery reached 99.89%. Numbers and geometry in docs/findings.md (2026-08-30 entries). .181 deliberately does NOT get this: SetAckResponder is unported on its 8733BU, so arming there is a logged degrade, not a responder. OpenIPC/devourer#406 ports it; revisit once vendored. Live-toggleable without a restart: POST /api/v1/air/ack_responder. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_015G1r7W361tynRf3aZqGPEC * spec(§11.6): a campaign's failure posture is scoped to its INTENT (Pass 199) Two operator-visible operations shared one campaign machine and one failure posture. §11.6's revert is right for a RETUNE (move the craft we already fly) and wrong for an ACQUIRE (leave this craft, take that one): reverting undoes the operator's explicit request, and lands them on a craft §15.5a has already aged out at 5 s of silence — so the "safety net" also removes the only visible target to retry from. Reported as "I cannot switch freely between my vehicles once I have claimed one". Ruled: - Campaigns carry CampaignIntent. kRetune reverts to prev_chan as before. kAcquire PARKS: stays on the target channel, adopts the target selection (a failed verify is not proof of absence, so §2 first-latch may still take it), drops the previous selection, reports select_failed. Both failure paths — kAbort (no CSA_ARMED) and kRevert (committed, no video) — take that posture. - An ACQUIRE does not move the craft. quickconnect claims on the channel the craft was FOUND on. It previously defaulted an absent target_chan to scout.emptiest(), silently making "switch to craft B" into "switch to craft B and move it elsewhere" — two operations, only the second of which can fail, and whose failure reverted the first. Pinning target == cand->chan is what makes PARK well defined: the craft is on that channel whether the campaign aborted before the commit or reverted after it, so both paths park somewhere the craft actually is. Spec-only; implementation follows in this branch. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_015G1r7W361tynRf3aZqGPEC * feat(§11.6): acquire parks instead of reverting, and never moves the craft Spec landed in the previous commit; this is the code. - CampaignIntent in core/include/wblink/csa.h; rx_node carries the intent of the campaign in flight, set at BOTH campaign start sites (do_claim => kAcquire, /api/v1/csa => kRetune) so no path can inherit a stale value. - kRevert and kAbort branch on it. On kAcquire the ears are already on the craft's channel, so parking costs no retune: adopt the pending selection, drop the previous one, report select_failed, and log which half failed using the issuer's existing armed/landed/video evidence. - do_claim pins target == cand->chan and refuses a foreign target_chan. Moving a craft is /api/v1/csa, once it is yours. Also fixes a defect the device matrix surfaced: a parked acquire whose craft then turned up stayed at select_failed forever while video flowed at 6 permille, because promotion to "latched" was gated on selection_state == "configured" (the boot state). The UI reported a failure the operator could see was not happening. The new promotion is narrower than the adoption beside it — only the originator we parked ON promotes — so it can never adopt a different craft that happens to share the channel. Device matrix on .242 against three crafts (17 @5540, 18 @5825, 19 @5700), full table in docs/findings.md 2026-08-31: S1 foreign target_chan refused, selection untouched S2 acquire with no target_chan keeps the craft on its own channel S3 acquire back S4 forced failure PARKS on the target, does not revert S4b parked acquire promotes to latched when the craft appears S5 NEGATIVE CONTROL: a failed retune still reverts S6 4/4 WebUI clicks across three crafts, via the hub proxy S7 one scout then three clicks, no re-scout: 3/3, list stayed complete Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_015G1r7W361tynRf3aZqGPEC * findings: the Pass 199 device matrix, and the scout's weak-craft gap Records the seven scenarios, the WebUI end-to-end verification, and the second defect S4 surfaced. Also names the still-OPEN scout issue, which is NOT Pass 199: the 1800 ms dwell extension is gated on a frame arriving inside a 300 ms base dwell, of which the 8812AU spends ~220-270 ms deaf after the retune, so a weak craft on a busy channel is intermittently dropped with rejects all-zero — never rejected, never heard. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_015G1r7W361tynRf3aZqGPEC --------- Co-authored-by: Claude Opus 5 (1M context) <noreply@anthropic.com>

josephnef

The code core traced clean across the review: the AckResponder register map was verified against the vendor tree (net_type REG_CR_8733B + 2 = 0x0102, MACID 0x0610, BSSID 0x0618), the arm/teardown exception paths are coherent, the harness's event parsing matches rxdemo's actual emission, and nothing new touches the per-frame send path. The measurements hold up. Requesting changes for one runtime gap (a config-requested arm whose failure is silently discarded — the exact failure shape this PR was filed about), one harness false-verdict hole, and a cluster of doc claims that overrun what was measured. Ten findings total, nine confirmed by an adversarial verify pass; details inline.

One finding has no diff line to anchor to because the file isn't touched by the PR:

src/DeviceConfig.h — the authoritative field docs were not extended. tx.retry_limit still enumerates "the 11ac generations and Kestrel" (omitting the 8733B this PR measures live), and tx.ack_timeout_us's register footnote still lists REG_ACKTO 0x640 only for the 11ac generations and Kestrel, though the 8733B now programs the same 0x640. Root CLAUDE.md names these doc-comments the per-var reference ("do not look for a second copy here") — a caller auditing coverage from them concludes both knobs are inert on the 8733B and budgets the link around the 33 µs vendor value, re-deriving exactly the pre-PR behavior this PR fixed.

…scope the claims Runtime - A config-requested arm no longer discards its result. `DEVOURER_ACK_RESPONDER` with a group MAC (easy to reach: the canonical TX SA 57:42:.. has the I/G bit set) previously gave a green init and a session running as a plain monitor, so the operator debugged the RF link instead of the config. It now fails the bring-up. - The disarm moves into `Halmac8733bMac::stop()`, unconditional. `stop()` clears only REG_CR's low half, so net_type at 0x0102 survives it; siting the clear there means no path can reach `stop()` and leave an unowned SIFS-timed transmitter airing. This deletes the `_ack_armed` flag, its latch-ordering subtlety and the flag-guarded special case at the device layer. - `ack::is_unicast()` and `ack::verify()` join `enable()`/`disable()`, so the register map lives in one file instead of being re-hardcoded by a backend that can drift from it. Jaguar1/2/3 adopt the unicast guard — they returned `true` for a group MAC, the "silently dead responder" shape AdapterCaps.h records from the 8821AU episode. They do NOT adopt verify(): a family whose 0x0102 does not read back would start refusing healthy arms, and that wants a bench cell per die (J1 and J3 verified here, no J2 on this bench). Harness - `FRAMES` gets a floor. The rx.txhit readout quantizes to <=99 airings whatever the frame count, so `FRAMES=60` scored a healthy retry=0 arm at 0.17 against a 0.6 band — a false FAIL, the same shape the script already refuses for an arm that did not run. Docs - The ACK claim scopes to what ran: normal-ACK response to unicast singles. BlockAck response was never measured; `tests/ampdu_ba_check.sh` against this die came back indeterminate (armed and disarmed both 0% delivered at retries 0 — the control did not separate), which is the documented consequence of CCX accounting not surviving AGG_EN, not a verdict on the chip. - The quantization attribution was arithmetically wrong for the retry=12 arm: the <=99-airing bound is <=0.066/frame and covers the 0 and 3 arms, but 12.27 against 13 is 0.73/frame (~1095 airings), an order of magnitude past it. That residue is monitor loss — which is why the ratio is a floor. - `docs/scheduled-mac.md`: the 8733B's single-shot cell is marked unmeasured with the closed-loop figure parenthetical, since that column's neighbours are single-shot rates and 100%-at-retry-12 is not comparable to them; and the "J1/2/3-only" line one row below is corrected. - `src/DeviceConfig.h`, the authoritative per-knob reference, now names the 8733B for both `retry_limit` and `ack_timeout_us`/REG_ACKTO. - Changelog framing removed from the new comments and docs; the rationale halves stand on their own. Verified: builds + ctest green on default / 8733B-only / 8733B-off / ASan+UBSan. On hardware, the new Jaguar guard refuses a group MAC and still arms a unicast one on both 8812AU and 8812CU, and the pre-existing responder path is unregressed (8812AU responder, 8812CU soliciting: on 1.00, retarget 1.00, off 0.00 with retries pinned at 12). Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01VjcZRPjNtsy2mxfDkAWNpd

snokvist · 2026-08-31T15:35:06Z

Addressed the unanchored src/DeviceConfig.h finding in 466bf5e: the authoritative docs for both tx.retry_limit and tx.ack_timeout_us / REG_ACKTO 0x640 now explicitly include RTL8733B.

josephnef

Re-reviewed at 466bf5e. All ten findings verified fixed against the actual delta, not the replies:

Config-arm discard — a refused DEVOURER_ACK_RESPONDER now throws out of bring_up_to_phy, failing the bring-up, and the unwind path is safe because the disarm is now unconditional in stop().
Disarm altitude — ack::disable moved into Halmac8733bMac::stop(), unconditional; _ack_armed, its latch-ordering subtlety and the Stop() special case are all deleted. Net −27 lines at the device layer.
Duplicated register recipe — ack::is_unicast() / ack::verify() now live beside enable()/disable() in AckResponder.h; the 8733B consumes both, and the deliberate scoping of verify() (MACID + net_type checked, BSSID written-not-checked, documented why) is the right call. The J1/J2/J3 adoption of the unicast guard — with verify() explicitly deferred pending a per-die bench cell — is exactly the split I'd have asked for, and the 8812AU/8812CU regression check covers it.
FRAMES floor — refuses below 1000 with the quantization rationale in the abort message; the header's error bound is restated at the floor (≤0.099/frame), which is correct.
Doc scope — BlockAck claim retracted to normal-ACK singles in both docs, and the indeterminate ampdu_ba_check.sh run is reported as indeterminate with the CCX/AGG_EN explanation rather than either suppressed or spun. The quantization attribution now splits the 0/3 arms (covered by the ≤0.066 bound) from the 12 arm's 0.73/frame residue, attributed to monitor loss and folded into the ratio-is-a-floor caveat. Arithmetic checks out.
scheduled-mac table — single-shot cell now unmeasured with the closed-loop figure parenthetical, matching the 8821AU row's convention; Kestrel line corrected.
DeviceConfig.h — both field docs now name the 8733B.
Conventions — issue tags and changelog framing gone from the new comments and docs (the one surviving issue # in DeviceConfig.h is a pre-existing line this PR never touched); the rationale halves stand alone as promised.

Independently verified on this end: default build + full ctest green (54/54) at 466bf5e.

One non-blocking observation, no action needed: the failed-verify path in SetAckResponder no longer attempts an immediate rollback — a mid-session runtime call that verify-fails with net_type latched would leave a half-armed engine until teardown. The comment documents the choice and the scenario needs a silent readback mismatch without a USB exception, so deferring to the unconditional stop() disarm is defensible; just noting the window exists.

qodo-free-for-open-source-projects Bot reviewed Aug 30, 2026

View reviewed changes

Comment thread CLAUDE.md Outdated

Comment thread src/rtl8733b/CLAUDE.md Outdated

Comment thread CLAUDE.md Outdated

Comment thread src/rtl8733b/Rtl8733bDevice.cpp

snokvist mentioned this pull request Aug 30, 2026

deploy: arm the §3.0 responder half on craft .232 snokvist/waybeam-link#259

Merged

josephnef requested changes Aug 31, 2026

View reviewed changes

josephnef approved these changes Aug 31, 2026

View reviewed changes

josephnef merged commit aa92276 into OpenIPC:master Aug 31, 2026
40 checks passed

snokvist mentioned this pull request Sep 1, 2026

rtl8733b: harden ARQ and qualify BlockAck response #407

Open

Navigation Menu

Uh oh!

Uh oh!

rtl8733b: port the hardware-ARQ surface — ACK responder, retry limit, honoured ACK window - #406

rtl8733b: port the hardware-ARQ surface — ACK responder, retry limit, honoured ACK window#406
josephnef merged 3 commits into
OpenIPC:masterfrom
snokvist:feat/8733b-hardware-arq

snokvist commented Aug 30, 2026

Uh oh!

qodo-free-for-open-source-projects Bot commented Aug 30, 2026

Uh oh!

qodo-free-for-open-source-projects Bot commented Aug 30, 2026 •

edited

Loading

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

josephnef left a comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

snokvist commented Aug 31, 2026

Uh oh!

josephnef left a comment

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

2 participants

Uh oh!

Uh oh!

Conversation

snokvist commented Aug 30, 2026

What changed

Counterparts

Verification

Uh oh!

qodo-free-for-open-source-projects Bot commented Aug 30, 2026

PR Summary by Qodo

Uh oh!

qodo-free-for-open-source-projects Bot commented Aug 30, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Code Review by Qodo

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

josephnef left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

snokvist commented Aug 31, 2026

Uh oh!

josephnef left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

2 participants

qodo-free-for-open-source-projects Bot commented Aug 30, 2026 •

edited

Loading