From c00c7feed1029271aa15e02ac790d4722d1749eb Mon Sep 17 00:00:00 2001 From: Raman Gupta <7243222+raman325@users.noreply.github.com> Date: Thu, 30 Mar 2023 01:32:29 -0400 Subject: [PATCH 01/13] Add support for is_opening/closing to zwave covers --- homeassistant/components/zwave_js/cover.py | 117 ++++++++++++++------- 1 file changed, 81 insertions(+), 36 deletions(-) diff --git a/homeassistant/components/zwave_js/cover.py b/homeassistant/components/zwave_js/cover.py index 4704718c804269..b26515a0b99882 100644 --- a/homeassistant/components/zwave_js/cover.py +++ b/homeassistant/components/zwave_js/cover.py @@ -66,36 +66,6 @@ def async_add_cover(info: ZwaveDiscoveryInfo) -> None: ) -def percent_to_zwave_position(value: int) -> int: - """Convert position in 0-100 scale to 0-99 scale. - - `value` -- (int) Position byte value from 0-100. - """ - if value > 0: - return max(1, round((value / 100) * 99)) - return 0 - - -def percent_to_zwave_tilt(value: int) -> int: - """Convert position in 0-100 scale to 0-99 scale. - - `value` -- (int) Position byte value from 0-100. - """ - if value > 0: - return round((value / 100) * 99) - return 0 - - -def zwave_tilt_to_percent(value: int) -> int: - """Convert 0-99 scale to position in 0-100 scale. - - `value` -- (int) Position byte value from 0-99. - """ - if value > 0: - return round((value / 99) * 100) - return 0 - - class ZWaveCover(ZWaveBaseEntity, CoverEntity): """Representation of a Z-Wave Cover device.""" @@ -115,6 +85,57 @@ def __init__( if self.info.platform_hint == "window_blind": self._attr_device_class = CoverDeviceClass.BLIND + # Set the previous value to the current value so that if there's an update, we + # can determine whether the cover is opening or closing. + self._prev_value: int | None = self.info.primary_value.value + + def percent_to_zwave_position(self, value: int) -> int: + """Convert position in 0-100 scale to closed_value-opened_value scale. + + `value` -- (int) Position byte value from 0-100. + """ + if value > self._closed_value: + return max(1, round((value / 100) * self._cover_range) + self._closed_value) + return self._closed_value + + def on_value_update(self) -> None: + """Handle primary value update.""" + new_value = self.info.primary_value.value + # If the cover is fully closed or opened, or if we don't know either the + # previous value or current value, we can't determine whether the cover + # is opening or closing + if ( + new_value in (self._closed_value, self._opened_value) + or self._prev_value is None + or new_value is None + ): + self._attr_is_closing = None + self._attr_is_opening = None + elif self._prev_value is not None and new_value is not None: + # If the current value is less than the previous value, the cover is + # closing, otherwise it is opening. + self._attr_is_closing = (new_value - self._prev_value) < 0 + self._attr_is_opening = not self._attr_is_closing + + self._prev_value = new_value + + @property + def _opened_value(self) -> int: + """Return fully opened value.""" + max_ = self.info.primary_value.metadata.max + return 99 if max_ is None else max_ + + @property + def _closed_value(self) -> int: + """Return fully closed value.""" + min_ = self.info.primary_value.metadata.min + return 0 if min_ is None else min_ + + @property + def _cover_range(self) -> int: + """Return range between fully opened and fully closed.""" + return self._opened_value - self._closed_value + @property def is_closed(self) -> bool | None: """Return true if cover is closed.""" @@ -129,27 +150,33 @@ def current_cover_position(self) -> int | None: if self.info.primary_value.value is None: # guard missing value return None - return round((cast(int, self.info.primary_value.value) / 99) * 100) + return round( + ( + (cast(int, self.info.primary_value.value) - self._closed_value) + / self._cover_range + ) + * 100 + ) async def async_set_cover_position(self, **kwargs: Any) -> None: """Move the cover to a specific position.""" target_value = self.get_zwave_value(TARGET_VALUE_PROPERTY) assert target_value is not None await self.info.node.async_set_value( - target_value, percent_to_zwave_position(kwargs[ATTR_POSITION]) + target_value, self.percent_to_zwave_position(kwargs[ATTR_POSITION]) ) async def async_open_cover(self, **kwargs: Any) -> None: """Open the cover.""" target_value = self.get_zwave_value(TARGET_VALUE_PROPERTY) assert target_value is not None - await self.info.node.async_set_value(target_value, 99) + await self.info.node.async_set_value(target_value, self._opened_value) async def async_close_cover(self, **kwargs: Any) -> None: """Close cover.""" target_value = self.get_zwave_value(TARGET_VALUE_PROPERTY) assert target_value is not None - await self.info.node.async_set_value(target_value, 0) + await self.info.node.async_set_value(target_value, self._closed_value) async def async_stop_cover(self, **kwargs: Any) -> None: """Stop cover.""" @@ -188,6 +215,24 @@ def __init__( CoverTiltDataTemplate, self.info.platform_data_template ) + def percent_to_zwave_tilt(self, value: int) -> int: + """Convert position in 0-100 scale to closed_value-opened_value scale. + + `value` -- (int) Position byte value from 0-100. + """ + if value > self._closed_value: + return round((value / 100) * self._cover_range) + self._closed_value + return self._closed_value + + def zwave_tilt_to_percent(self, value: int) -> int: + """Convert closed_value-opened_value scale to position in 0-100 scale. + + `value` -- (int) Position byte value from closed_value-opened_value. + """ + if value > self._closed_value: + return round(((value - self._closed_value) / self._cover_range) * 100) + return self._closed_value + @property def current_cover_tilt_position(self) -> int | None: """Return current position of cover tilt. @@ -197,7 +242,7 @@ def current_cover_tilt_position(self) -> int | None: value = self.data_template.current_tilt_value(self.info.platform_data) if value is None or value.value is None: return None - return zwave_tilt_to_percent(int(value.value)) + return self.zwave_tilt_to_percent(int(value.value)) async def async_set_cover_tilt_position(self, **kwargs: Any) -> None: """Move the cover tilt to a specific position.""" @@ -205,7 +250,7 @@ async def async_set_cover_tilt_position(self, **kwargs: Any) -> None: if tilt_value: await self.info.node.async_set_value( tilt_value, - percent_to_zwave_tilt(kwargs[ATTR_TILT_POSITION]), + self.percent_to_zwave_tilt(kwargs[ATTR_TILT_POSITION]), ) async def async_open_cover_tilt(self, **kwargs: Any) -> None: From 3dab4bf65b340894ac28d88486e62ab968868845 Mon Sep 17 00:00:00 2001 From: Raman Gupta <7243222+raman325@users.noreply.github.com> Date: Thu, 30 Mar 2023 01:44:09 -0400 Subject: [PATCH 02/13] Use constants in tests and rename property --- homeassistant/components/zwave_js/cover.py | 16 ++-- tests/components/zwave_js/test_cover.py | 98 ++++++++++++---------- 2 files changed, 60 insertions(+), 54 deletions(-) diff --git a/homeassistant/components/zwave_js/cover.py b/homeassistant/components/zwave_js/cover.py index b26515a0b99882..35775cd3e5dcfb 100644 --- a/homeassistant/components/zwave_js/cover.py +++ b/homeassistant/components/zwave_js/cover.py @@ -90,7 +90,7 @@ def __init__( self._prev_value: int | None = self.info.primary_value.value def percent_to_zwave_position(self, value: int) -> int: - """Convert position in 0-100 scale to closed_value-opened_value scale. + """Convert position in 0-100 scale to closed_value-open_value scale. `value` -- (int) Position byte value from 0-100. """ @@ -105,7 +105,7 @@ def on_value_update(self) -> None: # previous value or current value, we can't determine whether the cover # is opening or closing if ( - new_value in (self._closed_value, self._opened_value) + new_value in (self._closed_value, self._open_value) or self._prev_value is None or new_value is None ): @@ -120,7 +120,7 @@ def on_value_update(self) -> None: self._prev_value = new_value @property - def _opened_value(self) -> int: + def _open_value(self) -> int: """Return fully opened value.""" max_ = self.info.primary_value.metadata.max return 99 if max_ is None else max_ @@ -134,7 +134,7 @@ def _closed_value(self) -> int: @property def _cover_range(self) -> int: """Return range between fully opened and fully closed.""" - return self._opened_value - self._closed_value + return self._open_value - self._closed_value @property def is_closed(self) -> bool | None: @@ -170,7 +170,7 @@ async def async_open_cover(self, **kwargs: Any) -> None: """Open the cover.""" target_value = self.get_zwave_value(TARGET_VALUE_PROPERTY) assert target_value is not None - await self.info.node.async_set_value(target_value, self._opened_value) + await self.info.node.async_set_value(target_value, self._open_value) async def async_close_cover(self, **kwargs: Any) -> None: """Close cover.""" @@ -216,7 +216,7 @@ def __init__( ) def percent_to_zwave_tilt(self, value: int) -> int: - """Convert position in 0-100 scale to closed_value-opened_value scale. + """Convert position in 0-100 scale to closed_value-open_value scale. `value` -- (int) Position byte value from 0-100. """ @@ -225,9 +225,9 @@ def percent_to_zwave_tilt(self, value: int) -> int: return self._closed_value def zwave_tilt_to_percent(self, value: int) -> int: - """Convert closed_value-opened_value scale to position in 0-100 scale. + """Convert closed_value-open_value scale to position in 0-100 scale. - `value` -- (int) Position byte value from closed_value-opened_value. + `value` -- (int) Position byte value from closed_value-open_value. """ if value > self._closed_value: return round(((value - self._closed_value) / self._cover_range) * 100) diff --git a/tests/components/zwave_js/test_cover.py b/tests/components/zwave_js/test_cover.py index f1e9366593807e..bb4fe23d265fe1 100644 --- a/tests/components/zwave_js/test_cover.py +++ b/tests/components/zwave_js/test_cover.py @@ -10,14 +10,20 @@ from homeassistant.components.cover import ( ATTR_CURRENT_POSITION, ATTR_CURRENT_TILT_POSITION, + ATTR_POSITION, DOMAIN, SERVICE_CLOSE_COVER, + SERVICE_CLOSE_COVER_TILT, SERVICE_OPEN_COVER, + SERVICE_OPEN_COVER_TILT, + SERVICE_SET_COVER_POSITION, + SERVICE_STOP_COVER, CoverDeviceClass, ) from homeassistant.components.zwave_js.helpers import ZwaveValueMatcher from homeassistant.const import ( ATTR_DEVICE_CLASS, + ATTR_ENTITY_ID, STATE_CLOSED, STATE_CLOSING, STATE_OPEN, @@ -46,14 +52,14 @@ async def test_window_cover( assert state assert state.attributes[ATTR_DEVICE_CLASS] == CoverDeviceClass.WINDOW - assert state.state == "closed" + assert state.state == STATE_CLOSED assert state.attributes[ATTR_CURRENT_POSITION] == 0 # Test setting position await hass.services.async_call( - "cover", - "set_cover_position", - {"entity_id": WINDOW_COVER_ENTITY, "position": 50}, + DOMAIN, + SERVICE_SET_COVER_POSITION, + {ATTR_ENTITY_ID: WINDOW_COVER_ENTITY, ATTR_POSITION: 50}, blocking=True, ) @@ -72,9 +78,9 @@ async def test_window_cover( # Test setting position await hass.services.async_call( - "cover", - "set_cover_position", - {"entity_id": WINDOW_COVER_ENTITY, "position": 0}, + DOMAIN, + SERVICE_SET_COVER_POSITION, + {ATTR_ENTITY_ID: WINDOW_COVER_ENTITY, ATTR_POSITION: 0}, blocking=True, ) @@ -93,9 +99,9 @@ async def test_window_cover( # Test opening await hass.services.async_call( - "cover", - "open_cover", - {"entity_id": WINDOW_COVER_ENTITY}, + DOMAIN, + SERVICE_OPEN_COVER, + {ATTR_ENTITY_ID: WINDOW_COVER_ENTITY}, blocking=True, ) @@ -113,9 +119,9 @@ async def test_window_cover( client.async_send_command.reset_mock() # Test stop after opening await hass.services.async_call( - "cover", - "stop_cover", - {"entity_id": WINDOW_COVER_ENTITY}, + DOMAIN, + SERVICE_STOP_COVER, + {ATTR_ENTITY_ID: WINDOW_COVER_ENTITY}, blocking=True, ) @@ -152,13 +158,13 @@ async def test_window_cover( client.async_send_command.reset_mock() state = hass.states.get(WINDOW_COVER_ENTITY) - assert state.state == "open" + assert state.state == STATE_OPEN # Test closing await hass.services.async_call( - "cover", - "close_cover", - {"entity_id": WINDOW_COVER_ENTITY}, + DOMAIN, + SERVICE_CLOSE_COVER, + {ATTR_ENTITY_ID: WINDOW_COVER_ENTITY}, blocking=True, ) assert len(client.async_send_command.call_args_list) == 1 @@ -176,9 +182,9 @@ async def test_window_cover( # Test stop after closing await hass.services.async_call( - "cover", - "stop_cover", - {"entity_id": WINDOW_COVER_ENTITY}, + DOMAIN, + SERVICE_STOP_COVER, + {ATTR_ENTITY_ID: WINDOW_COVER_ENTITY}, blocking=True, ) @@ -215,10 +221,10 @@ async def test_window_cover( node.receive_event(event) state = hass.states.get(WINDOW_COVER_ENTITY) - assert state.state == "closed" + assert state.state == STATE_CLOSED -async def test_fibaro_FGR222_shutter_cover( +async def test_fibaro_fgr222_shutter_cover( hass: HomeAssistant, client, fibaro_fgr222_shutter, integration ) -> None: """Test tilt function of the Fibaro Shutter devices.""" @@ -226,14 +232,14 @@ async def test_fibaro_FGR222_shutter_cover( assert state assert state.attributes[ATTR_DEVICE_CLASS] == CoverDeviceClass.SHUTTER - assert state.state == "open" + assert state.state == STATE_OPEN assert state.attributes[ATTR_CURRENT_TILT_POSITION] == 0 # Test opening tilts await hass.services.async_call( - "cover", - "open_cover_tilt", - {"entity_id": FIBARO_SHUTTER_COVER_ENTITY}, + DOMAIN, + SERVICE_OPEN_COVER_TILT, + {ATTR_ENTITY_ID: FIBARO_SHUTTER_COVER_ENTITY}, blocking=True, ) @@ -252,9 +258,9 @@ async def test_fibaro_FGR222_shutter_cover( client.async_send_command.reset_mock() # Test closing tilts await hass.services.async_call( - "cover", - "close_cover_tilt", - {"entity_id": FIBARO_SHUTTER_COVER_ENTITY}, + DOMAIN, + SERVICE_CLOSE_COVER_TILT, + {ATTR_ENTITY_ID: FIBARO_SHUTTER_COVER_ENTITY}, blocking=True, ) @@ -306,14 +312,14 @@ async def test_aeotec_nano_shutter_cover( assert state assert state.attributes[ATTR_DEVICE_CLASS] == CoverDeviceClass.WINDOW - assert state.state == "closed" + assert state.state == STATE_CLOSED assert state.attributes[ATTR_CURRENT_POSITION] == 0 # Test opening await hass.services.async_call( - "cover", - "open_cover", - {"entity_id": AEOTEC_SHUTTER_COVER_ENTITY}, + DOMAIN, + SERVICE_OPEN_COVER, + {ATTR_ENTITY_ID: AEOTEC_SHUTTER_COVER_ENTITY}, blocking=True, ) @@ -331,9 +337,9 @@ async def test_aeotec_nano_shutter_cover( client.async_send_command.reset_mock() # Test stop after opening await hass.services.async_call( - "cover", - "stop_cover", - {"entity_id": AEOTEC_SHUTTER_COVER_ENTITY}, + DOMAIN, + SERVICE_STOP_COVER, + {ATTR_ENTITY_ID: AEOTEC_SHUTTER_COVER_ENTITY}, blocking=True, ) @@ -371,13 +377,13 @@ async def test_aeotec_nano_shutter_cover( client.async_send_command.reset_mock() state = hass.states.get(AEOTEC_SHUTTER_COVER_ENTITY) - assert state.state == "open" + assert state.state == STATE_OPEN # Test closing await hass.services.async_call( - "cover", - "close_cover", - {"entity_id": AEOTEC_SHUTTER_COVER_ENTITY}, + DOMAIN, + SERVICE_CLOSE_COVER, + {ATTR_ENTITY_ID: AEOTEC_SHUTTER_COVER_ENTITY}, blocking=True, ) assert len(client.async_send_command.call_args_list) == 1 @@ -395,9 +401,9 @@ async def test_aeotec_nano_shutter_cover( # Test stop after closing await hass.services.async_call( - "cover", - "stop_cover", - {"entity_id": AEOTEC_SHUTTER_COVER_ENTITY}, + DOMAIN, + SERVICE_STOP_COVER, + {ATTR_ENTITY_ID: AEOTEC_SHUTTER_COVER_ENTITY}, blocking=True, ) @@ -447,7 +453,7 @@ async def test_motor_barrier_cover( # Test open await hass.services.async_call( - DOMAIN, SERVICE_OPEN_COVER, {"entity_id": GDC_COVER_ENTITY}, blocking=True + DOMAIN, SERVICE_OPEN_COVER, {ATTR_ENTITY_ID: GDC_COVER_ENTITY}, blocking=True ) assert len(client.async_send_command.call_args_list) == 1 @@ -469,7 +475,7 @@ async def test_motor_barrier_cover( # Test close await hass.services.async_call( - DOMAIN, SERVICE_CLOSE_COVER, {"entity_id": GDC_COVER_ENTITY}, blocking=True + DOMAIN, SERVICE_CLOSE_COVER, {ATTR_ENTITY_ID: GDC_COVER_ENTITY}, blocking=True ) assert len(client.async_send_command.call_args_list) == 1 @@ -631,7 +637,7 @@ async def test_motor_barrier_cover_no_primary_value( assert ATTR_CURRENT_POSITION not in state.attributes -async def test_fibaro_FGR222_shutter_cover_no_tilt( +async def test_fibaro_fgr222_shutter_cover_no_tilt( hass: HomeAssistant, client, fibaro_fgr222_shutter_state, integration ) -> None: """Test tilt function of the Fibaro Shutter devices with tilt value is None.""" From 00284834e909a90e3d2ad6c37bd8d1eafc8e3013 Mon Sep 17 00:00:00 2001 From: Raman Gupta <7243222+raman325@users.noreply.github.com> Date: Thu, 30 Mar 2023 01:50:25 -0400 Subject: [PATCH 03/13] add tests for closing and opening --- tests/components/zwave_js/test_cover.py | 46 +++++++++++++++++++++++++ 1 file changed, 46 insertions(+) diff --git a/tests/components/zwave_js/test_cover.py b/tests/components/zwave_js/test_cover.py index bb4fe23d265fe1..2a592dc70cc8d9 100644 --- a/tests/components/zwave_js/test_cover.py +++ b/tests/components/zwave_js/test_cover.py @@ -136,6 +136,52 @@ async def test_window_cover( } assert not open_args["value"] + # Test that we see opening state from value updated event + event = Event( + type="value updated", + data={ + "source": "node", + "event": "value updated", + "nodeId": 6, + "args": { + "commandClassName": "Multilevel Switch", + "commandClass": 38, + "endpoint": 0, + "property": "currentValue", + "newValue": 50, + "prevValue": 0, + "propertyName": "currentValue", + }, + }, + ) + node.receive_event(event) + + state = hass.states.get(WINDOW_COVER_ENTITY) + assert state.state == STATE_OPENING + + # Test that we see closing state from value updated event + event = Event( + type="value updated", + data={ + "source": "node", + "event": "value updated", + "nodeId": 6, + "args": { + "commandClassName": "Multilevel Switch", + "commandClass": 38, + "endpoint": 0, + "property": "currentValue", + "newValue": 25, + "prevValue": 0, + "propertyName": "currentValue", + }, + }, + ) + node.receive_event(event) + + state = hass.states.get(WINDOW_COVER_ENTITY) + assert state.state == STATE_CLOSING + # Test position update from value updated event event = Event( type="value updated", From 83f7f0096d1feef66b038b8c72befbef5f4931dc Mon Sep 17 00:00:00 2001 From: Raman Gupta <7243222+raman325@users.noreply.github.com> Date: Thu, 30 Mar 2023 01:53:11 -0400 Subject: [PATCH 04/13] rename variable --- homeassistant/components/zwave_js/cover.py | 38 ++++++++++++---------- 1 file changed, 20 insertions(+), 18 deletions(-) diff --git a/homeassistant/components/zwave_js/cover.py b/homeassistant/components/zwave_js/cover.py index 35775cd3e5dcfb..bf29f2ce6b52af 100644 --- a/homeassistant/components/zwave_js/cover.py +++ b/homeassistant/components/zwave_js/cover.py @@ -94,9 +94,11 @@ def percent_to_zwave_position(self, value: int) -> int: `value` -- (int) Position byte value from 0-100. """ - if value > self._closed_value: - return max(1, round((value / 100) * self._cover_range) + self._closed_value) - return self._closed_value + if value > self._fully_closed_value: + return max( + 1, round((value / 100) * self._cover_range) + self._fully_closed_value + ) + return self._fully_closed_value def on_value_update(self) -> None: """Handle primary value update.""" @@ -105,7 +107,7 @@ def on_value_update(self) -> None: # previous value or current value, we can't determine whether the cover # is opening or closing if ( - new_value in (self._closed_value, self._open_value) + new_value in (self._fully_closed_value, self._fully_open_value) or self._prev_value is None or new_value is None ): @@ -120,21 +122,21 @@ def on_value_update(self) -> None: self._prev_value = new_value @property - def _open_value(self) -> int: - """Return fully opened value.""" + def _fully_open_value(self) -> int: + """Return value that represents fully opened.""" max_ = self.info.primary_value.metadata.max return 99 if max_ is None else max_ @property - def _closed_value(self) -> int: - """Return fully closed value.""" + def _fully_closed_value(self) -> int: + """Return value that represents fully closed.""" min_ = self.info.primary_value.metadata.min return 0 if min_ is None else min_ @property def _cover_range(self) -> int: """Return range between fully opened and fully closed.""" - return self._open_value - self._closed_value + return self._fully_open_value - self._fully_closed_value @property def is_closed(self) -> bool | None: @@ -152,7 +154,7 @@ def current_cover_position(self) -> int | None: return None return round( ( - (cast(int, self.info.primary_value.value) - self._closed_value) + (cast(int, self.info.primary_value.value) - self._fully_closed_value) / self._cover_range ) * 100 @@ -170,13 +172,13 @@ async def async_open_cover(self, **kwargs: Any) -> None: """Open the cover.""" target_value = self.get_zwave_value(TARGET_VALUE_PROPERTY) assert target_value is not None - await self.info.node.async_set_value(target_value, self._open_value) + await self.info.node.async_set_value(target_value, self._fully_open_value) async def async_close_cover(self, **kwargs: Any) -> None: """Close cover.""" target_value = self.get_zwave_value(TARGET_VALUE_PROPERTY) assert target_value is not None - await self.info.node.async_set_value(target_value, self._closed_value) + await self.info.node.async_set_value(target_value, self._fully_closed_value) async def async_stop_cover(self, **kwargs: Any) -> None: """Stop cover.""" @@ -220,18 +222,18 @@ def percent_to_zwave_tilt(self, value: int) -> int: `value` -- (int) Position byte value from 0-100. """ - if value > self._closed_value: - return round((value / 100) * self._cover_range) + self._closed_value - return self._closed_value + if value > self._fully_closed_value: + return round((value / 100) * self._cover_range) + self._fully_closed_value + return self._fully_closed_value def zwave_tilt_to_percent(self, value: int) -> int: """Convert closed_value-open_value scale to position in 0-100 scale. `value` -- (int) Position byte value from closed_value-open_value. """ - if value > self._closed_value: - return round(((value - self._closed_value) / self._cover_range) * 100) - return self._closed_value + if value > self._fully_closed_value: + return round(((value - self._fully_closed_value) / self._cover_range) * 100) + return self._fully_closed_value @property def current_cover_tilt_position(self) -> int | None: From 7c72be40b81cbaaa0ce8697e8e7ef5d76fd43ea3 Mon Sep 17 00:00:00 2001 From: Raman Gupta <7243222+raman325@users.noreply.github.com> Date: Thu, 30 Mar 2023 01:54:46 -0400 Subject: [PATCH 05/13] comments and bug --- homeassistant/components/zwave_js/cover.py | 8 +++++--- 1 file changed, 5 insertions(+), 3 deletions(-) diff --git a/homeassistant/components/zwave_js/cover.py b/homeassistant/components/zwave_js/cover.py index bf29f2ce6b52af..0a8a958365eaaa 100644 --- a/homeassistant/components/zwave_js/cover.py +++ b/homeassistant/components/zwave_js/cover.py @@ -104,10 +104,12 @@ def on_value_update(self) -> None: """Handle primary value update.""" new_value = self.info.primary_value.value # If the cover is fully closed or opened, or if we don't know either the - # previous value or current value, we can't determine whether the cover - # is opening or closing + # previous value or current value, or if they are the same value we can't + # make any determination whether the cover is opening or closing so we set + # them to None. if ( new_value in (self._fully_closed_value, self._fully_open_value) + or new_value == self._prev_value or self._prev_value is None or new_value is None ): @@ -117,7 +119,7 @@ def on_value_update(self) -> None: # If the current value is less than the previous value, the cover is # closing, otherwise it is opening. self._attr_is_closing = (new_value - self._prev_value) < 0 - self._attr_is_opening = not self._attr_is_closing + self._attr_is_opening = (new_value - self._prev_value) > 0 self._prev_value = new_value From 26f627a16ae92bf0367e25d3e7069bbfdc6bd494 Mon Sep 17 00:00:00 2001 From: Raman Gupta <7243222+raman325@users.noreply.github.com> Date: Thu, 30 Mar 2023 01:55:14 -0400 Subject: [PATCH 06/13] revert one change --- homeassistant/components/zwave_js/cover.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/homeassistant/components/zwave_js/cover.py b/homeassistant/components/zwave_js/cover.py index 0a8a958365eaaa..5f1ea8b9b82b5e 100644 --- a/homeassistant/components/zwave_js/cover.py +++ b/homeassistant/components/zwave_js/cover.py @@ -119,7 +119,7 @@ def on_value_update(self) -> None: # If the current value is less than the previous value, the cover is # closing, otherwise it is opening. self._attr_is_closing = (new_value - self._prev_value) < 0 - self._attr_is_opening = (new_value - self._prev_value) > 0 + self._attr_is_opening = not self._attr_is_closing self._prev_value = new_value From 45dc81a4505c523d6b1d20e28c18ee509251db3d Mon Sep 17 00:00:00 2001 From: Raman Gupta <7243222+raman325@users.noreply.github.com> Date: Thu, 30 Mar 2023 02:13:34 -0400 Subject: [PATCH 07/13] cleanup --- homeassistant/components/zwave_js/cover.py | 15 ++++++++------- 1 file changed, 8 insertions(+), 7 deletions(-) diff --git a/homeassistant/components/zwave_js/cover.py b/homeassistant/components/zwave_js/cover.py index 5f1ea8b9b82b5e..b207e4d52a1cf4 100644 --- a/homeassistant/components/zwave_js/cover.py +++ b/homeassistant/components/zwave_js/cover.py @@ -103,21 +103,22 @@ def percent_to_zwave_position(self, value: int) -> int: def on_value_update(self) -> None: """Handle primary value update.""" new_value = self.info.primary_value.value + # If the value update doesn't change anything, there is nothing to update. + if new_value == self._prev_value: + return # If the cover is fully closed or opened, or if we don't know either the - # previous value or current value, or if they are the same value we can't - # make any determination whether the cover is opening or closing so we set - # them to None. + # previous value or current value, we can't make any determination whether the + # cover is opening or closing so we set them to None. if ( new_value in (self._fully_closed_value, self._fully_open_value) - or new_value == self._prev_value or self._prev_value is None or new_value is None ): self._attr_is_closing = None self._attr_is_opening = None - elif self._prev_value is not None and new_value is not None: - # If the current value is less than the previous value, the cover is - # closing, otherwise it is opening. + # If the current value is less than the previous value, the cover is + # closing, otherwise it is opening. + else: self._attr_is_closing = (new_value - self._prev_value) < 0 self._attr_is_opening = not self._attr_is_closing From 7d20aa5d6c2768f9fc7e3b384eff37559592d654 Mon Sep 17 00:00:00 2001 From: Raman Gupta <7243222+raman325@users.noreply.github.com> Date: Thu, 30 Mar 2023 10:45:03 -0400 Subject: [PATCH 08/13] Add coverage --- tests/components/zwave_js/test_cover.py | 23 +++++++++++++++++++++++ 1 file changed, 23 insertions(+) diff --git a/tests/components/zwave_js/test_cover.py b/tests/components/zwave_js/test_cover.py index 2a592dc70cc8d9..79c234c54f84da 100644 --- a/tests/components/zwave_js/test_cover.py +++ b/tests/components/zwave_js/test_cover.py @@ -182,6 +182,29 @@ async def test_window_cover( state = hass.states.get(WINDOW_COVER_ENTITY) assert state.state == STATE_CLOSING + # Test that nothing changes when we send the exact same value + event = Event( + type="value updated", + data={ + "source": "node", + "event": "value updated", + "nodeId": 6, + "args": { + "commandClassName": "Multilevel Switch", + "commandClass": 38, + "endpoint": 0, + "property": "currentValue", + "newValue": 25, + "prevValue": 25, + "propertyName": "currentValue", + }, + }, + ) + node.receive_event(event) + + state = hass.states.get(WINDOW_COVER_ENTITY) + assert state.state == STATE_CLOSING + # Test position update from value updated event event = Event( type="value updated", From e9d7161b595e2edc13e25ac52d13f42e4b0f6c43 Mon Sep 17 00:00:00 2001 From: Raman Gupta <7243222+raman325@users.noreply.github.com> Date: Thu, 30 Mar 2023 14:16:34 -0400 Subject: [PATCH 09/13] If currentValue == targetValue, cover is not opening or closing either --- homeassistant/components/zwave_js/cover.py | 30 ++++++++++++++-------- 1 file changed, 19 insertions(+), 11 deletions(-) diff --git a/homeassistant/components/zwave_js/cover.py b/homeassistant/components/zwave_js/cover.py index b207e4d52a1cf4..2d06fbc9586c19 100644 --- a/homeassistant/components/zwave_js/cover.py +++ b/homeassistant/components/zwave_js/cover.py @@ -85,9 +85,9 @@ def __init__( if self.info.platform_hint == "window_blind": self._attr_device_class = CoverDeviceClass.BLIND - # Set the previous value to the current value so that if there's an update, we - # can determine whether the cover is opening or closing. - self._prev_value: int | None = self.info.primary_value.value + # Store the current value so that if there's an update, we can determine + # whether the cover is opening or closing. + self._curr_value: int | None = self.info.primary_value.value def percent_to_zwave_position(self, value: int) -> int: """Convert position in 0-100 scale to closed_value-open_value scale. @@ -104,14 +104,22 @@ def on_value_update(self) -> None: """Handle primary value update.""" new_value = self.info.primary_value.value # If the value update doesn't change anything, there is nothing to update. - if new_value == self._prev_value: + if new_value == self._curr_value: return - # If the cover is fully closed or opened, or if we don't know either the - # previous value or current value, we can't make any determination whether the + target_value = self.get_zwave_value(TARGET_VALUE_PROPERTY) + assert target_value is not None + # If the cover is fully closed or opened or at the target value, or if we don't + # know either the previous value or current value, either the cover is neither + # opening or closing anymore or we can't make any determination whether the # cover is opening or closing so we set them to None. if ( - new_value in (self._fully_closed_value, self._fully_open_value) - or self._prev_value is None + new_value + in ( + self._fully_closed_value, + self._fully_open_value, + target_value.value, + ) + or self._curr_value is None or new_value is None ): self._attr_is_closing = None @@ -119,10 +127,10 @@ def on_value_update(self) -> None: # If the current value is less than the previous value, the cover is # closing, otherwise it is opening. else: - self._attr_is_closing = (new_value - self._prev_value) < 0 + self._attr_is_closing = (new_value - self._curr_value) < 0 self._attr_is_opening = not self._attr_is_closing - self._prev_value = new_value + self._curr_value = new_value @property def _fully_open_value(self) -> int: @@ -147,7 +155,7 @@ def is_closed(self) -> bool | None: if self.info.primary_value.value is None: # guard missing value return None - return bool(self.info.primary_value.value == 0) + return bool(self.info.primary_value.value == self._fully_closed_value) @property def current_cover_position(self) -> int | None: From 384f5b197464477bc0176ce6afe59441f88c8f04 Mon Sep 17 00:00:00 2001 From: Raman Gupta <7243222+raman325@users.noreply.github.com> Date: Thu, 30 Mar 2023 14:46:42 -0400 Subject: [PATCH 10/13] swap --- homeassistant/components/zwave_js/cover.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/homeassistant/components/zwave_js/cover.py b/homeassistant/components/zwave_js/cover.py index 2d06fbc9586c19..3381c3ec3f3e67 100644 --- a/homeassistant/components/zwave_js/cover.py +++ b/homeassistant/components/zwave_js/cover.py @@ -119,8 +119,8 @@ def on_value_update(self) -> None: self._fully_open_value, target_value.value, ) - or self._curr_value is None or new_value is None + or self._curr_value is None ): self._attr_is_closing = None self._attr_is_opening = None From 7405c44fbfc82f200febccde66b027f34cce5e59 Mon Sep 17 00:00:00 2001 From: Raman Gupta <7243222+raman325@users.noreply.github.com> Date: Tue, 2 May 2023 11:24:30 -0400 Subject: [PATCH 11/13] fix --- homeassistant/components/zwave_js/cover.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/homeassistant/components/zwave_js/cover.py b/homeassistant/components/zwave_js/cover.py index bbd100b89181fc..6898a750295e0e 100644 --- a/homeassistant/components/zwave_js/cover.py +++ b/homeassistant/components/zwave_js/cover.py @@ -269,7 +269,7 @@ async def async_set_cover_tilt_position(self, **kwargs: Any) -> None: assert self._current_tilt_value await self.info.node.async_set_value( self._current_tilt_value, - percent_to_zwave_tilt(kwargs[ATTR_TILT_POSITION]), + self.percent_to_zwave_tilt(kwargs[ATTR_TILT_POSITION]), ) async def async_open_cover_tilt(self, **kwargs: Any) -> None: From a287f8d732ab8299899057caf3d2bea2bdfa7e5d Mon Sep 17 00:00:00 2001 From: Raman Gupta <7243222+raman325@users.noreply.github.com> Date: Tue, 2 May 2023 16:39:32 -0400 Subject: [PATCH 12/13] minor edit --- homeassistant/components/zwave_js/cover.py | 7 ++++--- 1 file changed, 4 insertions(+), 3 deletions(-) diff --git a/homeassistant/components/zwave_js/cover.py b/homeassistant/components/zwave_js/cover.py index 6898a750295e0e..9661e8572b8682 100644 --- a/homeassistant/components/zwave_js/cover.py +++ b/homeassistant/components/zwave_js/cover.py @@ -124,9 +124,9 @@ def on_value_update(self) -> None: target_value = self.get_zwave_value(TARGET_VALUE_PROPERTY) assert target_value is not None # If the cover is fully closed or opened or at the target value, or if we don't - # know either the previous value or current value, either the cover is neither - # opening or closing anymore or we can't make any determination whether the - # cover is opening or closing so we set them to None. + # know the previous value, current value, or target value, either the cover is + # neither opening or closing anymore or we can't make any determination whether + # the cover is opening or closing so we set them to None. if ( new_value in ( @@ -136,6 +136,7 @@ def on_value_update(self) -> None: ) or new_value is None or self._curr_value is None + or target_value.value is None ): self._attr_is_closing = None self._attr_is_opening = None From 67274ecffb779dccfc0a653230a9255fac81d7d7 Mon Sep 17 00:00:00 2001 From: Raman Gupta <7243222+raman325@users.noreply.github.com> Date: Mon, 22 May 2023 13:10:41 -0400 Subject: [PATCH 13/13] Remove is_opening/is_closing logic --- homeassistant/components/zwave_js/cover.py | 37 ------------ tests/components/zwave_js/test_cover.py | 69 ---------------------- 2 files changed, 106 deletions(-) diff --git a/homeassistant/components/zwave_js/cover.py b/homeassistant/components/zwave_js/cover.py index 9661e8572b8682..999c59cc23960e 100644 --- a/homeassistant/components/zwave_js/cover.py +++ b/homeassistant/components/zwave_js/cover.py @@ -100,10 +100,6 @@ def __init__( if self.info.platform_hint and self.info.platform_hint.startswith("blind"): self._attr_device_class = CoverDeviceClass.BLIND - # Store the current value so that if there's an update, we can determine - # whether the cover is opening or closing. - self._curr_value: int | None = self.info.primary_value.value - def percent_to_zwave_position(self, value: int) -> int: """Convert position in 0-100 scale to closed_value-open_value scale. @@ -115,39 +111,6 @@ def percent_to_zwave_position(self, value: int) -> int: ) return self._fully_closed_value - def on_value_update(self) -> None: - """Handle primary value update.""" - new_value = self.info.primary_value.value - # If the value update doesn't change anything, there is nothing to update. - if new_value == self._curr_value: - return - target_value = self.get_zwave_value(TARGET_VALUE_PROPERTY) - assert target_value is not None - # If the cover is fully closed or opened or at the target value, or if we don't - # know the previous value, current value, or target value, either the cover is - # neither opening or closing anymore or we can't make any determination whether - # the cover is opening or closing so we set them to None. - if ( - new_value - in ( - self._fully_closed_value, - self._fully_open_value, - target_value.value, - ) - or new_value is None - or self._curr_value is None - or target_value.value is None - ): - self._attr_is_closing = None - self._attr_is_opening = None - # If the current value is less than the previous value, the cover is - # closing, otherwise it is opening. - else: - self._attr_is_closing = (new_value - self._curr_value) < 0 - self._attr_is_opening = not self._attr_is_closing - - self._curr_value = new_value - @property def _fully_open_value(self) -> int: """Return value that represents fully opened.""" diff --git a/tests/components/zwave_js/test_cover.py b/tests/components/zwave_js/test_cover.py index 79c234c54f84da..bb4fe23d265fe1 100644 --- a/tests/components/zwave_js/test_cover.py +++ b/tests/components/zwave_js/test_cover.py @@ -136,75 +136,6 @@ async def test_window_cover( } assert not open_args["value"] - # Test that we see opening state from value updated event - event = Event( - type="value updated", - data={ - "source": "node", - "event": "value updated", - "nodeId": 6, - "args": { - "commandClassName": "Multilevel Switch", - "commandClass": 38, - "endpoint": 0, - "property": "currentValue", - "newValue": 50, - "prevValue": 0, - "propertyName": "currentValue", - }, - }, - ) - node.receive_event(event) - - state = hass.states.get(WINDOW_COVER_ENTITY) - assert state.state == STATE_OPENING - - # Test that we see closing state from value updated event - event = Event( - type="value updated", - data={ - "source": "node", - "event": "value updated", - "nodeId": 6, - "args": { - "commandClassName": "Multilevel Switch", - "commandClass": 38, - "endpoint": 0, - "property": "currentValue", - "newValue": 25, - "prevValue": 0, - "propertyName": "currentValue", - }, - }, - ) - node.receive_event(event) - - state = hass.states.get(WINDOW_COVER_ENTITY) - assert state.state == STATE_CLOSING - - # Test that nothing changes when we send the exact same value - event = Event( - type="value updated", - data={ - "source": "node", - "event": "value updated", - "nodeId": 6, - "args": { - "commandClassName": "Multilevel Switch", - "commandClass": 38, - "endpoint": 0, - "property": "currentValue", - "newValue": 25, - "prevValue": 25, - "propertyName": "currentValue", - }, - }, - ) - node.receive_event(event) - - state = hass.states.get(WINDOW_COVER_ENTITY) - assert state.state == STATE_CLOSING - # Test position update from value updated event event = Event( type="value updated",