Fix per-key state loss: invalid editor zones and power-cycle cookie skip

Three related fixes for per-key lighting state being lost on the wire:

1. The per-key canvas models layout gaps as placeholder cells with
   zone_id -1. Paint strokes sweeping a gap emitted -1 in the delta,
   which entered the setting's map, was persisted, and then aborted
   EVERY subsequent full-map repaint at frame pack time
   (OverflowError: can't convert negative int to unsigned) — the apply
   was logged and ignored, so keys silently kept stale colors or went
   dark. Filter gap cells from the delta before it reaches the sink.

2. PerKeyLighting now sanitizes zone keys against the device-reported
   zone bitmap in update/write/read paths, so an already-poisoned
   persisted map (saved by a build without fix 1) heals on read instead
   of permanently breaking repaints.

3. WIRELESS_DEVICE_STATUS power-on reconfig requests went through the
   ConfigChange-cookie dedup gate. On a G915 TKL the cookie survives a
   power cycle, so the gate concluded nothing changed and skipped
   apply_all_settings — but power-on wipes volatile state (RGB buffers,
   host-mode lighting), leaving the keyboard dark until manual
   intervention. A powered-on notification now forces the apply.

Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>

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

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)
 

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

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 = []

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