feat(hid): ELAD/WoodBoxRadio TMate 2 USB HID support#3401
Conversation
There was a problem hiding this comment.
Thanks @svabi79 — this is a clean, well-isolated addition. Nice work tracing the protocol back to USBPcap and OpenTMate2Lib, and the bitmask preservation in the digit-write helpers (the 0xF8 / 0xF0 / 0x0F / 0x1F masks) cleanly protects every indicator that shares a byte with a digit (DOT1@9, DOT2@15, RIT@13, W@20, HZ@23, DB_MINUS@28 all check out). CI is green across all 5 checks (build, macOS, Windows, CodeQL, analyze).
Conventions look good: AppSettings (not QSettings), QMetaObject::invokeMethod(m_hidEncoder, ...) for all cross-thread display writes, RAII via Qt parent ownership, new device added to kSupportedDevices[] + factory and gated by isTMate2() everywhere.
A few small things worth a follow-up commit:
1. Doc inconsistency in HidDeviceParser.h (TMate2 class block). The comment block has three different mappings for the encoder-push bits and they don't agree:
- "
[5..6] encoder 3 (main)" — calls bytes 5-6 the main encoder - "
encoderIndex: 0=enc1/main-tuning (bytes 1-2)" — calls bytes 1-2 the main encoder - "
bit6=…, bit7=encoder2 push, bit8=encoder1 push" then the next line says "bit8=$0100=enc3 push"
The code (kButtonMap, if (i == 0) diff = -diff for main tuning, and the TMate2KeyAction0… mapping in MainWindow.cpp) consistently treats encoder index 0 / bytes 1-2 as the main tuning wheel and bit8 → button 11 → encoder-3-push. Worth normalising the comment block to match — future maintainers will trip over the contradictions.
2. RadioSetupDialog.cpp capture-by-reference of the loop key. In the three new combo loops, the connect lambda captures key (a local QString) and the spin/timing helpers do the same:
connect(combo, &QComboBox::currentIndexChanged, this, [combo, key, this](int) {
s.setValue(key, combo->currentData().toString());
...
});key is captured by value here, which is correct — but the addTimingSpin lambda also captures key by value via its enclosing capture list, fine. No bug; just flagging that I checked because it's a common foot-gun in this shape of loop. (If you do a future cleanup pass, deduplicating the three near-identical combo blocks via a small helper would shrink the dialog by ~150 lines.)
3. PR base is behind main. Your base SHA 6a142807 predates the Reverse mouse-wheel tuning feature (#3333) that merged a few hours ago. A rebase on current main will produce a clean merge — no actual code conflict, just so you're not surprised.
On the flat-vs-nested settings question you raised: I'd keep the flat TMate2KeyAction0… keys as-is. The adjacent StreamDeck+ and HID encoder settings are flat too, so a nested-JSON blob here would be inconsistent with the immediate neighbourhood. If/when the maintainers do a broader migration to Principle V, this device can ride along.
Not blocking — these are all polish. The on-device verification + green CI on three platforms is strong evidence for a feature of this size.
🤖 aethersdr-agent · cost: $26.1400 · model: claude-opus-4-7
74175e0 to
af603de
Compare
|
Thanks for the thorough review, @aethersdr-agent — much appreciated. Addressed in the latest push ( 1. Doc inconsistency in 3. Base behind 2. Combo-loop captures / dedup — agree there's no bug (the And thanks for confirming the flat |
NF0T
left a comment
There was a problem hiding this comment.
Welcome to the codebase, @svabi79 — this is a thorough first contribution. The protocol implementation is correct: the encoder wrap-correction, active-low key handling, segment bit isolation, and the first-report seeding all check out against the source. Thread safety via QMetaObject::invokeMethod is correct throughout, all new config is properly gated behind #ifdef HAVE_HIDAPI, and the on-device validation gives real confidence. All 5 CI checks are green.
One item needs correcting before merge.
updateTMate2Display() — comment describes a formula the code doesn't implement
The doc comment currently reads:
// small_val mapping:
// RX: S-unit × 10 + fractional dB, clamped 0-999.
// S9 = -73 dBm (reference), each S unit = 6 dB below.
// Values above S9 shown as 90, 91, … (S9 + N dB → 90 + N).
// TX: forward power in watts from the last txMetersChanged sample.
// Displaying S-units on a 3-digit display gives a reading like "55" (S5+5dB)
// that maps naturally to a traditional S-meter without needing segment labels.
The actual code:
const int raw = 90 + (dbm - s9_dbm); // 90 at S9, +1 per dBm aboveThis is a linear dBm-from-S9 scale, not S-unit × 10 + fractional dB. At S5+5dB (-92 dBm) the code produces 90 + (-92 − (−73)) = 71, not 55. The example and the formula description are both wrong for the sub-S9 range. Above S9 they happen to agree, which is why it's easy to miss.
A stale comment with a specific wrong numeric example is a real hazard — a future developer or agent reading "55 (S5+5dB)" as a spec will write code to that interface and get incorrect values.
Please replace with text that describes what the code actually does:
// small_val mapping:
// RX: linear dBm offset from S9, clamped 0-999.
// S9 (-73 dBm) → 90; each dB above S9 adds 1 (S9+10 dB → 100);
// each dB below S9 subtracts 1 (S8 → 84, S5 → 66, S1 → 42).
// TX: forward power in watts from the last txMetersChanged sample.Informational (not blocking)
The new combo boxes use setStyleSheet(kEditStyle.replace("QLineEdit","QComboBox")) — this matches the existing FlexControl/HID encoder combos in the same file section exactly, so it's consistent with the immediate neighbourhood. The broader file uses AetherSDR::applyComboStyle() elsewhere. The HID section of RadioSetupDialog predates that convention and should be cleaned up in a follow-on pass; not something to resolve in this PR.
Happy to re-review promptly once the comment is updated.
73,
Ryan NF0T
af603de to
c08d491
Compare
|
Thanks Ryan — you're exactly right, and good catch. The Fixed in // small_val mapping:
// RX: linear dBm offset from S9, clamped 0-999.
// S9 (-73 dBm) → 90; each dB above S9 adds 1 (S9+10 dB → 100);
// each dB below S9 subtracts 1 (S8 → 84, S5 → 66, S1 → 42).
// TX: forward power in watts from the last txMetersChanged sample.Noted on the Ready for re-review whenever you have a moment. Thanks again, Jan (svabi79) |
Adds full TMate 2 (VID 0x1721 / PID 0x0614) integration: the device's three encoders, six function keys, and the LCDVector display are wired into the SDR. Protocol reverse-engineered from USB captures. Display layer (HidEncoderManager): - 44-byte LCDVector state buffer; every setter mutates it then sends the full report (prefixed with report-ID 0 so Windows hidapi does not shift the payload), so backlight / contrast / timing are never reset. - 9-digit main frequency + 3-digit small display, 15-segment S-meter bargraph, RX/TX, per-mode indicators (USB/LSB/AM/SAM/DSB/FM/CW/DIGU/ DIGL/RTTY), RIT/XIT, decimal dots. - applyModeSegs() shared by the live and overlay indicator paths. MainWindow wiring: - Frequency, S-meter, and TX power tracked from the active slice. The small display shows the signal strength in dBm (matching the AetherSDR UI) while the 15-bar bargraph shows the S-unit; RX/TX backlight colours switch on MOX. - Transient overlays (volume, RF power, CW speed, step size, RIT) shown for a configurable duration when the matching control changes. - Idle blanking: the LCD clears after a configurable no-interaction timeout and wakes on the next encoder / button event. - Encoder 0 direction inverted to match the physical tuning wheel. Config UI (RadioSetupDialog): per-key / per-encoder / per-push action mapping, RX and TX backlight RGB, and overlay / idle timing. All client-side AppSettings; no radio-authoritative state is persisted. Built clean on Linux (CI container) and Windows (MSVC), and tested against real TMate 2 hardware. Principle XI.
c08d491 to
10bceba
Compare
|
Heads-up @NF0T — 1. Small display now shows actual dBm, not the S9-offset scale. 2. Decimal dots + Hz unit now persist. Both were caught testing on the actual TMate 2. Sorry for moving the goalposts on the comment you just reviewed — this is the more correct behaviour. Ready for another look. |
NF0T
left a comment
There was a problem hiding this comment.
Thanks for the follow-through, @svabi79 — and for flagging the formula change
proactively rather than letting it quietly supersede the comment we just
corrected together. That kind of transparency makes re-review straightforward.
Original blocker — resolved (and improved) ✓
The misleading "S-unit × 10 + fractional dB / 55 (S5+5dB)" comment is gone.
More importantly, the underlying formula changed for the better: the small
display now shows actual dBm (`std::abs(dbm)`, clamped 0-999) instead of the
S9-offset scale. The reasoning is correct — the old `90 + (dbm − s9_dbm)`
value at −95 dBm rendered `68`, which with the `DBM` + `−` indicator segments
read as "−68 dBm", contradicting AetherSDR's −95 dBm readout. The new approach
reads "−95" to match. The new comment in `updateTMate2Display()` accurately
describes what the code does, including why the numeric and the bargraph are
complementary rather than redundant. No stale description anywhere.
Dot/Hz persistence fix ✓
Re-asserting `DOT1`, `DOT2`, `HZ` in `setTMate2Indicators()` rather than only
in `open()` is the correct fix. The WHY comment is accurate — overlay and
idle-blank paths clear those segments. On-device verified.
Additional overlay triggers ✓
RIT toggle/clear, volume, headphone volume, RF power, and CW speed now all
call `triggerTMate2Overlay()`. Correct hookups throughout.
Full diff checked — thread safety via `QMetaObject::invokeMethod` consistent,
`HAVE_HIDAPI` guards consistent, `kSeg_SMETER_DB_MINUS` correctly gated on
`!tx && smeter_dbm < 0.0f`, flat `TMate2KeyAction…` keys consistent with the
neighbouring StreamDeck+/HID encoder config. Nothing jumped out.
✅ Approving. Nice work getting this across the line with on-device
verification — hardware-tested HID contributions are always appreciated.
73, Ryan NF0T
…th HAVE_HIDAPI (#3436) ## Summary Fixes #3435. `applyFlexControlWheelAction` is declared outside any `#ifdef HAVE_HIDAPI` block (it handles wheel inputs from FlexControl, MIDI, and other non-HID sources), but PR #3401 added five call sites inside it that reference `triggerTMate2Overlay(TMate2Overlay::...)`. Both the `TMate2Overlay` enum class and `triggerTMate2Overlay` are declared inside the `#ifdef HAVE_HIDAPI` block in `MainWindow.h` (lines 573–623), so they are invisible to the compiler on configurations without HIDAPI — specifically the MinGW / no-`setup-hidapi.ps1` Windows developer build. **Affected actions:** `WheelRit`, `WheelVolume`, `WheelHeadphoneVolume`, `WheelPower`, `WheelCwSpeed` — all five call sites are wrapped with `#ifdef HAVE_HIDAPI` / `#endif`. Runtime behavior on HIDAPI-enabled builds is unchanged. This is the same guard pattern applied in PR #3344 to `UlanziDial`. ## Constitution principle honored Principle IX — Surface Only What Survives. Code on `main` must compile on all target platforms. The no-HIDAPI MinGW path is a required target (Windows developer builds before running `setup-hidapi.ps1`); surfaces that break that build do not survive the platform gate. ## Test plan - [x] Build without `HAVE_HIDAPI` passes — MinGW GCC 13.1.0 / Qt 6.11.0 / no hidapi, verified locally - [x] Build with `HAVE_HIDAPI` unaffected — same toolchain, guards compile out cleanly - [x] Existing tests pass (CI) ## Checklist - [x] Commits are signed (`docs/COMMIT-SIGNING.md`) - [x] No new flat-key `AppSettings` calls — N/A - [x] All meter UI uses `MeterSmoother` — N/A - [x] Documentation updated if user-visible behavior changed — no user-visible change - [x] Security-sensitive changes reference a GHSA if applicable — N/A Co-authored-by: Claude Sonnet 4.6 <noreply@anthropic.com>
Summary
Adds full support for the ELAD / WoodBoxRadio TMate 2 USB HID controller
(VID
0x1721/ PID0x0614) — three encoders, six function keys, and theLCDVector display, all wired into the SDR. The USB protocol was
reverse-engineered from capture analysis.
What's included
Display layer (
HidEncoderManager)report so backlight / contrast / timing are never reset. The report is
prefixed with report-ID
0so Windows hidapi does not shift the payload byone byte.
bargraph, RX/TX, per-mode indicators (USB/LSB/AM/SAM/DSB/FM/CW/DIGU/DIGL/
RTTY), RIT/XIT and decimal dots.
applyModeSegs()shared between the live and overlay indicator paths.MainWindow wiring
slice; RX/TX backlight colours switch on MOX.
configurable duration when the matching control changes.
wakes on the next encoder / button event.
Config UI (
RadioSetupDialog)overlay / idle timing. All client-side
AppSettings; no radio-authoritativestate is persisted.
Testing
backlight and idle blanking verified on-device.
Note for reviewers
The TMate 2 settings use flat
AppSettingskeys (TMate2KeyAction0…),matching the adjacent StreamDeck+/HID encoder config rather than the nested-JSON
form of Principle V. Happy to migrate these to a single nested-JSON blob if
you'd prefer the feature lead the way on that convention.
🤖 Generated with Claude Code