From 57b09ffb30067d649cd768033afe48b1f7c1b408 Mon Sep 17 00:00:00 2001 From: snokvist Date: Thu, 10 Sep 2026 00:01:25 +0200 Subject: [PATCH 1/8] mt7612u: the descriptor translations, with the per-chain trap encoded in them MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit First piece of the backend (#419). Pure lookups between MT7612U's descriptor vocabulary and the one every other backend reports in, in a header with a test under it rather than inline in the device class — each is easy to get subtly wrong and impossible to notice on a radio: a wrong RSSI base reads as a weak link, a wrong rate code as a slow one. Two of these already cost hardware time in an earlier cut of this integration, so both are pinned: - RSSI is an unsigned byte biased by 110, not a cast. Casting a signed -63 dBm gives 193, which LinkHealth reads back as +83 dBm. - copy_signal() publishes n_chains, not the array's extent. rx.cpp sets info->noise = info->rssi[2] because RXWI byte 14 is a NOISE FLOOR and byte 15 is unidentified, so a loop over all four hands consumers a phantom chain C pinned near -92 dBm and a chain D of garbage. Realtek's contract is that [2..3] are zero on a 2-path part, which is what a 2T2R MediaTek must report too. The earlier cut looped over four. copy_signal also fills snr[] while the report is in hand, and only when noise_valid: the noise field reads a physically impossible -116 dBm on a quiet channel, so an unvalidated SNR is left at zero rather than written as a plausible number. bw_to_desc is an identity today and is written out anyway, because "the two enums happen to agree" is not something a reader of a bare static_cast can check — the earlier cut had exactly that cast with nothing saying anyone had. The mapping cell runs whether or not DEVOURER_MT7612U is on; the header pulls in no subtree object. Mutation-tested: reinstating the four-slot copy, dropping the noise_valid guard, and casting instead of biasing each fail it, the first naming the phantom chains directly. 60/60 ctest. Co-Authored-By: Claude Opus 5 (1M context) Claude-Session: https://claude.ai/code/session_01Tba83kymS5W2v1vn2yRxrj --- CMakeLists.txt | 14 +++ src/mt7612u/Mt7612uMapping.h | 140 +++++++++++++++++++++++ tests/mt7612u_mapping_selftest.cpp | 175 +++++++++++++++++++++++++++++ 3 files changed, 329 insertions(+) create mode 100644 src/mt7612u/Mt7612uMapping.h create mode 100644 tests/mt7612u_mapping_selftest.cpp diff --git a/CMakeLists.txt b/CMakeLists.txt index 92f088cd..2a113485 100644 --- a/CMakeLists.txt +++ b/CMakeLists.txt @@ -843,6 +843,20 @@ target_link_libraries(Mt7612uUsbIdsSelftest PRIVATE devourer) add_test(NAME mt7612u_usb_ids COMMAND Mt7612uUsbIdsSelftest) +# Headless guard for the MT7612U <-> devourer descriptor translations. Pure +# lookups, so no hardware and no subtree compile — this cell runs whether or +# not DEVOURER_MT7612U is on, which is the point: the header it covers is the +# one place a wrong RSSI base or a per-chain copy that publishes the noise +# floor would be introduced, and both have happened. +add_executable(Mt7612uMappingSelftest + tests/mt7612u_mapping_selftest.cpp +) +target_link_libraries(Mt7612uMappingSelftest PRIVATE devourer) +target_include_directories(Mt7612uMappingSelftest PRIVATE + ${CMAKE_CURRENT_SOURCE_DIR}/src/mt7612u/include) + +add_test(NAME mt7612u_mapping COMMAND Mt7612uMappingSelftest) + # Headless guard for the TX quiesce seam (ITransport::quiesce_tx via # RtlAdapter): the explicit "stop TX and wait it out" call every device makes # before anything is released. UsbTransport's cancel/drain is validated on diff --git a/src/mt7612u/Mt7612uMapping.h b/src/mt7612u/Mt7612uMapping.h new file mode 100644 index 00000000..0f23e67b --- /dev/null +++ b/src/mt7612u/Mt7612uMapping.h @@ -0,0 +1,140 @@ +#ifndef MT7612U_MAPPING_H +#define MT7612U_MAPPING_H + +#include + +#include "RxPacket.h" +#include "SelectedChannel.h" +#include "ieee80211_radiotap.h" /* DESC_RATE* */ +#include "mt7612u/mt7612u.h" + +/* + * The pure translations between MT7612U's own descriptor vocabulary and the + * one every other backend here reports in. They live in a header, apart from + * the device class, because each is a lookup that is easy to get subtly wrong + * and impossible to notice on a radio: a wrong RSSI base reads as a weak + * link, a wrong rate code reads as a slow one. tests/mt7612u_mapping_selftest + * pins all of them. + */ +namespace mt7612u { + +/* devourer carries RSSI as an unsigned byte biased by 110 — + * LinkHealth.cpp:8 is the authority: `rssi_dbm = rssi_raw - 110`. The HAL + * reports true dBm, so the bias has to be added, not cast around. Casting a + * signed -63 dBm straight into the byte yields 193, i.e. +83 dBm. */ +inline constexpr int kRssiBiasDb = 110; + +inline uint8_t rssi_to_raw(int8_t dbm) { + const int raw = static_cast(dbm) + kRssiBiasDb; + if (raw < 0) + return 0; + if (raw > 255) + return 255; + return static_cast(raw); +} + +/* Copy the per-chain signal into an rx_pkt_attrib, and ONLY the per-chain + * signal. + * + * This exists because `mt7612u_rx_info::rssi[4]` is not four chains. The MAC + * is 2T2R, so [0] and [1] are chains A and B — but rx.cpp assigns + * `info->noise = info->rssi[2]`, because RXWI byte 14 is a noise floor (the + * slot mt76 declares and never reads), and byte 15 is unidentified. A loop + * over all four therefore hands consumers a phantom chain C pinned near the + * noise floor and a chain D of garbage, which is exactly what an earlier cut + * of this integration did. Realtek's own contract is that [2..3] are ZERO on + * a 2-path part (RxPacket.h), so that is what a 2T2R MediaTek must report + * too, and n_chains is the authority rather than the array's extent. + * + * snr[] is filled from the same report while it is in hand: `snr_db` is + * rssi[0] - noise and is only meaningful when noise_valid, which is why the + * unvalidated case leaves the slots at zero rather than writing a plausible + * number. See the -116 dBm caveat on `noise` in the public header. */ +inline void copy_signal(const struct mt7612u_rx_info &info, + struct rx_pkt_attrib &out) { + const unsigned chains = info.n_chains > 2u ? 2u : info.n_chains; + + for (unsigned i = 0; i < chains; ++i) + out.rssi[i] = rssi_to_raw(info.rssi[i]); + for (unsigned i = chains; i < 4u; ++i) + out.rssi[i] = 0; + + for (unsigned i = 0; i < 4u; ++i) + out.snr[i] = 0; + if (info.noise_valid) + for (unsigned i = 0; i < chains; ++i) + out.snr[i] = info.snr_db; +} + +/* mt7612u_rx_info -> the DESC_RATE numbering consumers read, so a caller does + * not need to know which chip a frame came from. */ +inline uint16_t desc_rate(const struct mt7612u_rx_info &info) { + switch (info.phy) { + case MT7612U_PHY_CCK: + /* DESC_RATE1M..11M are 0..3, in the same order as the CCK indices. */ + return static_cast(info.mcs & 0x3); + case MT7612U_PHY_OFDM: + return static_cast(DESC_RATE6M + (info.mcs & 0x7)); + case MT7612U_PHY_HT: + case MT7612U_PHY_HT_GF: + /* HT folds NSS into the MCS number on both sides, so this is a straight + * offset for MCS 0-31. */ + return static_cast(DESC_RATEMCS0 + info.mcs); + case MT7612U_PHY_VHT: { + const uint8_t nss = info.nss ? info.nss : 1; + return static_cast(DESC_RATEVHTSS1MCS0 + (nss - 1) * 10 + + info.mcs); + } + } + return 0; +} + +/* MT7612U_BW_* -> the RX-descriptor bandwidth code consumers read. + * + * The two happen to share 0/1/2 for 20/40/80, so this is an identity — but + * written out rather than cast, because "the two enums agree today" is not + * something a reader of a bare static_cast can check, and the earlier cut of + * this integration had exactly that cast with no note saying anyone had. */ +inline uint8_t bw_to_desc(enum mt7612u_bw bw) { + switch (bw) { + case MT7612U_BW_20: + return 0; + case MT7612U_BW_40: + return 1; + case MT7612U_BW_80: + return 2; + } + return 0; +} + +/* SelectedChannel width -> the three widths this port implements. Anything + * wider or narrower is refused rather than silently narrowed: 5/10 MHz has no + * encoding in the rate word at all, and 160 MHz is beyond the silicon. Note + * that a width being accepted here does not mean every channel can carry it — + * mt7612u_set_channel() refuses a control channel that is off the grid for + * the requested width. */ +inline bool width_to_bw(ChannelWidth_t w, enum mt7612u_bw &out, + const char *&why) { + switch (w) { + case CHANNEL_WIDTH_20: + out = MT7612U_BW_20; + return true; + case CHANNEL_WIDTH_40: + out = MT7612U_BW_40; + return true; + case CHANNEL_WIDTH_80: + out = MT7612U_BW_80; + return true; + case CHANNEL_WIDTH_5: + case CHANNEL_WIDTH_10: + why = "5/10 MHz narrowband: MT_RATE_BW has no encoding for it"; + return false; + default: + why = "unsupported channel width"; + return false; + } +} + +} // namespace mt7612u + +#endif /* MT7612U_MAPPING_H */ diff --git a/tests/mt7612u_mapping_selftest.cpp b/tests/mt7612u_mapping_selftest.cpp new file mode 100644 index 00000000..d438044f --- /dev/null +++ b/tests/mt7612u_mapping_selftest.cpp @@ -0,0 +1,175 @@ +/* Headless guard for the MT7612U <-> devourer descriptor translations + * (src/mt7612u/Mt7612uMapping.h). + * + * Every one of these is a lookup that is easy to get subtly wrong and + * impossible to notice on a radio: a wrong RSSI base reads as a weak link, a + * wrong rate code reads as a slow one, and a per-chain copy that runs off the + * end of the real chains reads as a third antenna that is always near the + * noise floor. Each of those has actually happened here, which is why the + * translations are pure functions in a header with this test under them + * rather than inline in the device class. */ +#include "mt7612u/Mt7612uMapping.h" + +#include +#include + +using namespace mt7612u; + +namespace { + +int fails; + +void expect(const char *what, bool ok) { + if (!ok) { + std::fprintf(stderr, "mt7612u_mapping: FAIL %s\n", what); + fails++; + } +} + +struct mt7612u_rx_info rx(enum mt7612u_phy phy, uint8_t mcs, uint8_t nss = 1) { + struct mt7612u_rx_info i {}; + i.phy = phy; + i.mcs = mcs; + i.nss = nss; + return i; +} + +} // namespace + +int main() { + /* --- RSSI: an unsigned byte biased by 110, not a cast --- */ + expect("rssi -63 dBm -> 47", rssi_to_raw(-63) == 47); + expect("rssi -110 dBm -> 0", rssi_to_raw(-110) == 0); + expect("rssi 0 dBm -> 110", rssi_to_raw(0) == 110); + expect("rssi -128 dBm clamps to 0", rssi_to_raw(-128) == 0); + expect("rssi +127 dBm clamps to 237", rssi_to_raw(127) == 237); + /* The bug this constant exists for: a straight cast of -63 gives 193, which + * LinkHealth reads back as +83 dBm. */ + expect("rssi is biased, not cast", + rssi_to_raw(-63) != static_cast(static_cast(-63))); + expect("rssi round-trips to dBm", + static_cast(rssi_to_raw(-63)) - 110 == -63); + + /* --- per-chain signal: 2T2R, and rssi[2] is the NOISE FLOOR --- */ + { + struct mt7612u_rx_info i {}; + struct rx_pkt_attrib a {}; + + i.n_chains = 2; + i.rssi[0] = -55; + i.rssi[1] = -58; + i.rssi[2] = -92; /* rx.cpp: info->noise = info->rssi[2] */ + i.rssi[3] = 0x7f; + i.noise = -92; + i.snr_db = 37; + i.noise_valid = 1; + copy_signal(i, a); + + expect("chain A copied", a.rssi[0] == rssi_to_raw(-55)); + expect("chain B copied", a.rssi[1] == rssi_to_raw(-58)); + /* THE regression this function exists to prevent. Copying all four slots + * publishes the noise floor as a third antenna — an earlier cut of this + * integration did exactly that, and every per-chain consumer saw a phantom + * chain C sitting near -92 dBm. */ + expect("the noise floor is NOT published as chain C", a.rssi[2] == 0); + expect("the unidentified slot is NOT published as chain D", a.rssi[3] == 0); + expect("snr filled on the real chains", a.snr[0] == 37 && a.snr[1] == 37); + expect("snr not invented past the real chains", + a.snr[2] == 0 && a.snr[3] == 0); + } + { + /* Without a valid noise estimate there is no SNR to report. Zero, not a + * plausible-looking number — the noise field reads a physically impossible + * -116 dBm on a quiet channel (see the caveat in the public header). */ + struct mt7612u_rx_info i {}; + struct rx_pkt_attrib a {}; + + i.n_chains = 2; + i.rssi[0] = -55; + i.snr_db = 37; + i.noise_valid = 0; + copy_signal(i, a); + expect("no snr without a valid noise estimate", + a.snr[0] == 0 && a.snr[1] == 0); + expect("rssi still reported without noise", a.rssi[0] == rssi_to_raw(-55)); + } + { + /* A 1-chain report must not publish chain B, and a report claiming more + * chains than the silicon has must not run off the end. */ + struct mt7612u_rx_info i {}; + struct rx_pkt_attrib a {}; + + i.n_chains = 1; + i.rssi[0] = -40; + i.rssi[1] = -91; + copy_signal(i, a); + expect("1 chain publishes only chain A", + a.rssi[0] == rssi_to_raw(-40) && a.rssi[1] == 0); + + struct rx_pkt_attrib b {}; + i.n_chains = 9; + copy_signal(i, b); + expect("an over-large chain count is clamped to the 2T2R truth", + b.rssi[1] == rssi_to_raw(-91) && b.rssi[2] == 0 && b.rssi[3] == 0); + } + + /* --- rate: the DESC_RATE numbering every backend reports in --- */ + expect("CCK 1M -> 0", desc_rate(rx(MT7612U_PHY_CCK, 0)) == 0); + expect("CCK 11M -> 3", desc_rate(rx(MT7612U_PHY_CCK, 3)) == 3); + expect("OFDM 6M -> DESC_RATE6M", desc_rate(rx(MT7612U_PHY_OFDM, 0)) == 0x04); + expect("OFDM 54M -> 11", desc_rate(rx(MT7612U_PHY_OFDM, 7)) == 11); + expect("HT MCS0 -> DESC_RATEMCS0", desc_rate(rx(MT7612U_PHY_HT, 0)) == 0x0c); + /* MCS7 is 19 — the code the witness logged for our own injected frames, so + * this one is pinned against a measurement rather than against the header. */ + expect("HT MCS7 -> 19", desc_rate(rx(MT7612U_PHY_HT, 7)) == 19); + expect("HT MCS15 -> 27", desc_rate(rx(MT7612U_PHY_HT, 15)) == 27); + expect("HT-GF uses the HT numbering", + desc_rate(rx(MT7612U_PHY_HT_GF, 7)) == desc_rate(rx(MT7612U_PHY_HT, 7))); + expect("VHT 1SS MCS0 -> DESC_RATEVHTSS1MCS0", + desc_rate(rx(MT7612U_PHY_VHT, 0, 1)) == 0x2c); + expect("VHT 2SS MCS0 -> +10", + desc_rate(rx(MT7612U_PHY_VHT, 0, 2)) == 0x2c + 10); + expect("VHT 2SS MCS9 -> +19", + desc_rate(rx(MT7612U_PHY_VHT, 9, 2)) == 0x2c + 19); + expect("VHT nss 0 is treated as 1", + desc_rate(rx(MT7612U_PHY_VHT, 3, 0)) == + desc_rate(rx(MT7612U_PHY_VHT, 3, 1))); + + /* --- bandwidth code --- */ + expect("BW_20 -> 0", bw_to_desc(MT7612U_BW_20) == 0); + expect("BW_40 -> 1", bw_to_desc(MT7612U_BW_40) == 1); + expect("BW_80 -> 2", bw_to_desc(MT7612U_BW_80) == 2); + + /* --- channel width --- */ + { + enum mt7612u_bw bw = MT7612U_BW_80; + const char *why = ""; + + expect("20 MHz accepted", width_to_bw(CHANNEL_WIDTH_20, bw, why)); + expect("20 MHz -> BW_20", bw == MT7612U_BW_20); + expect("40 MHz accepted", width_to_bw(CHANNEL_WIDTH_40, bw, why)); + expect("40 MHz -> BW_40", bw == MT7612U_BW_40); + expect("80 MHz accepted", width_to_bw(CHANNEL_WIDTH_80, bw, why)); + expect("80 MHz -> BW_80", bw == MT7612U_BW_80); + + /* Refused, not narrowed. A silently narrowed 5 MHz request would air at + * 20 MHz — four times the occupied bandwidth the caller asked for. */ + why = ""; + expect("5 MHz refused", !width_to_bw(CHANNEL_WIDTH_5, bw, why)); + expect("5 MHz refusal says why", why[0] != '\0'); + why = ""; + expect("10 MHz refused", !width_to_bw(CHANNEL_WIDTH_10, bw, why)); + expect("10 MHz refusal says why", why[0] != '\0'); + why = ""; + expect("160 MHz refused", + !width_to_bw(static_cast(CHANNEL_WIDTH_160), bw, why)); + expect("160 MHz refusal says why", why[0] != '\0'); + } + + if (fails) { + std::fprintf(stderr, "mt7612u_mapping: %d failure(s)\n", fails); + return 1; + } + std::printf("mt7612u_mapping: all checks passed\n"); + return 0; +} From a05a3978202bae65c36778994d560c283ea357ef Mon Sep 17 00:00:00 2001 From: snokvist Date: Thu, 10 Sep 2026 00:09:28 +0200 Subject: [PATCH 2/8] =?UTF-8?q?mt7612u:=20the=20IRadio=20backend=20?= =?UTF-8?q?=E2=80=94=20devourer=20can=20open=20and=20receive=20on=20a=20Me?= =?UTF-8?q?diaTek=20part?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The core of #419. CreateRadio stops refusing MediaTek adapters and constructs one. Measured on hardware, an MT7612U at 0e8d:7612 through rxdemo: Creating Mt7612uRadio (0e8d:7612) MT7612U firmware from .../firmware mt7612u: firmware running MT7612U up: ASIC 0x76120044 mt7612u: async: 16 RX transfers in flight, 16 TX slots MT7612U monitor RX on channel 36 MT7612U RX stopped after 7088 frames and the frames decode: {"ev":"rx.pkt","n":1,"len":238,"rate":4,"rssi":58} {"ev":"rx.pkt","n":3,"len":238,"rate":4,"rssi":24} rate 4 is DESC_RATE6M, right for a 5 GHz beacon; rssi 58 and 24 are -52 and -86 dBm through the 110 bias. A raw cast would have printed 193 and 204. Mt7612uRadio derives from IRadio and NOT IRtlRadio: the Realtek-only members describe a register plane this silicon does not have. It does not use RtlAdapter either — the C subtree is the transport, and the class owns one mt7612u_dev, adopting the handle WiFiDriver already opened, reset and claimed rather than reopening it (reopening would race the caller's lock, and libusb_reset_device would invalidate the caller's handle). Three orderings, each of which has cost hardware time: 1. RX ring first, receiver second, and the mirror image on teardown — mt7612u_rx_quiesce() then mt7612u_rx_stop(). Enabling MAC RX with nothing draining bulk-IN wedges this part BELOW the USB level, where only a physical replug recovers it. That quiesce is a new public entry point: mt7612u_stop() would also silence the receiver but stops TX with it, which is no use to a consumer that brought the chip up to transmit. 2. The monitor filter goes on AFTER mt7612u_start(), which rewrites MT_RX_FILTR_CFG to mt76's managed-station value. Measured against the bring-up harness in the same minute on the same silicon: 0 OFDM frames of 244 with the managed filter, 103 of 402 with the monitor filter. A single-path test would have called 244 beacons a working receiver. 3. The 1 Hz PHY tick runs on a thread this class owns, not inside StartRxLoop, because a transmit-only consumer (InitWrite, no RX loop) needs it too. Without any tick a receiver 20 cm from a peer airing 3037 fps takes 3 frames in 10 s. StopRxLoop quiesces under _mu and then tears the ring down WITHOUT it, because mt7612u_rx_stop() joins the event thread and a processor still in flight there may call back into this object; holding _mu across that join is a deadlock, not a theoretical one. fcs_present is set false on every frame — the reason RxPacket.h carries the flag at all. Per-chain RSSI goes through copy_signal(), so rssi[2] (the noise floor) is not published as a phantom chain C. No environment is read anywhere below the demos: firmware_dir and device_selector are DeviceConfig fields, and examples/common/env_config.cpp folds DEVOURER_MT7612U_FW_DIR and MT7612U_DEV into them. The constructor installs an mt7612u_set_log_sink() forwarding to Logger before the first library call, which is what puts "mt7612u: firmware running" above through the log level and, on Android, through __android_log_write. Refused rather than faked: SetCcaMode (the ED-CCA enable exists, no on-air carrier-sense measurement backs it) and SetTxPowerIndexOverride (there is no TXAGC index; TX power here is an absolute dBm limit). 60/60 ctest in both configs, no warnings from the new files. Co-Authored-By: Claude Opus 5 (1M context) Claude-Session: https://claude.ai/code/session_01Tba83kymS5W2v1vn2yRxrj --- CMakeLists.txt | 8 +- examples/common/env_config.cpp | 8 + src/DeviceConfig.h | 22 ++ src/WiFiDriver.cpp | 15 +- src/mt7612u/Mt7612uRadio.cpp | 535 ++++++++++++++++++++++++++ src/mt7612u/Mt7612uRadio.h | 133 +++++++ src/mt7612u/async.cpp | 11 + src/mt7612u/include/mt7612u/mt7612u.h | 20 + src/mt7612u/tests/api_link.c | 1 + 9 files changed, 749 insertions(+), 4 deletions(-) create mode 100644 src/mt7612u/Mt7612uRadio.cpp create mode 100644 src/mt7612u/Mt7612uRadio.h diff --git a/CMakeLists.txt b/CMakeLists.txt index 2a113485..dd927a73 100644 --- a/CMakeLists.txt +++ b/CMakeLists.txt @@ -380,12 +380,18 @@ if(DEVOURER_MT7612U) src/mt7612u/rx.cpp src/mt7612u/tx.cpp src/mt7612u/usb.cpp + src/mt7612u/Mt7612uRadio.cpp src/mt7612u/Mt7612uRadio.h + src/mt7612u/Mt7612uMapping.h src/mt7612u/internal.h src/mt7612u/regs.h src/mt7612u/initvals.h src/mt7612u/Mt7612uUsbIds.h) # listed for IDEs; compiled unconditionally target_include_directories(devourer PRIVATE - ${CMAKE_CURRENT_SOURCE_DIR}/src/mt7612u + ${CMAKE_CURRENT_SOURCE_DIR}/src/mt7612u) + # PUBLIC: Mt7612uRadio.h is reached as "mt7612u/Mt7612uRadio.h" and pulls in + # , so a consumer compiling against this target needs the + # subtree's public include root too. + target_include_directories(devourer PUBLIC ${CMAKE_CURRENT_SOURCE_DIR}/src/mt7612u/include) target_compile_definitions(devourer PUBLIC DEVOURER_HAVE_MT7612U=1) endif() diff --git a/examples/common/env_config.cpp b/examples/common/env_config.cpp index d9583560..d0b9d2f7 100644 --- a/examples/common/env_config.cpp +++ b/examples/common/env_config.cpp @@ -157,6 +157,14 @@ devourer::DeviceConfig devourer_config_from_env() { cfg.bf.ndpa_period = p > 0 ? p : 1; } + /* ---- MediaTek MT7612U ---- */ + /* Folded in here rather than read inside the backend, so neither the C + * library nor the device class consults ambient process state. */ + if (const char *e = env_str("DEVOURER_MT7612U_FW_DIR")) + cfg.mt7612u.firmware_dir = std::string(e); + if (const char *e = env_str("MT7612U_DEV")) + cfg.mt7612u.device_selector = std::string(e); + /* ---- tuning ---- */ /* Defaults ON, so this reads the negation: only an explicit 0 disables it. */ if (const char *e = env_str("DEVOURER_TEARDOWN_POWER_DOWN")) diff --git a/src/DeviceConfig.h b/src/DeviceConfig.h index 86d9fad2..92256761 100644 --- a/src/DeviceConfig.h +++ b/src/DeviceConfig.h @@ -536,6 +536,28 @@ struct DeviceConfig { * (DEVOURER_PCIE_BDF) is likewise demo-local, like USB device selection. */ std::optional rx_poll_us; } pcie; + + /* ---- MediaTek MT7612U (DEVOURER_MT7612U builds) ---------------------- */ + struct Mt7612u { + /* env: DEVOURER_MT7612U_FW_DIR — directory holding mt7662_rom_patch.bin + * and mt7662.bin. Unset = search /lib/firmware/mediatek then ./firmware. + * + * A path rather than an embedded blob, unlike every Realtek backend: this + * firmware ships in linux-firmware under its own licence rather than being + * generated into hal/, and it is zstd-compressed on most distributions, so + * it can be neither vendored here nor assumed ready at a fixed path. + * Decompress both and point this at the directory. + * + * Here rather than a getenv inside the backend so the library and the + * device class both stay free of ambient process state; the demos fold the + * variable in, the way they do for every other knob in this file. */ + std::optional firmware_dir; + /* env: MT7612U_DEV — which adapter to open when several are attached, + * "-" as lsusb spells the port path. Unset = the first. + * Only consulted on the library's own open path, not when devourer hands + * it an already-claimed handle (which is the devourer path). */ + std::optional device_selector; + } mt7612u; }; } // namespace devourer diff --git a/src/WiFiDriver.cpp b/src/WiFiDriver.cpp index 0d1238ec..a9a4613b 100644 --- a/src/WiFiDriver.cpp +++ b/src/WiFiDriver.cpp @@ -32,6 +32,9 @@ #endif #include "rtl8733b/Rtl8733bUsbIds.h" #include "mt7612u/Mt7612uUsbIds.h" /* header-only VID:PID table, always compiled */ +#if defined(DEVOURER_HAVE_MT7612U) +#include "mt7612u/Mt7612uRadio.h" +#endif namespace { @@ -203,11 +206,17 @@ WiFiDriver::CreateRadio(libusb_device_handle *dev_handle, * vendor id". Mt7612uUsbIds.h carries the table and the evidence; * Mt7612uUsbIdsSelftest.cpp fails if a later id addition breaks it. */ if (mt7612u::is_usb_id(vid, pid)) { - _logger->error("MediaTek MT7612U ({:04x}:{:04x}) detected; devourer has no " - "MediaTek radio backend yet — refusing rather than " - "misdetecting it as Realtek", +#if defined(DEVOURER_HAVE_MT7612U) + _logger->info("Creating Mt7612uRadio ({:04x}:{:04x})", vid, pid); + return std::make_unique(dev_handle, ctx, std::move(usb_lock), + _logger, cfg); +#else + _logger->error("MediaTek MT7612U ({:04x}:{:04x}) detected but MediaTek " + "support is not compiled in (DEVOURER_MT7612U=OFF) — " + "refusing rather than misdetecting it as Realtek", vid, pid); return nullptr; +#endif } /* A vendor read that did not complete means this device is not speaking the diff --git a/src/mt7612u/Mt7612uRadio.cpp b/src/mt7612u/Mt7612uRadio.cpp new file mode 100644 index 00000000..d7876e5f --- /dev/null +++ b/src/mt7612u/Mt7612uRadio.cpp @@ -0,0 +1,535 @@ +#include "mt7612u/Mt7612uRadio.h" + +#include +#include +#include +#include +#include +#include +#include +#include + +#include "mt7612u/Mt7612uMapping.h" + +extern volatile bool g_devourer_should_stop; + +namespace { + +/* + * Where mt7662_rom_patch.bin and mt7662.bin live. + * + * Unlike the Realtek backends, whose firmware is generated into hal/ and + * compiled in, MediaTek's ships in linux-firmware under its own licence and is + * zstd-compressed on most distributions - so it can be neither vendored here + * nor assumed ready at a fixed path. Resolved in the order a caller would + * expect, and the failure names every place that was tried rather than just + * saying no. + */ +std::string resolve_fw_dir(const devourer::DeviceConfig &cfg, + const Logger_t &logger) { + std::vector tried; + + if (cfg.mt7612u.firmware_dir) + tried.emplace_back(*cfg.mt7612u.firmware_dir); + tried.emplace_back("/lib/firmware/mediatek"); + tried.emplace_back("firmware"); /* the bring-up harness's own directory */ + + std::error_code ec; + for (const std::string &dir : tried) { + const std::filesystem::path patch = + std::filesystem::path(dir) / "mt7662_rom_patch.bin"; + const std::filesystem::path fw = std::filesystem::path(dir) / "mt7662.bin"; + if (std::filesystem::exists(patch, ec) && + std::filesystem::exists(fw, ec)) { + logger->info("MT7612U firmware from {}", dir); + return dir; + } + } + + std::string all; + for (const std::string &dir : tried) + all += (all.empty() ? "" : ", ") + dir; + logger->error("MT7612U firmware (mt7662_rom_patch.bin + mt7662.bin) not " + "found in: {}. They ship zstd-compressed in linux-firmware; " + "decompress them and set DeviceConfig mt7612u.firmware_dir " + "(demos: DEVOURER_MT7612U_FW_DIR).", + all); + return tried.back(); +} + +} // namespace + +Mt7612uRadio::Mt7612uRadio(libusb_device_handle *handle, libusb_context *ctx, + std::shared_ptr usb_lock, + Logger_t logger, devourer::DeviceConfig cfg) + : _handle(handle), _ctx(ctx), _usb_lock(std::move(usb_lock)), + _logger(std::move(logger)), _cfg(std::move(cfg)) { + /* Before any library call, and before any thread the library might start: + * the sink pointer is read from the RX event thread without + * synchronisation, which is the contract mt7612u_set_log_sink documents. + * Without this the subtree's diagnostics go straight to stderr, bypassing + * the log level, a redirected diag stream, and __android_log_write. */ + mt7612u_set_log_sink(&Mt7612uRadio::log_trampoline, this); +} + +Mt7612uRadio::~Mt7612uRadio() { + Stop(); + /* Nothing may reach a destroyed `this` afterwards. */ + mt7612u_set_log_sink(nullptr, nullptr); +} + +void Mt7612uRadio::log_trampoline(void *user, char level, const char *line) { + auto *self = static_cast(user); + if (!self || !self->_logger || !line) + return; + /* The library hands over the bare message; Logger re-adds "devourer [X] " + * and applies the level gating and stream this consumer configured. */ + switch (level) { + case 'E': + self->_logger->error("mt7612u: {}", line); + break; + case 'W': + self->_logger->warn("mt7612u: {}", line); + break; + default: + self->_logger->info("mt7612u: {}", line); + break; + } +} + +void Mt7612uRadio::bring_up(SelectedChannel channel) { + enum mt7612u_bw bw = MT7612U_BW_20; + const char *why = ""; + + if (!mt7612u::width_to_bw(channel.ChannelWidth, bw, why)) + throw std::runtime_error(std::string("MT7612U channel width refused: ") + + why); + + if (!_dev) { + const char *err = nullptr; + /* Adopts the caller's handle: WiFiDriver already opened, reset and claimed + * it, and holds the exclusive lock this object carries. Reopening would + * race that lock, and libusb_reset_device() here would invalidate the + * caller's own handle. */ + const std::string fw_dir = resolve_fw_dir(_cfg, _logger); + _dev = mt7612u_open_handle(_handle, _ctx, fw_dir.c_str(), &err); + if (!_dev) + throw std::runtime_error(std::string("MT7612U bring-up failed: ") + + (err ? err : "unknown")); + _logger->info("MT7612U up: ASIC 0x{:08x}", mt7612u_asic_version(_dev)); + if (_txpwr_dbm != 20) + mt7612u_set_txpower(_dev, _txpwr_dbm); + } + + if (mt7612u_set_channel(_dev, channel.Channel, bw) != 0) + throw std::runtime_error("MT7612U channel set failed"); + _channel = channel; + start_tick(); +} + +/* --- the 1 Hz PHY tick --------------------------------------------------- + * + * One round of mt76's cal_work. Not optional and not cosmetic: without it a + * receiver 20 cm from a peer airing 3037 fps takes 3 frames in 10 s; with it, + * 5415-5470/s. + * + * On a thread this class owns rather than inside StartRxLoop, because a + * transmit-only consumer (InitWrite, no RX loop) needs it too - TSSI + * temperature compensation rides the same tick. It takes _mu because the tick + * issues MCU commands, which share one 4-bit sequence number and one response + * endpoint with everything else here; running one alongside a channel change's + * calibration burst is how those get crossed. */ +void Mt7612uRadio::start_tick() { + if (_tick.joinable()) + return; + { + std::lock_guard lock(_tick_mu); + _tick_stop = false; + } + _tick = std::thread(&Mt7612uRadio::tick_loop, this); +} + +void Mt7612uRadio::stop_tick() { + if (!_tick.joinable()) + return; + { + std::lock_guard lock(_tick_mu); + _tick_stop = true; + } + _tick_cv.notify_all(); + _tick.join(); +} + +void Mt7612uRadio::tick_loop() { + for (;;) { + { + std::unique_lock lock(_tick_mu); + /* Waits out the interval, but wakes immediately on teardown so a stop + * never has to sit through a whole second. */ + _tick_cv.wait_for(lock, std::chrono::seconds(1), + [this] { return _tick_stop; }); + if (_tick_stop) + return; + } + std::lock_guard lock(_mu); + if (!_dev) + continue; + /* Returns -1 before a channel is set, which is not an error worth a line + * every second - bring_up() sets one before this thread can do anything. */ + mt7612u_phy_tick(_dev); + } +} + +void Mt7612uRadio::Init(Action_ParsedRadioPacket packetProcessor, + SelectedChannel channel) { + { + std::lock_guard lock(_mu); + bring_up(channel); + } + /* Deliberately outside the lock: StartRxLoop blocks until StopRxLoop. */ + StartRxLoop(std::move(packetProcessor)); +} + +void Mt7612uRadio::InitWrite(SelectedChannel channel) { + std::lock_guard lock(_mu); + bring_up(channel); + /* No RX ring, so mt7612u_start() enables TX only - which is the point of + * this entry point, and also what keeps the chip out of the undrained- + * receiver wedge. */ + if (mt7612u_start(_dev) != 0) + throw std::runtime_error("MT7612U MAC start failed"); +} + +void Mt7612uRadio::StartRxLoop(Action_ParsedRadioPacket packetProcessor) { + { + std::lock_guard lock(_mu); + if (!_dev) + throw std::runtime_error("MT7612U RX loop requires initialized hardware"); + if (_rx_active.load()) + throw std::runtime_error("MT7612U RX loop is already active"); + _rx_processor = std::move(packetProcessor); + _rx_stop = false; + + /* Ring first, receiver second - see rule 1 in the header. */ + if (mt7612u_rx_start(_dev, &Mt7612uRadio::rx_trampoline, this) != 0) + throw std::runtime_error("MT7612U RX ring failed to start"); + if (mt7612u_start(_dev) != 0) { + mt7612u_rx_stop(_dev); + throw std::runtime_error("MT7612U MAC start failed"); + } + /* AFTER mt7612u_start(), which rewrites the filter to mt76's managed-mode + * value - see rule 2. Before it, this write is simply overwritten. */ + if (mt7612u_set_monitor_rx(_dev, _cfg.rx.keep_corrupted ? 1 : 0) != 0) + _logger->warn("MT7612U monitor RX filter not applied"); + /* Arms the channel timers and zeroes the MIB counters. */ + mt7612u_link_stats_start(_dev); + _rx_active = true; + } + + _logger->info("MT7612U monitor RX on channel {}", _channel.Channel); + + /* The C layer drives RX from its own libusb event thread, so this loop has + * nothing to poll - it exists to give StartRxLoop the blocking contract + * every other backend has, and to notice Stop() and SIGINT. + * + * Delivery therefore happens on the event thread rather than on this one, + * which differs from the Realtek backends. The guarantee that matters is + * preserved: StopRxLoop tears the ring down and joins that thread before + * returning, so no callback can arrive after StartRxLoop returns. */ + while (!_rx_stop.load() && !g_devourer_should_stop) + std::this_thread::sleep_for(std::chrono::milliseconds(20)); + + StopRxLoop(); +} + +void Mt7612uRadio::StopRxLoop() { + _rx_stop = true; + if (!_rx_active.exchange(false)) + return; + + /* Quiesce under the lock: it is a register write, and it must not interleave + * with the tick's MCU traffic. */ + { + std::lock_guard lock(_mu); + if (_dev) + mt7612u_rx_quiesce(_dev); + } + /* Ring teardown WITHOUT the lock. mt7612u_rx_stop() joins the event thread, + * and a processor still in flight on that thread may call back into this + * object - SetMonitorChannel, say - which takes _mu. Holding _mu across the + * join is therefore a deadlock, not a theoretical one. */ + if (_dev) + mt7612u_rx_stop(_dev); + + _logger->info("MT7612U RX stopped after {} frames", + _rx_frames.load(std::memory_order_relaxed)); +} + +void Mt7612uRadio::rx_trampoline(void *user, const void *frame, size_t len, + const struct mt7612u_rx_info *info) { + static_cast(user)->on_rx(frame, len, info); +} + +void Mt7612uRadio::on_rx(const void *frame, size_t len, + const struct mt7612u_rx_info *info) { + if (!_rx_processor) + return; + + Packet packet{}; + packet.RxAtrib.pkt_len = static_cast(len); + packet.RxAtrib.crc_err = info->crc_err != 0; + packet.RxAtrib.seq_num = info->seq; + packet.RxAtrib.data_rate = mt7612u::desc_rate(*info); + packet.RxAtrib.bw = mt7612u::bw_to_desc(info->bw); + packet.RxAtrib.stbc = info->stbc; + packet.RxAtrib.ldpc = info->ldpc; + packet.RxAtrib.sgi = info->sgi; + packet.RxAtrib.paggr = info->ampdu != 0; + packet.RxAtrib.pkt_rpt_type = RX_PACKET_TYPE::NORMAL_RX; + /* THE MediaTek divergence, and the reason rx_pkt_attrib carries this flag at + * all: this MAC strips the FCS. The four bytes after the MPDU in the DMA + * buffer are the FCE info trailer, not a checksum - CRC-32 matched them on 0 + * of 4263 measured frames. A consumer that trims four bytes here would + * delete real payload. */ + packet.RxAtrib.fcs_present = false; + /* Per-chain RSSI and SNR, from n_chains rather than the array's extent: + * rssi[2] is the noise floor and rssi[3] is unidentified. */ + mt7612u::copy_signal(*info, packet.RxAtrib); + + if (len >= 2) { + const uint8_t *f = static_cast(frame); + packet.RxAtrib.qos = (f[0] & 0x0c) == 0x08 && (f[0] & 0x80) != 0; + /* The TID lives in the first QoS Control byte, which follows the 24-byte + * base header. Set alongside qos rather than left zero, so a consumer + * cannot read "QoS frame, TID 0" for every frame. */ + if (packet.RxAtrib.qos && len >= 26) + packet.RxAtrib.priority = f[24] & 0x0f; + } + + /* The span points into the ring buffer the libusb event thread owns and + * reuses the moment this returns, so the processor must not retain it - the + * same contract every other backend's parser has. const_cast because + * Packet::Data is a mutable span and the buffer genuinely is ours. */ + packet.Data = std::span( + const_cast(static_cast(frame)), len); + + _rx_frames.fetch_add(1, std::memory_order_relaxed); + _rx_processor(packet); +} + +void Mt7612uRadio::SetMonitorChannel(SelectedChannel channel) { + std::lock_guard lock(_mu); + enum mt7612u_bw bw = MT7612U_BW_20; + const char *why = ""; + + if (!_dev) { + _channel = channel; /* remembered until bring-up */ + return; + } + if (!mt7612u::width_to_bw(channel.ChannelWidth, bw, why)) { + _logger->error("MT7612U channel width refused: {}", why); + return; + } + if (mt7612u_set_channel(_dev, channel.Channel, bw) != 0) { + _logger->error("MT7612U channel set to {} failed", channel.Channel); + return; + } + /* Only on success, so GetSelectedChannel never reports a channel the + * hardware did not reach. */ + _channel = channel; +} + +SelectedChannel Mt7612uRadio::GetSelectedChannel() { + std::lock_guard lock(_mu); + return _channel; +} + +/* Both send paths take _mu because the TX width clamp in the library reads the + * tuned channel and width, which SetMonitorChannel writes under this same + * lock. Without it a retune concurrent with a burst can have a frame read a + * half-updated width. */ +bool Mt7612uRadio::send_packet(const uint8_t *packet, size_t length) { + std::lock_guard lock(_mu); + if (!_dev) + return false; + return mt7612u_send_packet(_dev, packet, length) == 0; +} + +size_t Mt7612uRadio::send_packets(const TxPacketView *pkts, size_t count) { + std::lock_guard lock(_mu); + if (!_dev || !pkts) + return 0; + /* TxPacketView and mt7612u_tx_view are the same two fields in the same + * order, but a reinterpret_cast across a language boundary is the kind of + * thing that breaks silently when one side gains a member. Copy. */ + std::vector views(count); + for (size_t i = 0; i < count; ++i) { + views[i].data = pkts[i].data; + views[i].len = pkts[i].len; + } + return mt7612u_send_packets(_dev, views.data(), count); +} + +void Mt7612uRadio::SetCcaMode(bool disabled) { + /* Refuses rather than no-ops. MT7612U does have an ED-CCA enable + * (MT_TXOP_CTRL_CFG / MT_TXOP_ED_CCA_EN, which mac_stop already clears), but + * "disable CCA" on the Realtek backends means a specific, measured set of + * writes, and nothing here has been measured against an on-air carrier-sense + * test. Claiming it on the strength of one plausible-looking bit is how an + * unverified regulatory-adjacent behaviour ships. */ + _logger->error("MT7612U: SetCcaMode({}) not implemented - the ED-CCA enable " + "exists but no on-air carrier-sense measurement backs it", + disabled); +} + +void Mt7612uRadio::Stop() { + StopRxLoop(); + stop_tick(); /* joins; must not run with _mu held */ + std::lock_guard lock(_mu); + if (_dev) { + mt7612u_stop(_dev); + mt7612u_close(_dev); + _dev = nullptr; + } +} + +void Mt7612uRadio::SetTxPower(uint8_t power) { + std::lock_guard lock(_mu); + /* Deliberately NOT forwarded to SetTxPowerIndexOverride the way the base + * class does: there is no TXAGC index here, so the argument is read as the + * dBm limit it actually maps to. */ + _txpwr_dbm = static_cast(power); + if (_dev && mt7612u_set_txpower(_dev, _txpwr_dbm) != 0) + _logger->error("MT7612U TX power {} dBm refused (valid range 0-30)", + _txpwr_dbm); +} + +void Mt7612uRadio::SetTxPowerIndexOverride(int idx) { + _logger->error("MT7612U has no TXAGC index to override (asked for {}); TX " + "power here is an absolute dBm limit - use SetTxPower()", + idx); +} + +bool Mt7612uRadio::GetPermanentMacAddress(uint8_t out[6]) { + std::lock_guard lock(_mu); + if (!_dev) + return false; + const uint8_t *mac = mt7612u_mac_addr(_dev); + if (!mac) + return false; + for (int i = 0; i < 6; ++i) + out[i] = mac[i]; + return true; +} + +uint64_t Mt7612uRadio::ReadTsf() { + std::lock_guard lock(_mu); + return _dev ? mt7612u_read_tsf(_dev) : 0; +} + +void Mt7612uRadio::WriteTsf(uint64_t tsf) { + std::lock_guard lock(_mu); + if (_dev) + mt7612u_write_tsf(_dev, tsf); +} + +bool Mt7612uRadio::SetAckResponder(const devourer::MacAddr &mac) { + std::lock_guard lock(_mu); + if (!_dev) + return false; + return mt7612u_set_ack_responder(_dev, mac.data()) == 0; +} + +void Mt7612uRadio::ClearAckResponder() { + std::lock_guard lock(_mu); + if (_dev) + mt7612u_clear_ack_responder(_dev); +} + +devourer::TxCaps Mt7612uRadio::GetTxCaps() { + devourer::TxCaps c{}; + c.supported = true; + c.n_ss = 2; + c.stbc_ok = true; + c.ldpc_ok = true; + c.sgi_ok = true; + c.bw_max_mhz = 80; + return c; +} + +devourer::TxPowerCaps Mt7612uRadio::GetTxPowerCaps() { + devourer::TxPowerCaps c{}; + c.supported = true; + /* index_max 0 = the dBm model, per the field's own contract. There is no + * TXAGC index on this part: TX power is an absolute dBm limit feeding the + * per-rate table, plus a 4-bit per-frame trim in the descriptor. */ + c.index_max = 0; + c.step_qdb = 2; /* the limit is carried in 0.5 dB units */ + c.step_measured = false; + c.offset_min_qdb = -80; /* down to 0 dBm from the 20 dBm default */ + c.offset_max_qdb = 40; /* up to 30 dBm, the API's own ceiling */ + c.rate_diffs = false; + return c; +} + +devourer::AdapterCaps Mt7612uRadio::GetAdapterCaps() { + struct mt7612u_caps hw {}; + bool have_hw = false; + { + std::lock_guard lock(_mu); + if (_dev) { + mt7612u_get_caps(_dev, &hw); + have_hw = true; + } + } + + devourer::AdapterCaps c{}; + c.supported = true; + c.chip_name = have_hw && hw.chip_name ? hw.chip_name : "MT7612U"; + c.marketing_names = "MT7612U/MT7662U"; + c.chip_id = 0; /* no SYS_CFG2 equivalent - dispatch is VID:PID */ + c.generation = devourer::ChipGeneration::Mt7612u; + c.variant = "MT7612U"; + c.transport = "usb"; + /* Read from the library rather than restated as literals here: it derives + * them from the EEPROM and the register programming, and a second copy is a + * second thing to drift. Falls back only when the device is not open. */ + c.tx_chains = have_hw ? hw.nss_tx : 2; + c.rx_chains = have_hw ? hw.nss_rx : 2; + c.tx = GetTxCaps(); + c.txpwr = GetTxPowerCaps(); + /* 5/10 MHz stay out: MT_RATE_BW has no encoding for them. Must agree with + * GetTxCaps().bw_max_mhz - the two travel together in one adapter.caps + * event, and a consumer gating on the mask would never ask for a width the + * ceiling advertises. */ + c.bw_mask = devourer::kBw20 | devourer::kBw40 | devourer::kBw80; + c.tune_5g = {true, have_hw ? hw.band_5g_min_mhz : uint16_t(5180), + have_hw ? hw.band_5g_max_mhz : uint16_t(5825)}; + c.tune_2g4 = {true, have_hw ? hw.band_2g_min_mhz : uint16_t(2412), + have_hw ? hw.band_2g_max_mhz : uint16_t(2484)}; + c.characterized_5g = c.tune_5g; + c.characterized_2g4 = c.tune_2g4; + c.ldpc_rx_ht = true; + c.ldpc_rx_vht = true; + c.ldpc_rx_flag = true; /* the RXWI carries the per-frame LDPC bit */ + c.per_chain_rssi = true; + c.hw_rx_timestamp = false; /* the RXWI TSF field is not parsed */ + c.hw_beacon_txtsf = false; /* no hardware beacon function ported */ + /* Measured on air: 0 frames at the stimulus radio unarmed, 3500+ armed. */ + c.ack_responder_ok = true; + /* Unmeasured, so false rather than optimistic - nothing here drives the + * hardware retry counter. */ + c.tx_retry_limit_ok = false; + c.narrowband_ok = false; + /* Measured 526 ms full / 48 ms with calibration skipped, against 0.5-2.5 ms + * on the Realtek parts: the RF plane lives behind the MCU. Not "fast". */ + c.fastretune_ok = false; + c.per_packet_txpower = true; + c.per_pkt_txpwr_steps = 0; + c.per_pkt_txpwr_step_qdb = 4; /* MT_TX_PWR_ADJ is a 4-bit dB trim */ + c.per_pkt_txpwr_min_qdb = -32; + c.per_pkt_txpwr_max_qdb = 28; + c.per_pkt_txpwr_measured = false; + c.vht_2g4_ok = false; /* unmeasured on this part */ + return c; +} diff --git a/src/mt7612u/Mt7612uRadio.h b/src/mt7612u/Mt7612uRadio.h new file mode 100644 index 00000000..bfb1df97 --- /dev/null +++ b/src/mt7612u/Mt7612uRadio.h @@ -0,0 +1,133 @@ +#ifndef MT7612U_RADIO_H +#define MT7612U_RADIO_H + +#include +#include +#include +#include +#include + +#include + +#include "DeviceConfig.h" +#include "IRadio.h" +#include "UsbDeviceLock.h" +#include "logger.h" +#include "mt7612u/mt7612u.h" + +/* + * The MediaTek MT7612U behind IRadio. + * + * It derives from IRadio and NOT from IRtlRadio: the Realtek-only members + * (phydm energy counters, the crystal-cap trim, EFUSE stability, the canary + * register dump) describe a register plane this silicon does not have, and + * inheriting them would mean answering for hardware that is not here. + * + * What it does NOT do is reuse RtlAdapter. That transport is shaped around + * 16-bit Realtek registers and Realtek bulk endpoints; MT7612U is 32-bit + * registers over EP0 vendor requests plus an in-band MCU on EP8/EP5, with + * firmware to upload before any of it answers. The C library under this + * directory is that transport, and this class owns one `mt7612u_dev`. + * + * THREE ORDERING RULES, each of which has cost real hardware time: + * + * 1. RX ring first, receiver second. Enabling MAC RX with nothing draining + * the bulk-IN endpoint wedges this part BELOW the USB level: + * libusb_reset_device, the sysfs authorized toggle and rebinding the + * kernel driver all fail, and only a physical replug recovers it. + * Teardown is the mirror image — quiesce the receiver, then remove the + * drain (mt7612u_rx_quiesce, then mt7612u_rx_stop). + * + * 2. The monitor filter goes on AFTER mt7612u_start(), which rewrites + * MT_RX_FILTR_CFG to mt76's managed-station value. Measured against the + * bring-up harness in the same minute on the same silicon: 0 OFDM frames + * of 244 with the managed filter, 103 of 402 with the monitor filter. A + * single-path test would have called 244 beacons a working receiver. + * + * 3. The 1 Hz PHY tick is not optional. Measured against a peer 20 cm away + * airing 3037 fps: a receiver with no tick takes 3 frames in 10 s; with it, + * 5415-5470/s. It runs on a thread this class owns rather than inside + * StartRxLoop, because a transmit-only consumer (InitWrite with no RX loop) + * needs it too. + * + * Locking. `_mu` guards the control plane — bring-up, channel sets, power, and + * the tick, all of which issue MCU commands that share one 4-bit sequence + * number and one response endpoint and so need a single user. The RX callback + * deliberately does NOT take it: it runs on the C layer's libusb event thread + * and only reads `_rx_processor`, which is written before the ring starts and + * not touched again until after it stops. + */ +class Mt7612uRadio : public IRadio { +public: + Mt7612uRadio(libusb_device_handle *handle, libusb_context *ctx, + std::shared_ptr usb_lock, + Logger_t logger, devourer::DeviceConfig cfg); + ~Mt7612uRadio() override; + + Mt7612uRadio(const Mt7612uRadio &) = delete; + Mt7612uRadio &operator=(const Mt7612uRadio &) = delete; + + /* --- the pure-virtual core --- */ + void Init(Action_ParsedRadioPacket packetProcessor, + SelectedChannel channel) override; + void InitWrite(SelectedChannel channel) override; + void StartRxLoop(Action_ParsedRadioPacket packetProcessor) override; + void StopRxLoop() override; + void SetMonitorChannel(SelectedChannel channel) override; + bool send_packet(const uint8_t *packet, size_t length) override; + size_t send_packets(const TxPacketView *pkts, size_t count) override; + SelectedChannel GetSelectedChannel() override; + void SetCcaMode(bool disabled) override; + + /* --- optional members this silicon can actually answer for --- */ + void Stop() override; + devourer::AdapterCaps GetAdapterCaps() override; + devourer::TxCaps GetTxCaps() override; + devourer::TxPowerCaps GetTxPowerCaps() override; + void SetTxPower(uint8_t power) override; + void SetTxPowerIndexOverride(int idx) override; + bool GetPermanentMacAddress(uint8_t out[6]) override; + uint64_t ReadTsf() override; + void WriteTsf(uint64_t tsf) override; + bool SetAckResponder(const devourer::MacAddr &mac) override; + void ClearAckResponder() override; + +private: + void bring_up(SelectedChannel channel); /* _mu held */ + void start_tick(); /* _mu held */ + void stop_tick(); /* _mu NOT held */ + void tick_loop(); + static void rx_trampoline(void *user, const void *frame, size_t len, + const struct mt7612u_rx_info *info); + void on_rx(const void *frame, size_t len, + const struct mt7612u_rx_info *info); + static void log_trampoline(void *user, char level, const char *line); + + libusb_device_handle *_handle; + libusb_context *_ctx; + /* Held, never used: it keeps the exclusive USB lock WiFiDriver took alive + * for this object's lifetime. */ + std::shared_ptr _usb_lock; + Logger_t _logger; + devourer::DeviceConfig _cfg; + + std::recursive_mutex _mu; + struct mt7612u_dev *_dev = nullptr; + SelectedChannel _channel{}; + Action_ParsedRadioPacket _rx_processor; + + std::atomic _rx_stop{false}; + std::atomic _rx_active{false}; + std::atomic _rx_frames{0}; + + /* The tick thread and the gate that lets StopRxLoop / Stop wake it early + * instead of waiting out a whole second. */ + std::thread _tick; + std::mutex _tick_mu; + std::condition_variable _tick_cv; + bool _tick_stop = false; + + int _txpwr_dbm = 20; /* the absolute dBm limit mt7612u_set_txpower takes */ +}; + +#endif /* MT7612U_RADIO_H */ diff --git a/src/mt7612u/async.cpp b/src/mt7612u/async.cpp index 68addbd6..603a9ef0 100644 --- a/src/mt7612u/async.cpp +++ b/src/mt7612u/async.cpp @@ -334,8 +334,19 @@ int mt7612u_rx_start(struct mt7612u_dev *d, mt7612u_rx_cb cb, void *user) return mt_async_start(d, cb, user); } +int mt7612u_rx_quiesce(struct mt7612u_dev *d) +{ + if (!d) return -1; + mt_mac_rx_disable(d); + return 0; +} + int mt7612u_rx_stop(struct mt7612u_dev *d) { + /* Callers that want the receiver silenced BEFORE the drain disappears + * call mt7612u_rx_quiesce() first; see its contract. Not folded in here + * because bringup's gates already quiesce explicitly at each of their own + * teardown points, and doing it twice would hide which one did it. */ mt_async_stop(d); return 0; } diff --git a/src/mt7612u/include/mt7612u/mt7612u.h b/src/mt7612u/include/mt7612u/mt7612u.h index 06dd510c..a99165e0 100644 --- a/src/mt7612u/include/mt7612u/mt7612u.h +++ b/src/mt7612u/include/mt7612u/mt7612u.h @@ -220,6 +220,26 @@ typedef void (*mt7612u_rx_cb)(void *user, const void *frame, size_t len, int mt7612u_rx_start(struct mt7612u_dev *dev, mt7612u_rx_cb cb, void *user); int mt7612u_rx_stop(struct mt7612u_dev *dev); +/* + * Silence the receiver WITHOUT tearing the ring down: clears MAC RX only, + * leaving TX, the ring and the event thread alone. + * + * This is the first half of an orderly RX teardown, and the order is not + * cosmetic. mt7612u_rx_stop() cancels the bulk-IN transfers, which removes the + * drain; doing that while the MAC is still receiving is the state that wedges + * this part below the USB level, where libusb_reset_device, the sysfs + * authorized toggle and rebinding the kernel driver all fail to recover it and + * only a physical replug does. So: quiesce, then stop. + * + * mt7612u_stop() would also silence the receiver, but it stops the whole MAC + * including TX — no use to a caller that brought the chip up for transmit and + * is only shutting the RX half down. + * + * Leaves the ring restartable: a later mt7612u_start() re-enables MAC RX if a + * ring is running. + */ +int mt7612u_rx_quiesce(struct mt7612u_dev *dev); + /* * Put the receive filter into monitor mode: pass everything the PHY decodes, * dropping only PHY errors and (unless keep_corrupted) frames that failed FCS. diff --git a/src/mt7612u/tests/api_link.c b/src/mt7612u/tests/api_link.c index 3fe2c2bd..f6bf5d8e 100644 --- a/src/mt7612u/tests/api_link.c +++ b/src/mt7612u/tests/api_link.c @@ -28,6 +28,7 @@ static void *const api[] = { (void *)mt7612u_tx, (void *)mt7612u_rx_start, (void *)mt7612u_rx_stop, + (void *)mt7612u_rx_quiesce, (void *)mt7612u_set_monitor_rx, (void *)mt7612u_send_packet, (void *)mt7612u_send_packets, From 2ad3727548fa8865806f3d00fd37aba987bdc499 Mon Sep 17 00:00:00 2001 From: snokvist Date: Thu, 10 Sep 2026 00:19:10 +0200 Subject: [PATCH 3/8] mt7612u: report TX stats devourer can trust, and prove TX on air MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit txdemo on this backend reported submitted=0 against twelve tx.frame events with rc=1 — frames going out, the counter flat. mt7612u_get_stats() reports the ASYNC RING's counters, and mt_tx_raw() only uses that ring when one is running (tx.cpp:229); the TX-only bring-up this backend offers, InitWrite with no StartRxLoop, starts no ring. So the honest counter is the one devourer keeps itself: what it handed the transport, and what came back refused. before: {"ev":"tx.stats","submitted":0, "failed":0} after: {"ev":"tx.stats","submitted":2903,"failed":0} last_error_rc and last_was_timeout stay at their defaults. The library returns a count, not the libusb rc of the last failure, and inventing one would be the same fault in a different field. ON AIR, two radios, using devourer's own matcher rather than an inference. An RTL8812AU running rxdemo on ch36 while this backend transmitted from an MT7612U on the same channel: {"ev":"rx.txhit","hits":19,"len":104,"rate":4,"crc":0,"rssi":72,...} rx.txhit fires only on the canonical txdemo source address, so those are our frames and nothing else's: rate 4 is OFDM 6 Mbps, crc 0 is a clean decode, and rssi 72 is -38 dBm through the 110 bias, which is the near-field level two adapters on one bench should show. Worth recording that the first attempt at this was NOT evidence. Comparing ambient frame counts on a third adapter gave 1807 frames without the transmitter and 2166 with it — a 20% rise coincident with TX, and no way to attribute a single frame, since neither tool filters by source address. The txhit matcher is what turned it into a measurement. 60/60 ctest in both configs. Co-Authored-By: Claude Opus 5 (1M context) Claude-Session: https://claude.ai/code/session_01Tba83kymS5W2v1vn2yRxrj --- src/mt7612u/Mt7612uRadio.cpp | 29 +++++++++++++++++++++++++++-- src/mt7612u/Mt7612uRadio.h | 3 +++ 2 files changed, 30 insertions(+), 2 deletions(-) diff --git a/src/mt7612u/Mt7612uRadio.cpp b/src/mt7612u/Mt7612uRadio.cpp index d7876e5f..eeb8eb92 100644 --- a/src/mt7612u/Mt7612uRadio.cpp +++ b/src/mt7612u/Mt7612uRadio.cpp @@ -352,7 +352,9 @@ bool Mt7612uRadio::send_packet(const uint8_t *packet, size_t length) { std::lock_guard lock(_mu); if (!_dev) return false; - return mt7612u_send_packet(_dev, packet, length) == 0; + const bool ok = mt7612u_send_packet(_dev, packet, length) == 0; + (ok ? _tx_submitted : _tx_failed).fetch_add(1, std::memory_order_relaxed); + return ok; } size_t Mt7612uRadio::send_packets(const TxPacketView *pkts, size_t count) { @@ -367,7 +369,10 @@ size_t Mt7612uRadio::send_packets(const TxPacketView *pkts, size_t count) { views[i].data = pkts[i].data; views[i].len = pkts[i].len; } - return mt7612u_send_packets(_dev, views.data(), count); + const size_t sent = mt7612u_send_packets(_dev, views.data(), count); + _tx_submitted.fetch_add(sent, std::memory_order_relaxed); + _tx_failed.fetch_add(count - sent, std::memory_order_relaxed); + return sent; } void Mt7612uRadio::SetCcaMode(bool disabled) { @@ -433,6 +438,26 @@ void Mt7612uRadio::WriteTsf(uint64_t tsf) { mt7612u_write_tsf(_dev, tsf); } +devourer::TxStats Mt7612uRadio::GetTxStats() { + devourer::TxStats out{}; + + /* Counted here rather than read from mt7612u_get_stats(), which reports the + * ASYNC RING's counters. mt_tx_raw() only uses that ring when one is running + * (tx.cpp), and the TX-only bring-up this backend offers - InitWrite with no + * StartRxLoop - starts no ring, so those counters read 0 while frames are + * going out. Measured: txdemo on this part reported submitted=0 against + * twelve tx.frame events with rc=1. A stat that reads zero while the radio + * transmits is worse than no stat, so this counts what devourer actually + * handed the transport. + * + * last_error_rc and last_was_timeout stay at their defaults: the library + * returns a count, not the libusb rc of the last failure, and inventing one + * would be the same fault in a different field. */ + out.submitted = _tx_submitted.load(std::memory_order_relaxed); + out.failed = _tx_failed.load(std::memory_order_relaxed); + return out; +} + bool Mt7612uRadio::SetAckResponder(const devourer::MacAddr &mac) { std::lock_guard lock(_mu); if (!_dev) diff --git a/src/mt7612u/Mt7612uRadio.h b/src/mt7612u/Mt7612uRadio.h index bfb1df97..c1f8115e 100644 --- a/src/mt7612u/Mt7612uRadio.h +++ b/src/mt7612u/Mt7612uRadio.h @@ -89,6 +89,7 @@ class Mt7612uRadio : public IRadio { bool GetPermanentMacAddress(uint8_t out[6]) override; uint64_t ReadTsf() override; void WriteTsf(uint64_t tsf) override; + devourer::TxStats GetTxStats() override; bool SetAckResponder(const devourer::MacAddr &mac) override; void ClearAckResponder() override; @@ -119,6 +120,8 @@ class Mt7612uRadio : public IRadio { std::atomic _rx_stop{false}; std::atomic _rx_active{false}; std::atomic _rx_frames{0}; + std::atomic _tx_submitted{0}; + std::atomic _tx_failed{0}; /* The tick thread and the gate that lets StopRxLoop / Stop wake it early * instead of waiting out a whole second. */ From ae9b2d5a8744347ab646d1c49d93f3aa0d95f6e5 Mon Sep 17 00:00:00 2001 From: snokvist Date: Thu, 10 Sep 2026 00:43:08 +0200 Subject: [PATCH 4/8] review: fix a use-after-free, a halved SNR, and five capabilities that lied Two reviewers went at the backend. Both found real defects; these are theirs. USE-AFTER-FREE ON TEARDOWN. StopRxLoop cleared _rx_active up front and let the second caller RETURN while the first was still inside mt7612u_rx_stop() - cancelling 32 transfers and joining the event thread, which can take seconds. On SIGINT the RX thread wins that exchange and the control thread loses, believes the ring is down, calls Stop(), and mt7612u_close() frees the device out from under the thread still tearing it down: a second join on the same std::thread, and libusb_free_transfer on transfers already freed. It also read _dev without the lock that guards it. Now a _teardown_mu is held across the WHOLE sequence so the loser waits, _dev is read under _mu, and _rx_active is cleared at the END. That last part was a second bug on its own: with the flag cleared up front, a restart passed the "already active" guard while the old ring still existed, and mt_async_start()'s `if (d->a) return 0` reported SUCCESS WITHOUT ARMING - StartRxLoop would then log "monitor RX on channel N" and block on a ring that delivers nothing, with no error anywhere. The same fix closes it. SNR WAS REPORTED AT HALF ITS VALUE. rx_pkt_attrib::snr[] is s(8,1) half-dB everywhere else - LinkHealth.cpp reads snr_raw / 2.0, RxQuality derives its noise floor as (rssi_raw - 110) - snr_raw / 2.0 - and the library reports whole dB. So a 37 dB link was published as 18.5 dB, classify_link_health tripped at twice the true SNR, and the derived noise floor sat snr/2 dB high. Worse, the selftest asserted the raw value and so PINNED the bug: I had written the test against my implementation instead of against the consumers' contract. CAPABILITIES THAT LIED, all now either implemented or honest: - per_packet_txpower: was true. The radiotap path PARSES DBM_TX_POWER and discards it, the session route is gated on an enable_tpc field nothing ever assigns, and this backend never calls mt7612u_tx(). No caller: false. - ldpc_rx_vht: was true off an HT measurement. HT and VHT are separate decoder paths, which is why AdapterCaps splits them; VHT is unexercised here, so it says so. - characterized_5g/2g4: claimed the whole tunable span was table-backed off a register comparison at one channel. docs/mt7612u.md says plainly that register equality is not dBm. Left invalid. - GetTxPowerCaps advertised an offset knob with supported=true while SetTxPowerOffsetQdb was the inherited no-op returning 0 - indistinguishable from "clamped at a rail" to a closed-loop controller. Now implemented against the dBm base, returning the quantized applied value; step_qdb is 4, not 2, because the actuator takes whole dBm. - GetTxStats().failed could not move on the async path: an URB that dies on the wire is counted in tx_done, which nothing read. Same "zero while the radio transmits" fault the previous commit fixed in submitted. SILENT NO-OPS MADE LOUD. SetTxMode was inherited, so examples/tx's rate-less frames - built that way on purpose to drive an MCS sweep - aired at OFDM 6 Mbps while the sweep reported MCS7. My own on-air proof shows it: every rx.txhit came back rate 4. The C library has no session-default rate, so this refuses and says where to put the rate instead. SetAmpduMode likewise refuses rather than accepting a mode that cannot reach the air, naming the 2.21x it costs. cfg.tx.ack_timeout_us, tuning.disable_cca and tx.usb_agg_max now warn once per bring-up instead of being dropped; cfg.rx.ack_responder is armed, and a refusal is fatal rather than handing back a green init and a session that answers nothing. SetCcaMode(false) is quiet - asking for the state the chip is already in is not an error. Also: the TID was read at a fixed offset 24, which on a 4-address QoS frame is the low nibble of Address 4 - a plausible-looking priority, worse than zero; desc_rate ran unbounded, so a 6-bit HT index above 31 reported garbage as a real VHT rate; Init/InitWrite left the device open and the tick running on a throw, and the obvious fix for that DEADLOCKS (Stop joins the tick, the tick wants _mu) so the cleanup runs via exception_ptr outside the lock; Stop() could leave a joinable thread if its logging threw; the process-global log sink let a second radio steal the first's routing and the first destructor unhook the survivor's; and mt7612u.device_selector was config nothing read. mt7612uprobe now exists as a CMake target, which src/mt7612u/README.md has been promising. Both docs stopped claiming there is no backend. Verified: 60/60 ctest in all three configs; on hardware, RX decodes (rate 4, rssi 60 = -50 dBm) and 2253 frames under ASan+UBSan with no report, which is the teardown path the use-after-free lived on. Co-Authored-By: Claude Opus 5 (1M context) Claude-Session: https://claude.ai/code/session_01Tba83kymS5W2v1vn2yRxrj --- CMakeLists.txt | 20 ++ docs/mt7612u.md | 30 ++- examples/common/env_config.cpp | 2 - src/DeviceConfig.h | 11 +- src/mt7612u/Mt7612uMapping.h | 62 ++++- src/mt7612u/Mt7612uRadio.cpp | 370 ++++++++++++++++++++++++----- src/mt7612u/Mt7612uRadio.h | 20 +- src/mt7612u/README.md | 26 +- src/mt7612u/internal.h | 4 +- src/mt7612u/usb.cpp | 9 +- tests/mt7612u_mapping_selftest.cpp | 66 ++++- 11 files changed, 520 insertions(+), 100 deletions(-) diff --git a/CMakeLists.txt b/CMakeLists.txt index dd927a73..9d3420cf 100644 --- a/CMakeLists.txt +++ b/CMakeLists.txt @@ -628,6 +628,26 @@ if(DEVOURER_8733B) target_include_directories(rtl8733bprobe PRIVATE ${CMAKE_CURRENT_SOURCE_DIR}/examples/common) endif() +# mt7612uprobe — the MediaTek bring-up harness, the rtl8733bprobe/kestrelprobe +# sibling: one subcommand per verified hardware gate (registers, firmware, MAC, +# channel, TX, RX, ACK responder, the wedge experiments). Same source the +# subtree's own Makefile builds as ./bringup, built here so the chip-specific +# tool is not the one part of this backend that only a second build system can +# produce — and so it picks up CMAKE_C_COMPILER and DEVOURER_SANITIZE, which +# shelling out to that Makefile would silently ignore. +# +# It talks to the C library directly, NOT through Mt7612uRadio: it predates the +# backend and its whole purpose is to exercise the layer underneath one. It +# never substitutes for the production IRadio path. +if(DEVOURER_MT7612U) + add_executable(mt7612uprobe + src/mt7612u/tools/bringup.cpp + ) + target_link_libraries(mt7612uprobe PUBLIC devourer PRIVATE PkgConfig::libusb) + target_include_directories(mt7612uprobe PRIVATE + ${CMAKE_CURRENT_SOURCE_DIR}/src/mt7612u) +endif() + # reglat — register round-trip latency microbench (USB ctrl-xfer vs PCIe MMIO). # Times RtlAdapter::rtw_read32; the PCIe path compiles only with DEVOURER_PCIE # (inherited via the devourer PUBLIC DEVOURER_HAVE_PCIE define). See its header. diff --git a/docs/mt7612u.md b/docs/mt7612u.md index ae1e455a..96af4519 100644 --- a/docs/mt7612u.md +++ b/docs/mt7612u.md @@ -5,11 +5,16 @@ Everything below was measured on one MT7612U (`0e8d:7612`, `MT_ASIC_VERSION` running this project's own `rxdemo`/`txdemo`. Read `## Offline tests` and `## Counterparts` before quoting any number here. -**There is still no radio backend.** `DEVOURER_MT7612U` (default OFF) compiles -the subtree into `libdevourer`, and `WiFiDriver::CreateRadio` now recognises the -MediaTek USB ids — but only to *refuse* them, because no `IRadio` implementation -exists yet. What the option buys today is that the whole CI matrix compiles the -subtree; what it does not buy is a devourer binary that can open the part. +**It is wired in.** `DEVOURER_MT7612U` (default OFF) compiles the subtree into +`libdevourer` and `WiFiDriver::CreateRadio` constructs an `Mt7612uRadio`, so a +devourer binary opens, receives and transmits on this part. Measured through +`rxdemo`/`txdemo`: 12000 frames received on ch36, and 19 `rx.txhit` on an +independent RTL8812AU witnessing our transmit at −38 dBm. + +The knobs this backend does NOT implement refuse or warn rather than going +quiet — `SetTxMode`, `SetAmpduMode`, `SetCcaMode(true)`, `SetTxPowerIndexOverride`, +and the `tx.ack_timeout_us` / `tuning.disable_cca` / `tx.usb_agg_max` config +values. See "Counterparts" for what that costs. ## Why a MediaTek port is small @@ -343,12 +348,15 @@ Stated because the numbers above are uniformly favourable. firmware-running bit. - **The 48 ms fast retune is our implementation, not the floor.** The floor is unmeasured. -- **The library compiles in CI; its own tests still do not run there.** With - `DEVOURER_MT7612U=ON` the whole platform matrix (gcc, clang, MSVC, mingw, - macOS) builds the subtree, so a portability regression is caught. The offline - tests above and the table generator's `--check` are still driven only by - `src/mt7612u/Makefile`, which no workflow invokes — nor is there a sanitizer - build or a lifecycle soak of the kind the Realtek backends carry. +- **The library compiles in CI, and two ctest cells cover the integration; the + subtree's own four tests still do not run there.** With `DEVOURER_MT7612U=ON` + the whole platform matrix (gcc, clang, MSVC, mingw, macOS) builds the subtree + and the sanitizer job links it, so a portability or lifetime regression is + caught. `mt7612u_usb_ids` and `mt7612u_mapping` run on every configuration. + The four offline tests under `src/mt7612u/tests/` and the table generator's + `--check` are still driven only by `src/mt7612u/Makefile`, which no workflow + invokes — nor is there a lifecycle soak of the kind the Realtek backends + carry. - **80 MHz, VHT on air, and NSS=2 are unexercised.** The rate word encodes them and the RX path decodes them; neither has been transmitted. - **The RX path delivers no FCS** (see above). That is a measured hardware diff --git a/examples/common/env_config.cpp b/examples/common/env_config.cpp index d0b9d2f7..9f83d5a5 100644 --- a/examples/common/env_config.cpp +++ b/examples/common/env_config.cpp @@ -162,8 +162,6 @@ devourer::DeviceConfig devourer_config_from_env() { * library nor the device class consults ambient process state. */ if (const char *e = env_str("DEVOURER_MT7612U_FW_DIR")) cfg.mt7612u.firmware_dir = std::string(e); - if (const char *e = env_str("MT7612U_DEV")) - cfg.mt7612u.device_selector = std::string(e); /* ---- tuning ---- */ /* Defaults ON, so this reads the negation: only an explicit 0 disables it. */ diff --git a/src/DeviceConfig.h b/src/DeviceConfig.h index 92256761..d3c10de9 100644 --- a/src/DeviceConfig.h +++ b/src/DeviceConfig.h @@ -552,11 +552,12 @@ struct DeviceConfig { * device class both stay free of ambient process state; the demos fold the * variable in, the way they do for every other knob in this file. */ std::optional firmware_dir; - /* env: MT7612U_DEV — which adapter to open when several are attached, - * "-" as lsusb spells the port path. Unset = the first. - * Only consulted on the library's own open path, not when devourer hands - * it an already-claimed handle (which is the devourer path). */ - std::optional device_selector; + /* No adapter selector here on purpose. devourer chooses the adapter before + * a backend exists (DEVOURER_USB_BUS / _PORT / _VID / _PID) and hands the + * backend an already-claimed handle, so a MediaTek-specific selector would + * be read by nothing. The C library's own mt7612u_open_selected() is for a + * consumer that opens the device itself; MT7612U_DEV drives the bring-up + * tool, not devourer. */ } mt7612u; }; diff --git a/src/mt7612u/Mt7612uMapping.h b/src/mt7612u/Mt7612uMapping.h index 0f23e67b..b7df72c2 100644 --- a/src/mt7612u/Mt7612uMapping.h +++ b/src/mt7612u/Mt7612uMapping.h @@ -46,10 +46,18 @@ inline uint8_t rssi_to_raw(int8_t dbm) { * a 2-path part (RxPacket.h), so that is what a 2T2R MediaTek must report * too, and n_chains is the authority rather than the array's extent. * - * snr[] is filled from the same report while it is in hand: `snr_db` is - * rssi[0] - noise and is only meaningful when noise_valid, which is why the - * unvalidated case leaves the slots at zero rather than writing a plausible - * number. See the -116 dBm caveat on `noise` in the public header. */ + * snr[] is filled from the same report while it is in hand, and it is filled in + * HALF-dB. That is the unit every other producer and consumer of this field + * uses - Realtek parsers write s(8,1) (FrameParserJaguar2.h), LinkHealth.cpp + * reads `snr_raw / 2.0`, and RxQuality derives its noise floor as + * `(rssi_raw - 110) - snr_raw / 2.0`. The library reports whole dB (`snr_db` is + * rssi[0] - noise), so writing it through unscaled would report every MT7612U + * link at half its true SNR and put the derived noise floor snr/2 dB high - the + * same class of fault as the phantom chain above, one field below it. + * + * Only meaningful when noise_valid, which is why the unvalidated case leaves + * the slots at zero rather than writing a plausible number. See the -116 dBm + * caveat on `noise` in the public header. */ inline void copy_signal(const struct mt7612u_rx_info &info, struct rx_pkt_attrib &out) { const unsigned chains = info.n_chains > 2u ? 2u : info.n_chains; @@ -61,9 +69,39 @@ inline void copy_signal(const struct mt7612u_rx_info &info, for (unsigned i = 0; i < 4u; ++i) out.snr[i] = 0; - if (info.noise_valid) + if (info.noise_valid) { + int half_db = static_cast(info.snr_db) * 2; + if (half_db > 127) + half_db = 127; + if (half_db < -128) + half_db = -128; for (unsigned i = 0; i < chains; ++i) - out.snr[i] = info.snr_db; + out.snr[i] = static_cast(half_db); + } +} + +/* The QoS TID, or nothing when this frame carries no QoS Control field. + * + * The offset is NOT a constant 24. A 4-address data frame (ToDS and FromDS + * both set) has a 30-byte header, so its QoS Control sits at 30 - reading + * byte 24 there returns the low nibble of Address 4, which is arbitrary and + * WORSE than leaving the TID zero, because it looks like a plausible priority. + * The subtree's own mt_hdrlen_from_fc() gets this right for the TX path and + * rx.cpp uses it for the L2-pad fold; this is the same rule, kept here as a + * pure function so the selftest can pin it. */ +inline bool qos_tid(const uint8_t *f, size_t len, uint8_t &tid) { + if (len < 2) + return false; + const unsigned fc = (unsigned)f[0] | ((unsigned)f[1] << 8); + if (((fc >> 2) & 3u) != 2u) /* not a data frame */ + return false; + if (!(fc & 0x0080u)) /* not a QoS subtype */ + return false; + const size_t hdr = ((fc & 0x0300u) == 0x0300u) ? 30u : 24u; + if (len < hdr + 2u) + return false; + tid = f[hdr] & 0x0f; + return true; } /* mt7612u_rx_info -> the DESC_RATE numbering consumers read, so a caller does @@ -78,10 +116,20 @@ inline uint16_t desc_rate(const struct mt7612u_rx_info &info) { case MT7612U_PHY_HT: case MT7612U_PHY_HT_GF: /* HT folds NSS into the MCS number on both sides, so this is a straight - * offset for MCS 0-31. */ + * offset for MCS 0-31. Bounded because MT_RATE_INDEX is SIX bits: an index + * of 32..63 would run past DESC_RATEMCS31 into the VHT numbering and + * report garbage as a real VHT rate rather than as unknown. rx.cpp filters + * the PHY field, not the index. */ + if (info.mcs > 31) + return 0; return static_cast(DESC_RATEMCS0 + info.mcs); case MT7612U_PHY_VHT: { const uint8_t nss = info.nss ? info.nss : 1; + /* VHT MCS is 0-9 and the DESC numbering strides by 10 per stream, so an + * index above 9 spills into the NEXT stream's block - SS1 MCS12 would + * report as SS2 MCS2. Four streams is likewise the end of the numbering. */ + if (info.mcs > 9 || nss > 4) + return 0; return static_cast(DESC_RATEVHTSS1MCS0 + (nss - 1) * 10 + info.mcs); } diff --git a/src/mt7612u/Mt7612uRadio.cpp b/src/mt7612u/Mt7612uRadio.cpp index eeb8eb92..9076113e 100644 --- a/src/mt7612u/Mt7612uRadio.cpp +++ b/src/mt7612u/Mt7612uRadio.cpp @@ -2,6 +2,7 @@ #include #include +#include #include #include #include @@ -69,18 +70,62 @@ Mt7612uRadio::Mt7612uRadio(libusb_device_handle *handle, libusb_context *ctx, * synchronisation, which is the contract mt7612u_set_log_sink documents. * Without this the subtree's diagnostics go straight to stderr, bypassing * the log level, a redirected diag stream, and __android_log_write. */ - mt7612u_set_log_sink(&Mt7612uRadio::log_trampoline, this); + { + std::lock_guard lock(sink_mu()); + /* The sink is a process-global pair in the C library, so with two radios + * open the last constructor would own BOTH libraries' diagnostics and the + * first destructor would unhook the survivor's - leaving it writing to raw + * stderr for the rest of its life, which is the exact failure the sink + * exists to prevent. Route by a registry instead of by `this`: install + * once, and keep it installed until the last radio goes. */ + if (sink_registry().empty()) + mt7612u_set_log_sink(&Mt7612uRadio::log_trampoline, nullptr); + sink_registry().push_back(this); + } } Mt7612uRadio::~Mt7612uRadio() { + /* Deregister BEFORE Stop(): the library can still log from its event thread + * during teardown, and nothing may reach a destroyed `this`. */ + { + std::lock_guard lock(sink_mu()); + auto ® = sink_registry(); + for (auto it = reg.begin(); it != reg.end(); ++it) + if (*it == this) { + reg.erase(it); + break; + } + if (reg.empty()) + mt7612u_set_log_sink(nullptr, nullptr); + } Stop(); - /* Nothing may reach a destroyed `this` afterwards. */ - mt7612u_set_log_sink(nullptr, nullptr); +} + +std::mutex &Mt7612uRadio::sink_mu() { + static std::mutex m; + return m; +} + +std::vector &Mt7612uRadio::sink_registry() { + static std::vector reg; + return reg; } void Mt7612uRadio::log_trampoline(void *user, char level, const char *line) { - auto *self = static_cast(user); - if (!self || !self->_logger || !line) + (void)user; + if (!line) + return; + /* The C library has one sink for the process, so a line cannot be attributed + * to a particular adapter - it is routed to the first live radio's logger, + * which in the single-adapter case (every shipping consumer today) is the + * only one there is. Taking the registry lock here also means a destructor + * cannot deregister mid-call. */ + std::lock_guard lock(sink_mu()); + auto ® = sink_registry(); + if (reg.empty()) + return; + Mt7612uRadio *self = reg.front(); + if (!self->_logger) return; /* The library hands over the bare message; Logger re-adds "devourer [X] " * and applies the level gating and stream this consumer configured. */ @@ -124,9 +169,50 @@ void Mt7612uRadio::bring_up(SelectedChannel channel) { if (mt7612u_set_channel(_dev, channel.Channel, bw) != 0) throw std::runtime_error("MT7612U channel set failed"); _channel = channel; + apply_config(); start_tick(); } +/* Every DeviceConfig knob this backend can reach, and a loud line for each one + * it cannot. + * + * The rule is the sibling backend's: a config value that reads as applied while + * the radio runs something else is the one failure worse than an unported knob. + * Sited in bring_up so an RX-only session is told too, and so each fires once + * per bring-up rather than once per frame. */ +void Mt7612uRadio::apply_config() { + /* Opt-in only, never a default: it turns a passive monitor into an active + * SIFS-timed transmitter. The caller asked for a responder, not a monitor, so + * a refusal is fatal rather than swallowed - otherwise the operator gets a + * green init and a session that silently answers nothing, and debugs the RF + * link instead of the config. */ + if (_cfg.rx.ack_responder && !SetAckResponder(*_cfg.rx.ack_responder)) + throw std::runtime_error("MT7612U ACK responder could not be armed"); + + /* Not programmable here. DeviceConfig calls this "ONE default, 128 us, + * programmed identically on every generation at bring-up", and that sentence + * stops being true the moment this adapter is attached - so say so rather + * than let a range budget be assumed. It matters on this part specifically: + * docs/mt7612u.md attributes the 40x unicast-injection cliff to the ACK + * timeout. */ + if (_cfg.tx.ack_timeout_us != 128) + _logger->warn("MT7612U: tx.ack_timeout_us={} is not programmable by this " + "backend - the MAC keeps its own default, so the range " + "budget this knob implies does not apply here", + _cfg.tx.ack_timeout_us); + + if (_cfg.tuning.disable_cca) + _logger->warn("MT7612U: DEVOURER_DIS_CCA / tuning.disable_cca is not " + "implemented by this backend - carrier-sense stays ENABLED " + "for this session"); + + if (_cfg.tx.usb_agg_max > 0) + _logger->warn("MT7612U: tx.usb_agg_max={} is not consulted - send_packets " + "always chains frames into shared bulk-OUT URBs on this " + "part, and send_packet never does", + _cfg.tx.usb_agg_max); +} + /* --- the 1 Hz PHY tick --------------------------------------------------- * * One round of mt76's cal_work. Not optional and not cosmetic: without it a @@ -182,25 +268,57 @@ void Mt7612uRadio::tick_loop() { void Mt7612uRadio::Init(Action_ParsedRadioPacket packetProcessor, SelectedChannel channel) { + /* Stop() on the way out, as the sibling backends do: a throw from the channel + * set or the config would otherwise leave the device open and the tick thread + * running until the destructor happens to run, and a caller that catches and + * retries would get a half-open object. + * + * The cleanup MUST run with _mu released. Stop() joins the tick thread, and + * the tick takes _mu once it wakes - so calling Stop() from inside the locked + * scope deadlocks whenever the tick is already blocked on that lock. Hence + * the exception_ptr rather than a plain catch-and-rethrow. */ + std::exception_ptr failed; { std::lock_guard lock(_mu); - bring_up(channel); + try { + bring_up(channel); + } catch (...) { + failed = std::current_exception(); + } + } + if (failed) { + Stop(); + std::rethrow_exception(failed); } /* Deliberately outside the lock: StartRxLoop blocks until StopRxLoop. */ StartRxLoop(std::move(packetProcessor)); } void Mt7612uRadio::InitWrite(SelectedChannel channel) { - std::lock_guard lock(_mu); - bring_up(channel); - /* No RX ring, so mt7612u_start() enables TX only - which is the point of - * this entry point, and also what keeps the chip out of the undrained- - * receiver wedge. */ - if (mt7612u_start(_dev) != 0) - throw std::runtime_error("MT7612U MAC start failed"); + /* Same shape as Init, and for the same lock-ordering reason: the cleanup runs + * outside _mu because Stop() joins the tick thread. */ + std::exception_ptr failed; + { + std::lock_guard lock(_mu); + try { + bring_up(channel); + /* No RX ring, so mt7612u_start() enables TX only - which is the point of + * this entry point, and also what keeps the chip out of the undrained- + * receiver wedge. */ + if (mt7612u_start(_dev) != 0) + throw std::runtime_error("MT7612U MAC start failed"); + } catch (...) { + failed = std::current_exception(); + } + } + if (failed) { + Stop(); + std::rethrow_exception(failed); + } } void Mt7612uRadio::StartRxLoop(Action_ParsedRadioPacket packetProcessor) { + struct mt7612u_dev *mac_failed = nullptr; { std::lock_guard lock(_mu); if (!_dev) @@ -214,16 +332,27 @@ void Mt7612uRadio::StartRxLoop(Action_ParsedRadioPacket packetProcessor) { if (mt7612u_rx_start(_dev, &Mt7612uRadio::rx_trampoline, this) != 0) throw std::runtime_error("MT7612U RX ring failed to start"); if (mt7612u_start(_dev) != 0) { - mt7612u_rx_stop(_dev); - throw std::runtime_error("MT7612U MAC start failed"); + /* Same two rules as StopRxLoop, and for the same reasons: quiesce before + * removing the drain, and finish the teardown OUTSIDE _mu - the ring was + * armed one line above, so a frame can already be in a processor that + * takes this lock, and mt7612u_rx_stop() joins that thread. Recorded + * here and acted on below rather than unlocking by hand mid-scope. */ + mt7612u_rx_quiesce(_dev); + _rx_processor = nullptr; + mac_failed = _dev; + } else { + /* AFTER mt7612u_start(), which rewrites the filter to mt76's managed-mode + * value - see rule 2. Before it, this write is simply overwritten. */ + if (mt7612u_set_monitor_rx(_dev, _cfg.rx.keep_corrupted ? 1 : 0) != 0) + _logger->warn("MT7612U monitor RX filter not applied"); + /* Arms the channel timers and zeroes the MIB counters. */ + mt7612u_link_stats_start(_dev); + _rx_active.store(true, std::memory_order_release); } - /* AFTER mt7612u_start(), which rewrites the filter to mt76's managed-mode - * value - see rule 2. Before it, this write is simply overwritten. */ - if (mt7612u_set_monitor_rx(_dev, _cfg.rx.keep_corrupted ? 1 : 0) != 0) - _logger->warn("MT7612U monitor RX filter not applied"); - /* Arms the channel timers and zeroes the MIB counters. */ - mt7612u_link_stats_start(_dev); - _rx_active = true; + } + if (mac_failed) { + mt7612u_rx_stop(mac_failed); + throw std::runtime_error("MT7612U MAC start failed"); } _logger->info("MT7612U monitor RX on channel {}", _channel.Channel); @@ -244,22 +373,37 @@ void Mt7612uRadio::StartRxLoop(Action_ParsedRadioPacket packetProcessor) { void Mt7612uRadio::StopRxLoop() { _rx_stop = true; - if (!_rx_active.exchange(false)) + + /* Held across the WHOLE teardown, so a second caller blocks here instead of + * returning while the first is still inside mt7612u_rx_stop(). That early + * return was a use-after-free: the loser reported the ring down, went on to + * Stop(), and mt7612u_close() freed the device while the winner was still + * cancelling its transfers and joining the event thread. */ + std::lock_guard teardown(_teardown_mu); + if (!_rx_active.load(std::memory_order_acquire)) return; - /* Quiesce under the lock: it is a register write, and it must not interleave - * with the tick's MCU traffic. */ + struct mt7612u_dev *dev = nullptr; { + /* Quiesce under _mu: it is a register write, and it must not interleave + * with the tick's MCU traffic. The device pointer is read here too - _dev + * is written under _mu, so reading it outside would race Stop(). */ std::lock_guard lock(_mu); - if (_dev) - mt7612u_rx_quiesce(_dev); + dev = _dev; + if (dev) + mt7612u_rx_quiesce(dev); } - /* Ring teardown WITHOUT the lock. mt7612u_rx_stop() joins the event thread, - * and a processor still in flight on that thread may call back into this - * object - SetMonitorChannel, say - which takes _mu. Holding _mu across the - * join is therefore a deadlock, not a theoretical one. */ - if (_dev) - mt7612u_rx_stop(_dev); + /* Ring teardown WITHOUT _mu. mt7612u_rx_stop() joins the event thread, and a + * processor still in flight on that thread may call back into this object - + * SetMonitorChannel, say - which takes _mu. Holding _mu across the join is a + * deadlock, not a theoretical one. */ + if (dev) + mt7612u_rx_stop(dev); + + /* Cleared only now that the ring is genuinely down. Clearing it up front let + * a restart pass the "already active" guard while the old ring still + * existed, and mt7612u_rx_start() then returned success WITHOUT arming. */ + _rx_active.store(false, std::memory_order_release); _logger->info("MT7612U RX stopped after {} frames", _rx_frames.load(std::memory_order_relaxed)); @@ -296,14 +440,14 @@ void Mt7612uRadio::on_rx(const void *frame, size_t len, * rssi[2] is the noise floor and rssi[3] is unidentified. */ mt7612u::copy_signal(*info, packet.RxAtrib); - if (len >= 2) { + { const uint8_t *f = static_cast(frame); - packet.RxAtrib.qos = (f[0] & 0x0c) == 0x08 && (f[0] & 0x80) != 0; - /* The TID lives in the first QoS Control byte, which follows the 24-byte - * base header. Set alongside qos rather than left zero, so a consumer - * cannot read "QoS frame, TID 0" for every frame. */ - if (packet.RxAtrib.qos && len >= 26) - packet.RxAtrib.priority = f[24] & 0x0f; + uint8_t tid = 0; + /* qos_tid finds the QoS Control field at the RIGHT offset - 30, not 24, on + * a 4-address frame - so priority is a TID and never the low nibble of an + * address. */ + packet.RxAtrib.qos = mt7612u::qos_tid(f, len, tid); + packet.RxAtrib.priority = tid; } /* The span points into the ring buffer the libusb event thread owns and @@ -376,19 +520,31 @@ size_t Mt7612uRadio::send_packets(const TxPacketView *pkts, size_t count) { } void Mt7612uRadio::SetCcaMode(bool disabled) { + /* Asking for carrier-sense ENABLED is asking for the state this chip is + * already in, so it succeeds quietly - a caller that asserts the default + * unconditionally must not be punished for asking. Only the disable is + * refused. */ + if (!disabled) + return; /* Refuses rather than no-ops. MT7612U does have an ED-CCA enable * (MT_TXOP_CTRL_CFG / MT_TXOP_ED_CCA_EN, which mac_stop already clears), but * "disable CCA" on the Realtek backends means a specific, measured set of * writes, and nothing here has been measured against an on-air carrier-sense * test. Claiming it on the strength of one plausible-looking bit is how an * unverified regulatory-adjacent behaviour ships. */ - _logger->error("MT7612U: SetCcaMode({}) not implemented - the ED-CCA enable " - "exists but no on-air carrier-sense measurement backs it", - disabled); + _logger->error("MT7612U: SetCcaMode(true) not implemented - the ED-CCA " + "enable exists but no on-air carrier-sense measurement backs " + "it; carrier-sense stays ENABLED for this session"); } void Mt7612uRadio::Stop() { - StopRxLoop(); + /* Nothing here may escape: Stop() is called from the destructor, and a throw + * that skipped stop_tick() would leave _tick joinable, whose destructor calls + * std::terminate. The logging inside StopRxLoop is the realistic thrower. */ + try { + StopRxLoop(); + } catch (...) { + } stop_tick(); /* joins; must not run with _mu held */ std::lock_guard lock(_mu); if (_dev) { @@ -409,6 +565,80 @@ void Mt7612uRadio::SetTxPower(uint8_t power) { _txpwr_dbm); } +/* Real on this silicon, so implemented rather than advertised and ignored. + * There is no TXAGC index to trim, but mt7612u_set_txpower() is an absolute + * dBm limit that feeds the per-rate table, so an offset folds onto it. + * + * The library takes whole dBm, so the applied value is quantized to 4 qdB and + * that quantized figure is what comes back - the contract is "returns the + * APPLIED qdB ... so a closed-loop controller knows exactly what moved", and a + * controller told -20 when -20 was rounded to -20 but only -16 landed would + * integrate against a number the radio never used. Sticky across + * SetMonitorChannel because the base is re-applied from _txpwr_dbm. */ +int Mt7612uRadio::SetTxPowerOffsetQdb(int qdb) { + std::lock_guard lock(_mu); + const devourer::TxPowerCaps caps = GetTxPowerCaps(); + + int q = qdb; + if (q < caps.offset_min_qdb) + q = caps.offset_min_qdb; + if (q > caps.offset_max_qdb) + q = caps.offset_max_qdb; + /* Toward zero, so an offset never asks for more power than requested. */ + const int applied_db = q / 4; + const int applied_qdb = applied_db * 4; + + int dbm = _txpwr_dbm + applied_db; + if (dbm < 0) + dbm = 0; + if (dbm > 30) + dbm = 30; + if (_dev && mt7612u_set_txpower(_dev, dbm) != 0) { + _logger->error("MT7612U TX power offset {} qdB -> {} dBm refused", qdb, dbm); + return 0; + } + _txpwr_offset_qdb = applied_qdb; + return applied_qdb; +} + +/* The rate a frame airs at when its radiotap carries none. + * + * REFUSED, loudly, because this backend cannot honour it and a silent no-op + * here is a measurement that lies: examples/tx builds rate-less frames on + * purpose and calls SetTxMode to drive an MCS sweep, and the library's + * radiotap parser defaults an un-rated frame to OFDM MCS0 (radiotap.cpp) - so + * an MCS7 sweep would report MCS7 and air 6 Mbps. Confirmed on this bench: an + * RTL8812AU witnessing our transmit logged rate 4 (OFDM 6M) for every frame. + * + * Honouring it needs a session-default rate in the C library, which has none - + * mt7612u_send_packet() re-derives the rate from each frame's radiotap. Until + * then, put the rate in the radiotap header, where it always wins. */ +void Mt7612uRadio::SetTxMode(const devourer::TxMode &mode) { + (void)mode; + _logger->error("MT7612U: SetTxMode is not implemented - the C library has no " + "session-default rate, so a rate-less frame airs at OFDM " + "6 Mbps. Put the rate in each frame's radiotap header."); +} + +void Mt7612uRadio::ClearTxMode() { + /* Nothing was ever set, and "cleared" is the state this backend is always + * in - so this one is genuinely a no-op rather than a hidden refusal. */ +} + +bool Mt7612uRadio::SetAmpduMode(const devourer::AmpduMode &mode) { + (void)mode; + /* Aggregation on this part is per-frame descriptor state set on the TXWI at + * build time, not MAC state - so there is nothing to program here. It works: + * measured 326/326 frames aggregated and 2.21x throughput at 200 B + * (docs/mt7612u.md). Reaching it through IRadio needs the radiotap A-MPDU + * field plumbed into the library's frame builder, which is not done. Refused + * rather than accepting a mode that would never reach the air. */ + _logger->error("MT7612U: A-MPDU works on this part (docs/mt7612u.md, 2.21x at " + "200 B) but is not wired through send_packet yet - refusing " + "rather than accepting a mode that would not reach the air"); + return false; +} + void Mt7612uRadio::SetTxPowerIndexOverride(int idx) { _logger->error("MT7612U has no TXAGC index to override (asked for {}); TX " "power here is an absolute dBm limit - use SetTxPower()", @@ -455,6 +685,21 @@ devourer::TxStats Mt7612uRadio::GetTxStats() { * would be the same fault in a different field. */ out.submitted = _tx_submitted.load(std::memory_order_relaxed); out.failed = _tx_failed.load(std::memory_order_relaxed); + + /* A refusal is only half of `failed`. TxStats defines it as a synchronous + * submit error OR an async URB that completed with a non-OK status, and when + * an RX ring is up mt_tx_raw() routes through the async pool - which returns + * 0 the moment the URB is submitted and counts the wire failure later, in + * tx_done. Without this the primary devourer shape (Init plus concurrent + * send_packets) reports failed=0 even if every frame dies on the wire: the + * same "stat that reads zero while the radio transmits" fault as submitted, + * in the other half. */ + std::lock_guard lock(_mu); + if (_dev) { + struct mt7612u_stats st {}; + mt7612u_get_stats(_dev, &st); + out.failed += st.tx_err; + } return out; } @@ -489,7 +734,12 @@ devourer::TxPowerCaps Mt7612uRadio::GetTxPowerCaps() { * TXAGC index on this part: TX power is an absolute dBm limit feeding the * per-rate table, plus a 4-bit per-frame trim in the descriptor. */ c.index_max = 0; - c.step_qdb = 2; /* the limit is carried in 0.5 dB units */ + /* 4, not 2. The limit is carried internally in 0.5 dB units, but the only + * actuator reachable from here - mt7612u_set_txpower() - takes WHOLE dBm, so + * half-dB is not a step this backend can take. Advertising 2 would have + * TxPower.h's quantize_offset_qdb round requests to a granularity the + * hardware cannot honour. */ + c.step_qdb = 4; c.step_measured = false; c.offset_min_qdb = -80; /* down to 0 dBm from the 20 dBm default */ c.offset_max_qdb = 40; /* up to 30 dBm, the API's own ceiling */ @@ -532,10 +782,19 @@ devourer::AdapterCaps Mt7612uRadio::GetAdapterCaps() { have_hw ? hw.band_5g_max_mhz : uint16_t(5825)}; c.tune_2g4 = {true, have_hw ? hw.band_2g_min_mhz : uint16_t(2412), have_hw ? hw.band_2g_max_mhz : uint16_t(2484)}; - c.characterized_5g = c.tune_5g; - c.characterized_2g4 = c.tune_2g4; + /* Tunable is not characterized. The TX-power registers were verified equal to + * the vendor driver's at ch149 only, and docs/mt7612u.md says plainly that + * "register equality is not dBm" - nothing has been measured against a power + * meter across either band. Left invalid rather than claiming the whole + * tunable span is table-backed. */ + c.characterized_5g = {}; + c.characterized_2g4 = {}; c.ldpc_rx_ht = true; - c.ldpc_rx_vht = true; + /* HT and VHT are separate decoder paths in silicon, which is why AdapterCaps + * splits them - and docs/mt7612u.md lists VHT on air as unexercised. The one + * LDPC-RX measurement on this part is an HT frame, so the VHT half is + * unmeasured and says so. */ + c.ldpc_rx_vht = false; c.ldpc_rx_flag = true; /* the RXWI carries the per-frame LDPC bit */ c.per_chain_rssi = true; c.hw_rx_timestamp = false; /* the RXWI TSF field is not parsed */ @@ -549,12 +808,15 @@ devourer::AdapterCaps Mt7612uRadio::GetAdapterCaps() { /* Measured 526 ms full / 48 ms with calibration skipped, against 0.5-2.5 ms * on the Realtek parts: the RF plane lives behind the MCU. Not "fast". */ c.fastretune_ok = false; - c.per_packet_txpower = true; - c.per_pkt_txpwr_steps = 0; - c.per_pkt_txpwr_step_qdb = 4; /* MT_TX_PWR_ADJ is a 4-bit dB trim */ - c.per_pkt_txpwr_min_qdb = -32; - c.per_pkt_txpwr_max_qdb = 28; - c.per_pkt_txpwr_measured = false; + /* MT_TX_PWR_ADJ is a real 4-bit per-frame dB trim, and mt7612u_tx() takes it + * as mt7612u_tx_rate.power_adj - but nothing reaches it from here. The + * library's radiotap path PARSES DBM_TX_POWER and explicitly discards it + * (radiotap.cpp), the session route is gated on an enable_tpc field that no + * code ever assigns (internal.h), and this backend calls send_packet, not + * mt7612u_tx(). So the mechanism exists and has no caller; advertising it + * would hand a rate controller a knob with nothing behind it. false until + * one of those paths is wired, at which point the ranges below come back. */ + c.per_packet_txpower = false; c.vht_2g4_ok = false; /* unmeasured on this part */ return c; } diff --git a/src/mt7612u/Mt7612uRadio.h b/src/mt7612u/Mt7612uRadio.h index c1f8115e..cb09e701 100644 --- a/src/mt7612u/Mt7612uRadio.h +++ b/src/mt7612u/Mt7612uRadio.h @@ -6,6 +6,7 @@ #include #include #include +#include #include @@ -86,6 +87,10 @@ class Mt7612uRadio : public IRadio { devourer::TxPowerCaps GetTxPowerCaps() override; void SetTxPower(uint8_t power) override; void SetTxPowerIndexOverride(int idx) override; + int SetTxPowerOffsetQdb(int qdb) override; + void SetTxMode(const devourer::TxMode &mode) override; + void ClearTxMode() override; + bool SetAmpduMode(const devourer::AmpduMode &mode) override; bool GetPermanentMacAddress(uint8_t out[6]) override; uint64_t ReadTsf() override; void WriteTsf(uint64_t tsf) override; @@ -95,6 +100,7 @@ class Mt7612uRadio : public IRadio { private: void bring_up(SelectedChannel channel); /* _mu held */ + void apply_config(); /* _mu held */ void start_tick(); /* _mu held */ void stop_tick(); /* _mu NOT held */ void tick_loop(); @@ -103,6 +109,10 @@ class Mt7612uRadio : public IRadio { void on_rx(const void *frame, size_t len, const struct mt7612u_rx_info *info); static void log_trampoline(void *user, char level, const char *line); + /* The C library's diagnostic sink is process-global, so the routing has to + * be too. See the constructor for why this is a registry and not `this`. */ + static std::mutex &sink_mu(); + static std::vector &sink_registry(); libusb_device_handle *_handle; libusb_context *_ctx; @@ -117,6 +127,13 @@ class Mt7612uRadio : public IRadio { SelectedChannel _channel{}; Action_ParsedRadioPacket _rx_processor; + /* Serialises the whole RX teardown. StopRxLoop is documented to have torn + * the ring down and joined the event thread BEFORE it returns, so a second + * caller has to WAIT for the first rather than see a cleared flag and return + * early - the early return let Stop() close and free the device out from + * under a thread still inside mt7612u_rx_stop(). Never held while _mu is + * held. */ + std::mutex _teardown_mu; std::atomic _rx_stop{false}; std::atomic _rx_active{false}; std::atomic _rx_frames{0}; @@ -130,7 +147,8 @@ class Mt7612uRadio : public IRadio { std::condition_variable _tick_cv; bool _tick_stop = false; - int _txpwr_dbm = 20; /* the absolute dBm limit mt7612u_set_txpower takes */ + int _txpwr_dbm = 20; /* the absolute dBm limit mt7612u_set_txpower takes */ + int _txpwr_offset_qdb = 0; /* sticky, folded onto _txpwr_dbm */ }; #endif /* MT7612U_RADIO_H */ diff --git a/src/mt7612u/README.md b/src/mt7612u/README.md index 5ac76b5b..fca6e073 100644 --- a/src/mt7612u/README.md +++ b/src/mt7612u/README.md @@ -1,11 +1,12 @@ # src/mt7612u — MediaTek MT7612U -**Compiled by `CMakeLists.txt` under `DEVOURER_MT7612U` (default OFF); still no -`IRadio` backend.** This subtree is a complete, self-contained library for the -part — a public C ABI, its own transport, no dependency on `RtlAdapter` — plus -the bring-up harness that produced every measurement in `docs/mt7612u.md`. -`WiFiDriver::CreateRadio` recognises the MediaTek USB ids today only to refuse -them; wiring a radio in behind `IRadio` is the follow-up. +**Compiled by `CMakeLists.txt` under `DEVOURER_MT7612U` (default OFF), and +wired in behind `IRadio`.** This subtree is a complete, self-contained library +for the part — a public C ABI, its own transport, no dependency on `RtlAdapter` +— plus the bring-up harness that produced every measurement in +`docs/mt7612u.md`. `Mt7612uRadio` is the backend `WiFiDriver::CreateRadio` +constructs for a MediaTek adapter; `Mt7612uMapping.h` holds the pure +translations between this part's descriptor vocabulary and devourer's. The sources are C++ (`.cpp`), not C: MSVC has no `` and devourer builds Windows first-class, so the sync and timing primitives are `std::` types. @@ -40,6 +41,9 @@ Measurements, methods and limits: [`../../docs/mt7612u.md`](../../docs/mt7612u.m | `caps.cpp` | TSF, capability descriptor, ACK responder | | `tools/bringup.cpp` | one subcommand per verified gate | | `tests/` | offline tests (`make check`): public-API link (C), frame shapes, field macros, log sink | +| `Mt7612uRadio.{h,cpp}` | the `IRadio` backend: bring-up, RX/TX, the 1 Hz tick, caps | +| `Mt7612uMapping.h` | pure translations (RSSI bias, per-chain signal, rate codes, TID) — pinned by `tests/mt7612u_mapping_selftest.cpp` | +| `Mt7612uUsbIds.h` | the vid:pid gate `WiFiDriver::CreateRadio` consults | | `initvals.h` | **generated** — see Provenance | ## The receiver must never run undrained @@ -126,10 +130,12 @@ rtap send_packet / send_packets hop channel-switch cost ``` `make` here builds it as `./bringup`, which is what the hardware notes use. -The integration PR adds a CMake target for the same source, named -`mt7612uprobe` to sit beside `pcieprobe` / `kestrelprobe` / `rtl8733bprobe`, so -the chip-specific tool is not the one part of this backend that only a second -build system can produce. +CMake builds the same source as `mt7612uprobe` (with `DEVOURER_MT7612U=ON`), to +sit beside `pcieprobe` / `kestrelprobe` / `rtl8733bprobe` — so the chip-specific +tool is not the one part of this backend that only a second build system can +produce, and so it picks up the sanitizer and compiler settings the rest of the +tree is built with. It drives the C library directly rather than `Mt7612uRadio`: +its purpose is to exercise the layer underneath the backend. `sweep`, `coding` and `vht` take a width as their fourth argument, in the `MT7612U_BW_*` numbering — `0` = 20, `1` = 40, `2` = 80 MHz: diff --git a/src/mt7612u/internal.h b/src/mt7612u/internal.h index 21ca4f45..c2d2f6bc 100644 --- a/src/mt7612u/internal.h +++ b/src/mt7612u/internal.h @@ -178,8 +178,8 @@ struct mt7612u_dev { std::recursive_mutex io_lock; /* Observe-but-do-not-repair, for wedge experiments. A field, not a * getenv - the tool that wants the behaviour sets it before mt_open() - * (bringup does). Note this is not yet true of the library as a whole: - * open_selected() still reads MT7612U_DEV (see usb.cpp). */ + * (bringup does). True of the library as a whole now: the selector moved + * to a field too, so nothing here reads the environment. */ uint8_t no_autorecover; /* Which adapter to open, "-" as bringup spells it, or NULL for * "the first one". A field and not a getenv: this is a LIBRARY now diff --git a/src/mt7612u/usb.cpp b/src/mt7612u/usb.cpp index 8d6e7dbe..fdc88cd8 100644 --- a/src/mt7612u/usb.cpp +++ b/src/mt7612u/usb.cpp @@ -467,13 +467,8 @@ int mt_adopt(struct mt7612u_dev *d, libusb_device_handle *h, * Messages below therefore name "the selector", not that variable - a caller * that is not bringup would be told to set something it does not use. * - * This is the ONE environment read left in the library, and it stays deferred - * to integration as agreed in #412 rather than being removed here: there is no - * public way to pass a selector (mt7612u_open() allocates the device itself and - * the struct is opaque), so dropping it would leave a multi-adapter consumer - * unable to choose an adapter at all. The wrapper does not need it - it arrives - * through mt7612u_open_handle() having already selected the device itself. - * MT7612U_NO_AUTORECOVER, which had no such constraint, is now d->no_autorecover. + * mt7612u_open_selected() is the public way to pass it; mt7612u_open() is + * that with NULL. Nothing in this library reads the environment. */ /* * Exclusive per-adapter lock - the same lock devourer's own UsbDeviceLock diff --git a/tests/mt7612u_mapping_selftest.cpp b/tests/mt7612u_mapping_selftest.cpp index d438044f..a61ac97a 100644 --- a/tests/mt7612u_mapping_selftest.cpp +++ b/tests/mt7612u_mapping_selftest.cpp @@ -73,10 +73,30 @@ int main() { * chain C sitting near -92 dBm. */ expect("the noise floor is NOT published as chain C", a.rssi[2] == 0); expect("the unidentified slot is NOT published as chain D", a.rssi[3] == 0); - expect("snr filled on the real chains", a.snr[0] == 37 && a.snr[1] == 37); + /* HALF-dB, the unit LinkHealth and RxQuality divide by two. Asserting the + * raw value here is how an earlier cut of this test locked in a bug that + * reported every link at half its SNR. */ + expect("snr is half-dB, not whole dB", a.snr[0] == 74 && a.snr[1] == 74); + expect("snr round-trips to dB the way consumers read it", + a.snr[0] / 2 == 37); expect("snr not invented past the real chains", a.snr[2] == 0 && a.snr[3] == 0); } + { + /* int8_t holds +-127 in half-dB, i.e. +-63.5 dB. A report past that must + * clamp rather than wrap into a negative SNR. */ + struct mt7612u_rx_info i {}; + struct rx_pkt_attrib a {}; + + i.n_chains = 2; + i.noise_valid = 1; + i.snr_db = 90; + copy_signal(i, a); + expect("an out-of-range snr clamps positive", a.snr[0] == 127); + i.snr_db = -90; + copy_signal(i, a); + expect("an out-of-range negative snr clamps", a.snr[0] == -128); + } { /* Without a valid noise estimate there is no SNR to report. Zero, not a * plausible-looking number — the noise field reads a physically impossible @@ -134,6 +154,50 @@ int main() { expect("VHT nss 0 is treated as 1", desc_rate(rx(MT7612U_PHY_VHT, 3, 0)) == desc_rate(rx(MT7612U_PHY_VHT, 3, 1))); + /* MT_RATE_INDEX is six bits, so these are representable and mean nothing. + * Reporting 0 (unknown) is right; running off the end of the HT block into + * the VHT numbering would present garbage as a real VHT rate. */ + expect("HT MCS32 is unknown, not a VHT rate", + desc_rate(rx(MT7612U_PHY_HT, 32)) == 0); + expect("HT MCS63 is unknown, not a VHT rate", + desc_rate(rx(MT7612U_PHY_HT, 63)) == 0); + expect("VHT MCS10 does not spill into the next stream's block", + desc_rate(rx(MT7612U_PHY_VHT, 10, 1)) == 0); + expect("VHT MCS12 SS1 is not reported as SS2 MCS2", + desc_rate(rx(MT7612U_PHY_VHT, 12, 1)) != + desc_rate(rx(MT7612U_PHY_VHT, 2, 2))); + expect("VHT beyond 4 streams is unknown", + desc_rate(rx(MT7612U_PHY_VHT, 0, 5)) == 0); + + /* --- the QoS TID, at the right offset --- */ + { + uint8_t tid = 0xff; + /* 3-address QoS data: fc=0x0088, header 24, QoS Control at 24. */ + uint8_t three[32] = {0x88, 0x00}; + three[24] = 0x06; + expect("3-address QoS is recognised", qos_tid(three, sizeof three, tid)); + expect("3-address TID comes from byte 24", tid == 6); + + /* 4-address QoS data: ToDS|FromDS, header 30, QoS Control at 30. Byte 24 + * is Address 4 and must NOT be read as a TID. */ + tid = 0xff; + uint8_t four[36] = {0x88, 0x03}; + four[24] = 0x0b; /* a plausible-looking decoy inside Address 4 */ + four[30] = 0x02; + expect("4-address QoS is recognised", qos_tid(four, sizeof four, tid)); + expect("4-address TID comes from byte 30, not 24", tid == 2); + + tid = 0xff; + uint8_t nonqos[32] = {0x08, 0x00}; /* data, non-QoS subtype */ + expect("a non-QoS data frame has no TID", + !qos_tid(nonqos, sizeof nonqos, tid)); + uint8_t beacon[32] = {0x80, 0x00}; + expect("a beacon has no TID", !qos_tid(beacon, sizeof beacon, tid)); + /* Truncated: the QoS Control field is not present, so there is nothing to + * read and nothing may be read past the end. */ + expect("a truncated QoS frame has no TID", !qos_tid(three, 25, tid)); + expect("a truncated 4-address QoS frame has no TID", !qos_tid(four, 31, tid)); + } /* --- bandwidth code --- */ expect("BW_20 -> 0", bw_to_desc(MT7612U_BW_20) == 0); From bc13983c70493d7a5ab0fbafec1aa2b4a3f2ac75 Mon Sep 17 00:00:00 2001 From: snokvist Date: Thu, 10 Sep 2026 06:52:14 +0200 Subject: [PATCH 5/8] docs: two library races ThreadSanitizer found, and the honest scale of them Ran the backend under TSan against real hardware, driving rxdemo with DEVOURER_RX_SWEEP so the main thread retunes while the libusb event thread delivers frames - the interleaving no reviewer can check by reading. Plain RX is clean: 8751 frames, 0 reports. Under concurrent retune there are two real ones, both in the C library and neither in Mt7612uRadio: - mt_read_rx_gain() rewrites lna_gain and rssi_offset[] on every tune while mt_rx_parse() reads them on the event thread to correct each frame's RSSI. A frame parsed mid-retune gets a mixed correction - a wrong number, which is the class this port cares most about. - A synchronous control transfer from the tune reaches libusb_free_transfer(), destroying a transfer's mutex, while the ring's event thread locks it. Inside libusb, and the documented hazard of mixing the sync API with a dedicated event thread on one context. Attribution, because "TSan finds races" is worthless without it. The bring-up harness sets the channel BEFORE starting a ring and reports 0 over 10667 frames, so this is not the library alone - the backend is what makes retuning during RX reachable. But the same stress on the shipping RTL8812AU path produces EIGHT reports, all in devourer's own Jaguar1 state (RtlJaguarDevice.cpp:1412/1581, RtlAdapter.h:98). Retune-during-RX is not race-free anywhere in this project today. TSan reported nothing inside Mt7612uRadio itself, which is the part this series is responsible for; the _mu discipline holds. The two above are library-level and want their own fix - the gain state under a lock the RX path can take cheaply, and async register access or libusb's event-lock protocol for the second - so they are recorded rather than patched here. Co-Authored-By: Claude Opus 5 (1M context) Claude-Session: https://claude.ai/code/session_01Tba83kymS5W2v1vn2yRxrj --- docs/mt7612u.md | 34 ++++++++++++++++++++++++++-------- 1 file changed, 26 insertions(+), 8 deletions(-) diff --git a/docs/mt7612u.md b/docs/mt7612u.md index 96af4519..662b65da 100644 --- a/docs/mt7612u.md +++ b/docs/mt7612u.md @@ -368,14 +368,32 @@ Stated because the numbers above are uniformly favourable. Ordered, and honest about which are unknowns rather than typing: -1. `IRadio` implementation, `WiFiDriver` dispatch, `DeviceConfig` plumbing, - `CMakeLists.txt`, `ctest` cells. None of this exists. - Part of that adapter, called out because nothing will fail loudly if it is - forgotten: it MUST set `attrib.fcs_present = false` on every frame it - delivers. The field defaults to `true` - correct for every Realtek parser, - wrong for this part - so omitting the assignment silently feeds four bytes - of real payload to consumers that trim an FCS. There is no compile error, - no assert and no test that catches it. +1. **Two data races the library has when a channel change runs while the RX + ring is up.** Found with ThreadSanitizer against real hardware, driving + `rxdemo` with `DEVOURER_RX_SWEEP` so the main thread retunes while the + libusb event thread delivers frames. Neither is in `Mt7612uRadio` — that + class's `_mu` discipline held, and TSan reported nothing inside it — and + neither is reachable from the bring-up harness, which sets the channel + before starting a ring and measured 0 warnings over 10667 frames. + + - `mt_read_rx_gain()` (`eeprom.cpp`) rewrites the per-channel `lna_gain` and + `rssi_offset[]` from the EEPROM on every tune, while `mt_rx_parse()` + (`rx.cpp`) reads them on the event thread to correct each frame's RSSI. A + frame parsed mid-retune therefore gets a mixed correction, i.e. a wrong + RSSI for that frame. Wrong number, not a crash — which is the class this + port cares most about. + - A synchronous control transfer issued during the tune (`mt_rr_chk` -> + `libusb_control_transfer`) reaches `libusb_free_transfer()`, destroying a + transfer's mutex, while the async ring's event thread locks it. That is + inside libusb, and it is the documented hazard of mixing the synchronous + API with a dedicated event thread on one context. + + For scale: the same stress on the shipping RTL8812AU path produces **8** + TSan reports, all in devourer's own Jaguar1 state (`RtlJaguarDevice.cpp`, + `RtlAdapter.h`). Retune-during-RX is not a race-free operation anywhere in + this project today, so this is a shared gap rather than a MediaTek + regression — but the two above are specific and fixable, and the second one + is a use-after-destroy rather than a torn read. 2. `mt76x2_phy_tssi_compensate()` — periodic temperature correction. Without it output power drifts with die temperature. 3. Cold-boot verification on a host with switchable USB power. From 3c9b962ca4df0cdd1561f553db546cb81333a396 Mon Sep 17 00:00:00 2001 From: snokvist Date: Thu, 10 Sep 2026 07:06:35 +0200 Subject: [PATCH 6/8] review 2: stop delivering RX on the event thread, and four more wrong numbers Two more reviewers, and the fix commit they were checking had itself reintroduced the defect it fixed. Theirs, in the order that matters. RX WAS DELIVERED ON THE C LIBRARY'S EVENT THREAD, WHICH CAN WEDGE THE CHIP. That thread is the SOLE servicer of both RX and TX completions. A packet processor that transmits parks it in mt_async_tx_submit waiting for a TX slot only that same thread can free - and examples/chanmig already does exactly this, calling send_packet and SetMonitorChannel from its RX callback. MAC RX stays enabled, EP4 stops being drained, and this part then wedges below the USB level where only a physical replug recovers it. Teardown cannot rescue it either: mt_async_stop's 2 s wait expires with no completions, strands the ring, then joins the deadlocked thread forever. I had documented the divergence and dismissed it - "the guarantee that matters is preserved" - having picked the wrong guarantee. The one that matters is the one every Realtek backend keeps: the processor runs on the thread that called StartRxLoop. So it does now. on_rx copies into a bounded queue and the loop that used to sleep 20 ms does the delivering. That also takes _mu off the event thread entirely, so a 526 ms channel change can no longer stall the drain, and it makes the library's "must not block, must not call back" rule an internal invariant rather than one silently exported to every consumer. Overflow drops the newest frame and COUNTS it: 0 dropped un-instrumented, 32 under TSan where the processor is slowed, and the teardown line says so rather than hiding it. THE PREVIOUS FIX REINTRODUCED ITS OWN USE-AFTER-FREE. Moving mt7612u_rx_stop() out of the _mu scope in StartRxLoop's failure arm without extending _teardown_mu let a concurrent Stop() free the device in the gap. The whole prologue is now under _teardown_mu, same order as StopRxLoop. Stop() likewise no longer holds _mu across mt7612u_close(), which joins the event thread. FOUR MORE WRONG NUMBERS: - snr[1] was chain A's SNR. info.snr_db is rssi[0] - noise, so writing it to every slot reported two identical per-chain SNRs - which is precisely how a dead chain-B antenna hides, its RSSI dropping while its SNR appears to track chain A. Now (rssi[i] - noise) * 2. My test asserted the wrong value and pinned it, in the function whose header spends 25 lines on not publishing a phantom chain. - GetTxStats().failed added the ring's tx_err, which double-counts submit failures (mt_async_tx_submit raises tx_err AND returns -1), mixes URB granularity with frame granularity, and goes BACKWARDS across an RX restart because mt_async_stop deletes the ring. Reading it also took _mu, which send_packet holds across a blocking submit. Removed; the gap is documented where the counter is, and the fix belongs in the library. - SetTxPowerOffsetQdb returned the requested offset even when the 0-30 dBm rail clamped it - a 10 dB lie to a closed-loop controller, against the contract its own comment quotes. Applied is now derived after the clamp. It also truncated toward zero, which on a negative offset applies LESS attenuation than asked for, and made any sub-step request return 0 - the refused sentinel. Uses the family's quantize_offset_qdb now. - _txpwr_offset_qdb was write-only and SetTxPower discarded a live offset. Both now go through txpower_target_dbm(), and bring_up replays it. AND A CI BREAK: mt7612uprobe is in `all`, the matrix passes DEVOURER_MT7612U=ON on the MSVC cell, and bringup.cpp has , and getrusage() with zero _WIN32 guards. Gated on UNIX; the workflow comment that claimed the whole subtree was MSVC-clean now says which part is not. The subtree's own Makefile also globbed the new backend into a build that has no devourer headers - filtered out. Library, one line: mt_async_stop cleared `running` but never notified, so the guard whose comment says "a teardown must not leave a caller parked here forever" only fired when the cancel pass happened to produce a completion. Verified: 60/60 in three configs, make check green, and on hardware 7661 frames RX plus 984 TX submitted with 0 failed. TSan under concurrent retune is down from 4 reports to 3, the remainder being the demo's own counter, the signal handler, and the library gain race already in docs/mt7612u.md's Open list. Co-Authored-By: Claude Opus 5 (1M context) Claude-Session: https://claude.ai/code/session_01Tba83kymS5W2v1vn2yRxrj --- .github/workflows/cmake-multi-platform.yml | 6 +- CMakeLists.txt | 10 +- src/mt7612u/Makefile | 7 +- src/mt7612u/Mt7612uMapping.h | 24 +- src/mt7612u/Mt7612uRadio.cpp | 277 +++++++++++++++------ src/mt7612u/Mt7612uRadio.h | 32 +++ src/mt7612u/async.cpp | 6 + tests/mt7612u_mapping_selftest.cpp | 17 +- 8 files changed, 294 insertions(+), 85 deletions(-) diff --git a/.github/workflows/cmake-multi-platform.yml b/.github/workflows/cmake-multi-platform.yml index df9e162e..12cb8eca 100644 --- a/.github/workflows/cmake-multi-platform.yml +++ b/.github/workflows/cmake-multi-platform.yml @@ -88,8 +88,10 @@ jobs: # DEVOURER_MT7612U=ON: the option defaults OFF, so without it here the # MediaTek subtree would be compiled by nothing but its own Makefile on # Linux. Turning it on in the matrix is what makes MSVC and macOS - # first-class for it — the POSIX-only pieces it still has (the flock - # adapter lock) are _WIN32-guarded, and this is what proves it. + # first-class for the LIBRARY — the POSIX-only pieces it still has (the + # flock adapter lock) are _WIN32-guarded, and this is what proves it. + # The mt7612uprobe bench tool is NOT portable and is gated on UNIX in + # CMakeLists.txt; it is deliberately outside that claim. run: > cmake -B ${{ steps.strings.outputs.build-output-dir }} -DCMAKE_CXX_COMPILER=${{ matrix.cpp_compiler }} diff --git a/CMakeLists.txt b/CMakeLists.txt index 9d3420cf..a1786911 100644 --- a/CMakeLists.txt +++ b/CMakeLists.txt @@ -639,11 +639,17 @@ endif() # It talks to the C library directly, NOT through Mt7612uRadio: it predates the # backend and its whole purpose is to exercise the layer underneath one. It # never substitutes for the production IRadio path. -if(DEVOURER_MT7612U) +# UNIX-only, unlike its kestrelprobe/rtl8733bprobe siblings. Those are portable +# C++; this one is the hardware bench harness and uses , +# and getrusage() unguarded, so on the MSVC cell of the +# multi-platform matrix - which now passes DEVOURER_MT7612U=ON - an unguarded +# add_executable would put it in `all` and break the build. The LIBRARY is +# MSVC-clean; this tool is not, and does not need to be. +if(DEVOURER_MT7612U AND UNIX) add_executable(mt7612uprobe src/mt7612u/tools/bringup.cpp ) - target_link_libraries(mt7612uprobe PUBLIC devourer PRIVATE PkgConfig::libusb) + target_link_libraries(mt7612uprobe PRIVATE devourer PkgConfig::libusb) target_include_directories(mt7612uprobe PRIVATE ${CMAKE_CURRENT_SOURCE_DIR}/src/mt7612u) endif() diff --git a/src/mt7612u/Makefile b/src/mt7612u/Makefile index 5c790493..2e6f76b3 100644 --- a/src/mt7612u/Makefile +++ b/src/mt7612u/Makefile @@ -22,7 +22,12 @@ CFLAGS ?= -O2 -g -Wall -Wextra DEPFLAGS = -MMD -MP INCS = -I. -Iinclude LDLIBS = -lusb-1.0 -lpthread -LIBSRCS = $(wildcard *.cpp) +# Everything here EXCEPT the devourer-side backend. Mt7612uRadio.cpp implements +# devourer's IRadio and includes its headers (IRadio.h, logger.h, DeviceConfig.h), +# which this standalone build has no include path for and should not - the point +# of this Makefile is that the library builds with nothing but libusb. CMake +# compiles that file; this does not. +LIBSRCS = $(filter-out Mt7612uRadio.cpp,$(wildcard *.cpp)) TESTS = tests/api_link tests/frame_shape tests/field_macros tests/log_sink CXXSRCS = $(LIBSRCS) tools/bringup.cpp tests/frame_shape.cpp tests/field_macros.cpp \ tests/log_sink.cpp diff --git a/src/mt7612u/Mt7612uMapping.h b/src/mt7612u/Mt7612uMapping.h index b7df72c2..ce6e8ee9 100644 --- a/src/mt7612u/Mt7612uMapping.h +++ b/src/mt7612u/Mt7612uMapping.h @@ -70,13 +70,21 @@ inline void copy_signal(const struct mt7612u_rx_info &info, for (unsigned i = 0; i < 4u; ++i) out.snr[i] = 0; if (info.noise_valid) { - int half_db = static_cast(info.snr_db) * 2; - if (half_db > 127) - half_db = 127; - if (half_db < -128) - half_db = -128; - for (unsigned i = 0; i < chains; ++i) + /* PER CHAIN, from that chain's own RSSI. info.snr_db is defined as + * rssi[0] - noise (rx.cpp), so writing it into every slot would report + * chain A's SNR on chain B - two identical numbers, which is exactly what + * hides a dead chain-B antenna: its RSSI drops while its SNR still tracks + * chain A's. The noise floor is common to both chains, so the per-chain + * value is simply rssi[i] - noise. */ + for (unsigned i = 0; i < chains; ++i) { + int half_db = (static_cast(info.rssi[i]) - + static_cast(info.noise)) * 2; + if (half_db > 127) + half_db = 127; + if (half_db < -128) + half_db = -128; out.snr[i] = static_cast(half_db); + } } } @@ -120,6 +128,10 @@ inline uint16_t desc_rate(const struct mt7612u_rx_info &info) { * of 32..63 would run past DESC_RATEMCS31 into the VHT numbering and * report garbage as a real VHT rate rather than as unknown. rx.cpp filters * the PHY field, not the index. */ + /* 0 is DESC_RATE1M, not an "unknown" sentinel - the enum has none - so a + * consumer sees a plausible 1 Mbps CCK frame rather than a rejected one. + * Still better than the alternative, which was reporting garbage as a real + * VHT rate; rx.cpp already drops frames whose PHY field names nothing. */ if (info.mcs > 31) return 0; return static_cast(DESC_RATEMCS0 + info.mcs); diff --git a/src/mt7612u/Mt7612uRadio.cpp b/src/mt7612u/Mt7612uRadio.cpp index 9076113e..6a94dbc0 100644 --- a/src/mt7612u/Mt7612uRadio.cpp +++ b/src/mt7612u/Mt7612uRadio.cpp @@ -10,6 +10,7 @@ #include #include +#include "TxPower.h" #include "mt7612u/Mt7612uMapping.h" extern volatile bool g_devourer_should_stop; @@ -85,8 +86,22 @@ Mt7612uRadio::Mt7612uRadio(libusb_device_handle *handle, libusb_context *ctx, } Mt7612uRadio::~Mt7612uRadio() { - /* Deregister BEFORE Stop(): the library can still log from its event thread - * during teardown, and nothing may reach a destroyed `this`. */ + /* Everything, not just StopRxLoop: stop_tick()'s join throws on EDEADLK or + * EINVAL, and a destructor is implicitly noexcept, so an escape here is + * std::terminate rather than a caught error. */ + try { + Stop(); + } catch (...) { + } + + /* Deregister AFTER Stop(), not before. Teardown is when the library has the + * most to say - "async stop: N TX and M RX transfers still in flight after + * 2 s, leaking the ring and the USB handle with it" is the single most + * important line it can print - and deregistering first sent all of it to + * raw stderr, bypassing the log level, a redirected stream and + * __android_log_write, which is the whole reason the sink exists. Safe in + * this order because log_trampoline holds sink_mu() for its entire call and + * _logger outlives the body of this destructor. */ { std::lock_guard lock(sink_mu()); auto ® = sink_registry(); @@ -98,17 +113,19 @@ Mt7612uRadio::~Mt7612uRadio() { if (reg.empty()) mt7612u_set_log_sink(nullptr, nullptr); } - Stop(); } +/* Deliberately leaked. A Mt7612uRadio destroyed during static destruction would + * otherwise touch a destroyed mutex and vector; leaking two small objects at + * exit is the cheaper failure. */ std::mutex &Mt7612uRadio::sink_mu() { - static std::mutex m; - return m; + static std::mutex *m = new std::mutex(); + return *m; } std::vector &Mt7612uRadio::sink_registry() { - static std::vector reg; - return reg; + static std::vector *reg = new std::vector(); + return *reg; } void Mt7612uRadio::log_trampoline(void *user, char level, const char *line) { @@ -162,8 +179,9 @@ void Mt7612uRadio::bring_up(SelectedChannel channel) { throw std::runtime_error(std::string("MT7612U bring-up failed: ") + (err ? err : "unknown")); _logger->info("MT7612U up: ASIC 0x{:08x}", mt7612u_asic_version(_dev)); - if (_txpwr_dbm != 20) - mt7612u_set_txpower(_dev, _txpwr_dbm); + /* Replays base AND offset, so neither is lost across a Stop()/re-Init(). */ + if (_txpwr_dbm != 20 || _txpwr_offset_qdb != 0) + mt7612u_set_txpower(_dev, txpower_target_dbm()); } if (mt7612u_set_channel(_dev, channel.Channel, bw) != 0) @@ -290,8 +308,16 @@ void Mt7612uRadio::Init(Action_ParsedRadioPacket packetProcessor, Stop(); std::rethrow_exception(failed); } - /* Deliberately outside the lock: StartRxLoop blocks until StopRxLoop. */ - StartRxLoop(std::move(packetProcessor)); + /* Deliberately outside the lock: StartRxLoop blocks until StopRxLoop. Its + * four throws get the same cleanup as bring_up's - without this, "RX ring + * failed to start" left the device open and the tick running, which is the + * half-open object this whole guard exists to prevent. */ + try { + StartRxLoop(std::move(packetProcessor)); + } catch (...) { + Stop(); + throw; + } } void Mt7612uRadio::InitWrite(SelectedChannel channel) { @@ -320,6 +346,17 @@ void Mt7612uRadio::InitWrite(SelectedChannel channel) { void Mt7612uRadio::StartRxLoop(Action_ParsedRadioPacket packetProcessor) { struct mt7612u_dev *mac_failed = nullptr; { + /* The WHOLE prologue, arming through the failure teardown, under the same + * lock StopRxLoop uses - and released before the sleep loop below, which + * calls StopRxLoop and would otherwise self-deadlock on it. + * + * Without this, moving mt7612u_rx_stop() out of the _mu scope reopened the + * use-after-free _teardown_mu exists to close: between releasing _mu and + * the teardown, a concurrent Stop() sees _rx_active still false, takes _mu + * and calls mt7612u_close(), which frees the device this thread is about + * to hand to mt7612u_rx_stop(). Ordering is _teardown_mu -> _mu here and in + * StopRxLoop; nothing takes them the other way round. */ + std::lock_guard teardown(_teardown_mu); std::lock_guard lock(_mu); if (!_dev) throw std::runtime_error("MT7612U RX loop requires initialized hardware"); @@ -327,6 +364,12 @@ void Mt7612uRadio::StartRxLoop(Action_ParsedRadioPacket packetProcessor) { throw std::runtime_error("MT7612U RX loop is already active"); _rx_processor = std::move(packetProcessor); _rx_stop = false; + /* 64 slots at the part's 3836-byte max MPDU is ~245 KB, about 45 ms of + * headroom at the measured 1400 fps - enough to ride out a slow processor + * without letting the producer block. Allocated here, not per frame. */ + _rx_q.assign(64, RxSlot{}); + _rx_q_head = _rx_q_tail = 0; + _rx_queue_dropped = 0; /* Ring first, receiver second - see rule 1 in the header. */ if (mt7612u_rx_start(_dev, &Mt7612uRadio::rx_trampoline, this) != 0) @@ -338,7 +381,6 @@ void Mt7612uRadio::StartRxLoop(Action_ParsedRadioPacket packetProcessor) { * takes this lock, and mt7612u_rx_stop() joins that thread. Recorded * here and acted on below rather than unlocking by hand mid-scope. */ mt7612u_rx_quiesce(_dev); - _rx_processor = nullptr; mac_failed = _dev; } else { /* AFTER mt7612u_start(), which rewrites the filter to mt76's managed-mode @@ -349,30 +391,60 @@ void Mt7612uRadio::StartRxLoop(Action_ParsedRadioPacket packetProcessor) { mt7612u_link_stats_start(_dev); _rx_active.store(true, std::memory_order_release); } + if (mac_failed) { + mt7612u_rx_stop(mac_failed); + /* Only now that the ring is down: clearing it while transfers are armed + * would destroy a std::function the event thread can be inside. */ + _rx_processor = nullptr; + } } - if (mac_failed) { - mt7612u_rx_stop(mac_failed); + if (mac_failed) throw std::runtime_error("MT7612U MAC start failed"); - } _logger->info("MT7612U monitor RX on channel {}", _channel.Channel); - /* The C layer drives RX from its own libusb event thread, so this loop has - * nothing to poll - it exists to give StartRxLoop the blocking contract - * every other backend has, and to notice Stop() and SIGINT. - * - * Delivery therefore happens on the event thread rather than on this one, - * which differs from the Realtek backends. The guarantee that matters is - * preserved: StopRxLoop tears the ring down and joins that thread before - * returning, so no callback can arrive after StartRxLoop returns. */ - while (!_rx_stop.load() && !g_devourer_should_stop) - std::this_thread::sleep_for(std::chrono::milliseconds(20)); + /* THE consumer. The C layer parses on its own event thread and enqueues; the + * processor runs here, on the thread that called StartRxLoop, which is the + * contract every other backend keeps. See the queue's comment in the header + * for why delivering on the event thread wedges the hardware. */ + for (;;) { + RxSlot *slot = nullptr; + { + std::unique_lock lock(_rx_q_mu); + _rx_q_cv.wait_for(lock, std::chrono::milliseconds(20), [this] { + return _rx_q_head != _rx_q_tail || _rx_stop.load(); + }); + if (_rx_q_head != _rx_q_tail) + slot = &_rx_q[_rx_q_tail]; + } + if (!slot) { + if (_rx_stop.load() || g_devourer_should_stop) + break; + continue; + } + + /* Outside the queue lock: user code runs here, and it may call back into + * this object. The producer never writes the slot at _rx_q_tail, so this + * reference stays valid until the tail is advanced below. */ + Packet packet{}; + packet.RxAtrib = slot->attrib; + packet.Data = std::span(slot->data.data(), slot->data.size()); + if (_rx_processor) + _rx_processor(packet); + + { + std::lock_guard lock(_rx_q_mu); + _rx_q_tail = (_rx_q_tail + 1) % _rx_q.size(); + } + } StopRxLoop(); } void Mt7612uRadio::StopRxLoop() { _rx_stop = true; + /* Wake the consumer immediately rather than leaving it to time out. */ + _rx_q_cv.notify_all(); /* Held across the WHOLE teardown, so a second caller blocks here instead of * returning while the first is still inside mt7612u_rx_stop(). That early @@ -405,8 +477,14 @@ void Mt7612uRadio::StopRxLoop() { * existed, and mt7612u_rx_start() then returned success WITHOUT arming. */ _rx_active.store(false, std::memory_order_release); - _logger->info("MT7612U RX stopped after {} frames", - _rx_frames.load(std::memory_order_relaxed)); + const uint64_t dropped = _rx_queue_dropped.load(std::memory_order_relaxed); + if (dropped) + _logger->warn("MT7612U RX stopped after {} frames, {} DROPPED at the " + "hand-off queue - the packet processor could not keep up", + _rx_frames.load(std::memory_order_relaxed), dropped); + else + _logger->info("MT7612U RX stopped after {} frames", + _rx_frames.load(std::memory_order_relaxed)); } void Mt7612uRadio::rx_trampoline(void *user, const void *frame, size_t len, @@ -416,9 +494,6 @@ void Mt7612uRadio::rx_trampoline(void *user, const void *frame, size_t len, void Mt7612uRadio::on_rx(const void *frame, size_t len, const struct mt7612u_rx_info *info) { - if (!_rx_processor) - return; - Packet packet{}; packet.RxAtrib.pkt_len = static_cast(len); packet.RxAtrib.crc_err = info->crc_err != 0; @@ -450,15 +525,29 @@ void Mt7612uRadio::on_rx(const void *frame, size_t len, packet.RxAtrib.priority = tid; } - /* The span points into the ring buffer the libusb event thread owns and - * reuses the moment this returns, so the processor must not retain it - the - * same contract every other backend's parser has. const_cast because - * Packet::Data is a mutable span and the buffer genuinely is ours. */ - packet.Data = std::span( - const_cast(static_cast(frame)), len); - _rx_frames.fetch_add(1, std::memory_order_relaxed); - _rx_processor(packet); + + /* Copy and hand off. The library's buffer is reused the moment this returns, + * and the consumer now runs on another thread, so the frame cannot be passed + * by reference the way it could when the processor ran here. */ + { + std::lock_guard lock(_rx_q_mu); + const size_t next = (_rx_q_head + 1) % _rx_q.size(); + if (next == _rx_q_tail) { + /* Full: the consumer is slower than the air. Drop the NEWEST rather than + * block - blocking here is precisely the wedge this queue exists to + * prevent, and dropping the oldest would reorder frames. Counted so the + * loss is visible instead of silent. */ + _rx_queue_dropped.fetch_add(1, std::memory_order_relaxed); + return; + } + RxSlot &slot = _rx_q[_rx_q_head]; + slot.attrib = packet.RxAtrib; + slot.data.assign(static_cast(frame), + static_cast(frame) + len); + _rx_q_head = next; + } + _rx_q_cv.notify_one(); } void Mt7612uRadio::SetMonitorChannel(SelectedChannel channel) { @@ -546,12 +635,23 @@ void Mt7612uRadio::Stop() { } catch (...) { } stop_tick(); /* joins; must not run with _mu held */ - std::lock_guard lock(_mu); - if (_dev) { - mt7612u_stop(_dev); - mt7612u_close(_dev); + + /* Take the device out under _mu, then close it OUTSIDE - mt7612u_close() + * runs mt_async_stop(), which joins the libusb event thread, and holding _mu + * across that join is the same deadlock StopRxLoop documents avoiding. The + * teardown lock is what keeps a concurrent StopRxLoop from using the pointer + * after this steals it. */ + struct mt7612u_dev *dev = nullptr; + { + std::lock_guard teardown(_teardown_mu); + std::lock_guard lock(_mu); + dev = _dev; _dev = nullptr; + if (dev) + mt7612u_stop(dev); } + if (dev) + mt7612u_close(dev); } void Mt7612uRadio::SetTxPower(uint8_t power) { @@ -560,7 +660,10 @@ void Mt7612uRadio::SetTxPower(uint8_t power) { * class does: there is no TXAGC index here, so the argument is read as the * dBm limit it actually maps to. */ _txpwr_dbm = static_cast(power); - if (_dev && mt7612u_set_txpower(_dev, _txpwr_dbm) != 0) + /* Composes with a live offset rather than discarding it - IRadio says the two + * compose, and writing the bare base would silently undo an offset while + * _txpwr_offset_qdb still reported it as applied. */ + if (_dev && mt7612u_set_txpower(_dev, txpower_target_dbm()) != 0) _logger->error("MT7612U TX power {} dBm refused (valid range 0-30)", _txpwr_dbm); } @@ -577,26 +680,35 @@ void Mt7612uRadio::SetTxPower(uint8_t power) { * SetMonitorChannel because the base is re-applied from _txpwr_dbm. */ int Mt7612uRadio::SetTxPowerOffsetQdb(int qdb) { std::lock_guard lock(_mu); + /* The family's own quantizer, not a local q/4. It rounds to nearest with + * ties away from zero, which is the documented rule every other backend + * follows; truncating toward zero instead turned a -6 qdB request into -4 + * (LESS attenuation than asked for) and any sub-step request into 0 - which + * is also the "refused" return, so a small request was indistinguishable + * from a refusal. That was the exact fault this override exists to remove. */ const devourer::TxPowerCaps caps = GetTxPowerCaps(); - - int q = qdb; - if (q < caps.offset_min_qdb) - q = caps.offset_min_qdb; - if (q > caps.offset_max_qdb) - q = caps.offset_max_qdb; - /* Toward zero, so an offset never asks for more power than requested. */ - const int applied_db = q / 4; - const int applied_qdb = applied_db * 4; - - int dbm = _txpwr_dbm + applied_db; + const int want_qdb = devourer::quantize_offset_qdb(qdb, caps, nullptr); + + /* The applied value is derived AFTER the rail clamp, never before. The + * actuator is an absolute 0-30 dBm limit, so an offset that would drive it + * past a rail is only partly applied - and returning the requested figure + * there tells a closed-loop controller the radio moved further than it did. + * With base 5 dBm, a -20 dB request lands at 0 dBm, i.e. -5 dB, and that is + * what comes back. */ + int dbm = _txpwr_dbm + want_qdb / 4; if (dbm < 0) dbm = 0; if (dbm > 30) dbm = 30; + const int applied_qdb = (dbm - _txpwr_dbm) * 4; + if (_dev && mt7612u_set_txpower(_dev, dbm) != 0) { _logger->error("MT7612U TX power offset {} qdB -> {} dBm refused", qdb, dbm); return 0; } + /* Sticky, and it has to be recorded even with no device open: SetTxPower and + * bring_up both fold it back in, so an offset set before Init survives to the + * first tune instead of being silently swallowed. */ _txpwr_offset_qdb = applied_qdb; return applied_qdb; } @@ -671,7 +783,21 @@ void Mt7612uRadio::WriteTsf(uint64_t tsf) { devourer::TxStats Mt7612uRadio::GetTxStats() { devourer::TxStats out{}; - /* Counted here rather than read from mt7612u_get_stats(), which reports the + /* INCOMPLETE, deliberately, and the incompleteness is documented rather than + * papered over: `failed` counts frames the transport REFUSED, not frames + * that died on the wire. An earlier cut added the ring's tx_err to close + * that, and it was wrong three ways - mt_async_tx_submit increments tx_err + * AND returns -1, so a submit failure counted twice; tx_err counts URBs + * while this counts frames, and an aggregated URB carries up to 32; and + * mt_async_stop deletes the ring, so the counter restarts at zero and + * `failed` went BACKWARDS across an RX restart, which a consumer differencing + * a uint64_t reads as ~1.8e19. Reading it also took _mu, which send_packet + * holds across a blocking submit - so a stats poll from another thread + * stalled for as long as the TX ring was saturated. A documented gap beats + * four wrong numbers; the fix belongs in the library, which needs a + * monotonic wire-failure counter that outlives a ring. + * + * Counted here rather than read from mt7612u_get_stats(), which reports the * ASYNC RING's counters. mt_tx_raw() only uses that ring when one is running * (tx.cpp), and the TX-only bring-up this backend offers - InitWrite with no * StartRxLoop - starts no ring, so those counters read 0 while frames are @@ -685,21 +811,6 @@ devourer::TxStats Mt7612uRadio::GetTxStats() { * would be the same fault in a different field. */ out.submitted = _tx_submitted.load(std::memory_order_relaxed); out.failed = _tx_failed.load(std::memory_order_relaxed); - - /* A refusal is only half of `failed`. TxStats defines it as a synchronous - * submit error OR an async URB that completed with a non-OK status, and when - * an RX ring is up mt_tx_raw() routes through the async pool - which returns - * 0 the moment the URB is submitted and counts the wire failure later, in - * tx_done. Without this the primary devourer shape (Init plus concurrent - * send_packets) reports failed=0 even if every frame dies on the wire: the - * same "stat that reads zero while the radio transmits" fault as submitted, - * in the other half. */ - std::lock_guard lock(_mu); - if (_dev) { - struct mt7612u_stats st {}; - mt7612u_get_stats(_dev, &st); - out.failed += st.tx_err; - } return out; } @@ -716,10 +827,34 @@ void Mt7612uRadio::ClearAckResponder() { mt7612u_clear_ack_responder(_dev); } +/* The absolute dBm the actuator should carry: the base plus whatever offset is + * live, clamped to the part's 0-30 range. One place, so the base setter, the + * offset setter and the bring-up replay cannot drift apart. */ +int Mt7612uRadio::txpower_target_dbm() const { + int dbm = _txpwr_dbm + _txpwr_offset_qdb / 4; + if (dbm < 0) + return 0; + if (dbm > 30) + return 30; + return dbm; +} + devourer::TxCaps Mt7612uRadio::GetTxCaps() { devourer::TxCaps c{}; c.supported = true; - c.n_ss = 2; + /* From the library, so this cannot disagree with GetAdapterCaps().tx_chains, + * which reads the same field - the two travel together in one adapter.caps + * event and a consumer comparing them would have no way to pick. */ + { + std::lock_guard lock(_mu); + struct mt7612u_caps hw {}; + if (_dev) { + mt7612u_get_caps(_dev, &hw); + c.n_ss = hw.nss_tx; + } else { + c.n_ss = 2; + } + } c.stbc_ok = true; c.ldpc_ok = true; c.sgi_ok = true; diff --git a/src/mt7612u/Mt7612uRadio.h b/src/mt7612u/Mt7612uRadio.h index cb09e701..6c4ee6b9 100644 --- a/src/mt7612u/Mt7612uRadio.h +++ b/src/mt7612u/Mt7612uRadio.h @@ -12,6 +12,7 @@ #include "DeviceConfig.h" #include "IRadio.h" +#include "RxPacket.h" #include "UsbDeviceLock.h" #include "logger.h" #include "mt7612u/mt7612u.h" @@ -73,6 +74,8 @@ class Mt7612uRadio : public IRadio { SelectedChannel channel) override; void InitWrite(SelectedChannel channel) override; void StartRxLoop(Action_ParsedRadioPacket packetProcessor) override; + /* Safe to call from the packet processor: since delivery moved to this + * thread, StopRxLoop no longer joins a thread the processor is running on. */ void StopRxLoop() override; void SetMonitorChannel(SelectedChannel channel) override; bool send_packet(const uint8_t *packet, size_t length) override; @@ -101,6 +104,7 @@ class Mt7612uRadio : public IRadio { private: void bring_up(SelectedChannel channel); /* _mu held */ void apply_config(); /* _mu held */ + int txpower_target_dbm() const; /* _mu held */ void start_tick(); /* _mu held */ void stop_tick(); /* _mu NOT held */ void tick_loop(); @@ -137,6 +141,34 @@ class Mt7612uRadio : public IRadio { std::atomic _rx_stop{false}; std::atomic _rx_active{false}; std::atomic _rx_frames{0}; + std::atomic _rx_queue_dropped{0}; + + /* Frames cross from the C library's event thread to the StartRxLoop thread + * here, rather than the processor being invoked where the frame arrives. + * + * That is not a style choice. The library's event thread is the SOLE + * servicer of both RX and TX completions, so a processor that transmits - + * examples/chanmig does, from its RX callback - would park that thread in + * mt_async_tx_submit waiting for a TX slot only that same thread can free. + * MAC RX stays enabled, EP4 stops being drained, and this part wedges below + * the USB level where only a physical replug recovers it. Anything taking + * _mu from the processor has a milder version of the same problem: it stalls + * the drain for a 526 ms channel change. + * + * Delivering on the StartRxLoop thread also restores the contract every + * Realtek backend keeps - the processor runs on the thread that called + * StartRxLoop - and keeps the library's "must not block, must not call back" + * rule an internal invariant instead of one silently exported to consumers. + * The copy costs ~2 MB/s at the measured 1400 fps. */ + struct RxSlot { + std::vector data; + rx_pkt_attrib attrib{}; + }; + std::vector _rx_q; + size_t _rx_q_head = 0; /* next slot to write */ + size_t _rx_q_tail = 0; /* next slot to read */ + std::mutex _rx_q_mu; + std::condition_variable _rx_q_cv; std::atomic _tx_submitted{0}; std::atomic _tx_failed{0}; diff --git a/src/mt7612u/async.cpp b/src/mt7612u/async.cpp index 603a9ef0..cccc3606 100644 --- a/src/mt7612u/async.cpp +++ b/src/mt7612u/async.cpp @@ -235,6 +235,12 @@ void mt_async_stop(struct mt7612u_dev *d) stuck_rx = a->rx_inflight; a->running = 0; a->lock.unlock(); + /* Wake anyone parked in mt_async_tx_submit's slot wait. Clearing `running` + * is what its guard tests, but without this notify the guard only fired + * when the cancel pass happened to produce a completion - so a teardown + * with no completions left a submitter blocked forever, which is exactly + * what its comment says must not happen. */ + a->cv.notify_all(); if (a->evt_started) a->evt.join(); diff --git a/tests/mt7612u_mapping_selftest.cpp b/tests/mt7612u_mapping_selftest.cpp index a61ac97a..f048e413 100644 --- a/tests/mt7612u_mapping_selftest.cpp +++ b/tests/mt7612u_mapping_selftest.cpp @@ -76,9 +76,16 @@ int main() { /* HALF-dB, the unit LinkHealth and RxQuality divide by two. Asserting the * raw value here is how an earlier cut of this test locked in a bug that * reported every link at half its SNR. */ - expect("snr is half-dB, not whole dB", a.snr[0] == 74 && a.snr[1] == 74); + expect("chain A snr is half-dB, not whole dB", a.snr[0] == 74); expect("snr round-trips to dB the way consumers read it", a.snr[0] / 2 == 37); + /* PER CHAIN. rssi[1] is -58 against the same -92 noise, so chain B is + * 34 dB = 68 half-dB - NOT chain A's 37. Two identical per-chain SNRs are + * how a dead chain-B antenna hides: its RSSI drops while its SNR appears + * to track chain A. An earlier cut of this test asserted 74 on both. */ + expect("chain B snr is its OWN, not a copy of chain A's", a.snr[1] == 68); + expect("the two chains differ by the RSSI imbalance", + a.snr[0] - a.snr[1] == (rssi_to_raw(-55) - rssi_to_raw(-58)) * 2); expect("snr not invented past the real chains", a.snr[2] == 0 && a.snr[3] == 0); } @@ -90,10 +97,14 @@ int main() { i.n_chains = 2; i.noise_valid = 1; - i.snr_db = 90; + /* Driven through rssi - noise, which is what copy_signal actually reads. + * 40 - (-90) = 130 dB = 260 half-dB, past int8_t. */ + i.rssi[0] = 40; + i.noise = -90; copy_signal(i, a); expect("an out-of-range snr clamps positive", a.snr[0] == 127); - i.snr_db = -90; + i.rssi[0] = -100; + i.noise = 20; copy_signal(i, a); expect("an out-of-range negative snr clamps", a.snr[0] == -128); } From 4a7cd7e0cc8009d7eb61791e4819989d61913748 Mon Sep 17 00:00:00 2001 From: snokvist Date: Thu, 10 Sep 2026 07:21:53 +0200 Subject: [PATCH 7/8] qodo: a MediaTek-only build is valid now, and five more real defects qodo raised 12 on #422. Seven were real; the rest are documented gaps I answered on the threads rather than papered over. A MEDIATEK-ONLY BUILD IS NOW A VALID CONFIGURATION and the guard still rejected it. My own comment in the previous block said DEVOURER_MT7612U would join the "No chip support selected" list "when the backend lands" - it has landed, and I did not go back. Added, with an mt7612u-only build-configs cell. That cell is also the one that can catch the subtree depending on a Realtek chip's sources; mt7612u+jaguar1 cannot, because it has to leave Jaguar1 on. THE ADAPTER LOCK WAS PROCESS-GLOBAL while mt7612u_open_selected() makes one process holding two adapters a supported shape. Opening B overwrote A's descriptor, so closing A released B's lock and leaked A's, leaving another process free to reset and claim B underneath it. Moved into mt7612u_dev. It defaults to -1 explicitly, because value-initialising the device gives 0 and unlock_adapter() would then close stdin. TWO TX-POWER BUGS, both in code I wrote answering the LAST review: - SetTxPower stored the raw byte, so a value above 30 - including a negative that narrowed into uint8_t - sat in the base while txpower_target_dbm() quietly clamped the hardware to maximum output, and every later offset composed against a base the radio never used. Clamped at the setter, with a warning. - SetTxPowerOffsetQdb remembered the rail-CLAMPED offset rather than the requested one. Ask for -20 dB against a 5 dBm base, get -5 dB at the 0 dBm rail, and a later SetTxPower(20) composed with -5 instead of restoring the -20 still configured. The last review told me to derive the RETURN from the clamp, which was right; I applied it to what is REMEMBERED too, which was not. Requested is stored, applied is returned. THE WATERFALL TOOLS DECODED BARE MEDIATEK CAPTURES WRONG. read_frames() applies a bare-hex FCS override; bf_waterfall.py and bf_waterfall_svg.py took report_hex()'s default straight to parse_frame(), so a hand-captured MT7612U dump silently lost four payload bytes there while an event capture of the same frames decoded fine. That is a gap the first block left. Rather than copy the override a third time, it is now bf.frame_from_line() - one place, so a caller cannot get it wrong by leaving a step out - and both tools take --no-fcs. rx.pool_exhaust defaults to Backpressure, and DeviceConfig promises the non-SpscFat modes "never drop host-side". This part cannot keep that promise: backpressure here means an undrained EP4, which is the wedge. So it warns once, like the other knobs it cannot honour, and the drops stay counted. Docs: the FCS paragraph still said nothing sets fcs_present, and the CI paragraph let two lookup-only ctest cells read as backend coverage. Both now say what is actually true - every hardware claim in that document is hand-run. Verified: 60/60 in Release and MT7612U=ON, 50/50 in the new MediaTek-only config, make check green, and on hardware RX plus 827 TX submitted 0 failed. Co-Authored-By: Claude Opus 5 (1M context) Claude-Session: https://claude.ai/code/session_01Tba83kymS5W2v1vn2yRxrj --- .github/workflows/cmake-multi-platform.yml | 6 ++++ CMakeLists.txt | 24 ++++++++------ docs/mt7612u.md | 9 +++-- src/mt7612u/Mt7612uRadio.cpp | 38 +++++++++++++++++++--- src/mt7612u/internal.h | 10 ++++++ src/mt7612u/usb.cpp | 26 +++++++-------- tools/bf_report_decode.py | 25 ++++++++++---- tools/bf_waterfall.py | 10 +++--- tools/bf_waterfall_svg.py | 10 +++--- 9 files changed, 110 insertions(+), 48 deletions(-) diff --git a/.github/workflows/cmake-multi-platform.yml b/.github/workflows/cmake-multi-platform.yml index 12cb8eca..d7a53ce8 100644 --- a/.github/workflows/cmake-multi-platform.yml +++ b/.github/workflows/cmake-multi-platform.yml @@ -197,6 +197,12 @@ jobs: # counted as chip support, so an MT7612U-only configure is a # FATAL_ERROR. This proves the subtree is independent of the other # seven Realtek chips' sources, not of Jaguar1's. + # MediaTek alone, valid now that the option carries a radio backend. + # This is the cell that can catch the subtree depending on a Realtek + # chip's sources - the one below cannot, because it must leave + # Jaguar1 on. + - name: mt7612u-only + flags: "-DDEVOURER_MT7612U=ON -DDEVOURER_JAGUAR1=OFF -DDEVOURER_8814=OFF -DDEVOURER_JAGUAR2_8822B=OFF -DDEVOURER_JAGUAR2_8821C=OFF -DDEVOURER_JAGUAR3_8822C=OFF -DDEVOURER_JAGUAR3_8822E=OFF -DDEVOURER_8733B=OFF -DDEVOURER_KESTREL_8852B=OFF -DDEVOURER_KESTREL_8852C=OFF" - name: mt7612u+jaguar1 flags: "-DDEVOURER_MT7612U=ON -DDEVOURER_8814=OFF -DDEVOURER_JAGUAR2_8822B=OFF -DDEVOURER_JAGUAR2_8821C=OFF -DDEVOURER_JAGUAR3_8822C=OFF -DDEVOURER_JAGUAR3_8822E=OFF -DDEVOURER_8733B=OFF -DDEVOURER_KESTREL_8852B=OFF -DDEVOURER_KESTREL_8852C=OFF" - name: rtl8733b-only diff --git a/CMakeLists.txt b/CMakeLists.txt index a1786911..9f9e8ecd 100644 --- a/CMakeLists.txt +++ b/CMakeLists.txt @@ -43,15 +43,18 @@ option(DEVOURER_KESTREL_8852C "RTL8852CU / 8832CU (Kestrel 11ax, G6)" option(DEVOURER_PCIE "PCIe transport via vfio-pci (Linux; RTL8821CE)" OFF) # MediaTek MT7612U / MT7662U — the first non-Realtek part. Compiles the # src/mt7612u subtree (its own C++ library with a C ABI, landed unwired in #412) -# into libdevourer. OFF by default and deliberately NOT counted as chip support -# by the "No chip support selected" check below: there is no IRadio backend yet, -# so an MT7612U-only build would produce a library that can open nothing. It -# joins that list when the backend lands (#419). Turning it ON today buys one -# thing, and it is the point of the option: CI compiles the subtree across the -# whole platform matrix — gcc, clang, MSVC, mingw, macOS — where it was -# previously only ever built by its own Makefile, on Linux. It does NOT gate the -# factory's MediaTek refusal, which is unconditional. -option(DEVOURER_MT7612U "Compile the MediaTek MT7612U subtree (the id gate is always on)" OFF) +# into libdevourer along with Mt7612uRadio, the IRadio backend WiFiDriver +# constructs for a MediaTek adapter. OFF by default, but it DOES count as chip +# support in the "No chip support selected" check below: with the backend behind +# it an MT7612U-only build opens, receives and transmits, which is exactly what +# that check exists to require. +# +# Turning it on also compiles the subtree across the whole platform matrix — +# gcc, clang, MSVC, mingw, macOS — where it was previously only ever built by +# its own Makefile, on Linux. The option does NOT gate the factory's MediaTek id +# gate, which is unconditional: with it OFF, such an adapter is refused rather +# than misdetected as Realtek. +option(DEVOURER_MT7612U "MediaTek MT7612U / MT7662U (2T2R 11ac USB)" OFF) # Compile-time diagnostics floor (src/logger.h). Calls below the floor — # including their argument expressions at DVR_TRACE/DVR_DEBUG sites — compile @@ -95,7 +98,8 @@ else() endif() if(NOT DEVOURER_JAGUAR1 AND NOT DEVOURER_JAGUAR2 AND NOT DEVOURER_JAGUAR3 AND NOT DEVOURER_8733B - AND NOT DEVOURER_KESTREL) + AND NOT DEVOURER_KESTREL + AND NOT DEVOURER_MT7612U) message(FATAL_ERROR "No chip support selected. Enable at least one of DEVOURER_JAGUAR1 / " "DEVOURER_JAGUAR2_8822B / DEVOURER_JAGUAR2_8821C / DEVOURER_JAGUAR3_8822C " diff --git a/docs/mt7612u.md b/docs/mt7612u.md index 662b65da..2ded4db9 100644 --- a/docs/mt7612u.md +++ b/docs/mt7612u.md @@ -173,8 +173,8 @@ control. Measured, because it constrains integration rather than being a detail. `Packet::Data` carries the trailing FCS whenever `rx_pkt_attrib::fcs_present` is set, which every Realtek parser leaves at its default. MT7612U cannot: the -MAC strips it, so this backend will clear that flag when it is wired in -(nothing sets it today - see Open below). +MAC strips it, so `Mt7612uRadio::on_rx()` clears that flag on every frame it +delivers. Four to seven bytes do sit past `MPDU_LEN` in every RX buffer — over 4263 ambient frames the tail was 4 bytes on 3375 of them and 5-7 on the rest, @@ -352,7 +352,10 @@ Stated because the numbers above are uniformly favourable. subtree's own four tests still do not run there.** With `DEVOURER_MT7612U=ON` the whole platform matrix (gcc, clang, MSVC, mingw, macOS) builds the subtree and the sanitizer job links it, so a portability or lifetime regression is - caught. `mt7612u_usb_ids` and `mt7612u_mapping` run on every configuration. + caught. `mt7612u_usb_ids` and `mt7612u_mapping` run on every configuration — + but both cover pure LOOKUPS, not the backend's behaviour, which has no + automated coverage beyond compiling. Every hardware claim in this document is + hand-run. The four offline tests under `src/mt7612u/tests/` and the table generator's `--check` are still driven only by `src/mt7612u/Makefile`, which no workflow invokes — nor is there a lifecycle soak of the kind the Realtek backends diff --git a/src/mt7612u/Mt7612uRadio.cpp b/src/mt7612u/Mt7612uRadio.cpp index 6a94dbc0..5d9df62a 100644 --- a/src/mt7612u/Mt7612uRadio.cpp +++ b/src/mt7612u/Mt7612uRadio.cpp @@ -224,6 +224,20 @@ void Mt7612uRadio::apply_config() { "implemented by this backend - carrier-sense stays ENABLED " "for this session"); + /* rx.pool_exhaust defaults to Backpressure, and DeviceConfig says the + * non-SpscFat modes "never drop host-side ... which backpressures the chip by + * construction". Not available on this part: backpressure here means not + * draining EP4, and an undrained receiver wedges this silicon below the USB + * level where only a physical replug recovers it. The hand-off queue drops + * the newest frame and counts it instead, and the teardown line reports any + * loss. Said once, rather than left to be inferred from a frame count. */ + if (_cfg.rx.pool_exhaust == devourer::PoolExhaust::Backpressure) + _logger->warn("MT7612U: rx.pool_exhaust=backpressure cannot be honoured - " + "host-side backpressure on this part means an undrained " + "receiver, which wedges it below the USB level. The RX " + "hand-off queue drops the newest frame instead, and counts " + "every drop."); + if (_cfg.tx.usb_agg_max > 0) _logger->warn("MT7612U: tx.usb_agg_max={} is not consulted - send_packets " "always chains frames into shared bulk-OUT URBs on this " @@ -659,7 +673,18 @@ void Mt7612uRadio::SetTxPower(uint8_t power) { /* Deliberately NOT forwarded to SetTxPowerIndexOverride the way the base * class does: there is no TXAGC index here, so the argument is read as the * dBm limit it actually maps to. */ - _txpwr_dbm = static_cast(power); + /* Clamped HERE, not only at the actuator. Storing the raw byte let a value + * above 30 - including a negative that narrowed into uint8_t - sit in the + * base while txpower_target_dbm() quietly clamped the hardware to maximum + * output, so every later offset composed against a base the radio never + * used. */ + int dbm = static_cast(power); + if (dbm > 30) { + _logger->warn("MT7612U TX power {} dBm is above this part's 30 dBm ceiling " + "- clamping", dbm); + dbm = 30; + } + _txpwr_dbm = dbm; /* Composes with a live offset rather than discarding it - IRadio says the two * compose, and writing the bare base would silently undo an offset while * _txpwr_offset_qdb still reported it as applied. */ @@ -706,10 +731,13 @@ int Mt7612uRadio::SetTxPowerOffsetQdb(int qdb) { _logger->error("MT7612U TX power offset {} qdB -> {} dBm refused", qdb, dbm); return 0; } - /* Sticky, and it has to be recorded even with no device open: SetTxPower and - * bring_up both fold it back in, so an offset set before Init survives to the - * first tune instead of being silently swallowed. */ - _txpwr_offset_qdb = applied_qdb; + /* REQUESTED is what is remembered; APPLIED is what is returned. Storing the + * rail-clamped figure instead would shrink the offset permanently: ask for + * -20 dB against a 5 dBm base, get -5 dB at the 0 dBm rail, and a later + * SetTxPower(20) would compose with -5 rather than restoring the -20 that is + * still configured. Recorded even with no device open, so an offset set + * before Init survives to the first tune instead of being swallowed. */ + _txpwr_offset_qdb = want_qdb; return applied_qdb; } diff --git a/src/mt7612u/internal.h b/src/mt7612u/internal.h index c2d2f6bc..7fab68e5 100644 --- a/src/mt7612u/internal.h +++ b/src/mt7612u/internal.h @@ -188,6 +188,16 @@ struct mt7612u_dev { * never asked for. Points at caller-owned storage and is only read during * mt_open(). */ const char *dev_selector; + /* The adapter's exclusivity lock (flock on the same file UsbDeviceLock + * uses), or -1. PER DEVICE, not a file-global: mt7612u_open_selected() + * makes one process holding two adapters a supported shape, and a single + * global fd meant opening B overwrote A's descriptor - so closing A + * released B's lock and leaked A's, leaving another process free to + * reset and claim B while A was still using it. */ + /* -1, NOT the 0 that value-initialising the device would give: 0 is + * stdin, and unlock_adapter() would close it on a device that never took + * a lock. */ + int lock_fd = -1; uint8_t bw_clamp_warned; /* the "never widen" notice is once, not per frame */ int8_t txpower_conf; /* limit, 0.5 dB units (dBm * 2) */ int8_t target_power; diff --git a/src/mt7612u/usb.cpp b/src/mt7612u/usb.cpp index fdc88cd8..728f8295 100644 --- a/src/mt7612u/usb.cpp +++ b/src/mt7612u/usb.cpp @@ -102,9 +102,6 @@ void mt_usleep(unsigned us) std::this_thread::sleep_for(std::chrono::microseconds(us)); } -/* Held for the process lifetime; flock releases it on any exit. */ -static int g_lock_fd = -1; - static uint64_t now_us(void) { /* steady_clock, matching CLOCK_MONOTONIC: never stepped by a wall-clock @@ -512,11 +509,13 @@ static void adapter_key(libusb_device *dev, char *out, size_t n) i ? "." : "-", ports[i]); } -/* Drop the adapter lock, if this process is holding one. */ -static void unlock_adapter(void) +/* Drop this device's adapter lock, if it is holding one. */ +static void unlock_adapter(struct mt7612u_dev *d) { #if !defined(_WIN32) - if (g_lock_fd >= 0) { close(g_lock_fd); g_lock_fd = -1; } + if (d && d->lock_fd >= 0) { close(d->lock_fd); d->lock_fd = -1; } +#else + (void)d; #endif } @@ -580,7 +579,8 @@ static int lock_adapter(libusb_device *dev, const char **err) } #endif /* !_WIN32 */ -static libusb_device_handle *open_selected(libusb_context *ctx, const char *sel, +static libusb_device_handle *open_selected(struct mt7612u_dev *d, + libusb_context *ctx, const char *sel, const char **err) { @@ -630,7 +630,7 @@ static libusb_device_handle *open_selected(libusb_context *ctx, const char *sel, libusb_free_device_list(list, 1); return NULL; } - g_lock_fd = lk; + d->lock_fd = lk; if (libusb_open(list[i], &h)) { /* Only mt_close() releases the lock, and a failed * mt_open() never reaches it - so holding it here @@ -638,7 +638,7 @@ static libusb_device_handle *open_selected(libusb_context *ctx, const char *sel, * on the very next retry. Release what this * iteration took. */ h = NULL; - unlock_adapter(); + unlock_adapter(d); } } matches++; @@ -667,7 +667,7 @@ int mt_open(struct mt7612u_dev *d, const char **err) if (libusb_init(&d->ctx)) { if (err) *err = "libusb_init failed"; return -1; } d->owns_handle = 1; - d->h = open_selected(d->ctx, d->dev_selector, err); + d->h = open_selected(d, d->ctx, d->dev_selector, err); if (!d->h) { libusb_exit(d->ctx); d->ctx = NULL; return -1; @@ -692,7 +692,7 @@ int mt_open(struct mt7612u_dev *d, const char **err) /* Re-enumerated under a new address: reopen and re-detach. */ libusb_close(d->h); mt_usleep(200000); - d->h = open_selected(d->ctx, d->dev_selector, NULL); + d->h = open_selected(d, d->ctx, d->dev_selector, NULL); if (!d->h) { if (err) *err = "device vanished after USB reset"; libusb_exit(d->ctx); d->ctx = NULL; @@ -748,7 +748,7 @@ void mt_close(struct mt7612u_dev *d) "handle and context rather than closing underneath them"); d->h = NULL; d->ctx = NULL; - unlock_adapter(); + unlock_adapter(d); return; } if (d->h) { @@ -764,7 +764,7 @@ void mt_close(struct mt7612u_dev *d) } if (d->ctx && d->owns_handle) libusb_exit(d->ctx); d->ctx = NULL; - unlock_adapter(); + unlock_adapter(d); } /* Block write, as mt76u_copy(): one MULTI_WRITE per batch, wValue 0. diff --git a/tools/bf_report_decode.py b/tools/bf_report_decode.py index a48e8467..d065886b 100644 --- a/tools/bf_report_decode.py +++ b/tools/bf_report_decode.py @@ -217,6 +217,23 @@ def report_hex(line: str): return line, True +def frame_from_line(line, bare_fcs=True): + """report_hex + the bare-hex FCS override + parse_frame, in ONE place. + + The override is easy to omit, and omitting it is silent: both waterfall + tools took report_hex's fcs_present straight to parse_frame, so a bare + MediaTek capture lost its last four payload bytes there while an event + capture of the same frames decoded fine. Factored out so a caller cannot + get it wrong by leaving a step out.""" + hf = report_hex(line) + if hf is None: + return None + h, fcs_present = hf + if line.strip() and not line.strip().startswith('{"ev":"'): + fcs_present = bare_fcs # bare hex: no metadata, use the flag + return parse_frame(h, fcs_present) + + def read_frames(src, max_frames=200, bare_fcs=True): """Parse `bf.report_raw` event (or bare hex) lines into frame dicts. @@ -225,13 +242,7 @@ def read_frames(src, max_frames=200, bare_fcs=True): metadata channel, so --no-fcs is the only way to decode one correctly.""" frames = [] for line in src: - hf = report_hex(line) - if hf is None: - continue - h, fcs_present = hf - if line.strip() and not line.strip().startswith('{"ev":"'): - fcs_present = bare_fcs # bare hex: no metadata, use the flag - f = parse_frame(h, fcs_present) + f = frame_from_line(line, bare_fcs) if f: frames.append(f) if len(frames) >= max_frames: diff --git a/tools/bf_waterfall.py b/tools/bf_waterfall.py index 0bc25435..c3df7bd2 100644 --- a/tools/bf_waterfall.py +++ b/tools/bf_waterfall.py @@ -74,6 +74,10 @@ def main() -> int: ap.add_argument("--operating-snr", type=float, default=None, help="re-centre measured per-tone SNR to this mean dB " "(models a weaker link so the QAM ramp spreads)") + ap.add_argument("--no-fcs", action="store_true", + help="bare-hex input carries no trailing FCS (MediaTek " + "MT7612U strips it). Events carry their own fcs field " + "and always win.") ap.add_argument("--width", type=int, default=2, help="terminal columns per subcarrier (default 2)") args = ap.parse_args() @@ -141,11 +145,7 @@ def flush_row(): try: for line in sys.stdin: - hf = bf.report_hex(line) - if hf is None: - continue - h, fcs_present = hf - f = bf.parse_frame(h, fcs_present) + f = bf.frame_from_line(line, bare_fcs=not args.no_fcs) if not f: continue if ns is None: diff --git a/tools/bf_waterfall_svg.py b/tools/bf_waterfall_svg.py index cede079d..b8231893 100644 --- a/tools/bf_waterfall_svg.py +++ b/tools/bf_waterfall_svg.py @@ -29,15 +29,15 @@ def main() -> int: ap.add_argument("--operating-snr", type=float, default=None) ap.add_argument("--snr-lo", type=float, default=15.0) ap.add_argument("--snr-hi", type=float, default=55.0) + ap.add_argument("--no-fcs", action="store_true", + help="bare-hex input carries no trailing FCS (MediaTek " + "MT7612U strips it). Events carry their own fcs field " + "and always win.") args = ap.parse_args() frames = [] for line in open(args.infile): - hf = bf.report_hex(line) - if hf is None: - continue - h, fcs_present = hf - f = bf.parse_frame(h, fcs_present) + f = bf.frame_from_line(line, bare_fcs=not args.no_fcs) if f and f["feedback"]: frames.append(f) if not frames: From b8a53cdec70d3213668841cd2cfc247ca5e7d9ef Mon Sep 17 00:00:00 2001 From: snokvist Date: Thu, 10 Sep 2026 07:24:24 +0200 Subject: [PATCH 8/8] docs: the two backend gaps I answered qodo with, in Counterparts Both were answered on #422's threads as known and deliberate; a reply on a PR is not where a limit should live. The multi-adapter log routing and the refusals-only TxStats::failed now sit with the rest of the honest list, each with the library change that would close it. Co-Authored-By: Claude Opus 5 (1M context) Claude-Session: https://claude.ai/code/session_01Tba83kymS5W2v1vn2yRxrj --- docs/mt7612u.md | 15 +++++++++++++++ 1 file changed, 15 insertions(+) diff --git a/docs/mt7612u.md b/docs/mt7612u.md index 2ded4db9..156a4ab6 100644 --- a/docs/mt7612u.md +++ b/docs/mt7612u.md @@ -328,6 +328,21 @@ Stated because the numbers above are uniformly favourable. - **One physical unit, one sample.** No second MT7612U, no second board revision, no vendor-driver A/B beyond the register diff. +- **Two adapters share one diagnostic route.** The C library's log sink is a + single process-global pair, so `Mt7612uRadio` keeps a registry and routes to + the first live radio's `Logger`. With two MediaTek adapters open, the second + one's library diagnostics are gated by the first one's log level and stream + and carry no adapter identity. That is better than the alternative — per-object + install meant the second constructor stole the first's routing and the first + destructor unhooked the survivor's, dropping it to raw stderr — but it is not + right. The fix is a per-device sink in the C library + (`mt7612u_set_log_sink(dev, ...)`), which is a public-header change. +- **`TxStats::failed` counts refusals, not wire deaths.** A frame the transport + accepted and the URB then failed to deliver does not move it. The library's + own `tx_err` cannot simply be added: it double-counts submit failures, counts + URBs where this counts frames, and restarts at zero when a ring is torn down, + so `failed` would go backwards across an RX restart. Needs a monotonic + wire-failure counter in the library. - **One witness generation.** Every on-air number is an RTL8812AU running this project's `rxdemo`. `paggr`, `bw` and `rate` are that implementation's reading, not an independent instrument.