From bae5a3dbd641e39ed9dfc237ebdefa1937ecd896 Mon Sep 17 00:00:00 2001 From: TheJulianJES Date: Thu, 3 Aug 2023 21:20:40 +0200 Subject: [PATCH] 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 | 12 ++-- tests/components/zha/test_light.py | 85 +++++++++++++++++++++++---- 2 files changed, 80 insertions(+), 17 deletions(-) diff --git a/homeassistant/components/zha/light.py b/homeassistant/components/zha/light.py index 9e71691aaa5..73955614c07 100644 --- a/homeassistant/components/zha/light.py +++ b/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 diff --git a/tests/components/zha/test_light.py b/tests/components/zha/test_light.py index 3abfd0e4f9c..c1f5cf04e35 100644 --- a/tests/components/zha/test_light.py +++ b/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,