Fix light effect not being turned off sometimes

tests/test_light.py

@@ -1838,6 +1838,140 @@ async def test_on_with_off_color(zha_gateway: Gateway) -> None:
     assert entity.state["color_mode"] == ColorMode.COLOR_TEMP
 
 
+@patch(
+    "zigpy.zcl.clusters.lighting.Color.request",
+    new=AsyncMock(return_value=[sentinel.data, zcl_f.Status.SUCCESS]),
+)
+@patch(
+    "zigpy.zcl.clusters.general.OnOff.request",
+    new=AsyncMock(return_value=[sentinel.data, zcl_f.Status.SUCCESS]),
+)
+async def test_turn_on_disables_effect_before_color_commands(
+    zha_gateway: Gateway,
+) -> None:
+    """Test switching away from colorloop disables it before changing color mode."""
+    device_light = await device_light_3_mock(zha_gateway)
+    entity = get_entity(device_light, platform=Platform.LIGHT)
+
+    cluster_on_off = device_light.device.endpoints[1].on_off
+    cluster_color = device_light.device.endpoints[1].light_color
+
+    entity.restore_external_state_attributes(
+        state=True,
+        off_with_transition=False,
+        off_brightness=None,
+        brightness=100,
+        color_temp=None,
+        xy_color=(0.5, 0.5),
+        color_mode=ColorMode.XY,
+        effect="colorloop",
+    )
+
+    cluster_on_off.request.reset_mock()
+    cluster_color.request.reset_mock()
+
+    await entity.async_turn_on(color_temp=235)
+
+    assert cluster_on_off.request.call_count == 1
+    assert cluster_on_off.request.await_count == 1
+    assert cluster_color.request.call_count == 2
+    assert cluster_color.request.await_count == 2
+
+    assert cluster_color.request.call_args_list[0] == call(
+        False,
+        cluster_color.commands_by_name["color_loop_set"].id,
+        cluster_color.commands_by_name["color_loop_set"].schema,
+        update_flags=lighting.Color.ColorLoopUpdateFlags.Action,
+        action=lighting.Color.ColorLoopAction.Deactivate,
+        direction=lighting.Color.ColorLoopDirection.Decrement,
+        time=0,
+        start_hue=0,
+        expect_reply=True,
+        manufacturer=None,
+    )
+    assert cluster_color.request.call_args_list[1] == call(
+        False,
+        cluster_color.commands_by_name["move_to_color_temp"].id,
+        cluster_color.commands_by_name["move_to_color_temp"].schema,
+        color_temp_mireds=235,
+        transition_time=0,
+        expect_reply=True,
+        manufacturer=None,
+    )
+
+    assert entity.state["effect"] == "off"
+    assert entity.state["color_mode"] == ColorMode.COLOR_TEMP
+    assert entity.state["color_temp"] == 235
+    assert entity.state["xy_color"] is None
+
+
+@patch(
+    "zigpy.zcl.clusters.lighting.Color.request",
+    new=AsyncMock(return_value=[sentinel.data, zcl_f.Status.SUCCESS]),
+)
+@patch(
+    "zigpy.zcl.clusters.general.LevelControl.request",
+    new=AsyncMock(return_value=[sentinel.data, zcl_f.Status.SUCCESS]),
+)
+async def test_turn_on_from_off_still_disables_stale_effect(
+    zha_gateway: Gateway,
+) -> None:
+    """Test stale colorloop state is cleared even when turn_on starts from off."""
+    device_light = await device_light_3_mock(zha_gateway)
+    entity = get_entity(device_light, platform=Platform.LIGHT)
+
+    cluster_level = device_light.device.endpoints[1].level
+    cluster_color = device_light.device.endpoints[1].light_color
+
+    entity.restore_external_state_attributes(
+        state=False,
+        off_with_transition=False,
+        off_brightness=None,
+        brightness=100,
+        color_temp=None,
+        xy_color=(0.5, 0.5),
+        color_mode=ColorMode.XY,
+        effect="colorloop",
+    )
+
+    cluster_level.request.reset_mock()
+    cluster_color.request.reset_mock()
+
+    await entity.async_turn_on(color_temp=235)
+
+    assert cluster_level.request.call_count == 2
+    assert cluster_level.request.await_count == 2
+    assert cluster_color.request.call_count == 2
+    assert cluster_color.request.await_count == 2
+
+    assert cluster_color.request.call_args_list[0] == call(
+        False,
+        cluster_color.commands_by_name["move_to_color_temp"].id,
+        cluster_color.commands_by_name["move_to_color_temp"].schema,
+        color_temp_mireds=235,
+        transition_time=0,
+        expect_reply=True,
+        manufacturer=None,
+    )
+    assert cluster_color.request.call_args_list[1] == call(
+        False,
+        cluster_color.commands_by_name["color_loop_set"].id,
+        cluster_color.commands_by_name["color_loop_set"].schema,
+        update_flags=lighting.Color.ColorLoopUpdateFlags.Action,
+        action=lighting.Color.ColorLoopAction.Deactivate,
+        direction=lighting.Color.ColorLoopDirection.Decrement,
+        time=0,
+        start_hue=0,
+        expect_reply=True,
+        manufacturer=None,
+    )
+
+    assert entity.state["effect"] == "off"
+    assert entity.state["color_mode"] == ColorMode.COLOR_TEMP
+    assert entity.state["color_temp"] == 235
+    assert entity.state["xy_color"] is None
+
+
 @patch(
     "zigpy.zcl.clusters.general.OnOff.request",
     new=AsyncMock(return_value=[sentinel.data, zcl_f.Status.SUCCESS]),

zha/application/platforms/light/__init__.py

@@ -454,7 +454,16 @@ async def _async_turn_on_impl(  # noqa: C901
         else:
             level = self._brightness or 254
 
-        t_log = {}
+        t_log: dict[str, Any] = {}
+        started_on = self._state
+        deactivate_effect_after_turn_on = (
+            self._color_cluster_handler is not None
+            and not started_on
+            and self._effect == EFFECT_COLORLOOP
+            and effect != EFFECT_COLORLOOP
+        )
+
+        await self._async_deactivate_effect_before_turn_on(effect, t_log)
 
         if new_color_provided_while_off:
             assert self._level_cluster_handler is not None
@@ -581,31 +590,23 @@ async def _async_turn_on_impl(  # noqa: C901
         # attribute reports after the completed transition).
         self.async_transition_start_timer(transition_time)
 
-        if self._color_cluster_handler is not None:
-            if effect == EFFECT_COLORLOOP:
-                result = await self._color_cluster_handler.color_loop_set(
-                    update_flags=(
-                        Color.ColorLoopUpdateFlags.Action
-                        | Color.ColorLoopUpdateFlags.Direction
-                        | Color.ColorLoopUpdateFlags.Time
-                    ),
-                    action=Color.ColorLoopAction.Activate_from_current_hue,
-                    direction=Color.ColorLoopDirection.Increment,
-                    time=transition if transition else 7,
-                    start_hue=0,
-                )
-                t_log["color_loop_set"] = result
-                self._effect = EFFECT_COLORLOOP
-            elif self._effect == EFFECT_COLORLOOP and effect != EFFECT_COLORLOOP:
-                result = await self._color_cluster_handler.color_loop_set(
-                    update_flags=Color.ColorLoopUpdateFlags.Action,
-                    action=Color.ColorLoopAction.Deactivate,
-                    direction=Color.ColorLoopDirection.Decrement,
-                    time=0,
-                    start_hue=0,
-                )
-                t_log["color_loop_set"] = result
-                self._effect = EFFECT_OFF
+        if deactivate_effect_after_turn_on:
+            await self._async_deactivate_effect_after_turn_on(t_log)
+
+        if self._color_cluster_handler is not None and effect == EFFECT_COLORLOOP:
+            result = await self._color_cluster_handler.color_loop_set(
+                update_flags=(
+                    Color.ColorLoopUpdateFlags.Action
+                    | Color.ColorLoopUpdateFlags.Direction
+                    | Color.ColorLoopUpdateFlags.Time
+                ),
+                action=Color.ColorLoopAction.Activate_from_current_hue,
+                direction=Color.ColorLoopDirection.Increment,
+                time=transition if transition else 7,
+                start_hue=0,
+            )
+            t_log["color_loop_set"] = result
+            self._effect = EFFECT_COLORLOOP
 
         if flash is not None:
             assert self._identify_cluster_handler is not None
@@ -620,6 +621,47 @@ async def _async_turn_on_impl(  # noqa: C901
         self.debug("turned on: %s", t_log)
         self.maybe_emit_state_changed_event()
 
+    async def _async_deactivate_effect_before_turn_on(
+        self,
+        effect: str | None,
+        t_log: dict[str, Any],
+    ) -> None:
+        """Disable an active effect before sending other state-changing commands."""
+        if (
+            self._color_cluster_handler is None
+            or not self._state
+            or self._effect != EFFECT_COLORLOOP
+            or effect == EFFECT_COLORLOOP
+        ):
+            return
+
+        result = await self._color_cluster_handler.color_loop_set(
+            update_flags=Color.ColorLoopUpdateFlags.Action,
+            action=Color.ColorLoopAction.Deactivate,
+            direction=Color.ColorLoopDirection.Decrement,
+            time=0,
+            start_hue=0,
+        )
+        t_log["color_loop_set"] = result
+        self._effect = EFFECT_OFF
+
+    async def _async_deactivate_effect_after_turn_on(
+        self,
+        t_log: dict[str, Any],
+    ) -> None:
+        """Disable an active effect after turn-on when the light started off."""
+        assert self._color_cluster_handler is not None
+
+        result = await self._color_cluster_handler.color_loop_set(
+            update_flags=Color.ColorLoopUpdateFlags.Action,
+            action=Color.ColorLoopAction.Deactivate,
+            direction=Color.ColorLoopDirection.Decrement,
+            time=0,
+            start_hue=0,
+        )
+        t_log["color_loop_set"] = result
+        self._effect = EFFECT_OFF
+
     async def async_turn_off(self, *, transition: float | None = None) -> None:
         """Turn the entity off."""
         brightness_supported = (