Skip to content
Open
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
103 changes: 81 additions & 22 deletions docs/rtl8733b.md
Original file line number Diff line number Diff line change
Expand Up @@ -53,21 +53,45 @@ factory path:
adapter-doctor checks.

Per-rate delivery, witnessed by an RTL8812AU (`0bda:8812`) in monitor mode on
channel 6, 300 submitted frames per rate. The witness's hit counter reports the
first ten hits then every hundredth, so these are floors at 100-frame
granularity:

| requested | captured | FCS errors |
channel 6, 300 submitted frames per rate. Counted per frame from the witness's
own RX events, so these are exact rather than sampled:

| requested | delivered | decoded as | FCS errors |
| --- | --- | --- | --- |
| 1 Mbps CCK | 298 / 300 | 1 Mbps | 0 |
| 2 Mbps CCK | 300 / 300 | 2 Mbps | 0 |
| 5.5 Mbps CCK | 300 / 300 | 5.5 Mbps | 0 |
| 11 Mbps CCK | 294 / 300 | 11 Mbps | 0 |
| 6 Mbps OFDM | 300 / 300 | 6 Mbps | 0 |
| HT MCS0 | 270 / 300 | MCS0 | 0 |
| HT MCS7 | 277 / 300 | MCS7 | 0 |

Every frame decoded at the rate it was requested at, and no captured frame
carried an FCS error at any rate. These are single probes, so per this repo's
own guidance they are worth about +/-3 points each.

The two HT readings sitting below the legacy ones are **not** a transmitter
effect, and the witness was qualified rather than assumed
(`tests/ground_station_qualify.sh` exists for exactly this question). Sweeping
the RTL8812AU witness across the HT ladder, 300 frames each:

| rate | delivered | witness RSSI |
| --- | --- | --- |
| 1 Mbps CCK | 300 / 300 | 0 |
| 6 Mbps OFDM | 300 / 300 | 0 |
| HT MCS7 | 300 / 300 | 0 |
| 11 Mbps CCK | >= 900 / 1000 | 0 |

11 Mbps was re-run at 1,000 frames because its first result sat on a counter
boundary; both repetitions landed in the same bucket, so the small deficit
relative to the other three rates is repeatable rather than noise — expected,
since 11 Mbps is the least robust CCK rate.
| MCS0 | 270 / 300 | 57.8 |
| MCS1 | 300 / 300 | 65.0 |
| MCS3 | 300 / 300 | 65.3 |
| MCS5 | 300 / 300 | 65.2 |
| MCS6 | 296 / 300 | 65.1 |
| MCS7 | 277 / 300 | 60.3 |

The ladder is flat at 100% through MCS5, so the witness is nowhere near its
modulation cliff and is fit to measure this rate range — a receiver measuring
itself would fall off progressively at the top, not dip at both ends. MCS0 is
the *most* robust rate in the set, so its being the lowest reading rules out a
cliff at either end. Both low readings also came with 5-7 units less RSSI than
the flat middle, which places the variation in the per-run link conditions
rather than in modulation. Treat the whole HT column as one flat band around
90-100% until something moves it by more than the probe noise.

A mixed run alternating all eight rates (1/2/5.5/11 Mbps CCK, 6 Mbps OFDM,
MCS0/4/7) submitted 250 frames through a CCK/OFDM TSSI table switch on every
Expand Down Expand Up @@ -101,14 +125,42 @@ transition-churn run submitted 1,000/1,000 frames through 118 CCK/OFDM TSSI
table changes; the witness captured 660 canonical frames across both rates,
all with valid FCS, while the same 32–33 thermal range held.

Switching the TSSI thermal table between its CCK and OFDM/HT variants costs
84 ms — 136 USB register round trips, no sleeps — because the backend tears
down and rebuilds the whole TSSI front-end to change 16 BB dwords.
`select_tssi_rate_table` early-returns when the rate class is unchanged, so a
single-rate stream pays nothing; only a stream that alternates CCK and OFDM
rates is affected, and it is capped at roughly 11 fps.
[OpenIPC/devourer#389](https://github.com/OpenIPC/devourer/issues/389) tracks
the measurement and a validated in-place alternative.
## TX power control

On a unit whose EFUSE is in TSSI-offset PG mode — the tested RTL8731BU is —
closed-loop TSSI *is* the TX-power control, so it runs by default. It is not
optional there: the flat fallback index (`kSafeTxAgcIndex8733b`) is a
conservative bring-up value, and on air it runs cold enough that HT rates do
not survive the link. Measured with the witness, 300 frames submitted at MCS7
with the flat index: zero captured, twice. A unit whose EFUSE carries no TSSI
calibration has nothing to drive the loop and takes the flat path.

The chip keeps two thermal-compensation curves, one for CCK and one for
OFDM/HT. The table is chosen once per channel set from the configured TX mode
and then left alone, which is what the vendor driver does —
`_halrf_tssi_set_tmeter_tbl_8733b` is only ever called from full TSSI setup,
keyed on `phydm_get_tx_rate` at that instant, and never re-selected at runtime.

There is deliberately no runtime switch, and the reason is worth recording so
nobody adds one back.

The two curves are **bit-identical for thermal deltas 0..+17** and first differ
at **+18** — the swing ramp starts there, and the words either side of the
baseline are zero in both tables. A five-minute max-duty MCS7 soak on the
validation unit plateaued at **+8** after two minutes and stopped climbing,
less than half the delta needed for the tables to differ by a single entry. So
on this silicon a runtime switch would spend 84 ms — 136 USB register round
trips, *inside `send_packet`*, capping a mixed-rate stream near 11 fps — to
install a table bit-identical to the one already loaded.

An opt-in knob for it was written, measured on air and then deleted on that
evidence. Room temperature agreed too: 300 frames of 1 Mbps CCK delivered
300/300 with the OFDM table loaded and 299/300 with the CCK table, zero FCS
errors either way.
[OpenIPC/devourer#389](https://github.com/OpenIPC/devourer/issues/389) has the
cost breakdown and a validated in-place alternative (13.8 ms, tracking left
enabled) — if a future board does reach +18, that is the implementation to
revisit, not the teardown/rebuild.

A related property of the closed loop, worth knowing before reading power
numbers from a fast rate-switching run: TSSI needs settling time. Alternating
Expand All @@ -123,6 +175,13 @@ state this backend's MAC bring-up leaves programmed — succeeds as a no-op.
gate has not been located and measured here, and the backend refuses rather
than silently pretending. The refusal leaves the session running.

The construction-time form of the same knob — `DeviceConfig::tuning::disable_cca`,
which every other generation applies during bring-up — cannot be honoured
either, and bring-up emits one warning saying so rather than dropping it
silently or failing to initialise. Silence is the only outcome that would let a
refused knob look like a granted one; a hard failure would be a harsh answer to
a request a caller may be making only through an inherited environment.

## Known gaps and deferred validation

These results have **not** been claimed:
Expand Down
38 changes: 26 additions & 12 deletions src/rtl8733b/CLAUDE.md
Original file line number Diff line number Diff line change
Expand Up @@ -48,18 +48,32 @@ unrelated register map.
either alone reintroduces the straddle, from opposite sides.
- **TSSI closed loop.** Power runs from a fixed safe target
(`kSafeTssiTargetQdbm8733b`); none of the runtime TX-power levers are ported.
The CCK and OFDM/HT variants of the thermal-compensation table are different
tables, and the table cannot be changed while tracking is enabled — so
`select_tssi_rate_table` disables tracking, rewrites, and re-enables, with
each stage verified by register readback. Note what the failure path is and
is not: `Phy8733b::enable_tssi_tracking` restores its own analog/BB snapshot
when its verdict fails, but the *transition* has no rollback to the previous
table — `select_tssi_rate_table` returns false with tracking left off, and
the caller responds by tearing the session down rather than transmitting at
an unverified power setting. That transition costs **84 ms / 136 USB
register round trips** (bench, one unit), so a stream alternating CCK and
OFDM rates is capped around 11 fps; a single-rate stream early-returns and
pays nothing.
On a TSSI-offset PG unit the loop **is** the TX-power control, so it is not
optional there — an attempt to make it opt-in with a fall back to the flat
`kSafeTxAgcIndex8733b` could not carry HT at all (witnessed: MCS7, 300/300
submitted, 0 captured, twice). A unit whose EFUSE carries no TSSI calibration
has nothing to drive the loop and takes the flat path.
- **The thermal table is chosen once per channel set, not per frame.** The CCK
and OFDM/HT variants of the thermal-compensation table are different tables.
`configure_tx_power` picks one from the configured TX mode and leaves it,
which is what the vendor does: `_halrf_tssi_set_tmeter_tbl_8733b` is
reachable only from full TSSI setup, keyed on `phydm_get_tx_rate` at that
instant, and never re-selected at runtime. So nothing reads or writes a
register per frame on the send path, matching the other four HALs.
- **The curve choice is inert at any temperature this part reaches, so do not
reintroduce runtime switching.** The CCK and OFDM/HT tables are
bit-identical for thermal deltas 0..+17 and first differ at **+18** (the
swing ramp starts there; the words either side of the baseline are zero in
both). A five-minute max-duty MCS7 soak on the validation unit plateaued at
**+8** after two minutes and stopped climbing — less than half the delta
needed for the tables to differ by a single entry. A runtime switch
therefore costs **84 ms / 136 USB register round trips** per crossing
(OpenIPC/devourer#389) to install a table that is bit-identical to the one
already loaded. An opt-in knob for it was written, measured and deleted on
that evidence. If a future board reaches +18 — a sealed module at high
ambient might — the switch is worth revisiting, but implement it as the
in-place rewrite validated in #389 (13.8 ms, tracking left enabled), not the
teardown/rebuild.
- **The loop needs settling time, so a fast rate-switching run misreports
power.** Alternating CCK and OFDM at a few ms per frame leaves CCK
transmitting above its settled level until the loop converges; pacing the
Expand Down
113 changes: 46 additions & 67 deletions src/rtl8733b/Rtl8733bDevice.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -73,6 +73,26 @@ void Rtl8733bDevice::bring_up_to_phy() {
_phy_ready = true;
_logger->info("RTL8733B factory path reached PHY-ready (cut {})",
_chip.cut);
/* Every other generation applies tuning.disable_cca during bring-up
* (jaguar1/2/3, kestrel all call SetCcaMode there), so a caller setting
* DEVOURER_DIS_CCA=1 reasonably expects it to take effect. This backend has
* not located and measured the HALMAC 87xx carrier-sense gate, so it cannot
* honour the request — say so once, loudly, rather than dropping it in
* silence, which is the one way a refused knob can look like a granted one:
* the operator would otherwise believe carrier-sense was off and read the
* resulting deferral as a transmitter problem.
*
* Deliberately a warning and not the throw SetCcaMode(true) raises: the knob
* is on by default for the streamtx FPV downlink, and failing bring-up
* outright is a harsh answer to a request the caller may be making only via
* an inherited environment. The session runs with standard carrier-sense.
*
* Sited in bring_up_to_phy rather than InitWrite so an RX-only session that
* set the knob is told too, and so it fires exactly once per bring-up. */
if (_cfg.tuning.disable_cca)
_logger->warn(
"RTL8733B: DEVOURER_DIS_CCA / tuning.disable_cca is not implemented by "
"this backend — carrier-sense stays ENABLED for this session");
}

void Rtl8733bDevice::Init(Action_ParsedRadioPacket packetProcessor,
Expand Down Expand Up @@ -110,18 +130,6 @@ void Rtl8733bDevice::InitWrite(SelectedChannel channel) {
throw std::runtime_error("RTL8733B monitor RX configuration failed");
_tx_ready = true;
_tx_submits = 0;
_tx_fatal = nullptr;
/* Say so rather than dropping it. SetCcaMode(true) refuses loudly, so the
* config path must not be the one door where the same request vanishes
* without a word — the operator would otherwise believe carrier-sense was
* off and read the resulting deferral as a transmitter problem. Warn
* rather than throw: the knob is on by default for the streamtx FPV
* downlink, and refusing to bring TX up over an unported optimisation is a
* worse trade than airing with standard carrier-sense. */
if (_cfg.tuning.disable_cca)
_logger->warn("RTL8733B: CCA disable (DEVOURER_DIS_CCA) is not "
"implemented by this backend; transmitting with "
"carrier-sense enabled");
/* One-shot thermal snapshot at bring-up — the PA-heating baseline for the
* session, same as the Kestrel InitWrite snapshot. Logged, never acted on:
* the meter is a PA-bias tracking index, not a calibrated °C sensor, and
Expand All @@ -143,46 +151,40 @@ void Rtl8733bDevice::InitWrite(SelectedChannel channel) {
bool Rtl8733bDevice::configure_tx_power(SelectedChannel channel) {
_tssi_tracking = false;
_tssi_cck = false;
/* Closed-loop TSSI is the TX-power control on a TSSI-offset PG unit, so it
* is not optional there: the flat fallback index below is a conservative
* bring-up value, and on-air it runs cold enough that HT rates do not
* survive the link (measured on the DUT: MCS7 undecodable by an RTL8812AU
* witness, 300/300 submitted, 0 captured). A unit whose EFUSE carries no
* TSSI calibration has nothing to drive the loop and takes the flat path. */
if (_efuse.tx_power_mode != rtl8733b::TxPowerPgMode8733b::TssiOffset)
return _phy.set_flat_tx_power(rtl8733b::kSafeTxAgcIndex8733b);

/* Start in the OFDM/HT thermal table. A first CCK submission switches the
* table through select_tssi_rate_table() before its descriptor reaches USB. */
/* Pick the thermal-compensation curve once, from the TX mode configured at
* this point, and leave it alone — the vendor's own setup keys the table on
* `phydm_get_tx_rate` at TSSI-setup time and never re-selects it at runtime.
* It is never re-selected per frame, so send_packet does no register I/O,
* like every other generation.
*
* The curve choice is inert on this silicon at any temperature the part
* reaches: the CCK and OFDM/HT tables are bit-identical for thermal deltas
* 0..+17 and first differ at +18, while a five-minute max-duty MCS7 soak
* plateaued at +8 after two minutes and stopped climbing. It is set from the
* rate class anyway because that costs nothing and is what the vendor does —
* but do not expect it to be measurable. */
const bool cck_table = _tx_mode_default.has_value() &&

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The comment says the curve "is set from the rate class anyway … which is what the vendor does", but in the canonical flow no mode is configured yet: txdemo calls InitWrite (examples/tx/main.cpp:998) before SetTxMode (:1058), so a DEVOURER_TX_RATE=1M session loads the OFDM table here, and the CCK branch is only reachable via a later SetMonitorChannel. Harmless by this PR's own evidence (inert below +18), but the vendor keys on the actual rate at setup while this usually reads an unset optional — one sentence acknowledging the ordering would keep the comment honest. (Alternatively move txdemo's SetTxMode above InitWrite; it's just a stored optional and valid pre-init.)

_tx_mode_default->mode ==
devourer::TxMode::Mode::Legacy &&
rtl8733b::is_cck_rate_500kbps(
Comment on lines +175 to +178

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Remediation recommended

4. Tssi table ignores settxmode 🐞 Bug ≡ Correctness

Rtl8733bDevice::configure_tx_power() programs the TSSI thermal table from _tx_mode_default during
InitWrite, but SetTxMode() later only stores the new mode and never reprograms the already-selected
table. Callers that set a Legacy CCK default after InitWrite (as examples/tx does) can therefore run
CCK traffic with the OFDM/HT curve unless they enable the per-frame switching knob.
Agent Prompt
## Issue description
`configure_tx_power()` selects the RTL8733B TSSI thermal-compensation curve once, based on `_tx_mode_default` at InitWrite time. However, `SetTxMode()` is a runtime API and (per the repo's own tx demo) is often called *after* InitWrite; the current implementation only updates `_tx_mode_default` and never updates the programmed TSSI curve.

As a result, when the runtime default TX mode is changed to a Legacy CCK rate after InitWrite, the device can keep using the OFDM/HT thermal table for the entire session unless per-frame switching is enabled.

## Issue Context
- The send path intentionally avoids register I/O when `tssi_rate_table` is off, so the fix should keep `send_packet()` free of register work.
- Changing the thermal table while tracking is enabled requires disabling/re-enabling tracking (similar to the existing `select_tssi_rate_table()` transition body).

## Fix Focus Areas
- src/rtl8733b/Rtl8733bDevice.cpp[152-216]
- src/rtl8733b/Rtl8733bDevice.cpp[462-470]
- examples/tx/main.cpp[998-1060]

## Suggested approach
1. In `Rtl8733bDevice::SetTxMode()` (and potentially `ClearTxMode()`), if:
   - EFUSE mode is `TssiOffset`,
   - `_tssi_tracking` is true (TX power loop active), and
   - `!_cfg.tuning.tssi_rate_table` (per-frame switching is OFF),
   then compute the desired table (`cck_table`) from the new mode and, if it differs from `_tssi_cck`, perform a one-time table transition (disable tracking -> `prepare_tssi_thermal()` -> re-enable tracking) under `_reg_mu`.
   - This keeps register I/O off the send path while ensuring the configured runtime default TX mode is honored.
2. Optionally update `examples/tx/main.cpp` to call `SetTxMode()` before `InitWrite()` to align demos with the intended “table chosen at setup” behavior, but still implement (1) to avoid relying on call order.

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

_tx_mode_default->legacy_rate_500kbps);
if (!_phy.prepare_tssi_bb(channel, _efuse) ||
!_phy.prepare_tssi_thermal(_efuse, false) ||
!_phy.prepare_tssi_thermal(_efuse, cck_table) ||
!_phy.prepare_tssi_offsets(channel, _efuse) ||
!_phy.enable_tssi_tracking(
channel, _efuse, rtl8733b::kSafeTssiTargetQdbm8733b))
return false;
_tssi_tracking = true;
return true;
}

bool Rtl8733bDevice::select_tssi_rate_table(bool cck) {
if (_efuse.tx_power_mode != rtl8733b::TxPowerPgMode8733b::TssiOffset)
return true;
if (!_tssi_tracking)
return false;
if (_tssi_cck == cck)
return true;

/* The vendor chooses this table from the current TX rate. The table cannot
* be changed while closed-loop tracking is enabled, so make the transition
* explicit and reversible: exact rollback, table readback, then a fresh
* capped enable verified by register readback. (prepare_tssi_thermal here is
* the vendor's TSSI thermal-*compensation table*, a hardware feature driven
* off the EFUSE baseline constant — not a live temperature read.) */
if (!_phy.disable_tssi_tracking())
return false;
_tssi_tracking = false;
if (!_phy.prepare_tssi_thermal(_efuse, cck) ||
!_phy.enable_tssi_tracking(
_channel, _efuse, rtl8733b::kSafeTssiTargetQdbm8733b))
return false;
_tssi_tracking = true;
_tssi_cck = cck;
_logger->info("RTL8733B TSSI rate table selected: {}",
cck ? "CCK" : "OFDM/HT");
_tssi_cck = cck_table;

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

With select_tssi_rate_table gone, _tssi_cck is now write-only — set here and cleared in Stop(), never read anywhere. Delete the member, or say what future reader it's being kept for.

return true;
}

Expand Down Expand Up @@ -344,14 +346,7 @@ void Rtl8733bDevice::SetMonitorChannel(SelectedChannel channel) {
bool Rtl8733bDevice::send_packet(const uint8_t *packet, size_t length) {
std::lock_guard<std::recursive_mutex> lock(_reg_mu);
if (!_phy_ready || !_mac_ready || !_tx_ready) {
/* Name the reason: a retry loop must be able to tell "InitWrite has not run
* yet" from "the session was deliberately stopped and will not recover". */
if (_tx_fatal != nullptr)
_logger->error("RTL8733B TX rejected: session stopped by {} — "
"re-run InitWrite to recover",
_tx_fatal);
else
_logger->error("RTL8733B TX rejected before InitWrite");
_logger->error("RTL8733B TX rejected before InitWrite");
return false;
}
if (packet == nullptr)
Expand Down Expand Up @@ -392,22 +387,6 @@ bool Rtl8733bDevice::send_packet(const uint8_t *packet, size_t length) {
const uint8_t bandwidth = (usb_frame[0x14] >> 5) & 0x3;
const uint8_t short_gi = (usb_frame[0x14] >> 4) & 0x1;
const uint8_t ldpc = (usb_frame[0x14] >> 7) & 0x1;
if (!select_tssi_rate_table(rate_hw <= 3)) {
/* Fatal, not transient. The CCK<->OFDM table switch disables closed-loop
* TSSI tracking, rewrites the thermal table and re-enables tracking; a
* failure anywhere in there means the readback no longer describes the
* loop that is driving the PA. Continuing to inject would be transmitting
* at an unverified power setting, so tear the session down (MAC stop +
* card disable). send_packet keeps its bool contract — no other generation
* throws per frame — so the reason is latched in _tx_fatal instead: a bare
* `false` is indistinguishable from a short bulk write and would send a
* retry loop straight back into the same state. */
_logger->error("RTL8733B TX aborted: TSSI rate-table transition failed — "
"closed-loop power state is unverified, stopping the card");
_tx_fatal = "a failed TSSI rate-table transition";
Stop();
return false;
}
const int sent = _device.bulk_send_sync_ep(
endpoint, usb_frame.data(), usb_frame.size(), 100);
if (sent != static_cast<int>(usb_frame.size())) {
Expand Down
5 changes: 0 additions & 5 deletions src/rtl8733b/Rtl8733bDevice.h
Original file line number Diff line number Diff line change
Expand Up @@ -50,7 +50,6 @@ class Rtl8733bDevice : public IRtlDevice {
private:
void bring_up_to_phy();
bool configure_tx_power(SelectedChannel channel);
bool select_tssi_rate_table(bool cck);
size_t build_tx_block(const uint8_t *packet, size_t length, uint8_t *out,
uint8_t packet_offset);

Expand All @@ -69,10 +68,6 @@ class Rtl8733bDevice : public IRtlDevice {
bool _mac_ready = false;
bool _phy_ready = false;
bool _tx_ready = false;
/* Non-null once a send-path failure was fatal enough to Stop() the card, so
* later rejections can say why instead of reading as "InitWrite never ran".
* Static string literal; cleared by InitWrite. */
const char *_tx_fatal = nullptr;
bool _tssi_tracking = false;
bool _tssi_cck = false;
std::atomic<bool> _rx_stop{false};
Expand Down
9 changes: 9 additions & 0 deletions src/rtl8733b/TxDescriptor8733b.h
Original file line number Diff line number Diff line change
Expand Up @@ -54,6 +54,15 @@ inline bool legacy_request_supported_8733b(unsigned bw_mhz, bool sgi, bool ldpc,
return bw_mhz == 20 && !sgi && !ldpc && !stbc;
}

/* Is a radiotap/TxMode legacy rate (500 kbps units) one of the four
* long-preamble CCK rates? 1M=2, 2M=4, 5.5M=11, 11M=22. Used to pick the TSSI
* thermal-compensation curve at setup, mirroring the vendor's rate-keyed
* table choice. */
inline bool is_cck_rate_500kbps(uint8_t rate_500kbps) {
return rate_500kbps == 2 || rate_500kbps == 4 || rate_500kbps == 11 ||
rate_500kbps == 22;
}

/* Whole-TxMode admission: the PPDU family plus its modulation parameters. VHT
* and HE fall through to false on every band. */
inline bool tx_mode_supported_8733b(const devourer::TxMode &mode) {
Expand Down
Loading