From d3ceaef09893d54405345eaa350a3d2139ef758c Mon Sep 17 00:00:00 2001 From: Michael Hansen <mike@rhasspy.org> Date: Wed, 26 Jun 2024 02:06:56 -0500 Subject: [PATCH] Allow timer management from any device (#120440) --- homeassistant/components/intent/timers.py | 47 ++----- homeassistant/helpers/llm.py | 2 +- .../conversation/test_default_agent.py | 20 ++- tests/components/intent/test_timers.py | 115 ++++++++---------- tests/helpers/test_llm.py | 2 +- 5 files changed, 77 insertions(+), 109 deletions(-) diff --git a/homeassistant/components/intent/timers.py b/homeassistant/components/intent/timers.py index 40b55134e92..82f6121da53 100644 --- a/homeassistant/components/intent/timers.py +++ b/homeassistant/components/intent/timers.py @@ -490,7 +490,7 @@ class FindTimerFilter(StrEnum): def _find_timer( hass: HomeAssistant, - device_id: str, + device_id: str | None, slots: dict[str, Any], find_filter: FindTimerFilter | None = None, ) -> TimerInfo: @@ -577,7 +577,7 @@ def _find_timer( return matching_timers[0] # Use device id - if matching_timers: + if matching_timers and device_id: matching_device_timers = [ t for t in matching_timers if (t.device_id == device_id) ] @@ -626,7 +626,7 @@ def _find_timer( def _find_timers( - hass: HomeAssistant, device_id: str, slots: dict[str, Any] + hass: HomeAssistant, device_id: str | None, slots: dict[str, Any] ) -> list[TimerInfo]: """Match multiple timers with constraints or raise an error.""" timer_manager: TimerManager = hass.data[TIMER_DATA] @@ -689,6 +689,10 @@ def _find_timers( # No matches return matching_timers + if not device_id: + # Can't order using area/floor + return matching_timers + # Use device id to order remaining timers device_registry = dr.async_get(hass) device = device_registry.async_get(device_id) @@ -861,12 +865,6 @@ class CancelTimerIntentHandler(intent.IntentHandler): timer_manager: TimerManager = hass.data[TIMER_DATA] slots = self.async_validate_slots(intent_obj.slots) - if not ( - intent_obj.device_id and timer_manager.is_timer_device(intent_obj.device_id) - ): - # Fail early - raise TimersNotSupportedError(intent_obj.device_id) - timer = _find_timer(hass, intent_obj.device_id, slots) timer_manager.cancel_timer(timer.id) return intent_obj.create_response() @@ -890,12 +888,6 @@ class IncreaseTimerIntentHandler(intent.IntentHandler): timer_manager: TimerManager = hass.data[TIMER_DATA] slots = self.async_validate_slots(intent_obj.slots) - if not ( - intent_obj.device_id and timer_manager.is_timer_device(intent_obj.device_id) - ): - # Fail early - raise TimersNotSupportedError(intent_obj.device_id) - total_seconds = _get_total_seconds(slots) timer = _find_timer(hass, intent_obj.device_id, slots) timer_manager.add_time(timer.id, total_seconds) @@ -920,12 +912,6 @@ class DecreaseTimerIntentHandler(intent.IntentHandler): timer_manager: TimerManager = hass.data[TIMER_DATA] slots = self.async_validate_slots(intent_obj.slots) - if not ( - intent_obj.device_id and timer_manager.is_timer_device(intent_obj.device_id) - ): - # Fail early - raise TimersNotSupportedError(intent_obj.device_id) - total_seconds = _get_total_seconds(slots) timer = _find_timer(hass, intent_obj.device_id, slots) timer_manager.remove_time(timer.id, total_seconds) @@ -949,12 +935,6 @@ class PauseTimerIntentHandler(intent.IntentHandler): timer_manager: TimerManager = hass.data[TIMER_DATA] slots = self.async_validate_slots(intent_obj.slots) - if not ( - intent_obj.device_id and timer_manager.is_timer_device(intent_obj.device_id) - ): - # Fail early - raise TimersNotSupportedError(intent_obj.device_id) - timer = _find_timer( hass, intent_obj.device_id, slots, find_filter=FindTimerFilter.ONLY_ACTIVE ) @@ -979,12 +959,6 @@ class UnpauseTimerIntentHandler(intent.IntentHandler): timer_manager: TimerManager = hass.data[TIMER_DATA] slots = self.async_validate_slots(intent_obj.slots) - if not ( - intent_obj.device_id and timer_manager.is_timer_device(intent_obj.device_id) - ): - # Fail early - raise TimersNotSupportedError(intent_obj.device_id) - timer = _find_timer( hass, intent_obj.device_id, slots, find_filter=FindTimerFilter.ONLY_INACTIVE ) @@ -1006,15 +980,8 @@ class TimerStatusIntentHandler(intent.IntentHandler): async def async_handle(self, intent_obj: intent.Intent) -> intent.IntentResponse: """Handle the intent.""" hass = intent_obj.hass - timer_manager: TimerManager = hass.data[TIMER_DATA] slots = self.async_validate_slots(intent_obj.slots) - if not ( - intent_obj.device_id and timer_manager.is_timer_device(intent_obj.device_id) - ): - # Fail early - raise TimersNotSupportedError(intent_obj.device_id) - statuses: list[dict[str, Any]] = [] for timer in _find_timers(hass, intent_obj.device_id, slots): total_seconds = timer.seconds_left diff --git a/homeassistant/helpers/llm.py b/homeassistant/helpers/llm.py index 4410de67ef2..ba307a785ac 100644 --- a/homeassistant/helpers/llm.py +++ b/homeassistant/helpers/llm.py @@ -355,7 +355,7 @@ class AssistAPI(API): if not llm_context.device_id or not async_device_supports_timers( self.hass, llm_context.device_id ): - prompt.append("This device does not support timers.") + prompt.append("This device is not able to start timers.") if exposed_entities: prompt.append( diff --git a/tests/components/conversation/test_default_agent.py b/tests/components/conversation/test_default_agent.py index dee7b4ca0ff..f8a021475d5 100644 --- a/tests/components/conversation/test_default_agent.py +++ b/tests/components/conversation/test_default_agent.py @@ -860,13 +860,27 @@ async def test_error_feature_not_supported(hass: HomeAssistant) -> None: @pytest.mark.usefixtures("init_components") -async def test_error_no_timer_support(hass: HomeAssistant) -> None: +async def test_error_no_timer_support( + hass: HomeAssistant, + area_registry: ar.AreaRegistry, + device_registry: dr.DeviceRegistry, +) -> None: """Test error message when a device does not support timers (no handler is registered).""" - device_id = "test_device" + area_kitchen = area_registry.async_create("kitchen") + + entry = MockConfigEntry() + entry.add_to_hass(hass) + device_kitchen = device_registry.async_get_or_create( + config_entry_id=entry.entry_id, + connections=set(), + identifiers={("demo", "device-kitchen")}, + ) + device_registry.async_update_device(device_kitchen.id, area_id=area_kitchen.id) + device_id = device_kitchen.id # No timer handler is registered for the device result = await conversation.async_converse( - hass, "pause timer", None, Context(), None, device_id=device_id + hass, "set a 5 minute timer", None, Context(), None, device_id=device_id ) assert result.response.response_type == intent.IntentResponseType.ERROR diff --git a/tests/components/intent/test_timers.py b/tests/components/intent/test_timers.py index 329db6e8b2b..c2efe5d39e2 100644 --- a/tests/components/intent/test_timers.py +++ b/tests/components/intent/test_timers.py @@ -64,6 +64,7 @@ async def test_start_finish_timer(hass: HomeAssistant, init_components) -> None: async_register_timer_handler(hass, device_id, handle_timer) + # A device that has been registered to handle timers is required result = await intent.async_handle( hass, "test", @@ -185,6 +186,27 @@ async def test_cancel_timer(hass: HomeAssistant, init_components) -> None: async with asyncio.timeout(1): await cancelled_event.wait() + # Cancel without a device + timer_name = None + started_event.clear() + result = await intent.async_handle( + hass, + "test", + intent.INTENT_START_TIMER, + { + "hours": {"value": 1}, + "minutes": {"value": 2}, + "seconds": {"value": 3}, + }, + device_id=device_id, + ) + + async with asyncio.timeout(1): + await started_event.wait() + + result = await intent.async_handle(hass, "test", intent.INTENT_CANCEL_TIMER, {}) + assert result.response_type == intent.IntentResponseType.ACTION_DONE + async def test_increase_timer(hass: HomeAssistant, init_components) -> None: """Test increasing the time of a running timer.""" @@ -260,7 +282,6 @@ async def test_increase_timer(hass: HomeAssistant, init_components) -> None: "minutes": {"value": 0}, "seconds": {"value": 0}, }, - device_id=device_id, ) assert result.response_type == intent.IntentResponseType.ACTION_DONE @@ -279,7 +300,6 @@ 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 @@ -293,7 +313,6 @@ async def test_increase_timer(hass: HomeAssistant, init_components) -> None: "test", intent.INTENT_CANCEL_TIMER, {"name": {"value": timer_name}}, - device_id=device_id, ) assert result.response_type == intent.IntentResponseType.ACTION_DONE @@ -375,7 +394,6 @@ async def test_decrease_timer(hass: HomeAssistant, init_components) -> None: "start_seconds": {"value": 3}, "seconds": {"value": 30}, }, - device_id=device_id, ) assert result.response_type == intent.IntentResponseType.ACTION_DONE @@ -389,7 +407,6 @@ async def test_decrease_timer(hass: HomeAssistant, init_components) -> None: "test", intent.INTENT_CANCEL_TIMER, {"name": {"value": timer_name}}, - device_id=device_id, ) assert result.response_type == intent.IntentResponseType.ACTION_DONE @@ -467,7 +484,6 @@ async def test_decrease_timer_below_zero(hass: HomeAssistant, init_components) - "start_seconds": {"value": 3}, "seconds": {"value": original_total_seconds + 1}, }, - device_id=device_id, ) assert result.response_type == intent.IntentResponseType.ACTION_DONE @@ -482,43 +498,25 @@ async def test_find_timer_failed(hass: HomeAssistant, init_components) -> None: """Test finding a timer with the wrong info.""" device_id = "test_device" - for intent_name in ( - intent.INTENT_START_TIMER, - intent.INTENT_CANCEL_TIMER, - intent.INTENT_PAUSE_TIMER, - intent.INTENT_UNPAUSE_TIMER, - intent.INTENT_INCREASE_TIMER, - intent.INTENT_DECREASE_TIMER, - intent.INTENT_TIMER_STATUS, - ): - if intent_name in ( + # No device id + with pytest.raises(TimersNotSupportedError): + await intent.async_handle( + hass, + "test", intent.INTENT_START_TIMER, - intent.INTENT_INCREASE_TIMER, - intent.INTENT_DECREASE_TIMER, - ): - slots = {"minutes": {"value": 5}} - else: - slots = {} + {"minutes": {"value": 5}}, + device_id=None, + ) - # No device id - with pytest.raises(TimersNotSupportedError): - await intent.async_handle( - hass, - "test", - intent_name, - slots, - device_id=None, - ) - - # Unregistered device - with pytest.raises(TimersNotSupportedError): - await intent.async_handle( - hass, - "test", - intent_name, - slots, - device_id=device_id, - ) + # Unregistered device + with pytest.raises(TimersNotSupportedError): + await intent.async_handle( + hass, + "test", + intent.INTENT_START_TIMER, + {"minutes": {"value": 5}}, + device_id=device_id, + ) # Must register a handler before we can do anything with timers @callback @@ -543,7 +541,6 @@ async def test_find_timer_failed(hass: HomeAssistant, init_components) -> None: "test", intent.INTENT_INCREASE_TIMER, {"name": {"value": "PIZZA "}, "minutes": {"value": 1}}, - device_id=device_id, ) assert result.response_type == intent.IntentResponseType.ACTION_DONE @@ -554,7 +551,6 @@ async def test_find_timer_failed(hass: HomeAssistant, init_components) -> None: "test", intent.INTENT_CANCEL_TIMER, {"name": {"value": "does-not-exist"}}, - device_id=device_id, ) # Right start time @@ -563,7 +559,6 @@ async def test_find_timer_failed(hass: HomeAssistant, init_components) -> None: "test", intent.INTENT_INCREASE_TIMER, {"start_minutes": {"value": 5}, "minutes": {"value": 1}}, - device_id=device_id, ) assert result.response_type == intent.IntentResponseType.ACTION_DONE @@ -574,7 +569,6 @@ async def test_find_timer_failed(hass: HomeAssistant, init_components) -> None: "test", intent.INTENT_CANCEL_TIMER, {"start_minutes": {"value": 1}}, - device_id=device_id, ) @@ -903,9 +897,7 @@ async def test_pause_unpause_timer(hass: HomeAssistant, init_components) -> None # Pause the timer expected_active = False - result = await intent.async_handle( - hass, "test", intent.INTENT_PAUSE_TIMER, {}, device_id=device_id - ) + result = await intent.async_handle(hass, "test", intent.INTENT_PAUSE_TIMER, {}) assert result.response_type == intent.IntentResponseType.ACTION_DONE async with asyncio.timeout(1): @@ -913,16 +905,12 @@ async def test_pause_unpause_timer(hass: HomeAssistant, init_components) -> None # Pausing again will fail because there are no running timers with pytest.raises(TimerNotFoundError): - await intent.async_handle( - hass, "test", intent.INTENT_PAUSE_TIMER, {}, device_id=device_id - ) + await intent.async_handle(hass, "test", intent.INTENT_PAUSE_TIMER, {}) # Unpause the timer updated_event.clear() expected_active = True - result = await intent.async_handle( - hass, "test", intent.INTENT_UNPAUSE_TIMER, {}, device_id=device_id - ) + result = await intent.async_handle(hass, "test", intent.INTENT_UNPAUSE_TIMER, {}) assert result.response_type == intent.IntentResponseType.ACTION_DONE async with asyncio.timeout(1): @@ -930,9 +918,7 @@ async def test_pause_unpause_timer(hass: HomeAssistant, init_components) -> None # Unpausing again will fail because there are no paused timers with pytest.raises(TimerNotFoundError): - await intent.async_handle( - hass, "test", intent.INTENT_UNPAUSE_TIMER, {}, device_id=device_id - ) + await intent.async_handle(hass, "test", intent.INTENT_UNPAUSE_TIMER, {}) async def test_timer_not_found(hass: HomeAssistant) -> None: @@ -1101,13 +1087,14 @@ async def test_timer_status_with_names(hass: HomeAssistant, init_components) -> await started_event.wait() # No constraints returns all timers - result = await intent.async_handle( - hass, "test", intent.INTENT_TIMER_STATUS, {}, device_id=device_id - ) - assert result.response_type == intent.IntentResponseType.ACTION_DONE - timers = result.speech_slots.get("timers", []) - assert len(timers) == 4 - assert {t.get(ATTR_NAME) for t in timers} == {"pizza", "cookies", "chicken"} + for handle_device_id in (device_id, None): + result = await intent.async_handle( + hass, "test", intent.INTENT_TIMER_STATUS, {}, device_id=handle_device_id + ) + assert result.response_type == intent.IntentResponseType.ACTION_DONE + timers = result.speech_slots.get("timers", []) + assert len(timers) == 4 + assert {t.get(ATTR_NAME) for t in timers} == {"pizza", "cookies", "chicken"} # Get status of cookie timer result = await intent.async_handle( diff --git a/tests/helpers/test_llm.py b/tests/helpers/test_llm.py index 872297b09ec..ad18aa53071 100644 --- a/tests/helpers/test_llm.py +++ b/tests/helpers/test_llm.py @@ -578,7 +578,7 @@ async def test_assist_api_prompt( "(what comes before the dot in its entity id). " "When controlling an area, prefer passing just area name and domain." ) - no_timer_prompt = "This device does not support timers." + no_timer_prompt = "This device is not able to start timers." area_prompt = ( "When a user asks to turn on all devices of a specific type, "