rgb_control: honor the off state — don't auto-claim, init, or shutdown LEDs
PerKeyLighting.write was force-claiming SW control via rgb_control.write(True) through _ensure_sw_control whenever a saved per-key map was applied. On startup with rgb_control persisted as False, the apply path would re-enable LED Control and overwrite the persister with True — making "off" impossible to keep across restarts. The fix treats rgb_control as the single gate for LED activity: when it's off, Solaar performs no SW claim, no per-key/zone wire writes, no SW release on apply (we never claimed), no profile-management restoration, and no shutdown-animation trigger on exit. This lets another tool (OpenRGB etc.) drive the LEDs without Solaar fighting it. Settings that don't actively change current lighting are still allowed: NV-config writes for startup/shutdown animations, brightness control (its own feature, no color push), and persister updates for per-key/zone state so colors survive an off→on toggle. UI: _apply_rgb_gates already greys per-key/zone/idle/sleep rows when rgb_control is off. Fix a race where the async-read completion callback in _update_setting_item would re-set sensitivity from the user lock-icon flag alone and undo the grey-out if rgb_control's read happened to complete first. Extract _gate_blocks as the single source of truth and AND-combine it into _update_setting_item's set_sensitive call.
This commit is contained in:
parent
3e88c73645
commit
4d1f9dc6c1
|
|
@ -324,8 +324,14 @@ def cleanup(device):
|
||||||
shutdown animation during the active→off transition. If the cap is
|
shutdown animation during the active→off transition. If the cap is
|
||||||
disabled, the firmware powers down LEDs silently. Matches LGHUB exit.
|
disabled, the firmware powers down LEDs silently. Matches LGHUB exit.
|
||||||
See solaar_shutdown_effect_trigger_spec.md.
|
See solaar_shutdown_effect_trigger_spec.md.
|
||||||
|
|
||||||
|
rgb_control is the gate: when LED Control is off, skip every wire write
|
||||||
|
here. We never claimed, so there's nothing to release; firing the shutdown
|
||||||
|
animation would visibly contradict the user's "leave my lighting alone".
|
||||||
"""
|
"""
|
||||||
stop(device)
|
stop(device)
|
||||||
|
if any(s.name == "rgb_control" and not s._value for s in getattr(device, "settings", []) or []):
|
||||||
|
return
|
||||||
try:
|
try:
|
||||||
device.feature_request(SupportedFeature.RGB_EFFECTS, 0x50, SW_RELEASE)
|
device.feature_request(SupportedFeature.RGB_EFFECTS, 0x50, SW_RELEASE)
|
||||||
if device.features and SupportedFeature.PROFILE_MANAGEMENT in device.features:
|
if device.features and SupportedFeature.PROFILE_MANAGEMENT in device.features:
|
||||||
|
|
|
||||||
|
|
@ -2053,8 +2053,13 @@ class RGBControl(settings.Setting):
|
||||||
logger.warning("%s: post-claim per-key repaint failed: %s", device, e)
|
logger.warning("%s: post-claim per-key repaint failed: %s", device, e)
|
||||||
|
|
||||||
def _release_sw_control(self, device):
|
def _release_sw_control(self, device):
|
||||||
# Stop software power management
|
# If we never claimed in this session, don't touch the device at all.
|
||||||
|
# The presence of an RGBPowerManager is the canonical "we claimed" signal
|
||||||
|
# — _claim_sw_control creates it via rgb_power.start, and stop() pops it.
|
||||||
|
had_claim = rgb_power.get_manager(device) is not None
|
||||||
rgb_power.stop(device)
|
rgb_power.stop(device)
|
||||||
|
if not had_claim:
|
||||||
|
return
|
||||||
# Release LED pipeline: SetSWControl(mode=0, flags=0)
|
# Release LED pipeline: SetSWControl(mode=0, flags=0)
|
||||||
device.feature_request(_F.RGB_EFFECTS, 0x50, rgb_power.SW_RELEASE)
|
device.feature_request(_F.RGB_EFFECTS, 0x50, rgb_power.SW_RELEASE)
|
||||||
# Restore firmware power management
|
# Restore firmware power management
|
||||||
|
|
@ -2456,6 +2461,10 @@ class RGBEffectSetting(LEDZoneSetting):
|
||||||
# Persist undimmed value first (single source of truth).
|
# Persist undimmed value first (single source of truth).
|
||||||
if self._value != value:
|
if self._value != value:
|
||||||
self.update(value, save)
|
self.update(value, save)
|
||||||
|
# rgb_control gate: skip wire when the user has LED Control off.
|
||||||
|
rgb_ctrl = next((s for s in device.settings if s.name == "rgb_control"), None)
|
||||||
|
if rgb_ctrl is not None and not rgb_ctrl._value:
|
||||||
|
return value
|
||||||
wire_value = self._translate_for_wire(value)
|
wire_value = self._translate_for_wire(value)
|
||||||
if wire_value is None: # SLEEPING — _wake() will re-push at full brightness.
|
if wire_value is None: # SLEEPING — _wake() will re-push at full brightness.
|
||||||
return value
|
return value
|
||||||
|
|
@ -2537,19 +2546,22 @@ class PerKeyLighting(settings.Settings):
|
||||||
def update_key_value(self, key, value, save=True):
|
def update_key_value(self, key, value, save=True):
|
||||||
super().update_key_value(key, self._wrap_color(value), save)
|
super().update_key_value(key, self._wrap_color(value), save)
|
||||||
|
|
||||||
def _ensure_sw_control(self):
|
def _sw_control_held(self):
|
||||||
"""Ensure SW control is claimed before writing per-key colors."""
|
"""Return True if it's safe to push LED bytes to the wire.
|
||||||
|
|
||||||
|
rgb_control is the gate: when the user has it off, LED writes must be
|
||||||
|
silent no-ops at the wire. Never auto-flip it on from here — doing so
|
||||||
|
rewrites the persister and turns the user-facing toggle into a lie."""
|
||||||
if getattr(self, "_has_rgb_effects", None) is None:
|
if getattr(self, "_has_rgb_effects", None) is None:
|
||||||
self._has_rgb_effects = bool(self._device.features and _F.RGB_EFFECTS in self._device.features)
|
self._has_rgb_effects = bool(self._device.features and _F.RGB_EFFECTS in self._device.features)
|
||||||
if not self._has_rgb_effects:
|
if not self._has_rgb_effects:
|
||||||
return # No autonomous effect engine, no claim needed
|
return True # No autonomous effect engine, no gate needed
|
||||||
for s in self._device.settings:
|
for s in self._device.settings:
|
||||||
if s.name == "rgb_control":
|
if s.name == "rgb_control":
|
||||||
# _value may be bool (current) or int (legacy persister value
|
# _value may be bool (current) or int 3/0 (legacy persister value
|
||||||
# before BooleanValidator migration); both coerce cleanly.
|
# before BooleanValidator migration); both coerce cleanly.
|
||||||
if not s._value: # Not already claimed by Solaar
|
return bool(s._value)
|
||||||
s.write(True) # Triggers full claim sequence in RGBControl
|
return True # rgb_control not on this device → no gate to enforce
|
||||||
return
|
|
||||||
|
|
||||||
# BUSY-retry backoff (ms).
|
# BUSY-retry backoff (ms).
|
||||||
_BUSY_BACKOFF_MS = (30, 60, 90)
|
_BUSY_BACKOFF_MS = (30, 60, 90)
|
||||||
|
|
@ -2719,9 +2731,10 @@ class PerKeyLighting(settings.Settings):
|
||||||
|
|
||||||
def write(self, map, save=True):
|
def write(self, map, save=True):
|
||||||
if self._device.online:
|
if self._device.online:
|
||||||
self._ensure_sw_control()
|
|
||||||
# Persist undimmed (single source of truth).
|
# Persist undimmed (single source of truth).
|
||||||
self.update(map, save)
|
self.update(map, save)
|
||||||
|
if not self._sw_control_held():
|
||||||
|
return map # gate is off — keep state in memory, skip the wire
|
||||||
# Per-key is a sub-mode of Static — when zone is animating, the
|
# Per-key is a sub-mode of Static — when zone is animating, the
|
||||||
# firmware engine owns the visible layer.
|
# firmware engine owns the visible layer.
|
||||||
if not rgb_power.zone_effect_is_static(self._device):
|
if not rgb_power.zone_effect_is_static(self._device):
|
||||||
|
|
@ -2752,13 +2765,14 @@ class PerKeyLighting(settings.Settings):
|
||||||
return map
|
return map
|
||||||
|
|
||||||
def write_key_value(self, key, value, save=True):
|
def write_key_value(self, key, value, save=True):
|
||||||
self._ensure_sw_control()
|
|
||||||
no_change = special_keys.COLORSPLUS["No change"]
|
no_change = special_keys.COLORSPLUS["No change"]
|
||||||
zone_id = int(key)
|
zone_id = int(key)
|
||||||
if value != no_change:
|
if value != no_change:
|
||||||
self.update_key_value(zone_id, value, save)
|
self.update_key_value(zone_id, value, save)
|
||||||
if not self._device.online:
|
if not self._device.online:
|
||||||
return value
|
return value
|
||||||
|
if not self._sw_control_held():
|
||||||
|
return value # gate is off — state stored, no wire push
|
||||||
# Per-key is a sub-mode of Static — defer to firmware animation.
|
# Per-key is a sub-mode of Static — defer to firmware animation.
|
||||||
if not rgb_power.zone_effect_is_static(self._device):
|
if not rgb_power.zone_effect_is_static(self._device):
|
||||||
return value
|
return value
|
||||||
|
|
@ -2788,6 +2802,8 @@ class PerKeyLighting(settings.Settings):
|
||||||
self.update_key_value(zone_id, no_change, save)
|
self.update_key_value(zone_id, no_change, save)
|
||||||
if not self._device.online:
|
if not self._device.online:
|
||||||
return no_change
|
return no_change
|
||||||
|
if not self._sw_control_held():
|
||||||
|
return no_change # gate is off — state stored, no wire push
|
||||||
if not rgb_power.zone_effect_is_static(self._device):
|
if not rgb_power.zone_effect_is_static(self._device):
|
||||||
return no_change
|
return no_change
|
||||||
zone_base = self._zone_base_color()
|
zone_base = self._zone_base_color()
|
||||||
|
|
|
||||||
|
|
@ -865,6 +865,17 @@ def _set_row_sensitive(device, name, can_function):
|
||||||
sbox._control.set_sensitive(user_allowed is True and can_function)
|
sbox._control.set_sensitive(user_allowed is True and can_function)
|
||||||
|
|
||||||
|
|
||||||
|
def _gate_blocks(device, name):
|
||||||
|
"""Single source of truth for "is this setting's row gated off?". Used by
|
||||||
|
`_apply_rgb_gates` to grey rows and by `_update_setting_item` so async-read
|
||||||
|
completions can't undo the grey-out when their callbacks land later."""
|
||||||
|
if name in _SW_CONTROL_DEPENDENT_NAMES or any(name.startswith(p) for p in _SW_CONTROL_DEPENDENT_PREFIXES):
|
||||||
|
return _sw_control_blocked(device)
|
||||||
|
if name == "per-key-lighting":
|
||||||
|
return _sw_control_blocked(device) or _zone_effect_blocks_perkey(device)
|
||||||
|
return False
|
||||||
|
|
||||||
|
|
||||||
def _apply_rgb_gates(device):
|
def _apply_rgb_gates(device):
|
||||||
"""Grey out RGB settings whose prerequisites aren't met. Visual-only:
|
"""Grey out RGB settings whose prerequisites aren't met. Visual-only:
|
||||||
leaves persister _sensitive flags (user lock-icon opt-ins) intact.
|
leaves persister _sensitive flags (user lock-icon opt-ins) intact.
|
||||||
|
|
@ -874,12 +885,14 @@ def _apply_rgb_gates(device):
|
||||||
- per-key-lighting needs LED Control = Solaar AND every zone effect on
|
- per-key-lighting needs LED Control = Solaar AND every zone effect on
|
||||||
Static (0x01), because non-Static zone animations mask per-key writes.
|
Static (0x01), because non-Static zone animations mask per-key writes.
|
||||||
"""
|
"""
|
||||||
sw_blocked = _sw_control_blocked(device)
|
|
||||||
for s in getattr(device, "settings", []) or []:
|
for s in getattr(device, "settings", []) or []:
|
||||||
if s.name in _SW_CONTROL_DEPENDENT_NAMES or any(s.name.startswith(p) for p in _SW_CONTROL_DEPENDENT_PREFIXES):
|
name = s.name
|
||||||
_set_row_sensitive(device, s.name, not sw_blocked)
|
if (
|
||||||
perkey_blocked = sw_blocked or _zone_effect_blocks_perkey(device)
|
name in _SW_CONTROL_DEPENDENT_NAMES
|
||||||
_set_row_sensitive(device, "per-key-lighting", not perkey_blocked)
|
or any(name.startswith(p) for p in _SW_CONTROL_DEPENDENT_PREFIXES)
|
||||||
|
or name == "per-key-lighting"
|
||||||
|
):
|
||||||
|
_set_row_sensitive(device, name, not _gate_blocks(device, name))
|
||||||
|
|
||||||
|
|
||||||
def _change_click(button, sbox):
|
def _change_click(button, sbox):
|
||||||
|
|
@ -1015,8 +1028,10 @@ def _create_sbox(s, _device):
|
||||||
def _update_setting_item(sbox, value, is_online=True, sensitive=True, null_okay=False):
|
def _update_setting_item(sbox, value, is_online=True, sensitive=True, null_okay=False):
|
||||||
sbox._spinner.stop()
|
sbox._spinner.stop()
|
||||||
sensitive = sbox._change_icon._allowed if sensitive is None else sensitive
|
sensitive = sbox._change_icon._allowed if sensitive is None else sensitive
|
||||||
|
name = sbox.setting.name
|
||||||
|
can_function = not _gate_blocks(sbox.setting._device, name)
|
||||||
if value is None and not null_okay:
|
if value is None and not null_okay:
|
||||||
sbox._control.set_sensitive(sensitive is True)
|
sbox._control.set_sensitive(sensitive is True and can_function)
|
||||||
_change_icon(sensitive, sbox._change_icon)
|
_change_icon(sensitive, sbox._change_icon)
|
||||||
sbox._failed.set_visible(is_online)
|
sbox._failed.set_visible(is_online)
|
||||||
return
|
return
|
||||||
|
|
@ -1026,10 +1041,9 @@ def _update_setting_item(sbox, value, is_online=True, sensitive=True, null_okay=
|
||||||
sbox._control.set_value(value)
|
sbox._control.set_value(value)
|
||||||
except TypeError as e:
|
except TypeError as e:
|
||||||
logger.warning("%s: error setting control value (%s): %s", sbox.setting.name, sbox.setting._device, repr(e))
|
logger.warning("%s: error setting control value (%s): %s", sbox.setting.name, sbox.setting._device, repr(e))
|
||||||
sbox._control.set_sensitive(sensitive is True)
|
sbox._control.set_sensitive(sensitive is True and can_function)
|
||||||
_change_icon(sensitive, sbox._change_icon)
|
_change_icon(sensitive, sbox._change_icon)
|
||||||
# rgb_control and rgb_zone_* state gate per-key sensitivity.
|
# rgb_control and rgb_zone_* state gate per-key sensitivity.
|
||||||
name = sbox.setting.name
|
|
||||||
if name == "rgb_control" or name.startswith("rgb_zone_"):
|
if name == "rgb_control" or name.startswith("rgb_zone_"):
|
||||||
_apply_rgb_gates(sbox.setting._device)
|
_apply_rgb_gates(sbox.setting._device)
|
||||||
|
|
||||||
|
|
|
||||||
|
|
@ -636,7 +636,6 @@ def test_perkey_write_skipped_when_zone_is_animation(monkeypatch):
|
||||||
s._device = device
|
s._device = device
|
||||||
s._value = {}
|
s._value = {}
|
||||||
s._has_rgb_effects = True
|
s._has_rgb_effects = True
|
||||||
s._ensure_sw_control = MagicMock()
|
|
||||||
s._send_with_retry = MagicMock(return_value=True)
|
s._send_with_retry = MagicMock(return_value=True)
|
||||||
s._fill_unset_zones_with_base_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, "translate_for_device", lambda d, c: c)
|
||||||
|
|
@ -663,7 +662,6 @@ def test_perkey_write_key_value_skipped_when_zone_is_animation(monkeypatch):
|
||||||
s._device = device
|
s._device = device
|
||||||
s._value = {}
|
s._value = {}
|
||||||
s._has_rgb_effects = True
|
s._has_rgb_effects = True
|
||||||
s._ensure_sw_control = MagicMock()
|
|
||||||
s._send_with_retry = MagicMock(return_value=True)
|
s._send_with_retry = MagicMock(return_value=True)
|
||||||
s._send_zone_color = MagicMock(return_value=True)
|
s._send_zone_color = MagicMock(return_value=True)
|
||||||
s._fill_unset_zones_with_base_color = MagicMock(return_value=True)
|
s._fill_unset_zones_with_base_color = MagicMock(return_value=True)
|
||||||
|
|
|
||||||
Loading…
Reference in New Issue