From c8a52ecd3cd861934c70936502e3455cf3a8ecde Mon Sep 17 00:00:00 2001 From: Ken Sanislo Date: Sun, 19 Apr 2026 00:12:52 -0700 Subject: [PATCH] Reject bridge responses for wrong sub-device function MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Field testing revealed cross-contamination between function calls on the same sub-device feature. A late-arriving MessageEvent for GetRGBZoneInfo (function 1) was being accepted as the response to a subsequent GetHostModeState (function 7) on the same feature (0x0620), because _is_bridge_response_for only matched on sub_feat_idx. Evidence from tester log: HeadsetRGBHostMode.write: before=b'\x08\x01\x02\x03\x04\x05\x06\x07\x08...' The "before" read of GetHostModeState returned what is clearly a GetRGBZoneInfo response (count=8, zones=[1..8]) queued from earlier. The device echoes our exact sub-device function byte (function<<4 | sw_id) in the response. Plumb that expected value from centurion_bridge_request through _is_bridge_response_for and reject any response whose sub_func_sw doesn't match. Also validate orig_func_sw for error responses. Also removes the zone_id==0 filter in HeadsetRGBColor._zone_ids — with proper response matching the device should now consistently report real zone IDs (G522 has 8 zones at IDs 1-8 per the delayed response capture), and filtering is no longer needed. --- lib/logitech_receiver/device.py | 26 ++++++++++++++++++--- lib/logitech_receiver/settings_templates.py | 5 ++-- 2 files changed, 25 insertions(+), 6 deletions(-) diff --git a/lib/logitech_receiver/device.py b/lib/logitech_receiver/device.py index 3fb7092b..f2f88c9e 100644 --- a/lib/logitech_receiver/device.py +++ b/lib/logitech_receiver/device.py @@ -766,6 +766,13 @@ class Device: if no_reply: return None + # The device echoes our exact sub-device function+swid byte in + # MessageEvent responses. Match on that to reject cross-contamination + # from late-arriving responses to other function calls on the same + # feature (e.g. GetRGBZoneInfo response showing up on a later + # GetHostModeState read). + expected_sub_func_sw = (sub_function & 0xF0) | sw_id + # Read ACK + MessageEvent response request_started = time.time() ack_received = False @@ -782,7 +789,7 @@ class Device: break if (func_sw >> 4) == 0x01 and (func_sw & 0x0F) == 0: # MessageEvent arrived before ACK — validate it's for our request - if self._is_bridge_response_for(reply_data, sub_feat_idx): + if self._is_bridge_response_for(reply_data, sub_feat_idx, expected_sub_func_sw): if logger.isEnabledFor(logging.DEBUG): logger.debug("bridge idx=%d fn=0x%02X -> OK", sub_feat_idx, sub_function) return self._parse_bridge_response(reply_data) @@ -800,7 +807,7 @@ class Device: if len(reply_data) >= 2 and reply_data[0] == bridge_idx: func_sw = reply_data[1] if (func_sw >> 4) == 0x01 and (func_sw & 0x0F) == 0: - if self._is_bridge_response_for(reply_data, sub_feat_idx): + if self._is_bridge_response_for(reply_data, sub_feat_idx, expected_sub_func_sw): if logger.isEnabledFor(logging.DEBUG): logger.debug("bridge idx=%d fn=0x%02X -> OK", sub_feat_idx, sub_function) return self._parse_bridge_response(reply_data) @@ -824,12 +831,19 @@ class Device: return False @staticmethod - def _is_bridge_response_for(reply_data, expected_sub_feat_idx): + def _is_bridge_response_for(reply_data, expected_sub_feat_idx, expected_sub_func_sw=None): """Check if a bridge MessageEvent is a response for our specific sub-feature request. Accepts both normal responses (sub_feat_idx matches) and error responses (sub_feat_idx=0xFF with original feat_idx in next byte). Unsolicited notifications (sub_cpl=0xFF) are rejected. + + If `expected_sub_func_sw` is provided, also matches on the echoed + sub-device function byte (`(function << 4) | sw_id`). This prevents + cross-talk between different function calls on the SAME feature, which + can happen when a late-arriving response for one function gets picked + up by a later request on the same feature (observed on G522 where a + GetRGBZoneInfo response contaminated a subsequent GetHostModeState). """ if len(reply_data) < 6: return False @@ -839,9 +853,15 @@ class Device: if sub_cpl != 0x00: return False if sub_feat_idx == expected_sub_feat_idx: + if expected_sub_func_sw is not None and len(reply_data) >= 7: + if reply_data[6] != expected_sub_func_sw: + return False return True # Error response: sub_feat_idx=0xFF, next byte is the original feat_idx that errored if sub_feat_idx == 0xFF and len(reply_data) >= 7 and reply_data[6] == expected_sub_feat_idx: + if expected_sub_func_sw is not None and len(reply_data) >= 8: + if reply_data[7] != expected_sub_func_sw: + return False return True return False diff --git a/lib/logitech_receiver/settings_templates.py b/lib/logitech_receiver/settings_templates.py index 709478c4..b9fc1da0 100644 --- a/lib/logitech_receiver/settings_templates.py +++ b/lib/logitech_receiver/settings_templates.py @@ -2077,9 +2077,9 @@ class HeadsetRGBColor(settings.Setting): # Tight format observed on G522: [count, zone_ids...] with no reserved gap. # The protocol doc shows 3-byte + 1-byte reserved gaps before zone IDs, but # the G522 sub-device packs them immediately after the count. + # Don't filter zone_id==0 — on some devices 0 is a valid zone ID. We trust + # the device to report sane zone IDs and let the device reject nonsense. tight = list(resp[1 : 1 + zone_count]) if 1 <= zone_count <= len(resp) - 1 else [] - # Filter out zero bytes that are just padding (zone IDs should be non-zero). - tight = [z for z in tight if z != 0] if tight and len(tight) == zone_count: logger.info( "HeadsetRGBColor: discovered %d zone(s) %s (tight format, raw resp=%s)", @@ -2091,7 +2091,6 @@ class HeadsetRGBColor(settings.Setting): return tight # Try the doc's format (with reserved gap) as fallback gap = list(resp[5 : 5 + zone_count]) if len(resp) >= 5 + zone_count else [] - gap = [z for z in gap if z != 0] if gap and len(gap) == zone_count: logger.info( "HeadsetRGBColor: discovered %d zone(s) %s (doc format, raw resp=%s)",