Fix HomeKit cover direction when moved from outside HomeKit (#180889)

homeassistant/components/homekit/type_covers.py

@@ -329,6 +329,28 @@ class OpeningDevice(OpeningDeviceBase, HomeAccessory):
         params = {ATTR_ENTITY_ID: self.entity_id, ATTR_POSITION: value}
         self.async_call_service(COVER_DOMAIN, SERVICE_SET_COVER_POSITION, params, value)
 
+    @callback
+    def _async_update_target_position_while_moving(
+        self, state: str, current_position: int
+    ) -> None:
+        """Aim the target position at the end of travel while moving.
+
+        The Home app reads the direction of travel from the target position
+        relative to the current one, so a target the cover has already passed
+        makes it report the opposite direction. Only a passed target is
+        replaced: one the cover is still travelling towards, or has just
+        reached, is what HomeKit asked for.
+        """
+        if not self.features & CoverEntityFeature.SET_POSITION:
+            # Tilt-only covers lock the target position to closed.
+            return
+        target_position = self.char_target_position.value
+        if state == CoverState.OPENING:
+            if target_position < current_position:
+                self.char_target_position.set_value(100)
+        elif state == CoverState.CLOSING and target_position > current_position:
+            self.char_target_position.set_value(0)
+
     @callback
     @override
     def async_update_state(self, new_state: State) -> None:
@@ -339,9 +361,11 @@ class OpeningDevice(OpeningDeviceBase, HomeAccessory):
         if isinstance(current_position, (float, int)):
             current_position = int(current_position)
             self.char_current_position.set_value(current_position)
-            # Writing target_position on a moving cover
-            # will break the moving state in HK.
-            if new_state.state not in MOVING_STATES:
+            if new_state.state in MOVING_STATES:
+                self._async_update_target_position_while_moving(
+                    new_state.state, current_position
+                )
+            else:
                 self.char_target_position.set_value(current_position)
 
         position_state = _hass_state_to_position_start(new_state.state)

tests/components/homekit/test_type_covers.py

@@ -228,7 +228,8 @@ async def test_windowcovering_set_cover_position(
     )
     await hass.async_block_till_done()
     assert acc.char_current_position.value == 60
-    assert acc.char_target_position.value == 0
+    # Aimed at the end of travel, not left behind at the starting position.
+    assert acc.char_target_position.value == 100
     assert acc.char_position_state.value == 1
 
     hass.states.async_set(
@@ -241,7 +242,7 @@ async def test_windowcovering_set_cover_position(
     )
     await hass.async_block_till_done()
     assert acc.char_current_position.value == 70
-    assert acc.char_target_position.value == 0
+    assert acc.char_target_position.value == 100
     assert acc.char_position_state.value == 1
 
     hass.states.async_set(
@@ -296,6 +297,109 @@ async def test_windowcovering_set_cover_position(
     assert events[-1].data[ATTR_VALUE] == 75
 
 
+async def test_windowcovering_target_position_while_moving(
+    hass: HomeAssistant, hk_driver
+) -> None:
+    """Test the target position while the cover is moving.
+
+    The Home app derives the direction of travel from the target position
+    relative to the current one, so a target left behind the cover makes it
+    report the opposite of what the cover is doing.
+    """
+    entity_id = "cover.window"
+    features = CoverEntityFeature.SET_POSITION
+
+    hass.states.async_set(
+        entity_id, CoverState.CLOSED, {ATTR_SUPPORTED_FEATURES: features}
+    )
+    await hass.async_block_till_done()
+    acc = WindowCovering(hass, hk_driver, "Cover", entity_id, 2, None)
+    acc.run()
+    await hass.async_block_till_done()
+
+    # Opened from outside HomeKit: the target still points at the start.
+    hass.states.async_set(
+        entity_id,
+        CoverState.OPENING,
+        {ATTR_SUPPORTED_FEATURES: features, ATTR_CURRENT_POSITION: 20},
+    )
+    await hass.async_block_till_done()
+    assert acc.char_target_position.value == 100
+
+    # Coming to rest hands the target back to the real position.
+    hass.states.async_set(
+        entity_id,
+        CoverState.OPEN,
+        {ATTR_SUPPORTED_FEATURES: features, ATTR_CURRENT_POSITION: 40},
+    )
+    await hass.async_block_till_done()
+    assert acc.char_target_position.value == 40
+
+    # Closed from outside HomeKit.
+    hass.states.async_set(
+        entity_id,
+        CoverState.CLOSING,
+        {ATTR_SUPPORTED_FEATURES: features, ATTR_CURRENT_POSITION: 30},
+    )
+    await hass.async_block_till_done()
+    assert acc.char_target_position.value == 0
+
+    # A partial move asked for from HomeKit must survive.
+    async_mock_service(hass, COVER_DOMAIN, "set_cover_position")
+    hass.states.async_set(
+        entity_id,
+        CoverState.CLOSED,
+        {ATTR_SUPPORTED_FEATURES: features, ATTR_CURRENT_POSITION: 0},
+    )
+    await hass.async_block_till_done()
+    acc.char_target_position.client_update_value(60)
+    await hass.async_block_till_done()
+    assert acc.char_target_position.value == 60
+
+    hass.states.async_set(
+        entity_id,
+        CoverState.OPENING,
+        {ATTR_SUPPORTED_FEATURES: features, ATTR_CURRENT_POSITION: 30},
+    )
+    await hass.async_block_till_done()
+    assert acc.char_target_position.value == 60
+
+    # Still reported as opening on arrival at the requested position: the
+    # target must not jump to the end of travel in that last moment.
+    hass.states.async_set(
+        entity_id,
+        CoverState.OPENING,
+        {ATTR_SUPPORTED_FEATURES: features, ATTR_CURRENT_POSITION: 60},
+    )
+    await hass.async_block_till_done()
+    assert acc.char_target_position.value == 60
+
+
+async def test_windowcovering_tilt_only_target_position_stays_closed(
+    hass: HomeAssistant, hk_driver
+) -> None:
+    """Test that a tilt-only cover keeps its target position locked at closed."""
+    entity_id = "cover.window"
+    features = CoverEntityFeature.OPEN_TILT | CoverEntityFeature.SET_TILT_POSITION
+
+    hass.states.async_set(
+        entity_id, CoverState.CLOSED, {ATTR_SUPPORTED_FEATURES: features}
+    )
+    await hass.async_block_till_done()
+    acc = WindowCovering(hass, hk_driver, "Cover", entity_id, 2, None)
+    acc.run()
+    await hass.async_block_till_done()
+
+    hass.states.async_set(
+        entity_id,
+        CoverState.OPENING,
+        {ATTR_SUPPORTED_FEATURES: features, ATTR_CURRENT_POSITION: 20},
+    )
+    await hass.async_block_till_done()
+    # Capped at 0 for tilt-only covers, so 100 would be out of range.
+    assert acc.char_target_position.value == 0
+
+
 async def test_window_instantiate_set_position(hass: HomeAssistant, hk_driver) -> None:
     """Test if Window accessory is instantiated correctly and can set position."""
     entity_id = "cover.window"