From 2e07b112ff47f48a2a99c75180b8448efb2f8f93 Mon Sep 17 00:00:00 2001 From: Ken Sanislo Date: Mon, 27 Apr 2026 18:10:32 -0700 Subject: [PATCH] G522 bug batch from log df178225 MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Five fixes from analysis of the latest user log: A. HeadsetMicMute.build() now matches device.product_id as the hex string ("0B18", "0B19") rather than ints. product_id is set as a str by hidapi_impl.f"{pid:04X}", so the int comparison was always False and the suppression never fired. The G522 was still being asked to write mic-mute on each connect and erroring out 0x0A. B. RGB-effects probe sub-device feature dump tolerates "unknown:HHHH" string features. _format_feature now detects that shape explicitly (and renders 0xHHHH from the suffix) instead of letting int(feat) raise ValueError mid-iteration and abort the whole table dump. C. HeadsetActiveEQPreset.write replaces the bare `_value = None` cache invalidation with a synchronous read(cached=False) so _value is a real dict before returning. Prevents a UI band-click crash with 'NoneType' object is not subscriptable when a user clicks an EQ band before the panel re-reads after a preset switch. D. New probe_advanced_eq_slots() iterates every advertised slot via getCustomEQ at build time, logs which respond, and caches a list of (slot, name, bands) on device._advanced_eq_working_slots. The HeadsetActiveEQPreset selector builds choices only from working slots and returns None from build() if ≤1 slot responds — G522's firmware advertises 16 slots but only honors slot 0, so the user no longer sees a 16-option dropdown they can't actually use. HeadsetAdvancedEQ.build now reuses the same probe (cached) rather than the old probe_all_presets path. The legacy alias is kept in the hidpp20 facade for any external callers still on it. E. (capture only — no parse change yet) get_advanced_eq_params and get_advanced_eq_defaults now log raw= on success too. The two functions decode the same slot to wildly different bands on G522 (likely a different header size between getCustomEQ's reply and getEQDefaults's reply); raw bytes will let us pin down the exact framing difference and adjust the parser in a follow-up. Fix path will land alongside docs/features.md notes documenting the per-call format variation, since other 0x020D headsets may diverge differently from G522. --- lib/logitech_receiver/advanced_para_eq.py | 82 ++++++++++++++------- lib/logitech_receiver/hidpp20.py | 1 + lib/logitech_receiver/rgb_effects_probe.py | 19 ++++- lib/logitech_receiver/settings_templates.py | 49 +++++++----- 4 files changed, 103 insertions(+), 48 deletions(-) diff --git a/lib/logitech_receiver/advanced_para_eq.py b/lib/logitech_receiver/advanced_para_eq.py index 91ca594b..93412a19 100644 --- a/lib/logitech_receiver/advanced_para_eq.py +++ b/lib/logitech_receiver/advanced_para_eq.py @@ -253,11 +253,16 @@ def get_advanced_eq_defaults(device, direction=DIRECTION_PLAYBACK, slot=0): result.hex(), ) return None + # Log raw=... too — getEQDefaults appears to use a different header + # framing than getCustomEQ on G522 (decoded values come out shifted + # by a byte). Capture the raw bytes so we can pin down the actual + # layout difference and adjust the parser accordingly. logger.info( - "AdvancedParaEQ getEQDefaults V2 (dir=%d slot=%d): %d band(s) %s", + "AdvancedParaEQ getEQDefaults V2 (dir=%d slot=%d): %d band(s) raw=%s %s", direction, slot, len(bands), + result.hex(), [_band_label(t, f) + f" {round(g, 2)}dB" for t, f, g in bands], ) return bands @@ -296,44 +301,62 @@ def get_advanced_eq_friendly_name(device, direction=DIRECTION_PLAYBACK, slot=0): return name -def probe_all_presets(device, direction=DIRECTION_PLAYBACK): - """Read every factory + custom preset slot and log name + band data at INFO. +def probe_advanced_eq_slots(device, direction=DIRECTION_PLAYBACK, info=None): + """Probe every advertised EQ slot via getCustomEQ and cache which respond. - Diagnostic probe — intended to run once at HeadsetAdvancedEQ.build() time. - The logged corpus is useful for spotting filter-type or frequency pattern - differences between named presets. + Some firmware (G522) advertises N slots via getEQInfos but only honors a + subset for getCustomEQ / setActiveEQ — the rest return 0x0B NOT_SUPPORTED. + This iterates 0..total-1 and records which slots actually have data. + + Result is cached on `device._advanced_eq_working_slots` as a list of + `(slot_index, name, bands)` tuples. The HeadsetActiveEQPreset selector + builds its choices from this list; the HeadsetAdvancedEQ panel uses it + to skip dead slots in its diagnostic output. + + Logs each working slot's bands at INFO and a summary line indicating + how many of the advertised slots are actually accessible. """ - info = getattr(device, "_advanced_eq_info", None) + cached = getattr(device, "_advanced_eq_working_slots", None) + if cached is not None: + return cached + if info is None: + info = getattr(device, "_advanced_eq_info", None) or get_advanced_eq_info(device) if not info: - return - ro_count = info.get("onboard_ro_preset_count", 0) - custom_count = info.get("onboard_custom_preset_count", 0) - for slot in range(ro_count): - name = get_advanced_eq_friendly_name(device, direction=direction, slot=slot) - bands = get_advanced_eq_defaults(device, direction=direction, slot=slot) - if bands is None: - continue - logger.info( - "AdvancedParaEQ factory preset %d/%d (dir=%d) name=%r: %s", - slot, - ro_count, - direction, - name, - [f"{_band_label(t, f)} {round(g, 2)}dB" for t, f, g in bands], - ) - for slot in range(custom_count): - name = get_advanced_eq_friendly_name(device, direction=direction, slot=slot) + return [] + ro_count = info.get("onboard_ro_preset_count", 0) or 0 + custom_count = info.get("onboard_custom_preset_count", 0) or 0 + total = ro_count + custom_count + if total == 0: + return [] + working = [] + for slot in range(total): bands = get_advanced_eq_params(device, direction=direction, slot=slot) if bands is None: continue + name = get_advanced_eq_friendly_name(device, direction=direction, slot=slot) + kind = "factory" if slot < ro_count else "custom" logger.info( - "AdvancedParaEQ custom preset %d/%d (dir=%d) name=%r: %s", + "AdvancedParaEQ %s preset slot=%d (dir=%d) name=%r: %s", + kind, slot, - custom_count, direction, name, [f"{_band_label(t, f)} {round(g, 2)}dB" for t, f, g in bands], ) + working.append((slot, name, bands)) + device._advanced_eq_working_slots = working + logger.info( + "AdvancedParaEQ working slots on dir=%d: %d of %d advertised %s", + direction, + len(working), + total, + [w[0] for w in working], + ) + return working + + +# Backward-compat alias kept until external callers are migrated. +probe_all_presets = probe_advanced_eq_slots def get_advanced_eq_params(device, direction=DIRECTION_PLAYBACK, slot=0): @@ -371,12 +394,15 @@ def get_advanced_eq_params(device, direction=DIRECTION_PLAYBACK, slot=0): ) return None step_db = info["step_db"] if info and "step_db" in info else 1.0 + # Log raw=... too so we can compare wire shapes across firmware + # variants and across get-fns (getCustomEQ vs getEQDefaults). logger.info( - "AdvancedParaEQ getCustomEQ V2 (dir=%d slot=%d): %d band(s) step_db=%.4f %s", + "AdvancedParaEQ getCustomEQ V2 (dir=%d slot=%d): %d band(s) step_db=%.4f raw=%s %s", direction, slot, len(bands), step_db, + result.hex(), [f"{_band_label(t, f)} {round(g, 2)}dB" for t, f, g in bands], ) return bands diff --git a/lib/logitech_receiver/hidpp20.py b/lib/logitech_receiver/hidpp20.py index 886f30cf..6b96d821 100644 --- a/lib/logitech_receiver/hidpp20.py +++ b/lib/logitech_receiver/hidpp20.py @@ -2328,6 +2328,7 @@ from .advanced_para_eq import get_advanced_eq_friendly_name # noqa: E402, F401 from .advanced_para_eq import get_advanced_eq_info # noqa: E402, F401 from .advanced_para_eq import get_advanced_eq_params # noqa: E402, F401 from .advanced_para_eq import parse_v2_bands # noqa: E402, F401 +from .advanced_para_eq import probe_advanced_eq_slots # noqa: E402, F401 from .advanced_para_eq import probe_all_presets as probe_advanced_eq_presets # noqa: E402, F401 from .onboard_eq import _build_set_eq_payload # noqa: E402, F401 from .onboard_eq import get_onboard_eq_info # noqa: E402, F401 diff --git a/lib/logitech_receiver/rgb_effects_probe.py b/lib/logitech_receiver/rgb_effects_probe.py index ea0f6229..a30ba932 100644 --- a/lib/logitech_receiver/rgb_effects_probe.py +++ b/lib/logitech_receiver/rgb_effects_probe.py @@ -22,13 +22,26 @@ def _hex_or_none(data) -> str | None: def _format_feature(feat) -> str: - """Render a feature for the log: 0x{id:04X}{:NAME} when known, else raw.""" + """Render a feature for the log: 0x{id:04X}{:NAME} when known, else raw. + + Unknown features are stored as the string "unknown:HHHH" by the feature + discovery code, so handle that shape explicitly — int(feat) on those + raises ValueError. Wrap the rest in a broad except so a future unhandled + feature shape can't kill the whole table dump. + """ if feat is None: return "?" + if isinstance(feat, str): + if feat.startswith("unknown:") and len(feat) > 8: + return f"0x{feat[8:].upper()}" + return feat try: return f"0x{int(feat):04X}:{feat.name}" - except (AttributeError, TypeError): - return f"0x{int(feat):04X}" if feat is not None else "?" + except (AttributeError, TypeError, ValueError): + try: + return f"0x{int(feat):04X}" + except (TypeError, ValueError): + return repr(feat) def _log_feature_table(device) -> None: diff --git a/lib/logitech_receiver/settings_templates.py b/lib/logitech_receiver/settings_templates.py index 3c07953c..fb389a55 100644 --- a/lib/logitech_receiver/settings_templates.py +++ b/lib/logitech_receiver/settings_templates.py @@ -1628,7 +1628,9 @@ class HeadsetMicMute(settings.Setting): # switch doesn't drive this feature anyway — G HUB attempts and # silently ignores failures. Hide the toggle on G522 PIDs (0x0B18 # wireless, 0x0B19 wired) so users don't see a permanently-broken UI. - if getattr(device, "product_id", None) in (0x0B18, 0x0B19): + # product_id is the uppercase hex string form set by hidapi_impl + # (`f"{pid:04X}"`), so compare against strings rather than ints. + if getattr(device, "product_id", None) in ("0B18", "0B19"): return None return super().build(device) @@ -1953,12 +1955,14 @@ class HeadsetAdvancedEQ(settings.RangeFieldSetting): gain_max, step_db, ) - # One-shot corpus probe — logs every factory + custom preset's - # band data at INFO. Useful diagnostic if a device turns up with - # filter types beyond the observed 0x00/0x78 pair. + # One-shot per-slot probe — logs band data for each slot the + # firmware actually honors and caches the working-slot list on + # `device._advanced_eq_working_slots`. Cheap if HeadsetActiveEQPreset + # already populated the cache (it usually has at this point); + # otherwise this is the first-time probe. if version >= 2: try: - hidpp20.probe_advanced_eq_presets(device, direction=0) + hidpp20.probe_advanced_eq_slots(device, direction=0, info=info) except Exception as e: logger.info("HeadsetAdvancedEQ.build: preset corpus probe failed: %s", e) return v @@ -2035,13 +2039,19 @@ class HeadsetActiveEQPreset(settings.Setting): if not info: return None ro_count = info.get("onboard_ro_preset_count", 0) or 0 - custom_count = info.get("onboard_custom_preset_count", 0) or 0 - total = ro_count + custom_count - if total == 0: + # Probe each advertised slot — getEQInfos may report capacity that + # the firmware doesn't actually back (G522 advertises 16 slots but + # only honors slot 0). Only include slots that responded with band + # data; the result is cached on device._advanced_eq_working_slots + # so HeadsetAdvancedEQ.build can reuse it without re-probing. + working = hidpp20.probe_advanced_eq_slots(device, direction=0, info=info) + if len(working) <= 1: + # One option (or zero) is meaningless as a selector — there's + # nothing for the user to choose between. The active EQ is + # whatever slot 0 has, no preset switching is available. return None choices = common.NamedInts() - for slot in range(total): - slot_name = hidpp20.get_advanced_eq_friendly_name(device, direction=0, slot=slot) + for slot, slot_name, _bands in working: if not slot_name: slot_name = _("Slot") + " " + str(slot) if slot < ro_count: @@ -2054,15 +2064,20 @@ class HeadsetActiveEQPreset(settings.Setting): def write(self, value, save=True): result = super().write(value, save) if result is not None: - # Drop the AdvancedParaEQ band-display cache so the panel's - # next read pulls the new active slot's bands. The visible - # values update on the next refresh of that panel (manual - # refresh icon, panel reopen, or device reconnect) — pushing - # the redraw automatically would need UI-side plumbing we - # don't currently expose from the settings layer. + # After setActiveEQ, repopulate the AdvancedParaEQ band-display + # cache so the panel reflects the newly-active slot. Force a + # fresh read so _value is a real dict — leaving it as None + # would let a UI band-click hit `_value[item]` on None and + # crash (config_panel.py:589 'NoneType' is not subscriptable). + # The visible widget redraw still waits for a manual refresh / + # panel reopen — auto-redraw would need UI-side plumbing. eq_panel = _headset_setting_by_name(self._device, HeadsetAdvancedEQ.name) if eq_panel is not None: - eq_panel._value = None + try: + eq_panel._value = None + eq_panel.read(cached=False) + except Exception as e: + logger.info("HeadsetActiveEQPreset: failed to refresh EQ panel: %s", e) return result