Fix handling of tplink light effects for scenes (#122965)

Co-authored-by: J. Nick Koston <nick@koston.org>

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_off_called = False
-            (effect := kwargs.get(ATTR_EFFECT))
+        if effect := kwargs.get(ATTR_EFFECT):
-            # Effect is unlikely to be LIGHT_EFFECTS_OFF but check for it anyway
+            if effect in {LightEffect.LIGHT_EFFECTS_OFF, EFFECT_OFF}:
-            and effect not in {LightEffect.LIGHT_EFFECTS_OFF, EFFECT_OFF}
+                if self._effect_module.effect is not LightEffect.LIGHT_EFFECTS_OFF:
-            and effect in self._effect_module.effect_list
+                    await self._effect_module.set_effect(LightEffect.LIGHT_EFFECTS_OFF)
-        ):
+                    effect_off_called = True
-            await self._effect_module.set_effect(
+                if len(kwargs) == 1:
-                kwargs[ATTR_EFFECT], brightness=brightness, transition=transition
+                    return
-            )
+            elif effect in self._effect_module.effect_list:
-        elif ATTR_COLOR_TEMP_KELVIN in kwargs:
+                await self._effect_module.set_effect(
-            if self.effect and self.effect != EFFECT_OFF:
+                    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

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))