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

src/core/HidDeviceParser.cpp

@@ -223,40 +223,42 @@ HidEvent StreamDeckPlusParser::parse(const uint8_t* buf, size_t len)
                     return {.type = HidEvent::Rotate, .steps = delta, .encoderIndex = i};
             }
         } else if (subtype == 0x00) {
-            // Encoder push — return first changed encoder button
+            // Encoder push — return first changed encoder button.
+            // We commit only the bit we're reporting (not all of newState) so
+            // that if two buttons change in the same HID report, the next
+            // poll() iteration's `changed` still flags the unreported bit and
+            // emits it.  See #3248 follow-up from PR #3236 review.
             uint8_t newState = 0;
             for (int i = 0; i < 4; ++i) {
                 if (buf[5 + i]) newState |= (1u << i);
             }
             uint8_t changed = newState ^ m_prevEncBtns;
-            if (changed) {
-                m_prevEncBtns = newState;
-                for (int i = 0; i < 4; ++i) {
-                    if (changed & (1u << i)) {
-                        const int act = (newState & (1u << i)) ? 0 : 1;
-                        return {.type = HidEvent::Button, .button = 9 + i, .action = act};
-                    }
+            for (int i = 0; i < 4; ++i) {
+                if (changed & (1u << i)) {
+                    m_prevEncBtns ^= (1u << i);  // consume this bit only
+                    const int act = (newState & (1u << i)) ? 0 : 1;
+                    return {.type = HidEvent::Button, .button = 9 + i, .action = act};
                 }
             }
         }
         return {};
     }
 
     if (type == 0x00) {
-        // LCD key — return first changed key
+        // LCD key — return first changed key.  Same one-bit-at-a-time
+        // commit pattern as encoder push above so simultaneous presses
+        // are not silently dropped. (#3248)
         if (len < 12) return {};
         uint8_t newState = 0;
         for (int i = 0; i < 8; ++i) {
             if (buf[4 + i]) newState |= (1u << i);
         }
         uint8_t changed = newState ^ m_prevKeys;
-        if (changed) {
-            m_prevKeys = newState;
-            for (int i = 0; i < 8; ++i) {
-                if (changed & (1u << i)) {
-                    const int act = (newState & (1u << i)) ? 0 : 1;
-                    return {.type = HidEvent::Button, .button = i + 1, .action = act};
-                }
+        for (int i = 0; i < 8; ++i) {
+            if (changed & (1u << i)) {
+                m_prevKeys ^= (1u << i);  // consume this bit only
+                const int act = (newState & (1u << i)) ? 0 : 1;
+                return {.type = HidEvent::Button, .button = i + 1, .action = act};
             }
         }
         return {};

src/core/HidEncoderManager.cpp

@@ -183,7 +183,18 @@ void HidEncoderManager::setKeyImage(int key, const QByteArray& jpegData)
         pkt[7] = static_cast<uint8_t>((pageNumber >> 8) & 0xFF);
         std::memcpy(pkt + HEADER_SIZE, jpegData.constData() + offset, chunkLen);
 
-        hid_write(m_device, pkt, PACKET_SIZE);
+        // Bail on write failure so we don't spin through the remaining packets
+        // writing into a dead handle.  The next poll() will catch the bad
+        // handle via hid_read() < 0 and trigger close() + hotplug reopen,
+        // which correlates the user-visible "deck went blank" with logs. (#3248)
+        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;
+        }
 
         offset     += chunkLen;
         pageNumber++;
@@ -229,7 +240,14 @@ void HidEncoderManager::setTouchscreenImage(const QByteArray& jpegData,
         pkt[15] = 0x00;
         std::memcpy(pkt + HEADER_SIZE, jpegData.constData() + offset, chunkLen);
 
-        hid_write(m_device, pkt, PACKET_SIZE);
+        // Same bail-on-failure pattern as setKeyImage above. (#3248)
+        const int written = hid_write(m_device, pkt, PACKET_SIZE);
+        if (written < 0) {
+            qCWarning(lcDevices) << "HidEncoderManager::setTouchscreenImage: hid_write failed"
+                                 << "page=" << pageNumber
+                                 << "— device disconnected? Will retry on hotplug.";
+            return;
+        }
 
         offset     += chunkLen;
         pageNumber++;

src/core/HidEncoderManager.h

@@ -4,6 +4,7 @@
 #include <QObject>
 #include <QTimer>
 #include <QByteArray>
+#include <atomic>
 #include <memory>
 #include <hidapi/hidapi.h>
 #include "HidDeviceParser.h"
@@ -26,12 +27,21 @@ class HidEncoderManager : public QObject {
 
     bool open(uint16_t vid, uint16_t pid);
     void close();
-    bool isOpen() const { return m_device != nullptr; }
+    // These getters are called from the main thread (e.g. refreshStreamDeckLabels,
+    // status snapshot) while the worker thread on m_extCtrlThread mutates the
+    // backing members in open()/close()/hotplugCheck().  Underlying state is
+    // std::atomic so the cross-thread reads are well-defined; relaxed ordering
+    // is sufficient because callers treat the result as a hint and the real
+    // gate is re-checked inside the queued slot. (#3248)
+    bool isOpen() const { return m_device.load(std::memory_order_relaxed) != nullptr; }
     QString deviceName() const { return m_deviceName; }
-    uint16_t vendorId() const { return m_openVid; }
-    uint16_t productId() const { return m_openPid; }
+    uint16_t vendorId() const { return m_openVid.load(std::memory_order_relaxed); }
+    uint16_t productId() const { return m_openPid.load(std::memory_order_relaxed); }
     int encoderCount() const { return m_parser ? m_parser->encoderCount() : 1; }
-    bool isStreamDeckPlus() const { return m_openVid == 0x0FD9 && m_openPid == 0x0084; }
+    bool isStreamDeckPlus() const {
+        return m_openVid.load(std::memory_order_relaxed) == 0x0FD9
+            && m_openPid.load(std::memory_order_relaxed) == 0x0084;
+    }
 
     void setInvertDirection(bool invert) { m_invertDirection = invert; }
 
@@ -58,11 +68,17 @@ private slots:
     void hotplugCheck();
 
 private:
-    hid_device* m_device{nullptr};
+    // m_device + m_openVid + m_openPid are read from the main thread
+    // (isOpen / isStreamDeckPlus / vendorId / productId) and written from
+    // m_extCtrlThread (open / close / hotplugCheck).  std::atomic makes
+    // those cross-thread reads well-defined.  m_deviceName is also
+    // touched cross-thread but it's a const-after-open string used only
+    // in diagnostics so a brief stale read is benign. (#3248)
+    std::atomic<hid_device*> m_device{nullptr};
     std::unique_ptr<HidDeviceParser> m_parser;
     QString m_deviceName;
-    uint16_t m_openVid{0};
-    uint16_t m_openPid{0};
+    std::atomic<uint16_t> m_openVid{0};
+    std::atomic<uint16_t> m_openPid{0};
     bool m_invertDirection{false};
 
     QTimer* m_pollTimer{nullptr};