HeadsetAdvancedEQ: validate persister against live read on apply
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 (commitsbde3c3bc,41db76bc,59e3dcb7) with different strides and gain encodings before settling at7c73c888. 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 (untilbe047fd9on 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.
This commit is contained in:
parent
f4345b9ce0
commit
0baeb87294
|
|
@ -2142,6 +2142,79 @@ class HeadsetAdvancedEQ(settings.RangeFieldSetting):
|
||||||
return None
|
return None
|
||||||
return map
|
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):
|
class HeadsetActiveEQPreset(settings.Setting):
|
||||||
"""Choose which AdvancedParaEQ slot drives live audio.
|
"""Choose which AdvancedParaEQ slot drives live audio.
|
||||||
|
|
|
||||||
Loading…
Reference in New Issue