Repository navigation
Include device_on in set_brightness, set_color_temp, and set_hsv - #1680
fluffyspace wants to merge 5 commits into
Conversation
Sorry, something went wrong.
Codecov Report✅ All modified and coverable lines are covered by tests. @@ Coverage Diff @@
## master #1680 +/- ##
==========================================
+ Coverage 93.31% 93.33% +0.02%
==========================================
Files 158 158
Lines 9977 9984 +7
Branches 1025 1028 +3
==========================================
+ Hits 9310 9319 +9
Misses 472 472
+ Partials 195 193 -2 ☔ View full report in Codecov by Harness.
|
Sorry, something went wrong.
|
Updated in 85d8d38 to address the regression risk against #1532 / #1066. Original approach forced device_on: True unconditionally, which would have brought back the "setting brightness turns on the light" behavior that those issues are explicitly asking to preserve. New approach: device_on: True is injected only when self._device.is_on is already true. That still re-asserts power state for the L900-5 zombie case (device reports on but LEDs are off → flag is still sent → LEDs come up) while leaving a deliberately-off device alone. PR description updated with the full matrix; new parametrized tests in tests/smart/modules/test_light_device_on_payload.py lock in both branches. |
Sorry, something went wrong.
|
There hasn't been any activity on this pull request recently. This pull request has been automatically marked as stale because of that and will be closed if no further activity occurs within 7 days. |
Sorry, something went wrong.
Some Tapo devices (confirmed on L900-5 LED strip) treat power state and brightness/color as independent parameters. When set_device_info is called with only brightness or color parameters (without device_on), the device accepts the command, updates its internal state, and reports success — but the LEDs do not physically turn on. This causes a particularly bad failure mode: the device enters a "zombie state" where it reports on/brightness=100 to all queries while the LEDs remain off. The device stays in this state until power cycled, and python-kasa has no way to detect the discrepancy since every protocol response indicates success. Adding device_on: True to set_brightness(), set_color_temp(), and set_hsv() ensures the device always receives an explicit power state with light property changes. This matches the behavior of set_state() which already includes device_on, and is consistent with how the official Tapo app sends commands. Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
Update test_light_effect_brightness assertions in both test_light_effect.py and test_light_strip_effect.py to match the new set_brightness payload that includes device_on: True. Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
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.
Fixes mypy: `dev` is typed as `Device` (the abstract base) which has no `_info` attribute. Patching the public `is_on` property has the same effect on the brightness module's logic and is type-clean.
|
Hi! Just a gentle ping on this one — I've rebased the branch onto the latest master and CI is green again (Codecov reports coverage at 93.24%). I'd love to get this merged if there's anything I can do to help it along. Quick recap for context: this fixes a set_device_info quirk where brightness/color changes on certain Tapo strips (e.g. L900-5) get silently dropped when the power state isn't re-asserted — the LED stays off even though the module reports the change as applied. The device_on: True is injected only when the device is already on, so it avoids the regression in #1532/#1066 (changing brightness on an off device must not turn it on). Happy to adjust the approach, add fixtures, or split the changes if that makes it easier to review. |
Sorry, something went wrong.
Summary
Problem
Some Tapo devices (confirmed on the L900-5 LED light strip) do not implicitly turn on when receiving brightness or color commands via set_device_info. The device:
This creates a "zombie state" where the device reports it's on but the LEDs are off. The state persists until the device is power-cycled. Since every protocol response indicates success, python-kasa (and Home Assistant) have no way to detect the failure.
Root cause
Meanwhile, Light.set_state() always includes device_on in the payload, and works correctly.
Fix
Include "device_on": True in all three methods' set_device_info payloads, only when the device is currently on. This re-asserts the power state for the zombie-state case (the device thinks it's on, so the flag is still sent and the LEDs physically come up) while leaving a deliberately-off device alone.
This matches the spirit of set_state() (which sends device_on explicitly) without contradicting the "set brightness without turning on" use case from #1532 / #1066.
Testing
Tested on a Tapo L900-5 (firmware current, KLAP v2 transport) controlled via Home Assistant's tplink integration:
Unit tests
New tests/smart/modules/test_light_device_on_payload.py parametrizes set_brightness, set_color_temp, and set_hsv over is_on=[True, False] and locks in:
Updated existing test_light_effect_brightness cases to force dev._info["device_on"] = True so the asserted payload is deterministic across fixtures (some have device_on: false baked in).
Closes the regression risk for #1532 / #1066.
🤖 Generated with Claude Code