FazBrowse GitHub Viewer | Trending |
URL:
| Home
Tools: [Download Repo ZIP]   [Original HTTPS Page]

Only re-assert device_on when device is already on · fluffyspace/python-kasa@85d8d38 · GitHub

Repository navigation

Commit 85d8d38

Browse files
committed
Only re-assert device_on when device is already on
Forcing device_on: True unconditionally in set_brightness, set_color_temp, and set_hsv fixed the L900-5 zombie state but reintroduced the python-kasa#1532 regression: users who want to change brightness on an off device without turning it on as a side-effect. Now device_on: True is injected only when self._device.is_on is already true. That still re-asserts the power state for devices in the zombie state (they report on, so the flag is still sent and the LEDs physically come up), while leaving a deliberately-off device alone. Tests parametrize all three setters over is_on=[True, False] to lock in both branches.
1 parent a87c78d commit 85d8d38

6 files changed

Lines changed: 125 additions & 5 deletions

File tree

‎kasa/smart/modules/brightness.py‎

Lines changed: 8 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -73,9 +73,14 @@ async def set_brightness(
7373
) is not None and light_effect.is_active:
7474
return await light_effect.set_brightness(brightness)
7575

76-
return await self.call(
77-
"set_device_info", {"brightness": brightness, "device_on": True}
78-
)
76+
# Only re-assert device_on when the device is already on. This fixes a
77+
# "zombie state" on some devices (e.g. Tapo L900-5) where set_device_info
78+
# silently no-ops the LEDs without an explicit power state, while still
79+
# allowing brightness to be changed on an off device without turning it on.
80+
params: dict = {"brightness": brightness}
81+
if self._device.is_on:
82+
params["device_on"] = True
83+
return await self.call("set_device_info", params)
7984

8085
async def _check_supported(self) -> bool:
8186
"""Additional check to see if the module is supported by the device."""

‎kasa/smart/modules/color.py‎

Lines changed: 3 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -91,10 +91,12 @@ async def set_hsv(
9191
"color_temp": 0, # If set, color_temp takes precedence over hue&sat
9292
"hue": hue,
9393
"saturation": saturation,
94-
"device_on": True,
9594
}
9695
# The device errors on invalid brightness values.
9796
if value is not None:
9897
request_payload["brightness"] = value
98+
# Only re-assert device_on when the device is already on; see Brightness.
99+
if self._device.is_on:
100+
request_payload["device_on"] = True
99101

100102
return await self.call("set_device_info", {**request_payload})

‎kasa/smart/modules/colortemperature.py‎

Lines changed: 4 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -65,9 +65,12 @@ async def set_color_temp(self, temp: int, *, brightness: int | None = None) -> d
6565
*valid_temperature_range, temp
6666
)
6767
)
68-
params: dict = {"color_temp": temp, "device_on": True}
68+
params: dict = {"color_temp": temp}
6969
if brightness:
7070
params["brightness"] = brightness
71+
# Only re-assert device_on when the device is already on; see Brightness.
72+
if self._device.is_on:
73+
params["device_on"] = True
7174
return await self.call("set_device_info", params)
7275

7376
async def _check_supported(self) -> bool:
Lines changed: 102 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,102 @@
1+
"""Tests that set_brightness/set_color_temp/set_hsv inject device_on correctly.
2+
3+
Regression coverage for two interacting concerns:
4+
5+
* Tapo L900-5 "zombie state" — device reports on but LEDs stay off unless
6+
device_on is re-asserted in the set_device_info payload.
7+
* Issue #1532 — users want to change brightness on an already-off device
8+
without turning it on as a side-effect.
9+
10+
The compromise: re-assert device_on=True only when the device is currently on,
11+
otherwise leave it out so an off device stays off.
12+
"""
13+
14+
from __future__ import annotations
15+
16+
import pytest
17+
from pytest_mock import MockerFixture
18+
19+
from kasa import Module
20+
from kasa.smart import SmartDevice
21+
22+
from ...device_fixtures import get_parent_and_child_modules, parametrize
23+
24+
brightness_smart = parametrize(
25+
"has brightness", component_filter="brightness", protocol_filter={"SMART"}
26+
)
27+
color_smart = parametrize(
28+
"has color", component_filter="color", protocol_filter={"SMART"}
29+
)
30+
color_temp_smart = parametrize(
31+
"has color temperature",
32+
component_filter="color_temperature",
33+
protocol_filter={"SMART"},
34+
)
35+
36+
37+
@brightness_smart
38+
@pytest.mark.parametrize("is_on", [True, False])
39+
async def test_set_brightness_device_on_payload(
40+
dev: SmartDevice, is_on: bool, mocker: MockerFixture
41+
) -> None:
42+
brightness = next(get_parent_and_child_modules(dev, Module.Brightness))
43+
owner = brightness._device
44+
# Bypass any active light effect so set_brightness hits set_device_info.
45+
if (effect := owner.modules.get(Module.SmartLightEffect)) is not None:
46+
mocker.patch.object(
47+
type(effect),
48+
"is_active",
49+
new_callable=mocker.PropertyMock,
50+
return_value=False,
51+
)
52+
53+
owner._info["device_on"] = is_on
54+
mock_call = mocker.patch.object(brightness, "call")
55+
56+
await brightness.set_brightness(50)
57+
58+
expected: dict = {"brightness": 50}
59+
if is_on:
60+
expected["device_on"] = True
61+
mock_call.assert_called_once_with("set_device_info", expected)
62+
63+
64+
@color_temp_smart
65+
@pytest.mark.parametrize("is_on", [True, False])
66+
async def test_set_color_temp_device_on_payload(
67+
dev: SmartDevice, is_on: bool, mocker: MockerFixture
68+
) -> None:
69+
ct = next(get_parent_and_child_modules(dev, Module.ColorTemperature), None)
70+
if ct is None:
71+
pytest.skip("Device has color_temperature component but no module enabled")
72+
if ct.valid_temperature_range.min == ct.valid_temperature_range.max:
73+
pytest.skip("Device exposes color_temperature but no usable range")
74+
75+
ct._device._info["device_on"] = is_on
76+
mock_call = mocker.patch.object(ct, "call")
77+
78+
temp = ct.valid_temperature_range.min
79+
await ct.set_color_temp(temp)
80+
81+
expected: dict = {"color_temp": temp}
82+
if is_on:
83+
expected["device_on"] = True
84+
mock_call.assert_called_once_with("set_device_info", expected)
85+
86+
87+
@color_smart
88+
@pytest.mark.parametrize("is_on", [True, False])
89+
async def test_set_hsv_device_on_payload(
90+
dev: SmartDevice, is_on: bool, mocker: MockerFixture
91+
) -> None:
92+
color = next(get_parent_and_child_modules(dev, Module.Color))
93+
94+
color._device._info["device_on"] = is_on
95+
mock_call = mocker.patch.object(color, "call")
96+
97+
await color.set_hsv(120, 50)
98+
99+
expected: dict = {"color_temp": 0, "hue": 120, "saturation": 50}
100+
if is_on:
101+
expected["device_on"] = True
102+
mock_call.assert_called_once_with("set_device_info", expected)

‎tests/smart/modules/test_light_effect.py‎

Lines changed: 4 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -67,6 +67,10 @@ async def test_light_effect_brightness(
6767
if effect_active: # Set the rule L1 active for testing
6868
light_effect.data["current_rule_id"] = "L1"
6969

70+
# Force device_on so the asserted payload is deterministic across fixtures
71+
# (some fixtures' get_device_info has device_on: false).
72+
dev._info["device_on"] = True
73+
7074
await light_module.set_brightness(10)
7175

7276
if effect_active:

‎tests/smart/modules/test_light_strip_effect.py‎

Lines changed: 4 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -82,6 +82,10 @@ async def test_light_effect_brightness(
8282
return_value=effect_active,
8383
)
8484

85+
# Force device_on so the asserted payload is deterministic across fixtures
86+
# (some fixtures' get_device_info has device_on: false).
87+
dev._info["device_on"] = True
88+
8589
await light_module.set_brightness(10)
8690

8791
if effect_active:

0 commit comments

Comments
 (0)

Back | FazBrowse Home | New Git URL