fix(hid): three StreamDeck+ robustness gaps from #3236 review (#3248)

[ten9876]

Closes #3248.

Implements the three small robustness fixes flagged during PR #3236's review and tracked in #3248. Each is independent and scoped to one file.

1. `StreamDeckPlusParser::parse` — don't drop simultaneous events

Both the encoder-push and LCD-key paths overwrote `m_prevEncBtns` / `m_prevKeys` with `newState` before the loop that finds the first changed bit, so any second-changed-bit got silently dropped (the next `poll()` saw no remaining change). Switch to one-bit-at-a-time commit:

```cpp
m_prevKeys ^= (1u << i); // consume just this bit
return {.button = i + 1, ...};
```

Now if two keys change in the same HID report, the first poll iteration emits one and the next poll iteration emits the other. Verified by hand-trace: press both → emit press 1 → emit press 2 → release both → emit release 1 → emit release 2. ✅

2. `HidEncoderManager` — check `hid_write` return on image writes

`setKeyImage` and `setTouchscreenImage` looped through 1024-byte feature reports ignoring the return. If the device unplugs mid-stream the loop spins through the rest writing into a dead handle with no log signal. Now:

```cpp
const int written = hid_write(m_device, pkt, PACKET_SIZE);
if (written < 0) {
qCWarning(lcDevices) << "HidEncoderManager::setKeyImage: hid_write failed"
<< "key=" << key << "page=" << pageNumber
<< "— device disconnected? Will retry on hotplug.";
return;
}
```

The next `poll()`'s `hid_read() < 0` triggers `close()` + hotplug reopen, which is now correlatable in logs with the user-visible "deck went blank".

3. `HidEncoderManager` — atomic gate-check members

`m_device` / `m_openVid` / `m_openPid` are read from the main thread (refreshStreamDeckLabels at MainWindow.cpp:6778, status snapshot at :7061+) and written from `m_extCtrlThread` (open / close / hotplugCheck). Wrap in `std::atomic` so cross-thread reads are well-defined. Relaxed memory order is sufficient because callers treat the result as a hint and the real gate is re-checked inside the queued slot.

`std::atomic<hid_device*>` and `std::atomic<uint16_t>` have implicit conversion / assignment operators, so no .cpp call-site needs explicit `.load()` / `.store()` — the source diff is purely the header change plus the existing reads continuing to compile under the new types.

Test plan

• Build clean on Linux (627/627 targets, no new warnings)
• Confirm check-windows and check-macos pass (now always run per ci: always run check-windows + check-macos on every PR #3244)
• Hand-trace the lost-events fix once more
• Optional: hotplug a StreamDeck+ during a label refresh and watch for the new `hid_write failed` warning

Diff stats: +61 / -25 across 3 files.

🤖 Generated with Claude Code

[ten9876]

@AetherClaude review this PR

[aethersdr-agent]

Review

Traced through all three fixes against src/core/HidDeviceParser.cpp:209-266, src/core/HidEncoderManager.cpp:157-237, and src/core/HidEncoderManager.h. Each one is correct and the rationale lines up with the actual call sites.

1. Parser — simultaneous-event fix ✅

The pre-fix bug is real and exactly as described: m_prevEncBtns = newState (line 233 pre-patch) commits all bit changes before the inner loop can iterate, so the second-changed bit gets swallowed because the next parse() sees changed == 0. The new m_prevEncBtns ^= (1u << i) consumes only the bit being reported, leaving the rest to be picked up next poll.

Hand-trace for "press dial 0 + dial 1 in the same HID report" (starting from m_prevEncBtns = 0b0000):

poll	buf bits	newState	changed	i=0?	action	m_prev after
1	1,1,0,0	0b0011	0b0011	yes	emit btn 9 press	0b0001
2	1,1,0,0	0b0011	0b0010	no, i=1	emit btn 10 press	0b0011
3	0,0,0,0	0b0000	0b0011	yes	emit btn 9 release	0b0010
4	0,0,0,0	0b0000	0b0010	no, i=1	emit btn 10 release	0b0000

Matches your trace — symmetric on press and release, no state drift if the user lifts before the queue drains.

2. hid_write return check ✅

Correctly bails on written < 0 so a mid-stream unplug stops chewing through the remaining ~10 KB of feature reports per key. The log message is correlatable with the user-visible symptom, which is the actual diagnostic win here. Next poll() → hid_read() < 0 → close() → hotplug reopen path is intact.

Minor (non-blocking) observation: setKeyImages at src/core/HidEncoderManager.cpp:151-155 just loops setKeyImage(i, ...) for all 8 keys. If the device disappears partway through key 0, keys 1–7 will still each be entered and each do their first failed hid_write (since m_device is non-null until the poll loop hits close()) before returning. That's 7 wasted ~1 KB writes and packet builds per refresh — not catastrophic, and they all log, but if you want to suppress the log spam you could set a "device looks dead" flag inside the failure branch and check it in setKeyImage's preamble. Easy follow-up, not a reason to hold this PR.

3. std::atomic gate-check members ✅

The cross-thread access is real — verified m_hidEncoder->moveToThread(m_extCtrlThread) at src/gui/MainWindow.cpp:3920 and the main-thread reads at MainWindow.cpp:6778 (isOpen() + isStreamDeckPlus() in refreshStreamDeckLabels). Relaxed ordering is the right choice given the queued-connection design — the worker thread re-checks m_device at the top of every slot (setKeyImage:159, setTouchscreenImage:197), so a stale true from the main thread just means one extra queued no-op call, never a use-after-close.

The header comment block calling out the threading invariant is genuinely useful — future readers won't have to grep for the moveToThread call to understand why those members are atomic.

Verdict

Looks good to land. The fixes are minimal, well-scoped, and the comments in the code itself explain the why for each one (referencing #3248), which is the right level of inline documentation for non-obvious threading and protocol behavior.

A maintainer can authorize the orchestrator to land this fix by adding the aetherclaude-eligible label.

73, Jeremy KK7GWY & Claude (AI dev partner)

---

_{🤖 aethersdr-agent · cost: $2.0181 · model: claude-opus-4-7}