From d441a62aa62ff1a916e6ca73b9189ae78f027863 Mon Sep 17 00:00:00 2001 From: Michael Hansen Date: Tue, 14 May 2024 14:48:24 -0500 Subject: [PATCH] Remove "device_id" slot from timers (#117460) Remove "device_id" slot --- .../components/conversation/default_agent.py | 13 +-- homeassistant/components/intent/timers.py | 41 ++++------ tests/components/intent/test_timers.py | 80 +++++++------------ 3 files changed, 47 insertions(+), 87 deletions(-) diff --git a/homeassistant/components/conversation/default_agent.py b/homeassistant/components/conversation/default_agent.py index c124ad96af8..98e8d07bd58 100644 --- a/homeassistant/components/conversation/default_agent.py +++ b/homeassistant/components/conversation/default_agent.py @@ -335,18 +335,13 @@ class DefaultAgent(ConversationEntity): assert lang_intents is not None # Slot values to pass to the intent - slots: dict[str, Any] = {} - - # Automatically add device id - if user_input.device_id is not None: - slots["device_id"] = user_input.device_id - - # Add entities from match - for entity in result.entities_list: - slots[entity.name] = { + slots: dict[str, Any] = { + entity.name: { "value": entity.value, "text": entity.text or entity.value, } + for entity in result.entities_list + } try: intent_response = await intent.async_handle( diff --git a/homeassistant/components/intent/timers.py b/homeassistant/components/intent/timers.py index 5aac199f32b..cca2e5a22ae 100644 --- a/homeassistant/components/intent/timers.py +++ b/homeassistant/components/intent/timers.py @@ -408,7 +408,9 @@ def async_register_timer_handler( # ----------------------------------------------------------------------------- -def _find_timer(hass: HomeAssistant, slots: dict[str, Any]) -> TimerInfo: +def _find_timer( + hass: HomeAssistant, slots: dict[str, Any], device_id: str | None +) -> TimerInfo: """Match a single timer with constraints or raise an error.""" timer_manager: TimerManager = hass.data[TIMER_DATA] matching_timers: list[TimerInfo] = list(timer_manager.timers.values()) @@ -463,10 +465,7 @@ def _find_timer(hass: HomeAssistant, slots: dict[str, Any]) -> TimerInfo: return matching_timers[0] # Use device id - device_id: str | None = None - if matching_timers and ("device_id" in slots): - device_id = slots["device_id"]["value"] - assert device_id is not None + if matching_timers and device_id: matching_device_timers = [ t for t in matching_timers if (t.device_id == device_id) ] @@ -513,7 +512,9 @@ def _find_timer(hass: HomeAssistant, slots: dict[str, Any]) -> TimerInfo: raise TimerNotFoundError -def _find_timers(hass: HomeAssistant, slots: dict[str, Any]) -> list[TimerInfo]: +def _find_timers( + hass: HomeAssistant, slots: dict[str, Any], device_id: str | None +) -> list[TimerInfo]: """Match multiple timers with constraints or raise an error.""" timer_manager: TimerManager = hass.data[TIMER_DATA] matching_timers: list[TimerInfo] = list(timer_manager.timers.values()) @@ -559,12 +560,11 @@ def _find_timers(hass: HomeAssistant, slots: dict[str, Any]) -> list[TimerInfo]: # No matches return matching_timers - if "device_id" not in slots: + if not device_id: # Can't re-order based on area/floor return matching_timers # Use device id to order remaining timers - device_id: str = slots["device_id"]["value"] device_registry = dr.async_get(hass) device = device_registry.async_get(device_id) if (device is None) or (device.area_id is None): @@ -612,7 +612,6 @@ class StartTimerIntentHandler(intent.IntentHandler): slot_schema = { vol.Required(vol.Any("hours", "minutes", "seconds")): cv.positive_int, vol.Optional("name"): cv.string, - vol.Optional("device_id"): cv.string, } async def async_handle(self, intent_obj: intent.Intent) -> intent.IntentResponse: @@ -621,10 +620,6 @@ class StartTimerIntentHandler(intent.IntentHandler): timer_manager: TimerManager = hass.data[TIMER_DATA] slots = self.async_validate_slots(intent_obj.slots) - device_id: str | None = None - if "device_id" in slots: - device_id = slots["device_id"]["value"] - name: str | None = None if "name" in slots: name = slots["name"]["value"] @@ -646,7 +641,7 @@ class StartTimerIntentHandler(intent.IntentHandler): minutes, seconds, language=intent_obj.language, - device_id=device_id, + device_id=intent_obj.device_id, name=name, ) @@ -660,7 +655,6 @@ class CancelTimerIntentHandler(intent.IntentHandler): slot_schema = { vol.Any("start_hours", "start_minutes", "start_seconds"): cv.positive_int, vol.Optional("name"): cv.string, - vol.Optional("device_id"): cv.string, } async def async_handle(self, intent_obj: intent.Intent) -> intent.IntentResponse: @@ -669,7 +663,7 @@ class CancelTimerIntentHandler(intent.IntentHandler): timer_manager: TimerManager = hass.data[TIMER_DATA] slots = self.async_validate_slots(intent_obj.slots) - timer = _find_timer(hass, slots) + timer = _find_timer(hass, slots, intent_obj.device_id) timer_manager.cancel_timer(timer.id) return intent_obj.create_response() @@ -683,7 +677,6 @@ class IncreaseTimerIntentHandler(intent.IntentHandler): vol.Any("hours", "minutes", "seconds"): cv.positive_int, vol.Any("start_hours", "start_minutes", "start_seconds"): cv.positive_int, vol.Optional("name"): cv.string, - vol.Optional("device_id"): cv.string, } async def async_handle(self, intent_obj: intent.Intent) -> intent.IntentResponse: @@ -693,7 +686,7 @@ class IncreaseTimerIntentHandler(intent.IntentHandler): slots = self.async_validate_slots(intent_obj.slots) total_seconds = _get_total_seconds(slots) - timer = _find_timer(hass, slots) + timer = _find_timer(hass, slots, intent_obj.device_id) timer_manager.add_time(timer.id, total_seconds) return intent_obj.create_response() @@ -707,7 +700,6 @@ class DecreaseTimerIntentHandler(intent.IntentHandler): vol.Required(vol.Any("hours", "minutes", "seconds")): cv.positive_int, vol.Any("start_hours", "start_minutes", "start_seconds"): cv.positive_int, vol.Optional("name"): cv.string, - vol.Optional("device_id"): cv.string, } async def async_handle(self, intent_obj: intent.Intent) -> intent.IntentResponse: @@ -717,7 +709,7 @@ class DecreaseTimerIntentHandler(intent.IntentHandler): slots = self.async_validate_slots(intent_obj.slots) total_seconds = _get_total_seconds(slots) - timer = _find_timer(hass, slots) + timer = _find_timer(hass, slots, intent_obj.device_id) timer_manager.remove_time(timer.id, total_seconds) return intent_obj.create_response() @@ -730,7 +722,6 @@ class PauseTimerIntentHandler(intent.IntentHandler): slot_schema = { vol.Any("start_hours", "start_minutes", "start_seconds"): cv.positive_int, vol.Optional("name"): cv.string, - vol.Optional("device_id"): cv.string, } async def async_handle(self, intent_obj: intent.Intent) -> intent.IntentResponse: @@ -739,7 +730,7 @@ class PauseTimerIntentHandler(intent.IntentHandler): timer_manager: TimerManager = hass.data[TIMER_DATA] slots = self.async_validate_slots(intent_obj.slots) - timer = _find_timer(hass, slots) + timer = _find_timer(hass, slots, intent_obj.device_id) timer_manager.pause_timer(timer.id) return intent_obj.create_response() @@ -752,7 +743,6 @@ class UnpauseTimerIntentHandler(intent.IntentHandler): slot_schema = { vol.Any("start_hours", "start_minutes", "start_seconds"): cv.positive_int, vol.Optional("name"): cv.string, - vol.Optional("device_id"): cv.string, } async def async_handle(self, intent_obj: intent.Intent) -> intent.IntentResponse: @@ -761,7 +751,7 @@ class UnpauseTimerIntentHandler(intent.IntentHandler): timer_manager: TimerManager = hass.data[TIMER_DATA] slots = self.async_validate_slots(intent_obj.slots) - timer = _find_timer(hass, slots) + timer = _find_timer(hass, slots, intent_obj.device_id) timer_manager.unpause_timer(timer.id) return intent_obj.create_response() @@ -774,7 +764,6 @@ class TimerStatusIntentHandler(intent.IntentHandler): slot_schema = { vol.Any("start_hours", "start_minutes", "start_seconds"): cv.positive_int, vol.Optional("name"): cv.string, - vol.Optional("device_id"): cv.string, } async def async_handle(self, intent_obj: intent.Intent) -> intent.IntentResponse: @@ -783,7 +772,7 @@ class TimerStatusIntentHandler(intent.IntentHandler): slots = self.async_validate_slots(intent_obj.slots) statuses: list[dict[str, Any]] = [] - for timer in _find_timers(hass, slots): + for timer in _find_timers(hass, slots, intent_obj.device_id): total_seconds = timer.seconds_left minutes, seconds = divmod(total_seconds, 60) diff --git a/tests/components/intent/test_timers.py b/tests/components/intent/test_timers.py index b88112ab6c8..7e458fed47e 100644 --- a/tests/components/intent/test_timers.py +++ b/tests/components/intent/test_timers.py @@ -65,9 +65,9 @@ async def test_start_finish_timer(hass: HomeAssistant, init_components) -> None: intent.INTENT_START_TIMER, { "name": {"value": timer_name}, - "device_id": {"value": device_id}, "seconds": {"value": 0}, }, + device_id=device_id, ) assert result.response_type == intent.IntentResponseType.ACTION_DONE @@ -118,11 +118,11 @@ async def test_cancel_timer(hass: HomeAssistant, init_components) -> None: "test", intent.INTENT_START_TIMER, { - "device_id": {"value": device_id}, "hours": {"value": 1}, "minutes": {"value": 2}, "seconds": {"value": 3}, }, + device_id=device_id, ) async with asyncio.timeout(1): @@ -154,12 +154,12 @@ async def test_cancel_timer(hass: HomeAssistant, init_components) -> None: "test", intent.INTENT_START_TIMER, { - "device_id": {"value": device_id}, "name": {"value": timer_name}, "hours": {"value": 1}, "minutes": {"value": 2}, "seconds": {"value": 3}, }, + device_id=device_id, ) async with asyncio.timeout(1): @@ -225,12 +225,12 @@ async def test_increase_timer(hass: HomeAssistant, init_components) -> None: "test", intent.INTENT_START_TIMER, { - "device_id": {"value": device_id}, "name": {"value": timer_name}, "hours": {"value": 1}, "minutes": {"value": 2}, "seconds": {"value": 3}, }, + device_id=device_id, ) assert result.response_type == intent.IntentResponseType.ACTION_DONE @@ -244,7 +244,6 @@ async def test_increase_timer(hass: HomeAssistant, init_components) -> None: "test", intent.INTENT_INCREASE_TIMER, { - "device_id": {"value": device_id}, "start_hours": {"value": 1}, "start_minutes": {"value": 2}, "start_seconds": {"value": 3}, @@ -252,6 +251,7 @@ async def test_increase_timer(hass: HomeAssistant, init_components) -> None: "minutes": {"value": 5}, "seconds": {"value": 30}, }, + device_id=device_id, ) assert result.response_type == intent.IntentResponseType.ACTION_DONE @@ -321,12 +321,12 @@ async def test_decrease_timer(hass: HomeAssistant, init_components) -> None: "test", intent.INTENT_START_TIMER, { - "device_id": {"value": device_id}, "name": {"value": timer_name}, "hours": {"value": 1}, "minutes": {"value": 2}, "seconds": {"value": 3}, }, + device_id=device_id, ) assert result.response_type == intent.IntentResponseType.ACTION_DONE @@ -340,12 +340,12 @@ async def test_decrease_timer(hass: HomeAssistant, init_components) -> None: "test", intent.INTENT_DECREASE_TIMER, { - "device_id": {"value": device_id}, "start_hours": {"value": 1}, "start_minutes": {"value": 2}, "start_seconds": {"value": 3}, "seconds": {"value": 30}, }, + device_id=device_id, ) assert result.response_type == intent.IntentResponseType.ACTION_DONE @@ -535,7 +535,8 @@ async def test_disambiguation( hass, "test", intent.INTENT_START_TIMER, - {"device_id": {"value": device_alice_study.id}, "minutes": {"value": 3}}, + {"minutes": {"value": 3}}, + device_id=device_alice_study.id, ) assert result.response_type == intent.IntentResponseType.ACTION_DONE @@ -544,16 +545,14 @@ async def test_disambiguation( hass, "test", intent.INTENT_START_TIMER, - {"device_id": {"value": device_bob_kitchen_1.id}, "minutes": {"value": 3}}, + {"minutes": {"value": 3}}, + device_id=device_bob_kitchen_1.id, ) assert result.response_type == intent.IntentResponseType.ACTION_DONE # Alice should hear her timer listed first result = await intent.async_handle( - hass, - "test", - intent.INTENT_TIMER_STATUS, - {"device_id": {"value": device_alice_study.id}}, + hass, "test", intent.INTENT_TIMER_STATUS, {}, device_id=device_alice_study.id ) assert result.response_type == intent.IntentResponseType.ACTION_DONE timers = result.speech_slots.get("timers", []) @@ -563,10 +562,7 @@ async def test_disambiguation( # Bob should hear his timer listed first result = await intent.async_handle( - hass, - "test", - intent.INTENT_TIMER_STATUS, - {"device_id": {"value": device_bob_kitchen_1.id}}, + hass, "test", intent.INTENT_TIMER_STATUS, {}, device_id=device_bob_kitchen_1.id ) assert result.response_type == intent.IntentResponseType.ACTION_DONE timers = result.speech_slots.get("timers", []) @@ -589,10 +585,7 @@ async def test_disambiguation( # Alice: cancel my timer result = await intent.async_handle( - hass, - "test", - intent.INTENT_CANCEL_TIMER, - {"device_id": {"value": device_alice_study.id}}, + hass, "test", intent.INTENT_CANCEL_TIMER, {}, device_id=device_alice_study.id ) assert result.response_type == intent.IntentResponseType.ACTION_DONE @@ -606,10 +599,7 @@ async def test_disambiguation( # Cancel Bob's timer result = await intent.async_handle( - hass, - "test", - intent.INTENT_CANCEL_TIMER, - {"device_id": {"value": device_bob_kitchen_1.id}}, + hass, "test", intent.INTENT_CANCEL_TIMER, {}, device_id=device_bob_kitchen_1.id ) assert result.response_type == intent.IntentResponseType.ACTION_DONE @@ -645,7 +635,8 @@ async def test_disambiguation( hass, "test", intent.INTENT_START_TIMER, - {"device_id": {"value": device_alice_study.id}, "minutes": {"value": 3}}, + {"minutes": {"value": 3}}, + device_id=device_alice_study.id, ) assert result.response_type == intent.IntentResponseType.ACTION_DONE @@ -654,10 +645,8 @@ async def test_disambiguation( hass, "test", intent.INTENT_START_TIMER, - { - "device_id": {"value": device_alice_bedroom.id}, - "minutes": {"value": 3}, - }, + {"minutes": {"value": 3}}, + device_id=device_alice_bedroom.id, ) assert result.response_type == intent.IntentResponseType.ACTION_DONE @@ -666,7 +655,8 @@ async def test_disambiguation( hass, "test", intent.INTENT_START_TIMER, - {"device_id": {"value": device_bob_kitchen_1.id}, "minutes": {"value": 3}}, + {"minutes": {"value": 3}}, + device_id=device_bob_kitchen_1.id, ) assert result.response_type == intent.IntentResponseType.ACTION_DONE @@ -675,17 +665,15 @@ async def test_disambiguation( hass, "test", intent.INTENT_START_TIMER, - {"device_id": {"value": device_bob_living_room.id}, "minutes": {"value": 3}}, + {"minutes": {"value": 3}}, + device_id=device_bob_living_room.id, ) assert result.response_type == intent.IntentResponseType.ACTION_DONE # Alice should hear the timer in her area first, then on her floor, then # elsewhere. result = await intent.async_handle( - hass, - "test", - intent.INTENT_TIMER_STATUS, - {"device_id": {"value": device_alice_study.id}}, + hass, "test", intent.INTENT_TIMER_STATUS, {}, device_id=device_alice_study.id ) assert result.response_type == intent.IntentResponseType.ACTION_DONE timers = result.speech_slots.get("timers", []) @@ -699,10 +687,7 @@ async def test_disambiguation( cancelled_event.clear() timer_info = None result = await intent.async_handle( - hass, - "test", - intent.INTENT_CANCEL_TIMER, - {"device_id": {"value": device_alice_study.id}}, + hass, "test", intent.INTENT_CANCEL_TIMER, {}, device_id=device_alice_study.id ) assert result.response_type == intent.IntentResponseType.ACTION_DONE @@ -727,10 +712,7 @@ async def test_disambiguation( cancelled_event.clear() timer_info = None result = await intent.async_handle( - hass, - "test", - intent.INTENT_CANCEL_TIMER, - {"device_id": {"value": device_alice_study.id}}, + hass, "test", intent.INTENT_CANCEL_TIMER, {}, device_id=device_alice_study.id ) assert result.response_type == intent.IntentResponseType.ACTION_DONE @@ -756,10 +738,7 @@ async def test_disambiguation( cancelled_event.clear() timer_info = None result = await intent.async_handle( - hass, - "test", - intent.INTENT_CANCEL_TIMER, - {"device_id": {"value": device_bob_kitchen_2.id}}, + hass, "test", intent.INTENT_CANCEL_TIMER, {}, device_id=device_bob_kitchen_2.id ) assert result.response_type == intent.IntentResponseType.ACTION_DONE @@ -774,10 +753,7 @@ async def test_disambiguation( cancelled_event.clear() timer_info = None result = await intent.async_handle( - hass, - "test", - intent.INTENT_CANCEL_TIMER, - {"device_id": {"value": device_bob_kitchen_2.id}}, + hass, "test", intent.INTENT_CANCEL_TIMER, {}, device_id=device_bob_kitchen_2.id ) assert result.response_type == intent.IntentResponseType.ACTION_DONE