From 3bca71244ed4a6fa1c94380746d3e8eb23a58d3f Mon Sep 17 00:00:00 2001 From: Xavi Date: Sun, 17 May 2020 14:08:16 +0200 Subject: [PATCH] fix(light-controller): add `add_transition_turn_toggle` attribute Fixes #79 --- Pipfile | 8 +- .../controllerx/core/type/light_controller.py | 44 ++++++---- docs/faq.md | 23 +++++- docs/start/type-configuration.md | 27 ++++--- tests/core/type/light_controller_test.py | 80 ++++++++++++++----- 5 files changed, 130 insertions(+), 52 deletions(-) diff --git a/Pipfile b/Pipfile index d2591c03..cfb573ef 100644 --- a/Pipfile +++ b/Pipfile @@ -5,15 +5,15 @@ verify_ssl = true [dev-packages] black = "==19.10b0" -pytest = "==5.4.1" +pytest = "==5.4.2" pytest-asyncio = "==0.12.0" pytest-cov = "==2.8.1" pytest-mock = "==3.1.0" mock = "==4.0.2" -pre-commit = "==2.3.0" -commitizen = "==1.19.3" +pre-commit = "==2.4.0" +commitizen = "==1.22.0" mypy = "==0.770" -flake8 = "==3.7.9" +flake8 = "==3.8.1" controllerx = {path = ".",editable = true} [packages] diff --git a/apps/controllerx/core/type/light_controller.py b/apps/controllerx/core/type/light_controller.py index cbb7b703..43c94d99 100644 --- a/apps/controllerx/core/type/light_controller.py +++ b/apps/controllerx/core/type/light_controller.py @@ -107,6 +107,9 @@ async def initialize(self) -> None: "smooth_power_on", self.supports_smooth_power_on() ) self.add_transition = self.args.get("add_transition", True) + self.add_transition_turn_toggle = self.args.get( + "add_transition_turn_toggle", True + ) bitfield = await self.get_entity_state( self.light["name"], attribute="supported_features" @@ -264,49 +267,62 @@ def get_light(self, light: Union[str, dict]) -> LightEntity: f"Type {type(light)} is not supported for `light` attribute" ) - async def call_light_service(self, service: str, **attributes) -> None: + async def call_light_service( + self, service: str, turned_toggle: bool, **attributes + ) -> None: + if "transition" not in attributes: attributes["transition"] = self.transition / 1000 if ( self.supported_features.not_supported(LightSupport.TRANSITION) or not self.add_transition + or (turned_toggle and not self.add_transition_turn_toggle) ): del attributes["transition"] await self.call_service(service, entity_id=self.light["name"], **attributes) @action - async def on(self, **attributes) -> None: - await self.call_light_service("light/turn_on", **attributes) + async def on(self, light_on: bool = None, **attributes) -> None: + if light_on is None: + light_state = await self.get_entity_state(self.light["name"]) + light_on = light_state == "on" + await self.call_light_service( + "light/turn_on", turned_toggle=not light_on, **attributes + ) @action async def off(self, **attributes) -> None: - await self.call_light_service("light/turn_off", **attributes) + await self.call_light_service( + "light/turn_off", turned_toggle=True, **attributes + ) @action async def toggle(self, **attributes) -> None: - await self.call_light_service("light/toggle", **attributes) + await self.call_light_service("light/toggle", turned_toggle=True, **attributes) @action - async def set_value(self, attribute: str, fraction: float) -> None: + async def set_value( + self, attribute: str, fraction: float, light_on: bool = None + ) -> None: fraction = max(0, min(fraction, 1)) stepper = self.automatic_steppers[attribute] if isinstance(stepper, MinMaxStepper): min_ = stepper.minmax.min max_ = stepper.minmax.max value = (max_ - min_) * fraction + min_ - await self.on(**{attribute: value}) + await self.on(light_on=light_on, **{attribute: value}) @action - async def on_full(self, attribute: str) -> None: + async def on_full(self, attribute: str, light_on: bool = None) -> None: stepper = self.automatic_steppers[attribute] stepper.previous_direction = Stepper.TOGGLE_UP - await self.set_value(attribute, 1) + await self.set_value(attribute, 1, light_on=light_on) @action - async def on_min(self, attribute: str) -> None: + async def on_min(self, attribute: str, light_on: bool = None) -> None: stepper = self.automatic_steppers[attribute] stepper.previous_direction = Stepper.TOGGLE_DOWN - await self.set_value(attribute, 0) + await self.set_value(attribute, 0, light_on=light_on) @action async def sync(self) -> None: @@ -436,7 +452,7 @@ async def change_light_state( attributes = {attribute: xy_color} if action_type == "hold": attributes["transition"] = self.delay / 1000 - await self.on(**attributes) + await self.on(**attributes, light_on=True) # In case of xy_color mode it never finishes the loop, the hold loop # will only stop if the hold action is called when releasing the button. # I haven't experimented any problems with it, but a future implementation @@ -445,14 +461,14 @@ async def change_light_state( if self.check_smooth_power_on( attribute, direction, await self.get_entity_state(self.light["name"]) ): - await self.on_min(attribute) + await self.on_min(attribute, light_on=False) # # After smooth power on, the light should not brighten up. return True new_state_attribute, exceeded = stepper.step(old, direction) attributes = {attribute: new_state_attribute} if action_type == "hold": attributes["transition"] = self.delay / 1000 - await self.on(**attributes) + await self.on(**attributes, light_on=True) self.value_attribute = new_state_attribute return exceeded diff --git a/docs/faq.md b/docs/faq.md index 8067c009..7488c7f0 100644 --- a/docs/faq.md +++ b/docs/faq.md @@ -23,4 +23,25 @@ This does not mean that any other integration will not work, but they might not #### 5. Error: "Value for X attribute could not be retrieved from light Y" -This error is shown when the light has support for the X attribute (e.g. brightness or color_temp) and the attribute is not in the state attribute of the entity. You can check whether the attribute X is shown in the state attributes from the "Developer Tools > States". \ No newline at end of file +This error is shown when the light has support for the X attribute (e.g. brightness or color_temp) and the attribute is not in the state attribute of the entity. You can check whether the attribute X is shown in the state attributes from the "Developer Tools > States". + +#### 6. Light is not turning on to the previous brightness + +Zigbee does not support transition natively to lights, so this attribute depends on the integration you have installed for your light. If you encountered this problem is because ControllerX, by default, sends the `transition` attribute to the light(s) through an HA call and the integration for your light does not support transition when turning on or off and it leads to an unexpected behaviour. In fact, if you go to AppDaemon logs, you will be able to see the service call that ControllerX does when pressing the buttons. You can then, replicate those calls on "Developer Tools > Services". These are the issues created related to this problem on the different integrations: + +- [Zigbee2MQTT](https://github.com/Koenkk/zigbee-herdsman-converters/issues/1073) (FIXED) +- [Hue integration](https://github.com/home-assistant/core/issues/32894) (OPEN) + +However, while the problem is not in the scope of ControllerX, there is a workaround that will help you fix this problem while losing the transition when turning on/off or toggeling. For this, you could add `add_transition_turn_toggle: false` to your controller configuration. This is an example: + +```yaml +problem_fixed: + module: controllerx + class: E1810Controller + controller: sensor.livingroom_controller_action + integration: z2m + light: light.bedroom + add_transition_turn_toggle: false +``` + +This will keep using transition when changing brightness or color, but not when turning on/off the light. diff --git a/docs/start/type-configuration.md b/docs/start/type-configuration.md index ba474f1f..c8b2d933 100644 --- a/docs/start/type-configuration.md +++ b/docs/start/type-configuration.md @@ -17,19 +17,20 @@ This controller allows the devices to control light or group of lights. This all - Smooth increase/decrease (holding button) of brightness and color - Color loop changing if the light supports xy color. -| key | type | value | description | -| ----------------- | -------------------- | ----------------------------------------------- | -------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------- | -| `light`\* | string \| dictionary | `group.livingroom_lights` or `light.kitchen` | The light (or group of lights) you want to control | -| `manual_steps` | int | 10 | Number of steps to go from min to max when clicking. If the value is 2 with one click you will set the light to 50% and with another one to 100%. | -| `automatic_steps` | int | 10 | Number of steps to go from min to max when smoothing. If the value is 2 with one click you will set the light to 50% and with another one to 100%. | -| `min_brightness` | int | 1 | The minimum brightness to set to the light. | -| `max_brightness` | int | 255 | The maximum brightness to set to the light. | -| `min_color_temp` | int | 153 | The minimum color temperature to set to the light. | -| `max_color_temp` | int | 500 | The maximum color temperature to set to the light. | -| `smooth_power_on` | boolean | False | If `True` the associated light will be set to minimum brightness when brightness up is clicked or hold ad light is off. | -| `delay` | int | [Controller specific](/controllerx/controllers) | Delay in milliseconds that takes between sending the instructions to the light (for the smooth functionality). Note that the maximum value is 1000 and if leaving to 0, you might get uncommon behavior. | -| `transition` | int | 300 | Time in milliseconds that takes the light to transition from one state to another one. | -| `add_transition` | boolean | True | If `true` adds transition if supported, otherwise it does not adds the `transition` attribute. | +| key | type | value | description | +| ---------------------------- | -------------------- | ----------------------------------------------- | ----------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------- | +| `light`\* | string \| dictionary | `group.livingroom_lights` or `light.kitchen` | The light (or group of lights) you want to control | +| `manual_steps` | int | 10 | Number of steps to go from min to max when clicking. If the value is 2 with one click you will set the light to 50% and with another one to 100%. | +| `automatic_steps` | int | 10 | Number of steps to go from min to max when smoothing. If the value is 2 with one click you will set the light to 50% and with another one to 100%. | +| `min_brightness` | int | 1 | The minimum brightness to set to the light. | +| `max_brightness` | int | 255 | The maximum brightness to set to the light. | +| `min_color_temp` | int | 153 | The minimum color temperature to set to the light. | +| `max_color_temp` | int | 500 | The maximum color temperature to set to the light. | +| `smooth_power_on` | boolean | False | If `True` the associated light will be set to minimum brightness when brightness up is clicked or hold ad light is off. | +| `delay` | int | [Controller specific](/controllerx/controllers) | Delay in milliseconds that takes between sending the instructions to the light (for the smooth functionality). Note that the maximum value is 1000 and if leaving to 0, you might get uncommon behavior. | +| `transition` | int | 300 | Time in milliseconds that takes the light to transition from one state to another one. | +| `add_transition` | boolean | True | If `true` adds transition if supported, otherwise it does not adds the `transition` attribute. | +| `add_transition_turn_toggle` | boolean | True | If `false` does not add transition when turning on/off or toggling, otherwise it adds the `transition` attribute to the call. See [FAQ #6](/controllerx/faq#6-light-is-not-turning-on-to-the-previous-brightness) for a further explanation on the use of this parameter. | _\* Required fields_ diff --git a/tests/core/type/light_controller_test.py b/tests/core/type/light_controller_test.py index a919e051..cc301c9b 100644 --- a/tests/core/type/light_controller_test.py +++ b/tests/core/type/light_controller_test.py @@ -229,21 +229,42 @@ async def fake_get_entity_state(*args, **kwargs): @pytest.mark.parametrize( - "attributes_input, transition_support, add_transition, attributes_expected", + "attributes_input, transition_support, turned_toggle, add_transition, add_transition_turn_toggle, attributes_expected", [ - ({"test": "test"}, True, True, {"test": "test", "transition": 0.3}), - ({"test": "test"}, False, True, {"test": "test"}), + ({"test": "test"}, True, True, True, True, {"test": "test", "transition": 0.3}), + ({"test": "test"}, False, True, True, True, {"test": "test"}), ( {"test": "test", "transition": 0.5}, True, True, + True, + True, + {"test": "test", "transition": 0.5}, + ), + ( {"test": "test", "transition": 0.5}, + False, + True, + True, + True, + {"test": "test"}, ), - ({"test": "test", "transition": 0.5}, False, True, {"test": "test"}), - ({}, True, True, {"transition": 0.3}), - ({}, False, True, {}), - ({}, False, True, {}), - ({}, False, False, {}), + ({}, True, True, True, True, {"transition": 0.3}), + ({}, True, True, True, False, {}), + ({}, True, True, False, True, {}), + ({}, True, True, False, False, {}), + ({}, True, False, True, True, {"transition": 0.3}), + ({}, True, False, True, False, {"transition": 0.3}), + ({}, True, False, False, True, {}), + ({}, True, False, False, False, {}), + ({}, False, True, True, True, {}), + ({}, False, True, True, False, {}), + ({}, False, True, False, True, {}), + ({}, False, True, False, False, {}), + ({}, False, False, True, True, {}), + ({}, False, False, True, False, {}), + ({}, False, False, False, True, {}), + ({}, False, False, False, False, {}), ], ) @pytest.mark.asyncio @@ -252,28 +273,42 @@ async def test_call_light_service( mocker, attributes_input, transition_support, + turned_toggle, add_transition, + add_transition_turn_toggle, attributes_expected, ): called_service_patch = mocker.patch.object(sut, "call_service") sut.transition = 300 sut.add_transition = add_transition + sut.add_transition_turn_toggle = add_transition_turn_toggle supported_features = {LightSupport.TRANSITION} if transition_support else set() sut.supported_features = LightSupport(FeatureSupport.encode(supported_features)) - await sut.call_light_service("test_service", **attributes_input) + await sut.call_light_service( + "test_service", turned_toggle=turned_toggle, **attributes_input + ) called_service_patch.assert_called_once_with( "test_service", entity_id=sut.light["name"], **attributes_expected ) +@pytest.mark.parametrize( + "light_on, light_state, expected_turned_toggle", + [(True, any, False), (False, any, True), (None, "on", False), (None, "off", True)], +) @pytest.mark.asyncio -async def test_on(sut, mocker, monkeypatch): +async def test_on( + sut, mocker, monkeypatch, light_on, light_state, expected_turned_toggle +): monkeypatch.setattr(sut, "call_light_service", fake_async_function()) + mocker.patch.object(sut, "get_entity_state", fake_async_function(light_state)) call_light_service_patch = mocker.patch.object(sut, "call_light_service") attributes = {"test": 0} - await sut.on(**attributes) - call_light_service_patch.assert_called_once_with("light/turn_on", **attributes) + await sut.on(light_on=light_on, **attributes) + call_light_service_patch.assert_called_once_with( + "light/turn_on", turned_toggle=expected_turned_toggle, **attributes + ) @pytest.mark.asyncio @@ -283,7 +318,9 @@ async def test_off(sut, mocker, monkeypatch): attributes = {"test": 0} await sut.off(**attributes) - call_light_service_patch.assert_called_once_with("light/turn_off", **attributes) + call_light_service_patch.assert_called_once_with( + "light/turn_off", turned_toggle=True, **attributes + ) @pytest.mark.asyncio @@ -293,7 +330,9 @@ async def test_toggle(sut, mocker, monkeypatch): attributes = {"test": 0} await sut.toggle(**attributes) - call_light_service_patch.assert_called_once_with("light/toggle", **attributes) + call_light_service_patch.assert_called_once_with( + "light/toggle", turned_toggle=True, **attributes + ) @pytest.mark.parametrize( @@ -318,12 +357,12 @@ async def test_set_value( sut.automatic_steppers = {attribute: stepper} # SUT - await sut.set_value(attribute, fraction) + await sut.set_value(attribute, fraction, light_on=False) # Checks assert on_patch.call_count == expected_calls if expected_calls > 0: - on_patch.assert_called_with(**{attribute: expected_value}) + on_patch.assert_called_with(light_on=False, **{attribute: expected_value}) @pytest.mark.asyncio @@ -336,10 +375,10 @@ async def test_on_full(sut, mocker): sut.automatic_steppers = {attribute: stepper} # SUT - await sut.on_full(attribute) + await sut.on_full(attribute, light_on=False) # Checks - on_patch.assert_called_once_with(**{attribute: max_}) + on_patch.assert_called_once_with(light_on=False, **{attribute: max_}) assert stepper.previous_direction == Stepper.TOGGLE_UP @@ -353,10 +392,10 @@ async def test_on_min(sut, mocker): sut.automatic_steppers = {attribute: stepper} # SUT - await sut.on_min(attribute) + await sut.on_min(attribute, light_on=False) # Checks - on_patch.assert_called_once_with(**{attribute: min_}) + on_patch.assert_called_once_with(light_on=False, **{attribute: min_}) assert stepper.previous_direction == Stepper.TOGGLE_DOWN @@ -376,6 +415,7 @@ async def test_sync( sut.light = {"name": "test_light"} sut.transition = 300 sut.add_transition = True + sut.add_transition_turn_toggle = True sut.supported_features = LightSupport( FeatureSupport.encode({LightSupport.TRANSITION}) )