Merge b89d7729ea into 5e4ae7261e
This commit is contained in:
grechmarlon
GitHub
commit
41b2839e87
|
|
@ -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,17 @@ 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 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
|
||||
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
|
||||
|
|
|
|||
|
|
@ -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.
|
||||
|
||||
|
|
|
|||
|
|
@ -322,15 +322,21 @@ 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
|
||||
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.
|
||||
device.apply_settings_if_needed()
|
||||
# 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)
|
||||
|
||||
|
|
|
|||
|
|
@ -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:
|
||||
|
|
@ -3871,12 +3888,37 @@ 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 gap
|
||||
placeholder, or junk from an older persisted config). A single bad
|
||||
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
|
||||
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 +4045,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 +4109,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 +4146,17 @@ 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 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:
|
||||
self.update_key_value(zone_id, value, save)
|
||||
if not self._device.online:
|
||||
|
|
|
|||
|
|
@ -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,6 +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; 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 = []
|
||||
|
|
|
|||
|
|
@ -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"
|
||||
|
|
|
|||
|
|
@ -672,3 +672,121 @@ 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()
|
||||
|
||||
|
||||
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
|
||||
|
||||
|
||||
# --- 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()
|
||||
|
|
|
|||
Loading…
Reference in New Issue