Assume cover or valve is always "running" in google assistant when the state is assumed or the position is reported to allow it to be be stopped (#158919)

homeassistant/components/google_assistant/trait.py

@@ -908,12 +908,21 @@ class StartStopTrait(_Trait):
             }
 
         if domain in COVER_VALVE_DOMAINS:
+            assumed_state_or_set_position = bool(
+                (
+                    self.state.attributes.get(ATTR_SUPPORTED_FEATURES, 0)
+                    & COVER_VALVE_SET_POSITION_FEATURE[domain]
+                )
+                or self.state.attributes.get(ATTR_ASSUMED_STATE)
+            )
+
             return {
                 "isRunning": state
                 in (
                     COVER_VALVE_STATES[domain]["closing"],
                     COVER_VALVE_STATES[domain]["opening"],
                 )
+                or assumed_state_or_set_position
             }
 
         raise NotImplementedError(f"Unsupported domain {domain}")
@@ -975,11 +984,23 @@ class StartStopTrait(_Trait):
         """Execute a StartStop command."""
         domain = self.state.domain
         if command == COMMAND_START_STOP:
+            assumed_state_or_set_position = bool(
+                (
+                    self.state.attributes.get(ATTR_SUPPORTED_FEATURES, 0)
+                    & COVER_VALVE_SET_POSITION_FEATURE[domain]
+                )
+                or self.state.attributes.get(ATTR_ASSUMED_STATE)
+            )
+
             if params["start"] is False:
-                if self.state.state in (
+                if (
-                    COVER_VALVE_STATES[domain]["closing"],
+                    self.state.state
-                    COVER_VALVE_STATES[domain]["opening"],
+                    in (
-                ) or self.state.attributes.get(ATTR_ASSUMED_STATE):
+                        COVER_VALVE_STATES[domain]["closing"],
+                        COVER_VALVE_STATES[domain]["opening"],
+                    )
+                    or assumed_state_or_set_position
+                ):
                     await self.hass.services.async_call(
                         domain,
                         SERVICE_STOP_COVER_VALVE[domain],
@@ -992,7 +1013,14 @@ class StartStopTrait(_Trait):
                         ERR_ALREADY_STOPPED,
                         f"{FRIENDLY_DOMAIN[domain]} is already stopped",
                     )
-            else:
+            elif (
+                self.state.state
+                in (
+                    COVER_VALVE_STATES[domain]["open"],
+                    COVER_VALVE_STATES[domain]["closed"],
+                )
+                or assumed_state_or_set_position
+            ):
                 await self.hass.services.async_call(
                     domain,
                     SERVICE_TOGGLE_COVER_VALVE[domain],

tests/components/google_assistant/test_trait.py

@@ -693,7 +693,7 @@ async def test_startstop_lawn_mower(hass: HomeAssistant) -> None:
         ),
     ],
 )
-async def test_startstop_cover_valve(
+async def test_startstop_cover_valve_no_assumed_state(
     hass: HomeAssistant,
     domain: str,
     state_open: str,
@@ -706,14 +706,14 @@ async def test_startstop_cover_valve(
     service_stop: str,
     service_toggle: str,
 ) -> None:
-    """Test startStop trait support."""
+    """Test startStop trait support and no assumed state."""
     assert helpers.get_google_type(domain, None) is not None
     assert trait.StartStopTrait.supported(domain, supported_features, None, None)
 
     state = State(
         f"{domain}.bla",
         state_closed,
-        {ATTR_SUPPORTED_FEATURES: supported_features},
+        {ATTR_SUPPORTED_FEATURES: supported_features, ATTR_ASSUMED_STATE: False},
     )
 
     trt = trait.StartStopTrait(
@@ -773,6 +773,168 @@ async def test_startstop_cover_valve(
         await trt.execute(trait.COMMAND_PAUSE_UNPAUSE, BASIC_DATA, {"start": True}, {})
 
 
+@pytest.mark.parametrize(
+    (
+        "domain",
+        "state_open",
+        "state_closed",
+        "state_opening",
+        "state_closing",
+        "supported_features",
+        "service_close",
+        "service_open",
+        "service_stop",
+        "service_toggle",
+        "assumed_state",
+    ),
+    [
+        (
+            cover.DOMAIN,
+            cover.CoverState.OPEN,
+            cover.CoverState.CLOSED,
+            cover.CoverState.OPENING,
+            cover.CoverState.CLOSING,
+            CoverEntityFeature.STOP
+            | CoverEntityFeature.OPEN
+            | CoverEntityFeature.CLOSE,
+            cover.SERVICE_OPEN_COVER,
+            cover.SERVICE_CLOSE_COVER,
+            cover.SERVICE_STOP_COVER,
+            cover.SERVICE_TOGGLE,
+            True,
+        ),
+        (
+            valve.DOMAIN,
+            valve.ValveState.OPEN,
+            valve.ValveState.CLOSED,
+            valve.ValveState.OPENING,
+            valve.ValveState.CLOSING,
+            ValveEntityFeature.STOP
+            | ValveEntityFeature.OPEN
+            | ValveEntityFeature.CLOSE,
+            valve.SERVICE_OPEN_VALVE,
+            valve.SERVICE_CLOSE_VALVE,
+            valve.SERVICE_STOP_VALVE,
+            cover.SERVICE_TOGGLE,
+            True,
+        ),
+        (
+            cover.DOMAIN,
+            cover.CoverState.OPEN,
+            cover.CoverState.CLOSED,
+            cover.CoverState.OPENING,
+            cover.CoverState.CLOSING,
+            CoverEntityFeature.STOP
+            | CoverEntityFeature.OPEN
+            | CoverEntityFeature.CLOSE
+            | CoverEntityFeature.SET_POSITION,
+            cover.SERVICE_OPEN_COVER,
+            cover.SERVICE_CLOSE_COVER,
+            cover.SERVICE_STOP_COVER,
+            cover.SERVICE_TOGGLE,
+            False,
+        ),
+        (
+            valve.DOMAIN,
+            valve.ValveState.OPEN,
+            valve.ValveState.CLOSED,
+            valve.ValveState.OPENING,
+            valve.ValveState.CLOSING,
+            ValveEntityFeature.STOP
+            | ValveEntityFeature.OPEN
+            | ValveEntityFeature.CLOSE
+            | ValveEntityFeature.SET_POSITION,
+            valve.SERVICE_OPEN_VALVE,
+            valve.SERVICE_CLOSE_VALVE,
+            valve.SERVICE_STOP_VALVE,
+            cover.SERVICE_TOGGLE,
+            False,
+        ),
+    ],
+)
+async def test_startstop_cover_valve_with_assumed_state_or_reports_position(
+    hass: HomeAssistant,
+    domain: str,
+    state_open: str,
+    state_closed: str,
+    state_opening: str,
+    state_closing: str,
+    supported_features: str,
+    service_open: str,
+    service_close: str,
+    service_stop: str,
+    service_toggle: str,
+    assumed_state: bool,
+) -> None:
+    """Test startStop trait support without an assumed state or reporting position."""
+    assert helpers.get_google_type(domain, None) is not None
+    assert trait.StartStopTrait.supported(domain, supported_features, None, None)
+
+    state = State(
+        f"{domain}.bla",
+        state_closed,
+        {
+            ATTR_SUPPORTED_FEATURES: supported_features,
+            ATTR_ASSUMED_STATE: assumed_state,
+        },
+    )
+
+    trt = trait.StartStopTrait(
+        hass,
+        state,
+        BASIC_CONFIG,
+    )
+
+    assert trt.sync_attributes() == {}
+
+    for state_value in (state_closing, state_opening):
+        state.state = state_value
+        assert trt.query_attributes()["isRunning"] is True
+
+    stop_calls = async_mock_service(hass, domain, service_stop)
+    open_calls = async_mock_service(hass, domain, service_open)
+    close_calls = async_mock_service(hass, domain, service_close)
+    toggle_calls = async_mock_service(hass, domain, service_toggle)
+    await trt.execute(trait.COMMAND_START_STOP, BASIC_DATA, {"start": False}, {})
+    assert len(stop_calls) == 1
+    assert stop_calls[0].data == {ATTR_ENTITY_ID: f"{domain}.bla"}
+
+    # Trait attr isRunning always returns True,
+    # so the cover or valve can always be stopped
+    for state_value in (state_closing, state_opening, state_closed, state_open):
+        state.state = state_value
+        assert trt.query_attributes()["isRunning"] is True
+
+    state.state = state_open
+
+    # Stop does not raise because we assume the state
+    # or the position is reported
+    await trt.execute(trait.COMMAND_START_STOP, BASIC_DATA, {"start": False}, {})
+    assert len(stop_calls) == 2
+
+    # Start triggers toggle open
+    state.state = state_closed
+    await trt.execute(trait.COMMAND_START_STOP, BASIC_DATA, {"start": True}, {})
+    assert len(open_calls) == 0
+    assert len(close_calls) == 0
+    assert len(toggle_calls) == 1
+    assert toggle_calls[0].data == {ATTR_ENTITY_ID: f"{domain}.bla"}
+    # Second start triggers toggle close
+    state.state = state_open
+    await trt.execute(trait.COMMAND_START_STOP, BASIC_DATA, {"start": True}, {})
+    assert len(open_calls) == 0
+    assert len(close_calls) == 0
+    assert len(toggle_calls) == 2
+    assert toggle_calls[1].data == {ATTR_ENTITY_ID: f"{domain}.bla"}
+
+    state.state = state_closed
+    with pytest.raises(
+        SmartHomeError,
+        match="Command action.devices.commands.PauseUnpause is not supported",
+    ):
+        await trt.execute(trait.COMMAND_PAUSE_UNPAUSE, BASIC_DATA, {"start": True}, {})
+
+
 @pytest.mark.parametrize(
     (
         "domain",