diff --git a/src/core/HidDeviceParser.cpp b/src/core/HidDeviceParser.cpp index 45f24cb8..fe0351aa 100644 --- a/src/core/HidDeviceParser.cpp +++ b/src/core/HidDeviceParser.cpp @@ -223,19 +223,21 @@ 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}; } } } @@ -243,20 +245,20 @@ HidEvent StreamDeckPlusParser::parse(const uint8_t* buf, size_t len) } 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 {}; diff --git a/src/core/HidEncoderManager.cpp b/src/core/HidEncoderManager.cpp index 1f91be6a..2a715726 100644 --- a/src/core/HidEncoderManager.cpp +++ b/src/core/HidEncoderManager.cpp @@ -183,7 +183,18 @@ void HidEncoderManager::setKeyImage(int key, const QByteArray& jpegData) pkt[7] = static_cast((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++; diff --git a/src/core/HidEncoderManager.h b/src/core/HidEncoderManager.h index 63b40441..984adabb 100644 --- a/src/core/HidEncoderManager.h +++ b/src/core/HidEncoderManager.h @@ -4,6 +4,7 @@ #include #include #include +#include #include #include #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 m_device{nullptr}; std::unique_ptr m_parser; QString m_deviceName; - uint16_t m_openVid{0}; - uint16_t m_openPid{0}; + std::atomic m_openVid{0}; + std::atomic m_openPid{0}; bool m_invertDirection{false}; QTimer* m_pollTimer{nullptr};