From e844d0611db4bb45d469863dc438bf6ccf50a35c Mon Sep 17 00:00:00 2001 From: grechmarlon <55017324+grechmarlon@users.noreply.github.com> Date: Thu, 11 Jun 2026 18:22:25 +0200 Subject: [PATCH 1/4] Fix crash writing per-key lighting from the CLI PerKeyLighting.write_key_value dereferenced self._value without the initialization guard that the base Settings.write_key_value has. On a fresh CLI invocation (solaar config per-key-lighting ) nothing has read the setting yet, so _value is still None and the write crashes with 'NoneType' object does not support item assignment. The GUI never hits this because it reads all settings on attach. Initialize via read(), which for this write-only 0x8081 setting loads the persisted map or fabricates the all-"No change" sentinel map. Co-Authored-By: Claude Fable 5 --- lib/logitech_receiver/settings_templates.py | 2 ++ tests/logitech_receiver/test_rgb_power.py | 33 +++++++++++++++++++++ 2 files changed, 35 insertions(+) diff --git a/lib/logitech_receiver/settings_templates.py b/lib/logitech_receiver/settings_templates.py index a3891b35..fbaf109f 100644 --- a/lib/logitech_receiver/settings_templates.py +++ b/lib/logitech_receiver/settings_templates.py @@ -4101,6 +4101,8 @@ class PerKeyLighting(settings.Settings): no_change = special_keys.COLORSPLUS["No change"] zone_id = int(key) if value != no_change: + if self._value is None: + self.read() # 0x8081 is write-only — read() loads persisted state or the sentinel map self.update_key_value(zone_id, value, save) if not self._device.online: return value diff --git a/tests/logitech_receiver/test_rgb_power.py b/tests/logitech_receiver/test_rgb_power.py index cafe6d59..45dbae42 100644 --- a/tests/logitech_receiver/test_rgb_power.py +++ b/tests/logitech_receiver/test_rgb_power.py @@ -672,3 +672,36 @@ def test_perkey_write_key_value_skipped_when_zone_is_animation(monkeypatch): s.write_key_value(7, 0xFF0000) s._send_zone_color.assert_not_called() + + +def test_perkey_write_key_value_initializes_unread_value(monkeypatch): + """The CLI writes a single key without ever reading the setting, so + _value is still None. write_key_value must initialize it via read() + (persisted state or the sentinel map — 0x8081 is write-only) instead + of crashing with a None dereference in update_key_value.""" + 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] + s._device = device + s._value = None # fresh CLI invocation: nothing has read the setting + s._validator = MagicMock() + s._validator.choices = [7] + 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) + + s.write_key_value(7, 0xFF0000) + + assert s._value[7] == 0xFF0000 + s._send_zone_color.assert_called_once() From c26f3cd6eced98a8f471c864008224683bb6cd15 Mon Sep 17 00:00:00 2001 From: grechmarlon <55017324+grechmarlon@users.noreply.github.com> Date: Thu, 11 Jun 2026 20:11:00 +0200 Subject: [PATCH 2/4] Fix per-key state loss: invalid editor zones and power-cycle cookie skip MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit 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 --- lib/logitech_receiver/device.py | 8 ++- lib/logitech_receiver/notifications.py | 9 ++-- lib/logitech_receiver/settings_templates.py | 33 +++++++++++- lib/solaar/ui/perkey/canvas.py | 3 ++ tests/logitech_receiver/test_rgb_power.py | 58 +++++++++++++++++++++ 5 files changed, 105 insertions(+), 6 deletions(-) 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 From 19865f6b54200869b977c4bef66a44442d8f54c4 Mon Sep 17 00:00:00 2001 From: grechmarlon <55017324+grechmarlon@users.noreply.github.com> Date: Fri, 12 Jun 2026 06:27:29 +0200 Subject: [PATCH 3/4] address review: name gap sentinel, hoist init guard, scope power-on force MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit - GAP_ZONE_ID names the canvas' phantom gap-cell zone and the delta filter tests keys against it explicitly, so it can't be confused with the -1 "No change" color value; _sanitize_map's docstring states it filters keys only. - The _value init guard in PerKeyLighting.write_key_value moves above both branches — the un-set branch also mutates _value — matching the base class, with a note on why it tests `is None`. - A powered-on reconfig forces the apply only when the device was already marked active: on the inactive→active transition changed() just applied everything, so forcing would push a redundant duplicate frame. Regression test covers both cases. Co-Authored-By: Claude Fable 5 --- lib/logitech_receiver/device.py | 9 ++++++--- lib/logitech_receiver/notifications.py | 17 ++++++++++------- lib/logitech_receiver/settings_templates.py | 15 +++++++++++---- lib/solaar/ui/perkey/canvas.py | 14 ++++++++++---- tests/logitech_receiver/test_notifications.py | 16 ++++++++++++++++ 5 files changed, 53 insertions(+), 18 deletions(-) diff --git a/lib/logitech_receiver/device.py b/lib/logitech_receiver/device.py index 4bff8bdf..575d6c7a 100644 --- a/lib/logitech_receiver/device.py +++ b/lib/logitech_receiver/device.py @@ -473,9 +473,12 @@ class Device: 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). + when the device powered on while already marked active — no + activation apply ran in changed(), yet volatile state (RGB buffers, + host-mode lighting) is gone even on devices whose ConfigChange + cookie survives the power cycle (e.g. G915 TKL). When the device + was inactive, changed() just applied everything, so forcing here + would only push a redundant duplicate. Returns True if apply ran, False if it was skipped.""" if not self.online: return False diff --git a/lib/logitech_receiver/notifications.py b/lib/logitech_receiver/notifications.py index 76516783..132fa354 100644 --- a/lib/logitech_receiver/notifications.py +++ b/lib/logitech_receiver/notifications.py @@ -326,14 +326,17 @@ def _process_feature_notification(device: Device, notification: HIDPPNotificatio reason = "powered on" if powered_on else None if notification.data[1] == 1: # device is asking for software reconfiguration alert = Alert.NONE + was_active = bool(device.online) # before changed() updates it 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. 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) + # changed(active=True) applies all settings unconditionally on + # the inactive→active transition; for follow-up reconfig + # notifications on an already-active device, fire the gate here + # so the cookie comparison decides whether to re-push. A + # power-on while already marked active forces the apply: no + # activation apply ran, yet volatile state (RGB buffers, + # host-mode lighting) is gone even when the cookie survived + # the cycle (e.g. G915 TKL). + device.apply_settings_if_needed(force=powered_on and was_active) 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 e35ebe3b..3bbc4896 100644 --- a/lib/logitech_receiver/settings_templates.py +++ b/lib/logitech_receiver/settings_templates.py @@ -3878,9 +3878,11 @@ class PerKeyLighting(settings.Settings): 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 + """Drop zone keys the device never reported (e.g. the editor's gap placeholder, or junk from an older persisted config). A single bad - key would otherwise abort the whole frame at pack time.""" + key would otherwise abort the whole frame at pack time. + Filters map KEYS only — the -1 "No change" sentinel as a map VALUE + is legitimate and handled separately in _send_perkey_frame.""" valid = self._valid_zones() if not isinstance(value, dict) or valid is None: return value @@ -4131,9 +4133,14 @@ class PerKeyLighting(settings.Settings): 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 self._value is None: + # 0x8081 is write-only — read() loads persisted state or the all- + # "No change" sentinel map. Both branches below mutate _value, so + # guard here (like the base class does). `is None` rather than the + # base class's falsy test: read() never yields an empty map here, + # so None is precisely "never initialized". + self.read() if value != no_change: - if self._value is None: - self.read() # 0x8081 is write-only — read() loads persisted state or the sentinel map self.update_key_value(zone_id, value, save) if not self._device.online: return value diff --git a/lib/solaar/ui/perkey/canvas.py b/lib/solaar/ui/perkey/canvas.py index bdead4bc..cea32d07 100644 --- a/lib/solaar/ui/perkey/canvas.py +++ b/lib/solaar/ui/perkey/canvas.py @@ -54,6 +54,11 @@ GUTTER_PX = 4 STRIP_GAP_PX = 16 PADDING_PX = 8 +# Phantom anchor zone for matrix gaps (positions with no key). Never a real +# zone, never sent to the device — distinct from the -1 "No change" color +# VALUE (special_keys.COLORSPLUS / control.NO_CHANGE). +GAP_ZONE_ID = -1 + class KeyboardCanvas(Gtk.DrawingArea): __gsignals__ = { @@ -178,7 +183,7 @@ class KeyboardCanvas(Gtk.DrawingArea): col = int((x - PADDING_PX) // (CELL_PX + GUTTER_PX)) row = int((y - PADDING_PX) // (CELL_PX + GUTTER_PX)) if 0 <= col < cols and 0 <= row < rows: - return BoundCell(cell=Cell(zone_id=-1, row=row, col=col), bound=False) + return BoundCell(cell=Cell(zone_id=GAP_ZONE_ID, row=row, col=col), bound=False) return None # ---- draw ---- @@ -438,9 +443,10 @@ 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} + # Strokes can sweep over layout gaps; drop their phantom cells so + # they never reach the sink. This filters KEYS — "No change" color + # VALUES (-1) pass through untouched. + delta = {z: c for z, c in delta.items() if z != GAP_ZONE_ID} self._press_cell = None self._motion_cell = None self._brush_path = [] diff --git a/tests/logitech_receiver/test_notifications.py b/tests/logitech_receiver/test_notifications.py index cbe51aab..8dc8a4f4 100644 --- a/tests/logitech_receiver/test_notifications.py +++ b/tests/logitech_receiver/test_notifications.py @@ -279,6 +279,22 @@ def test_process_feature_notification(mocker, hidpp_notification, feature): assert result is True +@pytest.mark.parametrize("was_active, expected_force", [(True, True), (False, False)]) +def test_wireless_power_on_reconfig_forces_apply_only_when_already_active(mocker, was_active, expected_force): + """A powered-on reconfig request must bypass the config-cookie gate + exactly when changed() ran no activation apply (device already active). + On the inactive→active transition changed() just applied everything, so + forcing there would push a redundant duplicate.""" + device = mocker.Mock() + device.online = was_active # state before changed() updates it + device.features.get_feature.return_value = SupportedFeature.WIRELESS_DEVICE_STATUS + hidpp_notification = HIDPPNotification(0, 0, sub_id=0x05, address=0x00, data=b"\x00\x01\x01") + + notifications._process_feature_notification(device, hidpp_notification) + + device.apply_settings_if_needed.assert_called_once_with(force=expected_force) + + def test_process_receiver_notification_invalid(mocker): invalid_sub_id = 0x30 notification_data = b"\x02" From 73adad2a522fab0d90935f3aed8a4bb47a5e0033 Mon Sep 17 00:00:00 2001 From: grechmarlon <55017324+grechmarlon@users.noreply.github.com> Date: Mon, 15 Jun 2026 11:27:23 +0200 Subject: [PATCH 4/4] =?UTF-8?q?G915=20TKL:=20device-gated=20RGB-claim=20qu?= =?UTF-8?q?irk=20=E2=80=94=20keep=20onboard=20mode=20on=20claim?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit A usbmon capture of G HUB's bring-up (receiver passed to a Win11 VM) showed the root cause of the dead F-row/media keys AND the F4 blackout: the RGB SW-control claim switched the keyboard to host mode (ONBOARD_PROFILES 0x10 0x02). On the G915 TKL host mode disables the F-row/media keys (#1100), and the host-mode transition made the firmware re-assert the M/MR indicator (overlaid on F1-F4 on this keyless model) over the per-key colours. G HUB drives the LEDs from onboard mode via the RGB SetSWControl claim alone, never switching. Gated by modelId in device_quirks.py so no other device changes: - rgb_claim_keeps_onboard_mode(): _claim_sw_control skips the onboard-> host switch for listed models -> F-keys/media keys work and the power-on F4 blackout is gone, with no timer and no G-key divert. - rgb_repaint_on_profile_change(): re-assert + repaint on a reported profile change so an accidental switch doesn't strand the scheme. Documented residual (cannot be fixed without breaking the keys): F1-F4 are the M1/M2/M3/MR indicator zones on the TKL. The firmware re-asserts the active-profile indicator on them on profile switch and after idle, so those keys can show the indicator instead of the user colour. Claiming that subsystem (G-key divert) would stop it but disables the F-row/media keys, so onboard mode + working keys is the chosen trade-off. Co-Authored-By: Claude Fable 5 --- lib/logitech_receiver/device_quirks.py | 32 +++++++++++++++++ lib/logitech_receiver/settings_templates.py | 39 +++++++++++++++------ tests/logitech_receiver/test_rgb_power.py | 27 ++++++++++++++ 3 files changed, 87 insertions(+), 11 deletions(-) diff --git a/lib/logitech_receiver/device_quirks.py b/lib/logitech_receiver/device_quirks.py index e2c09009..6fac2c52 100644 --- a/lib/logitech_receiver/device_quirks.py +++ b/lib/logitech_receiver/device_quirks.py @@ -69,6 +69,38 @@ HEADSET_SIGNATURE_EFFECTS_ALLOWED: dict[str, dict[int, set[str]]] = { } +# Keyboards whose firmware breaks the F-row / media keys when the RGB +# SW-control claim switches them to host mode (ONBOARD_PROFILES fn 0x10 mode +# 0x02) — Solaar #1100. A USB capture of G HUB's bring-up shows it drives the +# LEDs from onboard mode via the RGB SetSWControl claim alone, never making +# that switch. For these models the claim keeps onboard mode, which also avoids +# the host-mode transition that made the firmware re-assert the M/MR indicator +# over a per-key cell (the G915 TKL F4 blackout). +RGB_CLAIM_KEEPS_ONBOARD_MODE: set[str] = { + "B35F408EC343", # G915 TKL LIGHTSPEED — verified: host mode disables F-keys/media keys +} + +# Keyboards where switching onboard profiles drops the software per-key/zone +# paint (the firmware loads the switched-to profile's own onboard lighting). +# For these, re-assert the claim + repaint on a profile-change notification so +# an accidental profile switch doesn't strand the user's software scheme. +RGB_REPAINT_ON_PROFILE_CHANGE: set[str] = { + "B35F408EC343", # G915 TKL LIGHTSPEED +} + + +def rgb_claim_keeps_onboard_mode(device) -> bool: + """True when the RGB SW-control claim must NOT switch this model to host + mode (it would disable the F-row / media keys).""" + return (getattr(device, "modelId", None) or "") in RGB_CLAIM_KEEPS_ONBOARD_MODE + + +def rgb_repaint_on_profile_change(device) -> bool: + """True when a profile-change notification should re-apply the claimed + software RGB state on this model (the switch drops the per-key paint).""" + return (getattr(device, "modelId", None) or "") in RGB_REPAINT_ON_PROFILE_CHANGE + + def rgb_effects_nvconfig_allowed_fields(device, cap_id: int) -> set[str] | None: """Color fields to expose for an 0x8071 NvConfig boot effect. diff --git a/lib/logitech_receiver/settings_templates.py b/lib/logitech_receiver/settings_templates.py index 3bbc4896..8cc6957d 100644 --- a/lib/logitech_receiver/settings_templates.py +++ b/lib/logitech_receiver/settings_templates.py @@ -542,6 +542,18 @@ def profile_change(device, profile_sector): device.setting_callback(device, AdjustableDpi, [profile.resolutions[resolution_index]]) device.setting_callback(device, ReportRate, [profile.report_rate]) break + # A profile switch reloads that profile's onboard lighting, dropping the + # software per-key/zone paint. On models prone to this, re-assert the claim + # and repaint (only when Solaar already holds the LED claim) so an + # accidental profile switch doesn't strand the user's scheme. + if device_quirks.rgb_repaint_on_profile_change(device): + for s in device.settings or []: + if s.name == "rgb_control" and getattr(s, "_value", None): + try: + s._claim_sw_control(device) + except Exception as e: + logger.warning("%s: repaint after profile change failed: %s", device, e) + break class OnboardProfiles(settings.Setting): @@ -3330,11 +3342,16 @@ class RGBControl(settings.Setting): return value def _claim_sw_control(self, device): - # Disable firmware power management via profile management or onboard profiles - if device.features and _F.PROFILE_MANAGEMENT in device.features: - device.feature_request(_F.PROFILE_MANAGEMENT, 0x60, b"\x05") - elif device.features and _F.ONBOARD_PROFILES in device.features: - device.feature_request(_F.ONBOARD_PROFILES, 0x10, b"\x02") + # Disable firmware power management via profile management or onboard + # profiles. Skipped on models where the onboard->host mode switch + # disables the F-row / media keys (device_quirks, Solaar #1100): they + # drive the LEDs from onboard mode via the SetSWControl claim alone, + # exactly as G HUB does. + if not device_quirks.rgb_claim_keeps_onboard_mode(device): + if device.features and _F.PROFILE_MANAGEMENT in device.features: + device.feature_request(_F.PROFILE_MANAGEMENT, 0x60, b"\x05") + elif device.features and _F.ONBOARD_PROFILES in device.features: + device.feature_request(_F.ONBOARD_PROFILES, 0x10, b"\x02") # Claim LED pipeline: SetSWControl(mode=3, flags=5) device.feature_request(_F.RGB_EFFECTS, 0x50, rgb_power.SW_ACTIVE) # Reset per-key one-shot flags so the first write after this claim @@ -3349,15 +3366,15 @@ class RGBControl(settings.Setting): # Register cleanup for graceful release on device close if rgb_power.cleanup not in device.cleanups: device.cleanups.append(rgb_power.cleanup) - # Repaint LEDs with Solaar's saved state. Without this the firmware's - # last-active onboard profile keeps showing until the user changes - # something — the takeover would look like it did nothing. + # Repaint saved zone effects and the per-key buffer after the claim. + # (No settle-repaint timer needed: keeping onboard mode on the affected + # models avoids the host-mode transition that caused the F4 blackout.) self._repaint_after_claim(device) def _repaint_after_claim(self, device): - """Push saved zone effects and (if opted in) per-key buffer to the - device after a fresh SW claim. Best-effort: individual failures get - logged but don't abort the rest of the repaint.""" + """Push saved zone effects and the per-key buffer to the device after + a fresh SW claim. Best-effort: individual failures get logged but + don't abort the rest of the repaint.""" for s in device.settings: if s.name.startswith("rgb_zone_") and s._value is not None: try: diff --git a/tests/logitech_receiver/test_rgb_power.py b/tests/logitech_receiver/test_rgb_power.py index 667a8595..d614f598 100644 --- a/tests/logitech_receiver/test_rgb_power.py +++ b/tests/logitech_receiver/test_rgb_power.py @@ -763,3 +763,30 @@ def test_perkey_read_heals_poisoned_persisted_map(monkeypatch): assert -1 not in result assert result[7] == 0x00FF00 + + +# --- post-claim repaint ------------------------------------------------------ + + +def test_repaint_after_claim_pushes_zones_and_perkey(monkeypatch): + """The post-claim repaint pushes saved zone effects and the per-key + buffer. (The F4/MR blackout is handled structurally by keeping affected + models in onboard mode — device_quirks.rgb_claim_keeps_onboard_mode — not + by any timed repaint.)""" + from unittest.mock import MagicMock + + from logitech_receiver import settings_templates + + ctl = settings_templates.RGBControl.__new__(settings_templates.RGBControl) + zone = MagicMock() + zone.name = "rgb_zone_1" + zone._value = object() + device = MagicMock() + device.settings = [zone] + perkey = MagicMock() + perkey._value = {1: 0xFF0000} + monkeypatch.setattr(rgb_power, "perkey_has_paint", lambda d: (perkey, True)) + + ctl._repaint_after_claim(device) + zone.write.assert_called_once() + perkey.write.assert_called_once()