Tracking three concerns flagged during review of PR #3236 (feat(hid): Elgato StreamDeck+ support). The feature shipped — these are robustness follow-ups, not blockers.

1. StreamDeckPlusParser::parse drops simultaneous events (silent input loss)

Both the encoder-push path and the LCD-key path in HidDeviceParser.cpp overwrite the previous-state mask before iterating to find changed bits, then return on the first one. If two keys (or two encoder pushes) change state in the same HID report, only the first event fires; the second is permanently silently dropped because the next poll() sees m_prevKeys == newState (no change).

Current shape (paraphrased):

uint8_t changed = newState ^ m_prevKeys;
if (changed) {
    m_prevKeys = newState;          // ← overwrites BEFORE the loop
    for (int i = 0; i < 8; ++i) {
        if (changed & (1u << i)) {
            return {.button = i + 1, ...};   // returns on FIRST changed bit
        }
    }
}

Suggested fix

Consume one bit at a time so subsequent polls re-enter and pick up the rest:

for (int i = 0; i < 8; ++i) {
    if (changed & (1u << i)) {
        m_prevKeys ^= (1u << i);     // flip just this bit
        return {.button = i + 1, ...};
    }
}

Or, alternatively, have parse() return a small QList<HidEvent> and let HidEncoderManager::poll() drain it.

Severity

Low frequency, but a silent failure mode that's frustrating to debug if a user ever notices a missed press. Recommend the bit-flip fix — three lines per path.

2. hid_write return values ignored on hot-unplug

HidEncoderManager::setKeyImage (~line 182) and setTouchscreenImage (~line 232) both loop through 1024-byte feature reports and ignore the hid_write return:

while (offset < totalBytes) {
    // ... build packet ...
    hid_write(m_device, pkt, PACKET_SIZE);
    offset += chunkLen;
}

If the device is unplugged mid-stream, the loop spins through all remaining packets writing into a dead handle with zero diagnostics. The user sees "deck went blank" but the log shows nothing.

Suggested fix

int rc = hid_write(m_device, pkt, PACKET_SIZE);
if (rc < 0) {
    qCWarning(lcExtCtrl) << \"StreamDeck+ hid_write failed at page\" << pageNumber
                         << \"— closing handle for hotplugCheck to reopen\";
    closeDevice();   // or set a flag for the next hotplugCheck cycle
    return;
}

Plus optionally make hotplugCheck() reopen on a closed-by-write-failure handle.

3. Thread-safety race on isOpen() / isStreamDeckPlus() gate

refreshStreamDeckLabels() runs on the main thread and reads:

• m_hidEncoder->isOpen() → inspects m_device
• m_hidEncoder->isStreamDeckPlus() → inspects m_openVid / m_openPid

Those members are mutated on m_extCtrlThread in poll() / hotplugCheck() without synchronization. Actual image writes ARE correctly marshalled via QMetaObject::invokeMethod(…, Qt::QueuedConnection), so the worst case is a wasted queued call against a closed handle (the queued lambda re-checks m_device and returns early — safe).

Severity

Benign on common platforms but technically a data race. Mostly worth tightening for future-proofing if the rate of label refreshes ever grows (e.g. live S-meter on the touchscreen).

Suggested fix

Either:

• Make m_device / m_openVid / m_openPid std::atomic<…> with appropriate memory order, or
• Move the gate check inside the queued lambda so reads happen on the worker thread that owns the writes.

Refs

• PR feat(hid): Elgato StreamDeck+ support — encoders, LCD buttons, touchscreen labels (#1510) #3236 — original StreamDeck+ implementation
• Bot review on feat(hid): Elgato StreamDeck+ support — encoders, LCD buttons, touchscreen labels (#1510) #3236 — flagged all three independently
• src/core/HidDeviceParser.cpp — concerns PHONE applet: DEXP commands rejected on firmware v1.4.0.0 #1
• src/core/HidEncoderManager.cpp — concerns build(deps): Bump github/codeql-action from 3 to 4 #2 and build(deps): Bump actions/checkout from 4 to 6 #3

73, Jeremy KK7GWY & Claude (AI dev partner)