From 994d6f552cbca60d3436bc19f2c9e0899d3d6c1a Mon Sep 17 00:00:00 2001 From: Steven B <51370195+sdb9696@users.noreply.github.com> Date: Fri, 5 Jul 2024 10:19:04 +0100 Subject: [PATCH] Fix tplink light effect behaviour when activating a scene (#121288) --- homeassistant/components/tplink/light.py | 7 +- tests/components/tplink/__init__.py | 51 +++++++++++--- tests/components/tplink/test_light.py | 88 +++++++++++++++++++++++- 3 files changed, 135 insertions(+), 11 deletions(-) diff --git a/homeassistant/components/tplink/light.py b/homeassistant/components/tplink/light.py index a736a0ba1e1046..22e7c523d1a6e0 100644 --- a/homeassistant/components/tplink/light.py +++ b/homeassistant/components/tplink/light.py @@ -382,7 +382,12 @@ def _async_update_attrs(self) -> None: async def async_turn_on(self, **kwargs: Any) -> None: """Turn the light on.""" brightness, transition = self._async_extract_brightness_transition(**kwargs) - if ATTR_EFFECT in kwargs: + if ( + (effect := kwargs.get(ATTR_EFFECT)) + # Effect is unlikely to be LIGHT_EFFECTS_OFF but check for it anyway + and effect not in {LightEffect.LIGHT_EFFECTS_OFF, EFFECT_OFF} + and effect in self._effect_module.effect_list + ): await self._effect_module.set_effect( kwargs[ATTR_EFFECT], brightness=brightness, transition=transition ) diff --git a/tests/components/tplink/__init__.py b/tests/components/tplink/__init__.py index d12858017cca93..b554cf07015866 100644 --- a/tests/components/tplink/__init__.py +++ b/tests/components/tplink/__init__.py @@ -210,7 +210,8 @@ def _mocked_device( if modules: device.modules = { - module_name: MODULE_TO_MOCK_GEN[module_name]() for module_name in modules + module_name: MODULE_TO_MOCK_GEN[module_name](device) + for module_name in modules } if features: @@ -298,7 +299,7 @@ def _mocked_feature( return feature -def _mocked_light_module() -> Light: +def _mocked_light_module(device) -> Light: light = MagicMock(spec=Light, name="Mocked light module") light.update = AsyncMock() light.brightness = 50 @@ -314,26 +315,58 @@ def _mocked_light_module() -> Light: light.hsv = (10, 30, 5) light.valid_temperature_range = ColorTempRange(min=4000, max=9000) light.hw_info = {"sw_ver": "1.0.0", "hw_ver": "1.0.0"} - light.set_state = AsyncMock() - light.set_brightness = AsyncMock() - light.set_hsv = AsyncMock() - light.set_color_temp = AsyncMock() + + async def _set_state(state, *_, **__): + light.state = state + + light.set_state = AsyncMock(wraps=_set_state) + + async def _set_brightness(brightness, *_, **__): + light.state.brightness = brightness + light.state.light_on = brightness > 0 + + light.set_brightness = AsyncMock(wraps=_set_brightness) + + async def _set_hsv(h, s, v, *_, **__): + light.state.hue = h + light.state.saturation = s + light.state.brightness = v + light.state.light_on = True + + light.set_hsv = AsyncMock(wraps=_set_hsv) + + async def _set_color_temp(temp, *_, **__): + light.state.color_temp = temp + light.state.light_on = True + + light.set_color_temp = AsyncMock(wraps=_set_color_temp) light.protocol = _mock_protocol() return light -def _mocked_light_effect_module() -> LightEffect: +def _mocked_light_effect_module(device) -> LightEffect: effect = MagicMock(spec=LightEffect, name="Mocked light effect") effect.has_effects = True effect.has_custom_effects = True effect.effect = "Effect1" effect.effect_list = ["Off", "Effect1", "Effect2"] - effect.set_effect = AsyncMock() + + async def _set_effect(effect_name, *_, **__): + assert ( + effect_name in effect.effect_list + ), f"set_effect '{effect_name}' not in {effect.effect_list}" + assert device.modules[ + Module.Light + ], "Need a light module to test set_effect method" + device.modules[Module.Light].state.light_on = True + effect.effect = effect_name + + effect.set_effect = AsyncMock(wraps=_set_effect) effect.set_custom_effect = AsyncMock() return effect -def _mocked_fan_module() -> Fan: +def _mocked_fan_module(effect) -> Fan: fan = MagicMock(auto_spec=Fan, name="Mocked fan") fan.fan_speed_level = 0 fan.set_fan_speed_level = AsyncMock() diff --git a/tests/components/tplink/test_light.py b/tests/components/tplink/test_light.py index 6fce04ec454c9b..bb814d1f5d377a 100644 --- a/tests/components/tplink/test_light.py +++ b/tests/components/tplink/test_light.py @@ -5,6 +5,7 @@ from datetime import timedelta from unittest.mock import MagicMock, PropertyMock +from freezegun.api import FrozenDateTimeFactory from kasa import ( AuthenticationError, DeviceType, @@ -36,7 +37,13 @@ ) from homeassistant.components.tplink.const import DOMAIN from homeassistant.config_entries import SOURCE_REAUTH -from homeassistant.const import ATTR_ENTITY_ID, CONF_HOST, STATE_OFF, STATE_ON +from homeassistant.const import ( + ATTR_ENTITY_ID, + CONF_HOST, + STATE_OFF, + STATE_ON, + STATE_UNKNOWN, +) from homeassistant.core import HomeAssistant from homeassistant.exceptions import HomeAssistantError from homeassistant.helpers import entity_registry as er @@ -920,3 +927,82 @@ async def test_light_child( assert child_entity assert child_entity.unique_id == f"{DEVICE_ID}0{light_id}" assert child_entity.device_id == entity.device_id + + +async def test_scene_effect_light( + hass: HomeAssistant, + freezer: FrozenDateTimeFactory, +) -> None: + """Test activating a scene works with effects. + + i.e. doesn't try to set the effect to 'off' + """ + already_migrated_config_entry = MockConfigEntry( + domain=DOMAIN, data={CONF_HOST: "127.0.0.1"}, unique_id=MAC_ADDRESS + ) + already_migrated_config_entry.add_to_hass(hass) + device = _mocked_device( + modules=[Module.Light, Module.LightEffect], alias="my_light" + ) + light_effect = device.modules[Module.LightEffect] + light_effect.effect = LightEffect.LIGHT_EFFECTS_OFF + + with _patch_discovery(device=device), _patch_connect(device=device): + assert await async_setup_component(hass, tplink.DOMAIN, {tplink.DOMAIN: {}}) + assert await async_setup_component(hass, "scene", {}) + await hass.async_block_till_done() + + entity_id = "light.my_light" + + await hass.services.async_call( + LIGHT_DOMAIN, "turn_on", {ATTR_ENTITY_ID: entity_id}, blocking=True + ) + await hass.async_block_till_done() + freezer.tick(5) + async_fire_time_changed(hass) + await hass.async_block_till_done() + + state = hass.states.get(entity_id) + assert state.state is STATE_ON + assert state.attributes["effect"] is EFFECT_OFF + + await hass.services.async_call( + "scene", + "create", + {"scene_id": "effect_off_scene", "snapshot_entities": [entity_id]}, + blocking=True, + ) + await hass.async_block_till_done() + scene_state = hass.states.get("scene.effect_off_scene") + assert scene_state.state is STATE_UNKNOWN + + await hass.services.async_call( + LIGHT_DOMAIN, "turn_off", {ATTR_ENTITY_ID: entity_id}, blocking=True + ) + await hass.async_block_till_done() + freezer.tick(5) + async_fire_time_changed(hass) + await hass.async_block_till_done() + + state = hass.states.get(entity_id) + assert state.state is STATE_OFF + + await hass.services.async_call( + "scene", + "turn_on", + { + "entity_id": "scene.effect_off_scene", + }, + blocking=True, + ) + await hass.async_block_till_done() + scene_state = hass.states.get("scene.effect_off_scene") + assert scene_state.state is not STATE_UNKNOWN + + freezer.tick(5) + async_fire_time_changed(hass) + await hass.async_block_till_done() + + state = hass.states.get(entity_id) + assert state.state is STATE_ON + assert state.attributes["effect"] is EFFECT_OFF