Move HassSetPosition to homeassistant domain (#111867)

* Move HassSetPosition to homeassistant domain

* Add test for unsupported domain with HassSetPosition

* Split service intent handler

* cleanup

---------

Co-authored-by: Paulus Schoutsen <balloob@gmail.com>

homeassistant/components/cover/intent.py

@@ -1,16 +1,11 @@
 """Intents for the cover integration."""
 
-import voluptuous as vol
 
-from homeassistant.const import (
+from homeassistant.const import SERVICE_CLOSE_COVER, SERVICE_OPEN_COVER
-    SERVICE_CLOSE_COVER,
-    SERVICE_OPEN_COVER,
-    SERVICE_SET_COVER_POSITION,
-)
 from homeassistant.core import HomeAssistant
 from homeassistant.helpers import intent
 
-from . import ATTR_POSITION, DOMAIN
+from . import DOMAIN
 
 INTENT_OPEN_COVER = "HassOpenCover"
 INTENT_CLOSE_COVER = "HassCloseCover"
@@ -30,12 +25,3 @@ async def async_setup_intents(hass: HomeAssistant) -> None:
             INTENT_CLOSE_COVER, DOMAIN, SERVICE_CLOSE_COVER, "Closed {}"
         ),
     )
-    intent.async_register(
-        hass,
-        intent.ServiceIntentHandler(
-            intent.INTENT_SET_POSITION,
-            DOMAIN,
-            SERVICE_SET_COVER_POSITION,
-            extra_slots={ATTR_POSITION: vol.All(vol.Range(min=0, max=100))},
-        ),
-    )

homeassistant/components/intent/__init__.py

@@ -10,9 +10,11 @@ import voluptuous as vol
 
 from homeassistant.components import http
 from homeassistant.components.cover import (
+    ATTR_POSITION,
     DOMAIN as COVER_DOMAIN,
     SERVICE_CLOSE_COVER,
     SERVICE_OPEN_COVER,
+    SERVICE_SET_COVER_POSITION,
 )
 from homeassistant.components.http.data_validator import RequestDataValidator
 from homeassistant.components.lock import (
@@ -24,6 +26,7 @@ from homeassistant.components.valve import (
     DOMAIN as VALVE_DOMAIN,
     SERVICE_CLOSE_VALVE,
     SERVICE_OPEN_VALVE,
+    SERVICE_SET_VALVE_POSITION,
 )
 from homeassistant.const import (
     ATTR_ENTITY_ID,
@@ -75,6 +78,7 @@ async def async_setup(hass: HomeAssistant, config: ConfigType) -> bool:
         hass,
         NevermindIntentHandler(),
     )
+    intent.async_register(hass, SetPositionIntentHandler())
 
     return True
 
@@ -89,14 +93,16 @@ class IntentPlatformProtocol(Protocol):
 class OnOffIntentHandler(intent.ServiceIntentHandler):
     """Intent handler for on/off that also supports covers, valves, locks, etc."""
 
-    async def async_call_service(self, intent_obj: intent.Intent, state: State) -> None:
+    async def async_call_service(
+        self, domain: str, service: str, intent_obj: intent.Intent, state: State
+    ) -> None:
         """Call service on entity with handling for special cases."""
         hass = intent_obj.hass
 
         if state.domain == COVER_DOMAIN:
             # on = open
             # off = close
-            if self.service == SERVICE_TURN_ON:
+            if service == SERVICE_TURN_ON:
                 service_name = SERVICE_OPEN_COVER
             else:
                 service_name = SERVICE_CLOSE_COVER
@@ -117,7 +123,7 @@ class OnOffIntentHandler(intent.ServiceIntentHandler):
         if state.domain == LOCK_DOMAIN:
             # on = lock
             # off = unlock
-            if self.service == SERVICE_TURN_ON:
+            if service == SERVICE_TURN_ON:
                 service_name = SERVICE_LOCK
             else:
                 service_name = SERVICE_UNLOCK
@@ -138,7 +144,7 @@ class OnOffIntentHandler(intent.ServiceIntentHandler):
         if state.domain == VALVE_DOMAIN:
             # on = opened
             # off = closed
-            if self.service == SERVICE_TURN_ON:
+            if service == SERVICE_TURN_ON:
                 service_name = SERVICE_OPEN_VALVE
             else:
                 service_name = SERVICE_CLOSE_VALVE
@@ -156,13 +162,13 @@ class OnOffIntentHandler(intent.ServiceIntentHandler):
             )
             return
 
-        if not hass.services.has_service(state.domain, self.service):
+        if not hass.services.has_service(state.domain, service):
             raise intent.IntentHandleError(
-                f"Service {self.service} does not support entity {state.entity_id}"
+                f"Service {service} does not support entity {state.entity_id}"
             )
 
         # Fall back to homeassistant.turn_on/off
-        await super().async_call_service(intent_obj, state)
+        await super().async_call_service(domain, service, intent_obj, state)
 
 
 class GetStateIntentHandler(intent.IntentHandler):
@@ -296,6 +302,29 @@ class NevermindIntentHandler(intent.IntentHandler):
         return intent_obj.create_response()
 
 
+class SetPositionIntentHandler(intent.DynamicServiceIntentHandler):
+    """Intent handler for setting positions."""
+
+    def __init__(self) -> None:
+        """Create set position handler."""
+        super().__init__(
+            intent.INTENT_SET_POSITION,
+            extra_slots={ATTR_POSITION: vol.All(vol.Range(min=0, max=100))},
+        )
+
+    def get_domain_and_service(
+        self, intent_obj: intent.Intent, state: State
+    ) -> tuple[str, str]:
+        """Get the domain and service name to call."""
+        if state.domain == COVER_DOMAIN:
+            return (COVER_DOMAIN, SERVICE_SET_COVER_POSITION)
+
+        if state.domain == VALVE_DOMAIN:
+            return (VALVE_DOMAIN, SERVICE_SET_VALVE_POSITION)
+
+        raise intent.IntentHandleError(f"Domain not supported: {state.domain}")
+
+
 async def _async_process_intent(
     hass: HomeAssistant, domain: str, platform: IntentPlatformProtocol
 ) -> None:

homeassistant/components/valve/intent.py

@@ -1,22 +0,0 @@
-"""Intents for the valve integration."""
-
-import voluptuous as vol
-
-from homeassistant.const import SERVICE_SET_VALVE_POSITION
-from homeassistant.core import HomeAssistant
-from homeassistant.helpers import intent
-
-from . import ATTR_POSITION, DOMAIN
-
-
-async def async_setup_intents(hass: HomeAssistant) -> None:
-    """Set up the valve intents."""
-    intent.async_register(
-        hass,
-        intent.ServiceIntentHandler(
-            intent.INTENT_SET_POSITION,
-            DOMAIN,
-            SERVICE_SET_VALVE_POSITION,
-            extra_slots={ATTR_POSITION: vol.All(vol.Range(min=0, max=100))},
-        ),
-    )

homeassistant/helpers/intent.py

@@ -2,6 +2,7 @@
 
 from __future__ import annotations
 
+from abc import abstractmethod
 import asyncio
 from collections.abc import Collection, Coroutine, Iterable
 import dataclasses
@@ -385,8 +386,8 @@ class IntentHandler:
         return f"<{self.__class__.__name__} - {self.intent_type}>"
 
 
-class ServiceIntentHandler(IntentHandler):
+class DynamicServiceIntentHandler(IntentHandler):
-    """Service Intent handler registration.
+    """Service Intent handler registration (dynamic).
 
     Service specific intent handler that calls a service by name/entity_id.
     """
@@ -404,15 +405,11 @@ class ServiceIntentHandler(IntentHandler):
     def __init__(
         self,
         intent_type: str,
-        domain: str,
-        service: str,
         speech: str | None = None,
         extra_slots: dict[str, vol.Schema] | None = None,
     ) -> None:
         """Create Service Intent Handler."""
         self.intent_type = intent_type
-        self.domain = domain
-        self.service = service
         self.speech = speech
         self.extra_slots = extra_slots
 
@@ -441,6 +438,13 @@ class ServiceIntentHandler(IntentHandler):
             extra=vol.ALLOW_EXTRA,
         )
 
+    @abstractmethod
+    def get_domain_and_service(
+        self, intent_obj: Intent, state: State
+    ) -> tuple[str, str]:
+        """Get the domain and service name to call."""
+        raise NotImplementedError()
+
     async def async_handle(self, intent_obj: Intent) -> IntentResponse:
         """Handle the hass intent."""
         hass = intent_obj.hass
@@ -536,7 +540,10 @@ class ServiceIntentHandler(IntentHandler):
 
         service_coros: list[Coroutine[Any, Any, None]] = []
         for state in states:
-            service_coros.append(self.async_call_service(intent_obj, state))
+            domain, service = self.get_domain_and_service(intent_obj, state)
+            service_coros.append(
+                self.async_call_service(domain, service, intent_obj, state)
+            )
 
         # Handle service calls in parallel, noting failures as they occur.
         failed_results: list[IntentResponseTarget] = []
@@ -558,7 +565,7 @@ class ServiceIntentHandler(IntentHandler):
             # If no entities succeeded, raise an error.
             failed_entity_ids = [target.id for target in failed_results]
             raise IntentHandleError(
-                f"Failed to call {self.service} for: {failed_entity_ids}"
+                f"Failed to call {service} for: {failed_entity_ids}"
             )
 
         response.async_set_results(
@@ -574,7 +581,9 @@ class ServiceIntentHandler(IntentHandler):
 
         return response
 
-    async def async_call_service(self, intent_obj: Intent, state: State) -> None:
+    async def async_call_service(
+        self, domain: str, service: str, intent_obj: Intent, state: State
+    ) -> None:
         """Call service on entity."""
         hass = intent_obj.hass
 
@@ -587,13 +596,13 @@ class ServiceIntentHandler(IntentHandler):
         await self._run_then_background(
             hass.async_create_task(
                 hass.services.async_call(
-                    self.domain,
+                    domain,
-                    self.service,
+                    service,
                     service_data,
                     context=intent_obj.context,
                     blocking=True,
                 ),
-                f"intent_call_service_{self.domain}_{self.service}",
+                f"intent_call_service_{domain}_{service}",
             )
         )
 
@@ -615,6 +624,32 @@ class ServiceIntentHandler(IntentHandler):
             raise
 
 
+class ServiceIntentHandler(DynamicServiceIntentHandler):
+    """Service Intent handler registration.
+
+    Service specific intent handler that calls a service by name/entity_id.
+    """
+
+    def __init__(
+        self,
+        intent_type: str,
+        domain: str,
+        service: str,
+        speech: str | None = None,
+        extra_slots: dict[str, vol.Schema] | None = None,
+    ) -> None:
+        """Create service handler."""
+        super().__init__(intent_type, speech=speech, extra_slots=extra_slots)
+        self.domain = domain
+        self.service = service
+
+    def get_domain_and_service(
+        self, intent_obj: Intent, state: State
+    ) -> tuple[str, str]:
+        """Get the domain and service name to call."""
+        return (self.domain, self.service)
+
+
 class IntentCategory(Enum):
     """Category of an intent."""
 

tests/components/conversation/test_default_agent_intents.py

@@ -8,7 +8,6 @@ from homeassistant.components.cover import intent as cover_intent
 from homeassistant.components.homeassistant.exposed_entities import async_expose_entity
 from homeassistant.components.media_player import intent as media_player_intent
 from homeassistant.components.vacuum import intent as vaccum_intent
-from homeassistant.components.valve import intent as valve_intent
 from homeassistant.const import STATE_CLOSED
 from homeassistant.core import Context, HomeAssistant
 from homeassistant.helpers import intent
@@ -84,8 +83,6 @@ async def test_valve_intents(
     init_components,
 ) -> None:
     """Test open/close/set position for valves."""
-    await valve_intent.async_setup_intents(hass)
-
     entity_id = f"{valve.DOMAIN}.main_valve"
     hass.states.async_set(entity_id, STATE_CLOSED)
     async_expose_entity(hass, conversation.DOMAIN, entity_id, True)

tests/components/cover/test_intent.py

@@ -11,6 +11,7 @@ from homeassistant.components.cover import (
 from homeassistant.const import STATE_CLOSED, STATE_OPEN
 from homeassistant.core import HomeAssistant
 from homeassistant.helpers import intent
+from homeassistant.setup import async_setup_component
 
 from tests.common import async_mock_service
 
@@ -60,7 +61,7 @@ async def test_close_cover_intent(hass: HomeAssistant) -> None:
 
 async def test_set_cover_position(hass: HomeAssistant) -> None:
     """Test HassSetPosition intent for covers."""
-    await cover_intent.async_setup_intents(hass)
+    assert await async_setup_component(hass, "intent", {})
 
     entity_id = f"{DOMAIN}.test_cover"
     hass.states.async_set(

tests/components/intent/test_init.py

@@ -432,3 +432,20 @@ async def test_get_state_intent(
                 "domain": {"value": "light"},
             },
         )
+
+
+async def test_set_position_intent_unsupported_domain(hass: HomeAssistant) -> None:
+    """Test that HassSetPosition intent fails with unsupported domain."""
+    assert await async_setup_component(hass, "homeassistant", {})
+    assert await async_setup_component(hass, "intent", {})
+
+    # Can't set position of lights
+    hass.states.async_set("light.test_light", "off")
+
+    with pytest.raises(intent.IntentHandleError):
+        await intent.async_handle(
+            hass,
+            "test",
+            "HassSetPosition",
+            {"name": {"value": "test light"}, "position": {"value": 100}},
+        )

tests/components/valve/test_intent.py

@@ -6,7 +6,6 @@ from homeassistant.components.valve import (
     SERVICE_CLOSE_VALVE,
     SERVICE_OPEN_VALVE,
     SERVICE_SET_VALVE_POSITION,
-    intent as valve_intent,
 )
 from homeassistant.const import STATE_CLOSED, STATE_OPEN
 from homeassistant.core import HomeAssistant
@@ -60,7 +59,7 @@ async def test_close_valve_intent(hass: HomeAssistant) -> None:
 
 async def test_set_valve_position(hass: HomeAssistant) -> None:
     """Test HassSetPosition intent for valves."""
-    await valve_intent.async_setup_intents(hass)
+    assert await async_setup_component(hass, "intent", {})
 
     entity_id = f"{DOMAIN}.test_valve"
     hass.states.async_set(