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

@@ -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

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):
+        target_door_state = DOOR_TARGET_HASS_TO_HK.get(hass_state)
-            current_state = 0 if hass_state == STATE_OPEN else 1
+        current_door_state = DOOR_CURRENT_HASS_TO_HK.get(hass_state)
-            self.char_current_state.set_value(current_state)
+
-            if not self._flag_state:
+        if (
-                self.char_target_state.set_value(current_state)
+            target_door_state is not None
-            self._flag_state = False
+            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")

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_current_state.value == HK_DOOR_OPEN
-    assert acc.char_target_state.value == 0
+    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_current_state.value == HK_DOOR_CLOSED
-    assert acc.char_target_state.value == 1
+    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_current_state.value == HK_DOOR_OPEN
-    assert acc.char_target_state.value == 0
+    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_current_state.value == HK_DOOR_OPEN
-    assert acc.char_target_state.value == 0
+    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_current_state.value == HK_DOOR_OPEN
-    assert acc.char_target_state.value == 0
+    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_current_state.value == HK_DOOR_CLOSING
-    assert acc.char_target_state.value == 1
+    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_current_state.value == HK_DOOR_CLOSED
-    assert acc.char_target_state.value == 1
+    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_current_state.value == HK_DOOR_OPENING
-    assert acc.char_target_state.value == 0
+    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_current_state.value == HK_DOOR_OPEN
-    assert acc.char_target_state.value == 0
+    assert acc.char_target_state.value == HK_DOOR_OPEN
     assert len(events) == 4
     assert events[-1].data[ATTR_VALUE] is None