Fix ZHA turn_on issues with transition=0, improve tests (#97539)

* Fix turn_on ignoring transition=0 and brightness=None, add test

This fixes light.turn_on for ZHA lights ignoring a transition of 0 when no brightness is given at the same time.
It also adds a test for that case.

Fixes https://github.com/home-assistant/core/issues/93265

* Add test for "force on" lights

This test checks that "force on" lights also get an "on" command (in addition to the "move to level" command) when turn_on is called with only transition=0.

* Fix "on" command sent for transition=0 calls, fix FORCE_ON missing for transition=0

This fixes an issue where the "on" command is sent in addition to a "move_to_level_with_on_off" command, even though the latter one is enough (for non-FORCE_ON lights).
It also fixes the test to not expect the unnecessary "on" command (in addition to the expected "move_to_level_with_on_off" command).

The `brightness != 0` change is needed to fix an issue where FORCE_ON lights did not get the required "on" command (in addition to "move_to_level_with_on_off") if turn_on was called with only transition=0.
(It could have been `brightness not None`, but that would also send an "on" command if turn_on is called with brightness=0 which HA somewhat "supports". The brightness != 0 check avoids that issue.)

* Improve comments in ZHA light class

homeassistant/components/zha/light.py

@@ -329,7 +329,7 @@ class BaseLight(LogMixin, light.LightEntity):
                 return
 
         if (
-            (brightness is not None or transition)
+            (brightness is not None or transition is not None)
             and not new_color_provided_while_off
             and brightness_supported(self._attr_supported_color_modes)
         ):
@@ -350,11 +350,11 @@ class BaseLight(LogMixin, light.LightEntity):
                 self._attr_brightness = level
 
         if (
-            brightness is None
+            (brightness is None and transition is None)
             and not new_color_provided_while_off
-            or (self._FORCE_ON and brightness)
+            or (self._FORCE_ON and brightness != 0)
         ):
-            # since some lights don't always turn on with move_to_level_with_on_off,
+            # since FORCE_ON lights don't turn on with move_to_level_with_on_off,
             # we should call the on command on the on_off cluster
             # if brightness is not 0.
             result = await self._on_off_cluster_handler.on()
@@ -385,7 +385,7 @@ class BaseLight(LogMixin, light.LightEntity):
                 return
 
         if new_color_provided_while_off:
-            # The light is has the correct color, so we can now transition
+            # The light has the correct color, so we can now transition
             # it to the correct brightness level.
             result = await self._level_cluster_handler.move_to_level(
                 level=level, transition_time=int(10 * duration)
@@ -1076,7 +1076,7 @@ class HueLight(Light):
     manufacturers={"Jasco", "Quotra-Vision", "eWeLight", "eWeLink"},
 )
 class ForceOnLight(Light):
-    """Representation of a light which does not respect move_to_level_with_on_off."""
+    """Representation of a light which does not respect on/off for move_to_level_with_on_off commands."""
 
     _attr_name: str = "Light"
     _FORCE_ON = True

tests/components/zha/test_light.py

@@ -530,7 +530,78 @@ async def test_transitions(
     light2_state = hass.states.get(device_2_entity_id)
     assert light2_state.state == STATE_OFF
 
-    # first test 0 length transition with no color provided
+    # first test 0 length transition with no color and no brightness provided
+    dev1_cluster_on_off.request.reset_mock()
+    dev1_cluster_level.request.reset_mock()
+    await hass.services.async_call(
+        LIGHT_DOMAIN,
+        "turn_on",
+        {"entity_id": device_1_entity_id, "transition": 0},
+        blocking=True,
+    )
+    assert dev1_cluster_on_off.request.call_count == 0
+    assert dev1_cluster_on_off.request.await_count == 0
+    assert dev1_cluster_color.request.call_count == 0
+    assert dev1_cluster_color.request.await_count == 0
+    assert dev1_cluster_level.request.call_count == 1
+    assert dev1_cluster_level.request.await_count == 1
+    assert dev1_cluster_level.request.call_args == call(
+        False,
+        dev1_cluster_level.commands_by_name["move_to_level_with_on_off"].id,
+        dev1_cluster_level.commands_by_name["move_to_level_with_on_off"].schema,
+        level=254,  # default "full on" brightness
+        transition_time=0,
+        expect_reply=True,
+        manufacturer=None,
+        tsn=None,
+    )
+
+    light1_state = hass.states.get(device_1_entity_id)
+    assert light1_state.state == STATE_ON
+    assert light1_state.attributes["brightness"] == 254
+
+    # test 0 length transition with no color and no brightness provided again, but for "force on" lights
+    eWeLink_cluster_on_off.request.reset_mock()
+    eWeLink_cluster_level.request.reset_mock()
+    await hass.services.async_call(
+        LIGHT_DOMAIN,
+        "turn_on",
+        {"entity_id": eWeLink_light_entity_id, "transition": 0},
+        blocking=True,
+    )
+    assert eWeLink_cluster_on_off.request.call_count == 1
+    assert eWeLink_cluster_on_off.request.await_count == 1
+    assert eWeLink_cluster_on_off.request.call_args_list[0] == call(
+        False,
+        eWeLink_cluster_on_off.commands_by_name["on"].id,
+        eWeLink_cluster_on_off.commands_by_name["on"].schema,
+        expect_reply=True,
+        manufacturer=None,
+        tsn=None,
+    )
+    assert eWeLink_cluster_color.request.call_count == 0
+    assert eWeLink_cluster_color.request.await_count == 0
+    assert eWeLink_cluster_level.request.call_count == 1
+    assert eWeLink_cluster_level.request.await_count == 1
+    assert eWeLink_cluster_level.request.call_args == call(
+        False,
+        eWeLink_cluster_level.commands_by_name["move_to_level_with_on_off"].id,
+        eWeLink_cluster_level.commands_by_name["move_to_level_with_on_off"].schema,
+        level=254,  # default "full on" brightness
+        transition_time=0,
+        expect_reply=True,
+        manufacturer=None,
+        tsn=None,
+    )
+
+    eWeLink_state = hass.states.get(eWeLink_light_entity_id)
+    assert eWeLink_state.state == STATE_ON
+    assert eWeLink_state.attributes["brightness"] == 254
+
+    eWeLink_cluster_on_off.request.reset_mock()
+    eWeLink_cluster_level.request.reset_mock()
+
+    # test 0 length transition with brightness, but no color provided
     dev1_cluster_on_off.request.reset_mock()
     dev1_cluster_level.request.reset_mock()
     await hass.services.async_call(
@@ -1423,18 +1494,10 @@ async def async_test_level_on_off_from_hass(
         {"entity_id": entity_id, "transition": 10},
         blocking=True,
     )
-    assert on_off_cluster.request.call_count == 1
-    assert on_off_cluster.request.await_count == 1
+    assert on_off_cluster.request.call_count == 0
+    assert on_off_cluster.request.await_count == 0
     assert level_cluster.request.call_count == 1
     assert level_cluster.request.await_count == 1
-    assert on_off_cluster.request.call_args == call(
-        False,
-        on_off_cluster.commands_by_name["on"].id,
-        on_off_cluster.commands_by_name["on"].schema,
-        expect_reply=True,
-        manufacturer=None,
-        tsn=None,
-    )
     assert level_cluster.request.call_args == call(
         False,
         level_cluster.commands_by_name["move_to_level_with_on_off"].id,