From 9db42beadef9e7f49b35473878a72f1dffea6e64 Mon Sep 17 00:00:00 2001
From: "Steven B." <51370195+sdb9696@users.noreply.github.com>
Date: Wed, 31 Jul 2024 18:57:12 +0100
Subject: [PATCH] Fix handling of tplink light effects for scenes (#122965)

Co-authored-by: J. Nick Koston <nick@koston.org>
---
 homeassistant/components/tplink/light.py | 30 ++++++++++++-------
 tests/components/tplink/test_light.py    | 38 +++++++++++++++++++++++-
 2 files changed, 56 insertions(+), 12 deletions(-)

diff --git a/homeassistant/components/tplink/light.py b/homeassistant/components/tplink/light.py
index 9b7dd499c97..8d6ec27f81c 100644
--- a/homeassistant/components/tplink/light.py
+++ b/homeassistant/components/tplink/light.py
@@ -382,17 +382,25 @@ class TPLinkLightEffectEntity(TPLinkLightEntity):
     async def async_turn_on(self, **kwargs: Any) -> None:
         """Turn the light on."""
         brightness, transition = self._async_extract_brightness_transition(**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
-            )
-        elif ATTR_COLOR_TEMP_KELVIN in kwargs:
-            if self.effect and self.effect != EFFECT_OFF:
+        effect_off_called = False
+        if effect := kwargs.get(ATTR_EFFECT):
+            if effect in {LightEffect.LIGHT_EFFECTS_OFF, EFFECT_OFF}:
+                if self._effect_module.effect is not LightEffect.LIGHT_EFFECTS_OFF:
+                    await self._effect_module.set_effect(LightEffect.LIGHT_EFFECTS_OFF)
+                    effect_off_called = True
+                if len(kwargs) == 1:
+                    return
+            elif effect in self._effect_module.effect_list:
+                await self._effect_module.set_effect(
+                    kwargs[ATTR_EFFECT], brightness=brightness, transition=transition
+                )
+                return
+            else:
+                _LOGGER.error("Invalid effect %s for %s", effect, self._device.host)
+                return
+
+        if ATTR_COLOR_TEMP_KELVIN in kwargs:
+            if self.effect and self.effect != EFFECT_OFF and not effect_off_called:
                 # If there is an effect in progress
                 # we have to clear the effect
                 # before we can set a color temp
diff --git a/tests/components/tplink/test_light.py b/tests/components/tplink/test_light.py
index 590274b8405..6998d8fbcc7 100644
--- a/tests/components/tplink/test_light.py
+++ b/tests/components/tplink/test_light.py
@@ -505,7 +505,9 @@ async def test_dimmer_turn_on_fix(hass: HomeAssistant) -> None:
     light.set_state.reset_mock()
 
 
-async def test_smart_strip_effects(hass: HomeAssistant) -> None:
+async def test_smart_strip_effects(
+    hass: HomeAssistant, caplog: pytest.LogCaptureFixture
+) -> None:
     """Test smart strip effects."""
     already_migrated_config_entry = MockConfigEntry(
         domain=DOMAIN, data={CONF_HOST: "127.0.0.1"}, unique_id=MAC_ADDRESS
@@ -555,6 +557,40 @@ async def test_smart_strip_effects(hass: HomeAssistant) -> None:
         "Effect2", brightness=None, transition=None
     )
     light_effect.set_effect.reset_mock()
+    async_fire_time_changed(hass, dt_util.utcnow() + timedelta(seconds=30))
+    await hass.async_block_till_done()
+    state = hass.states.get(entity_id)
+    assert state.state == STATE_ON
+    assert state.attributes[ATTR_EFFECT] == "Effect2"
+
+    # Test setting light effect off
+    await hass.services.async_call(
+        LIGHT_DOMAIN,
+        "turn_on",
+        {ATTR_ENTITY_ID: entity_id, ATTR_EFFECT: "off"},
+        blocking=True,
+    )
+    async_fire_time_changed(hass, dt_util.utcnow() + timedelta(seconds=30))
+    await hass.async_block_till_done()
+    state = hass.states.get(entity_id)
+    assert state.state == STATE_ON
+    assert state.attributes[ATTR_EFFECT] == "off"
+    light.set_state.assert_not_called()
+
+    # Test setting light effect to invalid value
+    caplog.clear()
+    await hass.services.async_call(
+        LIGHT_DOMAIN,
+        "turn_on",
+        {ATTR_ENTITY_ID: entity_id, ATTR_EFFECT: "Effect3"},
+        blocking=True,
+    )
+    async_fire_time_changed(hass, dt_util.utcnow() + timedelta(seconds=30))
+    await hass.async_block_till_done()
+    state = hass.states.get(entity_id)
+    assert state.state == STATE_ON
+    assert state.attributes[ATTR_EFFECT] == "off"
+    assert "Invalid effect Effect3 for" in caplog.text
 
     light_effect.effect = LightEffect.LIGHT_EFFECTS_OFF
     async_fire_time_changed(hass, dt_util.utcnow() + timedelta(seconds=10))