From ad5a396c10bb707544f1ae03976bb988d19a41b0 Mon Sep 17 00:00:00 2001 From: "J. Nick Koston" Date: Sun, 12 Apr 2020 17:15:55 -0500 Subject: [PATCH] Fix reversed door closing/opening states in HomeKit (#34095) * Fix reversed door closing/opening states in HomeKit When we closed the door we would set state 2 which is "Opening" it should have been 3 which is "Closing" When we opened the door we would set state 3 which is "Closing" it should have been 2 which is "Opening" Add constants to make this easier to catch in the future. * Remove debug * Add note about target door state --- homeassistant/components/homekit/const.py | 7 +++ .../components/homekit/type_covers.py | 56 +++++++++++++++---- tests/components/homekit/test_type_covers.py | 44 ++++++++------- 3 files changed, 76 insertions(+), 31 deletions(-) diff --git a/homeassistant/components/homekit/const.py b/homeassistant/components/homekit/const.py index ccce082044b..1c748af8571 100644 --- a/homeassistant/components/homekit/const.py +++ b/homeassistant/components/homekit/const.py @@ -182,3 +182,10 @@ THRESHOLD_CO2 = 1000 # #### Default values #### DEFAULT_MIN_TEMP_WATER_HEATER = 40 # °C DEFAULT_MAX_TEMP_WATER_HEATER = 60 # °C + +# #### Door states #### +HK_DOOR_OPEN = 0 +HK_DOOR_CLOSED = 1 +HK_DOOR_OPENING = 2 +HK_DOOR_CLOSING = 3 +HK_DOOR_STOPPED = 4 diff --git a/homeassistant/components/homekit/type_covers.py b/homeassistant/components/homekit/type_covers.py index 97940952171..624f23d0423 100644 --- a/homeassistant/components/homekit/type_covers.py +++ b/homeassistant/components/homekit/type_covers.py @@ -37,10 +37,35 @@ from .const import ( CHAR_TARGET_POSITION, CHAR_TARGET_TILT_ANGLE, DEVICE_PRECISION_LEEWAY, + HK_DOOR_CLOSED, + HK_DOOR_CLOSING, + HK_DOOR_OPEN, + HK_DOOR_OPENING, SERV_GARAGE_DOOR_OPENER, SERV_WINDOW_COVERING, ) +DOOR_CURRENT_HASS_TO_HK = { + STATE_OPEN: HK_DOOR_OPEN, + STATE_CLOSED: HK_DOOR_CLOSED, + STATE_OPENING: HK_DOOR_OPENING, + STATE_CLOSING: HK_DOOR_CLOSING, +} + +# HomeKit only has two states for +# Target Door State: +# 0: Open +# 1: Closed +# Opening is mapped to 0 since the target is Open +# Closing is mapped to 1 since the target is Closed +DOOR_TARGET_HASS_TO_HK = { + STATE_OPEN: HK_DOOR_OPEN, + STATE_CLOSED: HK_DOOR_CLOSED, + STATE_OPENING: HK_DOOR_OPEN, + STATE_CLOSING: HK_DOOR_CLOSED, +} + + _LOGGER = logging.getLogger(__name__) @@ -55,7 +80,7 @@ class GarageDoorOpener(HomeAccessory): def __init__(self, *args): """Initialize a GarageDoorOpener accessory object.""" super().__init__(*args, category=CATEGORY_GARAGE_DOOR_OPENER) - self._flag_state = False + state = self.hass.states.get(self.entity_id) serv_garage_door = self.add_preload_service(SERV_GARAGE_DOOR_OPENER) self.char_current_state = serv_garage_door.configure_char( @@ -64,31 +89,38 @@ class GarageDoorOpener(HomeAccessory): self.char_target_state = serv_garage_door.configure_char( CHAR_TARGET_DOOR_STATE, value=0, setter_callback=self.set_state ) + self.update_state(state) def set_state(self, value): """Change garage state if call came from HomeKit.""" _LOGGER.debug("%s: Set state to %d", self.entity_id, value) - self._flag_state = True params = {ATTR_ENTITY_ID: self.entity_id} - if value == 0: + if value == HK_DOOR_OPEN: if self.char_current_state.value != value: - self.char_current_state.set_value(3) + self.char_current_state.set_value(HK_DOOR_OPENING) self.call_service(DOMAIN, SERVICE_OPEN_COVER, params) - elif value == 1: + elif value == HK_DOOR_CLOSED: if self.char_current_state.value != value: - self.char_current_state.set_value(2) + self.char_current_state.set_value(HK_DOOR_CLOSING) self.call_service(DOMAIN, SERVICE_CLOSE_COVER, params) def update_state(self, new_state): """Update cover state after state changed.""" hass_state = new_state.state - if hass_state in (STATE_OPEN, STATE_CLOSED): - current_state = 0 if hass_state == STATE_OPEN else 1 - self.char_current_state.set_value(current_state) - if not self._flag_state: - self.char_target_state.set_value(current_state) - self._flag_state = False + target_door_state = DOOR_TARGET_HASS_TO_HK.get(hass_state) + current_door_state = DOOR_CURRENT_HASS_TO_HK.get(hass_state) + + if ( + target_door_state is not None + and self.char_target_state.value != target_door_state + ): + self.char_target_state.set_value(target_door_state) + if ( + current_door_state is not None + and self.char_current_state.value != current_door_state + ): + self.char_current_state.set_value(current_door_state) @TYPES.register("WindowCovering") diff --git a/tests/components/homekit/test_type_covers.py b/tests/components/homekit/test_type_covers.py index 188a4ec99e3..b26379d576d 100644 --- a/tests/components/homekit/test_type_covers.py +++ b/tests/components/homekit/test_type_covers.py @@ -12,7 +12,13 @@ from homeassistant.components.cover import ( SUPPORT_SET_TILT_POSITION, SUPPORT_STOP, ) -from homeassistant.components.homekit.const import ATTR_VALUE +from homeassistant.components.homekit.const import ( + ATTR_VALUE, + HK_DOOR_CLOSED, + HK_DOOR_CLOSING, + HK_DOOR_OPEN, + HK_DOOR_OPENING, +) from homeassistant.const import ( ATTR_ENTITY_ID, ATTR_SUPPORTED_FEATURES, @@ -62,28 +68,28 @@ async def test_garage_door_open_close(hass, hk_driver, cls, events): assert acc.aid == 2 assert acc.category == 4 # GarageDoorOpener - assert acc.char_current_state.value == 0 - assert acc.char_target_state.value == 0 + assert acc.char_current_state.value == HK_DOOR_OPEN + assert acc.char_target_state.value == HK_DOOR_OPEN hass.states.async_set(entity_id, STATE_CLOSED) await hass.async_block_till_done() - assert acc.char_current_state.value == 1 - assert acc.char_target_state.value == 1 + assert acc.char_current_state.value == HK_DOOR_CLOSED + assert acc.char_target_state.value == HK_DOOR_CLOSED hass.states.async_set(entity_id, STATE_OPEN) await hass.async_block_till_done() - assert acc.char_current_state.value == 0 - assert acc.char_target_state.value == 0 + assert acc.char_current_state.value == HK_DOOR_OPEN + assert acc.char_target_state.value == HK_DOOR_OPEN hass.states.async_set(entity_id, STATE_UNAVAILABLE) await hass.async_block_till_done() - assert acc.char_current_state.value == 0 - assert acc.char_target_state.value == 0 + assert acc.char_current_state.value == HK_DOOR_OPEN + assert acc.char_target_state.value == HK_DOOR_OPEN hass.states.async_set(entity_id, STATE_UNKNOWN) await hass.async_block_till_done() - assert acc.char_current_state.value == 0 - assert acc.char_target_state.value == 0 + assert acc.char_current_state.value == HK_DOOR_OPEN + assert acc.char_target_state.value == HK_DOOR_OPEN # Set from HomeKit call_close_cover = async_mock_service(hass, DOMAIN, "close_cover") @@ -93,8 +99,8 @@ async def test_garage_door_open_close(hass, hk_driver, cls, events): await hass.async_block_till_done() assert call_close_cover assert call_close_cover[0].data[ATTR_ENTITY_ID] == entity_id - assert acc.char_current_state.value == 2 - assert acc.char_target_state.value == 1 + assert acc.char_current_state.value == HK_DOOR_CLOSING + assert acc.char_target_state.value == HK_DOOR_CLOSED assert len(events) == 1 assert events[-1].data[ATTR_VALUE] is None @@ -103,8 +109,8 @@ async def test_garage_door_open_close(hass, hk_driver, cls, events): await hass.async_add_executor_job(acc.char_target_state.client_update_value, 1) await hass.async_block_till_done() - assert acc.char_current_state.value == 1 - assert acc.char_target_state.value == 1 + assert acc.char_current_state.value == HK_DOOR_CLOSED + assert acc.char_target_state.value == HK_DOOR_CLOSED assert len(events) == 2 assert events[-1].data[ATTR_VALUE] is None @@ -112,8 +118,8 @@ async def test_garage_door_open_close(hass, hk_driver, cls, events): await hass.async_block_till_done() assert call_open_cover assert call_open_cover[0].data[ATTR_ENTITY_ID] == entity_id - assert acc.char_current_state.value == 3 - assert acc.char_target_state.value == 0 + assert acc.char_current_state.value == HK_DOOR_OPENING + assert acc.char_target_state.value == HK_DOOR_OPEN assert len(events) == 3 assert events[-1].data[ATTR_VALUE] is None @@ -122,8 +128,8 @@ async def test_garage_door_open_close(hass, hk_driver, cls, events): await hass.async_add_executor_job(acc.char_target_state.client_update_value, 0) await hass.async_block_till_done() - assert acc.char_current_state.value == 0 - assert acc.char_target_state.value == 0 + assert acc.char_current_state.value == HK_DOOR_OPEN + assert acc.char_target_state.value == HK_DOOR_OPEN assert len(events) == 4 assert events[-1].data[ATTR_VALUE] is None