From 0baeb87294706c80e4b5102e867af89cec601d0c Mon Sep 17 00:00:00 2001 From: Ken Sanislo Date: Fri, 15 May 2026 00:54:14 -0700 Subject: [PATCH] HeadsetAdvancedEQ: validate persister against live read on apply MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Setting.apply uses cached=True so the persister is treated as the source of truth and the device's live state never wins. That model is correct for most settings, but it created a destructive bug for the 0x020D AdvancedParaEQ: 1. The V2 wire parser went through several iterations during G522 bring-up (commits bde3c3bc, 41db76bc, 59e3dcb7) with different strides and gain encodings before settling at 7c73c888. Each intermediate produced different decoded values from the same wire response; whatever a user's Solaar was on when they last apply'd got stored to the persister. 2. PerKeyLighting-style `prepare_write` silently fills missing band keys with 0 dB and clamps out-of-range gain values to the [gain_min, gain_max] rail. A partial/stale persister dict therefore encodes as a complete wire payload — looking valid to the device. 3. Writes were disabled in the early V2 builds (until be047fd9 on May 10). Once writes shipped, the next apply read the stale persister, prepare_write filled+clamped it, and setCustomEQ slot 0 overwrote whatever the user had configured. Observed on a G325 LIGHTSPEED user log: persister carried `{0: -6, 1: 1094}` from an older build; apply pushed `-6 +6 0 0 0 0 0 0 0 0` to slot 0 (band 1 clamped from 1094 to the +6 dB rail, bands 2-9 zero-filled), wiping the user's hand-tuned EQ. Fix: override apply for HeadsetAdvancedEQ. Validate the persister value against the current validator's count + gain range. If it's well-formed, push it normally (preserves Solaar's "user config is authoritative" model). If it's malformed (wrong key count, missing indices, out-of-range gain), do a live device read and reseed both _value and the persister from the device — without writing the corrupt persister back. If both are invalid, log a warning and skip this setting only; apply_all_settings continues with the rest. --- lib/logitech_receiver/settings_templates.py | 73 +++++++++++++++++++++ 1 file changed, 73 insertions(+) diff --git a/lib/logitech_receiver/settings_templates.py b/lib/logitech_receiver/settings_templates.py index 6d5394f2..bdd42014 100644 --- a/lib/logitech_receiver/settings_templates.py +++ b/lib/logitech_receiver/settings_templates.py @@ -2142,6 +2142,79 @@ class HeadsetAdvancedEQ(settings.RangeFieldSetting): return None return map + def _is_valid_persisted_value(self, value): + """True iff `value` is a well-formed band-gain dict for the current + validator: a dict with exactly `count` int keys covering [0, count), + each mapped to an int within [min_value, max_value]. Used to detect + stale persister entries from older Solaar builds whose V2 parser had + a different stride/header (commits prior to 7c73c888) and produced + partial dicts or out-of-range gain values.""" + validator = getattr(self, "_validator", None) + if not isinstance(value, dict) or validator is None: + return False + count = getattr(validator, "count", 0) + if count == 0 or len(value) != count: + return False + mn = getattr(validator, "min_value", None) + mx = getattr(validator, "max_value", None) + if mn is None or mx is None: + return False + for i in range(count): + if i not in value: + return False + v = value[i] + if not isinstance(v, int) or v < mn or v > mx: + return False + return True + + def apply(self): + """Validate the persisted EQ against the live device state before + pushing. Setting.apply uses cached=True so the persister is treated + as authoritative — that's wrong here because (a) the EQ can be + changed externally (LGHUB, onboard preset buttons) and (b) older + Solaar V2 parsers stored partial/out-of-range dicts that + prepare_write silently fills with 0 dB, overwriting user EQ with + zeros. Strategy: if the persisted value is well-formed, apply it + normally (matches existing Solaar semantics). If it's corrupt, + treat the device's live read as truth and reseed the persister + from it without writing back. If both are invalid, skip this + setting only — apply_all_settings keeps going.""" + assert hasattr(self, "_value") + assert hasattr(self, "_device") + if not self._device.online: + return + persister = getattr(self._device, "persister", None) + persisted = persister.get(self.name) if persister else None + persisted_valid = self._is_valid_persisted_value(persisted) + try: + live = self.read(cached=False) + except Exception as e: + logger.warning("%s: live EQ read failed during apply (%s): %s", self.name, self._device, repr(e)) + live = None + live_valid = self._is_valid_persisted_value(live) + if persisted_valid: + try: + self.write(persisted, save=False) + except Exception as e: + logger.warning("%s: error applying %s (%s): %s", self.name, persisted, self._device, repr(e)) + elif live_valid: + logger.info( + "%s: rejecting stale persister value %r; reseeding from device live %r", + self.name, + persisted, + live, + ) + self._value = live + if persister is not None: + persister[self.name] = live + else: + logger.warning( + "%s: both persisted (%r) and live (%r) values invalid; skipping apply", + self.name, + persisted, + live, + ) + class HeadsetActiveEQPreset(settings.Setting): """Choose which AdvancedParaEQ slot drives live audio.