diff --git a/lib/logitech_receiver/device.py b/lib/logitech_receiver/device.py index 725a7259..4bff8bdf 100644 --- a/lib/logitech_receiver/device.py +++ b/lib/logitech_receiver/device.py @@ -463,7 +463,7 @@ class Device: if self.persister is not None: self.persister["_config_cookie"] = [cookie[0], cookie[1]] - def apply_settings_if_needed(self): + def apply_settings_if_needed(self, force=False): """Cookie-gated dedup helper for repeat WIRELESS_DEVICE_STATUS reconfig notifications on an already-active device. Skips when the live ConfigChange cookie matches the value stored by the most @@ -472,10 +472,14 @@ class Device: whose firmware resets the cookie to a fixed value would falsely match a stored cookie from a prior session and skip the apply the device actually needs. + `force` skips the cookie comparison (but still re-records): pass it + when the notification says the device just powered on — volatile + state (RGB buffers, host-mode lighting) is gone even on devices + whose ConfigChange cookie survives the power cycle (e.g. G915 TKL). Returns True if apply ran, False if it was skipped.""" if not self.online: return False - if self.protocol >= 2.0 and self.features and SupportedFeature.CONFIG_CHANGE in self.features: + if not force and self.protocol >= 2.0 and self.features and SupportedFeature.CONFIG_CHANGE in self.features: live = _hidpp20.get_configuration_cookie(self) if live and len(live) >= 2: stored = self.persister.get("_config_cookie") if self.persister else None diff --git a/lib/logitech_receiver/notifications.py b/lib/logitech_receiver/notifications.py index 933d2477..76516783 100644 --- a/lib/logitech_receiver/notifications.py +++ b/lib/logitech_receiver/notifications.py @@ -322,15 +322,18 @@ def _process_feature_notification(device: Device, notification: HIDPPNotificatio if notification.address == 0x00: if logger.isEnabledFor(logging.DEBUG): logger.debug("wireless status: %s", notification) - reason = "powered on" if notification.data[2] == 1 else None + powered_on = notification.data[2] == 1 + reason = "powered on" if powered_on else None if notification.data[1] == 1: # device is asking for software reconfiguration alert = Alert.NONE device.changed(active=True, alert=alert, reason=reason) # changed(active=True) already runs apply_settings_if_needed on # the first transition; for follow-up reconfig notifications # on an already-active device, fire the gate here so the - # cookie comparison decides whether to re-push. - device.apply_settings_if_needed() + # cookie comparison decides whether to re-push. A power-on + # forces the apply: volatile state (RGB buffers, host-mode + # lighting) is gone even when the cookie survived the cycle. + device.apply_settings_if_needed(force=powered_on) else: logger.warning("%s: unknown WIRELESS %s", device, notification) diff --git a/lib/logitech_receiver/settings_templates.py b/lib/logitech_receiver/settings_templates.py index fbaf109f..e35ebe3b 100644 --- a/lib/logitech_receiver/settings_templates.py +++ b/lib/logitech_receiver/settings_templates.py @@ -3871,12 +3871,35 @@ class PerKeyLighting(settings.Settings): return common.ColorInt(value) return value + def _valid_zones(self): + """Zone IDs the device reported, or None when no validator is + attached yet (then no filtering can happen).""" + choices = getattr(getattr(self, "_validator", None), "choices", None) + return {int(k) for k in choices} if choices is not None else None + + def _sanitize_map(self, value): + """Drop zone keys the device never reported (e.g. the editor's -1 + placeholder, or junk from an older persisted config). A single bad + key would otherwise abort the whole frame at pack time.""" + valid = self._valid_zones() + if not isinstance(value, dict) or valid is None: + return value + bad = [k for k in value if int(k) not in valid] + if bad: + logger.warning("%s: dropping invalid per-key zones %s", self.name, bad) + value = {k: v for k, v in value.items() if int(k) in valid} + return value + def update(self, value, save=True): if isinstance(value, dict): - value = {k: self._wrap_color(v) for k, v in value.items()} + value = {k: self._wrap_color(v) for k, v in self._sanitize_map(value).items()} super().update(value, save) def update_key_value(self, key, value, save=True): + valid = self._valid_zones() + if valid is not None and int(key) not in valid: + logger.warning("%s: ignoring write to invalid per-key zone %s", self.name, key) + return super().update_key_value(key, self._wrap_color(value), save) def _sw_control_held(self): @@ -4003,6 +4026,9 @@ class PerKeyLighting(settings.Settings): # live line — it now matches what's actually on the keyboard. self._pre_read(cached) if self._value is not None: + # The persisted map may carry zones from a buggy editor or an + # older config — heal it here so they age out on the next save. + self._value = self._sanitize_map(self._value) return self._value reply_map = {} for key in self._validator.choices: @@ -4064,6 +4090,7 @@ class PerKeyLighting(settings.Settings): def write(self, map, save=True): if self._device.online: + map = self._sanitize_map(map) # the frame pack below can't take invalid zones # Persist undimmed (single source of truth). self.update(map, save) if not self._sw_control_held(): @@ -4100,6 +4127,10 @@ class PerKeyLighting(settings.Settings): def write_key_value(self, key, value, save=True): no_change = special_keys.COLORSPLUS["No change"] zone_id = int(key) + valid = self._valid_zones() + if valid is not None and zone_id not in valid: + logger.warning("%s: ignoring write to invalid per-key zone %s", self.name, zone_id) + return value if value != no_change: if self._value is None: self.read() # 0x8081 is write-only — read() loads persisted state or the sentinel map diff --git a/lib/solaar/ui/perkey/canvas.py b/lib/solaar/ui/perkey/canvas.py index 995f0a50..bdead4bc 100644 --- a/lib/solaar/ui/perkey/canvas.py +++ b/lib/solaar/ui/perkey/canvas.py @@ -438,6 +438,9 @@ class KeyboardCanvas(Gtk.DrawingArea): delta: dict[int, int] = {} if tool is not None: delta = tool.compute(self._press_cell, self._motion_cell, list(self._brush_path), ctx) + # Strokes can sweep over layout gaps, which are placeholder cells + # with zone_id -1 — not real zones; they must never reach the sink. + delta = {z: c for z, c in delta.items() if int(z) >= 0} self._press_cell = None self._motion_cell = None self._brush_path = [] diff --git a/tests/logitech_receiver/test_rgb_power.py b/tests/logitech_receiver/test_rgb_power.py index 45dbae42..667a8595 100644 --- a/tests/logitech_receiver/test_rgb_power.py +++ b/tests/logitech_receiver/test_rgb_power.py @@ -705,3 +705,61 @@ def test_perkey_write_key_value_initializes_unread_value(monkeypatch): assert s._value[7] == 0xFF0000 s._send_zone_color.assert_called_once() + + +def _perkey_under_test(monkeypatch, choices): + """PerKeyLighting wired to a static-zone device with mocked wire sends.""" + from unittest.mock import MagicMock + + from logitech_receiver import settings_templates + + static_zone = _FakeZoneSetting("rgb_zone_1", _ValueWithID(0x01)) + s = settings_templates.PerKeyLighting.__new__(settings_templates.PerKeyLighting) + device = MagicMock() + device.online = True + device.settings = [static_zone] + device.kind = None # not a mouse — no artanis prep + s._device = device + s._value = None + s._validator = MagicMock() + s._validator.choices = choices + s._pre_read = lambda cached=True: None + s._has_rgb_effects = True + s._send_with_retry = MagicMock(return_value=True) + s._send_zone_color = MagicMock(return_value=True) + s._fill_unset_zones_with_base_color = MagicMock(return_value=True) + monkeypatch.setattr(rgb_power, "translate_for_device", lambda d, c: c) + monkeypatch.setattr(rgb_power, "get_manager", lambda d: None) + return s + + +def test_perkey_write_drops_invalid_zones(monkeypatch): + """A persisted or editor-supplied map can carry zones the device never + reported (e.g. the canvas' -1 gap placeholder). They must be dropped, + not abort the whole frame with OverflowError at pack time.""" + s = _perkey_under_test(monkeypatch, choices=[7]) + + s.write({-1: 0xFF0000, 7: 0x00FF00}) # must not raise + + assert -1 not in s._value + assert s._value[7] == 0x00FF00 + + +def test_perkey_write_key_value_ignores_invalid_zone(monkeypatch): + s = _perkey_under_test(monkeypatch, choices=[7]) + + s.write_key_value(-1, 0xFF0000) + + s._send_zone_color.assert_not_called() + + +def test_perkey_read_heals_poisoned_persisted_map(monkeypatch): + """A -1 zone saved by an older/buggy build must be dropped on read so + it ages out of the persisted config on the next save.""" + s = _perkey_under_test(monkeypatch, choices=[7]) + s._value = {-1: 0xFF0000, 7: 0x00FF00} # as loaded from persister + + result = s.read() + + assert -1 not in result + assert result[7] == 0x00FF00