Merge 012d7fd04d into 5e4ae7261e

lib/logitech_receiver/device.py

@@ -172,6 +172,7 @@ class Device:
         self.present = True  # used for devices that are integral with their receiver but that separately be disconnected
 
         self._feature_settings_checked = False
+        self._closed = False
         self._gestures_lock = threading.Lock()
         self._settings_lock = threading.Lock()
         self._persister_lock = threading.Lock()
@@ -1037,6 +1038,12 @@ class Device:
         pass
 
     def close(self):
+        # Idempotent: __del__ re-invokes close() at interpreter shutdown, where
+        # re-running cleanups can't safely build settings. Flag, not self.handle
+        # (None for receiver-paired devices that borrow the receiver's handle).
+        if getattr(self, "_closed", False):
+            return None
+        self._closed = True
         # Run device.cleanups before clearing self.handle — cleanup callbacks
         # typically need to issue final feature_request() writes (e.g. release
         # SW control, restore device-side state) and feature_request() relies

lib/logitech_receiver/rgb_power.py

@@ -316,6 +316,21 @@ def stop(device):
         mgr.stop()
 
 
+def _rgb_control_off(device):
+    # Reads already-built settings/persister, never device.settings (the lazy
+    # property): cleanup() runs from __del__, where a settings build fails at
+    # shutdown. Unknown state ⇒ False (proceed).
+    for s in getattr(device, "_settings", None) or []:
+        if s.name == "rgb_control":
+            return not s._value
+    persister = getattr(device, "persister", None)
+    if persister is not None:
+        value = persister.get("rgb_control")
+        if value is not None:
+            return value in (False, 0)
+    return False
+
+
 def cleanup(device):
     """device.cleanups handler — restore firmware control on device close.
 
@@ -330,7 +345,7 @@ def cleanup(device):
     animation would visibly contradict the user's "leave my lighting alone".
     """
     stop(device)
-    if any(s.name == "rgb_control" and not s._value for s in getattr(device, "settings", []) or []):
+    if _rgb_control_off(device):
         return
     try:
         device.feature_request(SupportedFeature.RGB_EFFECTS, 0x50, SW_RELEASE)

tests/logitech_receiver/test_device.py

@@ -423,3 +423,20 @@ def test_device_battery(device_info, responses, protocol, expected_battery, chan
     assert test_device.battery() == expected_battery
     test_device.read_battery()
     spy_changed.assert_called_with(**changed)
+
+
+def test_close_runs_cleanups_once():
+    """close() is idempotent: __del__ calls it again at interpreter shutdown,
+    and cleanups must not run a second time (a settings build there fails)."""
+    handle = 0x1
+    test_device = device.Device(
+        LowLevelInterfaceFake(fake_hidpp.r_empty), None, None, None, handle=handle, device_info=di_CCCC
+    )
+    calls = []
+    test_device.cleanups = [lambda d: calls.append(d)]
+
+    test_device.close()
+    test_device.close()  # simulates the __del__ re-entry
+    test_device.__del__()
+
+    assert calls == [test_device]

tests/logitech_receiver/test_rgb_power.py

@@ -672,3 +672,42 @@ 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()
+
+
+class _SettingsPropertyExplodes:
+    """A device whose `.settings` property raises — mirrors the interpreter
+    -shutdown case where forcing a lazy settings build fails. cleanup() must
+    never touch it."""
+
+    def __init__(self, settings_list=None, persister=None):
+        self._settings = settings_list
+        self.persister = persister
+
+    @property
+    def settings(self):
+        raise AssertionError("cleanup must not access the settings property")
+
+
+def _rgb_control_setting(value):
+    class _S:
+        name = "rgb_control"
+
+    s = _S()
+    s._value = value
+    return s
+
+
+def test_rgb_control_off_reads_built_setting_without_building():
+    dev = _SettingsPropertyExplodes(settings_list=[_rgb_control_setting(False)])
+    assert rgb_power._rgb_control_off(dev) is True
+
+    dev = _SettingsPropertyExplodes(settings_list=[_rgb_control_setting(True)])
+    assert rgb_power._rgb_control_off(dev) is False
+
+
+def test_rgb_control_off_falls_back_to_persister():
+    dev = _SettingsPropertyExplodes(settings_list=None, persister={"rgb_control": False})
+    assert rgb_power._rgb_control_off(dev) is True
+
+    dev = _SettingsPropertyExplodes(settings_list=None, persister={})
+    assert rgb_power._rgb_control_off(dev) is False