RomanNum3ral
/
Solaar
mirror of https://github.com/pwr-Solaar/Solaar.git
1
0
Fork 0
Code Issues Packages Projects Releases Wiki Activity

Reject bridge responses for wrong sub-device function 

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.
This commit is contained in:
Ken Sanislo 2026-04-19 00:12:52 -07:00
parent e09f0de107
commit c8a52ecd3c
2 changed files with 25 additions and 6 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

26
lib/logitech_receiver/device.py
View File

@ -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

5
lib/logitech_receiver/settings_templates.py
View File

@ -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)",