Address review feedback on timer lists

Behaviour fixes:

- Subtracting all remaining time from a *paused* timer now finishes it,
  like the active case already did, instead of leaving it paused at zero.
- A zero duration passed to add_time/subtract_time is a no-op. The
  service schema accepts it, so a direct call used to reschedule the
  timer and emit TIME_CHANGED with a delta of 0.
- _archive now cancels the pending finish callback itself, so every
  terminal path clears it rather than each caller remembering to.

Timer intents now check supported_features. A list may advertise only
some of TimerListEntityFeature, but every intent called its entity
method directly, so an unsupported action surfaced as NotImplementedError
instead of the "device does not support timers" response. Tool exposure
matches: HassCancelAllTimers moves out of the always-on LLM_INTENTS,
since it resolves the requesting device's own list and could only ever
fail without one, and each timer tool is now offered only if the
device's list supports it.

Timer list triggers no longer miss a list created after the automation.
Entity registry creation fires before EntityPlatform hands the entity to
the component, so the synchronous lookup found nothing and never retried;
wait for the entity's first state write and resolve then.

ESPHome only marks the timer_list platform as needed when the device
advertises the TIMERS feature, matching what timer_list.async_setup_entry
will actually create.

Docstring corrections: `timers` said archived timers are what voice
status reports, but _find_timers filters to active and paused; add_time
promised finishing on subtract-to-zero without qualifying that it now
holds for paused timers too, and said nothing about a zero duration.

Also drops a conditional from a parametrized test body, per the
repository test conventions.

Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
This commit is contained in:
Michael Hansen
2026-09-02 17:21:59 -05:00
co-authored by Claude Opus 5
parent 8c6e3f1464
commit 5045dd33ce
10 changed files with 243 additions and 41 deletions
@@ -44,6 +44,7 @@ from aioesphomeapi import (
UpdateInfo,
UserService,
ValveInfo,
VoiceAssistantFeature,
WaterHeaterInfo,
build_device_unique_id,
)
@@ -355,9 +356,15 @@ class RuntimeEntryData:
# and we don't want to load the update platform since it needs
# a complete device_info.
needed_platforms.add(Platform.UPDATE)
if self.device_info.voice_assistant_feature_flags_compat(self.api_version):
feature_flags = self.device_info.voice_assistant_feature_flags_compat(
self.api_version
)
if feature_flags:
needed_platforms.add(Platform.BINARY_SENSOR)
needed_platforms.add(Platform.SELECT)
if feature_flags & VoiceAssistantFeature.TIMERS:
# Matches what timer_list.async_setup_entry creates, so the
# platform is not marked loaded before it can make an entity.
needed_platforms.add(Platform.TIMER_LIST)
# Make a dict of the EntityInfo by type and send
+13 -10
View File
@@ -17,21 +17,23 @@ from homeassistant.helpers import (
from homeassistant.helpers.llm import LLM_API_ASSIST, IntentTool, LLMContext, Tool
from .const import DOMAIN
from .timers import async_device_supports_timers
from .timers import async_device_supports_timer_intent
# Generic intents exposed as LLM tools regardless of a timer-capable device.
LLM_INTENTS = (
intent.INTENT_TURN_ON,
intent.INTENT_TURN_OFF,
intent.INTENT_CANCEL_ALL_TIMERS,
intent.INTENT_SET_POSITION,
intent.INTENT_STOP_MOVING,
)
# Timer intents, only exposed for a device that supports timers.
# Timer intents, only exposed for a device that supports timers. All of them
# resolve the requesting device's own timer list, so without one they can only
# fail.
TIMER_INTENTS = (
intent.INTENT_START_TIMER,
intent.INTENT_CANCEL_TIMER,
intent.INTENT_CANCEL_ALL_TIMERS,
intent.INTENT_INCREASE_TIMER,
intent.INTENT_DECREASE_TIMER,
intent.INTENT_PAUSE_TIMER,
@@ -55,13 +57,14 @@ def async_get_tools(
if api_id != LLM_API_ASSIST:
return None
supports_timers = (
llm_context.device_id is not None
and async_device_supports_timers(hass, llm_context.device_id)
)
wanted = set(LLM_INTENTS)
if supports_timers:
wanted.update(TIMER_INTENTS)
if (device_id := llm_context.device_id) is not None:
# Offer only the timer tools the device's own list can actually serve.
wanted.update(
intent_type
for intent_type in TIMER_INTENTS
if async_device_supports_timer_intent(hass, device_id, intent_type)
)
exposed_domains = {
state.domain
@@ -116,6 +119,6 @@ def async_get_tools(
)
prompt_parts = [DEVICE_CONTROL_TOOL_USAGE_PROMPT, area_prompt]
if not supports_timers:
if intent.INTENT_START_TIMER not in wanted:
prompt_parts.append("This device is not able to start timers.")
return LLMTools(tools=tools, prompt="\n".join(prompt_parts))
+51 -10
View File
@@ -10,6 +10,7 @@ import voluptuous as vol
from homeassistant.components.timer_list import (
TimerItem,
TimerListEntity,
TimerListEntityFeature,
TimerStatus,
async_get_timer_list_entity,
)
@@ -55,21 +56,61 @@ class TimersNotSupportedError(intent.IntentHandleError):
)
# The entity feature each timer intent needs. INTENT_TIMER_STATUS is absent
# because it only reads the `timers` property, which every list provides.
_INTENT_FEATURES: dict[str, TimerListEntityFeature] = {
intent.INTENT_START_TIMER: TimerListEntityFeature.CREATE_TIMER,
intent.INTENT_CANCEL_TIMER: TimerListEntityFeature.CANCEL_TIMER,
intent.INTENT_CANCEL_ALL_TIMERS: TimerListEntityFeature.CANCEL_TIMER,
intent.INTENT_INCREASE_TIMER: TimerListEntityFeature.ADD_TIME,
intent.INTENT_DECREASE_TIMER: TimerListEntityFeature.ADD_TIME,
intent.INTENT_PAUSE_TIMER: TimerListEntityFeature.PAUSE_TIMER,
intent.INTENT_UNPAUSE_TIMER: TimerListEntityFeature.PAUSE_TIMER,
}
@callback
def _supports_intent(entity: TimerListEntity, intent_type: str) -> bool:
"""Return True if a timer list advertises what an intent needs."""
if (feature := _INTENT_FEATURES.get(intent_type)) is None:
return True
return bool((entity.supported_features or 0) & feature)
@callback
def async_device_supports_timers(hass: HomeAssistant, device_id: str) -> bool:
"""Return True if a device has a timer_list entity to manage timers."""
return async_get_timer_list_entity(hass, device_id) is not None
@callback
def async_device_supports_timer_intent(
hass: HomeAssistant, device_id: str, intent_type: str
) -> bool:
"""Return True if a device's timer list can serve a given timer intent."""
if (entity := async_get_timer_list_entity(hass, device_id)) is None:
return False
return _supports_intent(entity, intent_type)
# -----------------------------------------------------------------------------
@callback
def _get_timer_entity(hass: HomeAssistant, device_id: str | None) -> TimerListEntity:
"""Return the requesting device's timer_list entity or raise if it has none."""
def _get_timer_entity(
hass: HomeAssistant, device_id: str | None, intent_type: str
) -> TimerListEntity:
"""Return the requesting device's timer_list entity.
Raises if the device has no timer list, or if its list does not advertise
the feature the intent needs, so an unsupported action reports as
unsupported rather than reaching a base method that raises
``NotImplementedError``.
"""
if (
device_id is None
or (entity := async_get_timer_list_entity(hass, device_id)) is None
or not _supports_intent(entity, intent_type)
):
raise TimersNotSupportedError(device_id)
return entity
@@ -324,7 +365,7 @@ class StartTimerIntentHandler(intent.IntentHandler):
async def async_handle(self, intent_obj: intent.Intent) -> intent.IntentResponse:
"""Handle the intent."""
hass = intent_obj.hass
entity = _get_timer_entity(hass, intent_obj.device_id)
entity = _get_timer_entity(hass, intent_obj.device_id, self.intent_type)
slots = self.async_validate_slots(intent_obj.slots)
name: str | None = None
@@ -361,7 +402,7 @@ class CancelTimerIntentHandler(intent.IntentHandler):
async def async_handle(self, intent_obj: intent.Intent) -> intent.IntentResponse:
"""Handle the intent."""
hass = intent_obj.hass
entity = _get_timer_entity(hass, intent_obj.device_id)
entity = _get_timer_entity(hass, intent_obj.device_id, self.intent_type)
slots = self.async_validate_slots(intent_obj.slots)
timer = _find_timer(entity, slots)
@@ -379,7 +420,7 @@ class CancelAllTimersIntentHandler(intent.IntentHandler):
async def async_handle(self, intent_obj: intent.Intent) -> intent.IntentResponse:
"""Handle the intent."""
hass = intent_obj.hass
entity = _get_timer_entity(hass, intent_obj.device_id)
entity = _get_timer_entity(hass, intent_obj.device_id, self.intent_type)
slots = self.async_validate_slots(intent_obj.slots)
canceled = 0
@@ -408,7 +449,7 @@ class IncreaseTimerIntentHandler(intent.IntentHandler):
async def async_handle(self, intent_obj: intent.Intent) -> intent.IntentResponse:
"""Handle the intent."""
hass = intent_obj.hass
entity = _get_timer_entity(hass, intent_obj.device_id)
entity = _get_timer_entity(hass, intent_obj.device_id, self.intent_type)
slots = self.async_validate_slots(intent_obj.slots)
total_seconds = _get_total_seconds(slots)
@@ -435,7 +476,7 @@ class DecreaseTimerIntentHandler(intent.IntentHandler):
async def async_handle(self, intent_obj: intent.Intent) -> intent.IntentResponse:
"""Handle the intent."""
hass = intent_obj.hass
entity = _get_timer_entity(hass, intent_obj.device_id)
entity = _get_timer_entity(hass, intent_obj.device_id, self.intent_type)
slots = self.async_validate_slots(intent_obj.slots)
total_seconds = _get_total_seconds(slots)
@@ -461,7 +502,7 @@ class PauseTimerIntentHandler(intent.IntentHandler):
async def async_handle(self, intent_obj: intent.Intent) -> intent.IntentResponse:
"""Handle the intent."""
hass = intent_obj.hass
entity = _get_timer_entity(hass, intent_obj.device_id)
entity = _get_timer_entity(hass, intent_obj.device_id, self.intent_type)
slots = self.async_validate_slots(intent_obj.slots)
timer = _find_timer(entity, slots, find_filter=FindTimerFilter.ONLY_ACTIVE)
@@ -483,7 +524,7 @@ class UnpauseTimerIntentHandler(intent.IntentHandler):
async def async_handle(self, intent_obj: intent.Intent) -> intent.IntentResponse:
"""Handle the intent."""
hass = intent_obj.hass
entity = _get_timer_entity(hass, intent_obj.device_id)
entity = _get_timer_entity(hass, intent_obj.device_id, self.intent_type)
slots = self.async_validate_slots(intent_obj.slots)
timer = _find_timer(entity, slots, find_filter=FindTimerFilter.ONLY_INACTIVE)
@@ -505,7 +546,7 @@ class TimerStatusIntentHandler(intent.IntentHandler):
async def async_handle(self, intent_obj: intent.Intent) -> intent.IntentResponse:
"""Handle the intent."""
hass = intent_obj.hass
entity = _get_timer_entity(hass, intent_obj.device_id)
entity = _get_timer_entity(hass, intent_obj.device_id, self.intent_type)
slots = self.async_validate_slots(intent_obj.slots)
now = dt_util.utcnow()
@@ -302,8 +302,9 @@ class TimerListEntity(Entity):
def timers(self) -> list[TimerItem]:
"""Return the timers in the list.
Includes archived (``finished`` and ``cancelled``) timers, which are
what voice commands like "how long is left on my timer?" report on.
Includes archived (``finished`` and ``cancelled``) timers, which the
websocket API and ``get_timers`` report. Voice commands only ever
consider active and paused timers.
"""
raise NotImplementedError
@@ -368,7 +369,9 @@ class TimerListEntity(Entity):
Emits ``TIME_CHANGED`` carrying ``duration`` as the event's signed
``delta``. ``created_duration`` must not change, and ``total_duration``
must be raised so it stays at least the new remaining time. Subtracting
more than is left finishes the timer instead, emitting ``FINISHED``.
more than is left finishes the timer instead, emitting ``FINISHED``,
whether it was active or paused. A zero ``duration`` changes nothing
and emits no event.
"""
raise NotImplementedError
+9 -6
View File
@@ -109,7 +109,6 @@ class InMemoryTimerListEntity(TimerListEntity):
if timer.status in _FINISHED_STATUSES:
# Already archived (finished or cancelled); nothing to cancel.
return
self._unschedule(timer_id)
self._archive(timer, TimerStatus.CANCELLED)
@override
@@ -119,27 +118,30 @@ class InMemoryTimerListEntity(TimerListEntity):
if timer.status in _FINISHED_STATUSES:
# Already archived (finished or cancelled); nothing to finish.
return
self._unschedule(timer_id)
self._archive(timer, TimerStatus.FINISHED)
@override
async def async_add_time(self, timer_id: str, duration: timedelta) -> None:
"""Add (or, with a negative duration, subtract) time on a timer."""
timer = self._get_timer(timer_id)
if not duration:
return
now = dt_util.utcnow()
if timer.status == TimerStatus.ACTIVE and timer.finishes_at is not None:
finishes_at = timer.finishes_at + duration
if finishes_at <= now:
# Subtracted past the end; finish now rather than schedule the past.
self._unschedule(timer_id)
self._archive(timer, TimerStatus.FINISHED)
return
timer.finishes_at = finishes_at
self._schedule(timer)
elif timer.status == TimerStatus.PAUSED and timer.paused_remaining is not None:
timer.paused_remaining = max(
timedelta(0), timer.paused_remaining + duration
)
paused_remaining = timer.paused_remaining + duration
if paused_remaining <= timedelta(0):
# Same as the active case: nothing left to resume to.
self._archive(timer, TimerStatus.FINISHED)
return
timer.paused_remaining = paused_remaining
else:
return
# Rounded to whole seconds: timers are second-granularity, and the
@@ -187,6 +189,7 @@ class InMemoryTimerListEntity(TimerListEntity):
@callback
def _archive(self, timer: TimerItem, status: TimerStatus) -> None:
"""Move a timer to a terminal status and notify subscribers."""
self._unschedule(timer.timer_id)
timer.status = status
timer.finishes_at = None
timer.paused_remaining = None
@@ -10,6 +10,7 @@ from homeassistant.const import ATTR_ENTITY_ID, CONF_OPTIONS, CONF_TARGET
from homeassistant.core import CALLBACK_TYPE, HomeAssistant, callback, split_entity_id
from homeassistant.exceptions import HomeAssistantError
from homeassistant.helpers import config_validation as cv
from homeassistant.helpers.event import async_track_state_change_event
from homeassistant.helpers.target import TargetEntityChangeTracker, TargetSelection
from homeassistant.helpers.trigger import (
Trigger,
@@ -62,13 +63,26 @@ class TimerEventListener(TargetEntityChangeTracker):
self._unsubscribe_listeners = []
component = self._hass.data[DATA_COMPONENT]
pending: set[str] = set()
for entity_id in tracked_entities:
if (entity := component.get_entity(entity_id)) is None:
pending.add(entity_id)
continue
self._unsubscribe_listeners.append(
entity.async_subscribe_updates(partial(self._listener, entity_id))
)
if pending:
# Entity registry creation fires before EntityPlatform hands the
# entity to the component, so a targeted list added after this
# trigger is not resolvable yet. Its first state write is, so retry
# then instead of dropping it for the lifetime of the automation.
self._unsubscribe_listeners.append(
async_track_state_change_event(
self._hass, pending, self._handle_target_update
)
)
@override
@callback
def _unsubscribe(self) -> None:
+6 -1
View File
@@ -56,7 +56,11 @@ async def test_generic_intents_exposed(hass: HomeAssistant) -> None:
async def test_timer_intents_require_timer_device(hass: HomeAssistant) -> None:
"""Test timer intents are not exposed without a timer-capable device."""
assert "intent__HassStartTimer" not in await _tool_names(hass)
names = await _tool_names(hass)
assert "intent__HassStartTimer" not in names
# Cancel-all resolves the requesting device's list too, so it cannot work
# without one either.
assert "intent__HassCancelAllTimers" not in names
async def _add_timer_device(hass: HomeAssistant) -> str:
@@ -90,6 +94,7 @@ async def test_timer_intents_offered_for_timer_device(hass: HomeAssistant) -> No
names = {tool.name for tool in result.tools}
assert "intent__HassStartTimer" in names
assert "intent__HassTimerStatus" in names
assert "intent__HassCancelAllTimers" in names
async def test_set_position_requires_exposed_cover(hass: HomeAssistant) -> None:
+55
View File
@@ -3,6 +3,7 @@
import asyncio
from collections.abc import Callable
from datetime import timedelta
from typing import Any
from unittest.mock import MagicMock
import pytest
@@ -12,6 +13,7 @@ from homeassistant.components.intent.timers import (
TimerNotFoundError,
TimersNotSupportedError,
_round_time,
async_device_supports_timer_intent,
async_device_supports_timers,
)
from homeassistant.components.local_timer_list import LocalTimerListEntity
@@ -20,6 +22,7 @@ from homeassistant.components.timer_list import (
DOMAIN as TIMER_LIST_DOMAIN,
TimerItem,
TimerListEntity,
TimerListEntityFeature,
TimerListEvent,
TimerListEventType,
TimerStatus,
@@ -1106,6 +1109,58 @@ async def test_async_device_supports_timers(hass: HomeAssistant) -> None:
assert async_device_supports_timers(hass, device_id)
@pytest.mark.parametrize(
("intent_type", "slots"),
[
pytest.param(
intent.INTENT_START_TIMER, {"minutes": {"value": 5}}, id="start_timer"
),
pytest.param(intent.INTENT_CANCEL_TIMER, {}, id="cancel_timer"),
pytest.param(intent.INTENT_CANCEL_ALL_TIMERS, {}, id="cancel_all_timers"),
pytest.param(intent.INTENT_PAUSE_TIMER, {}, id="pause_timer"),
pytest.param(intent.INTENT_UNPAUSE_TIMER, {}, id="unpause_timer"),
pytest.param(
intent.INTENT_INCREASE_TIMER, {"minutes": {"value": 1}}, id="increase_timer"
),
pytest.param(
intent.INTENT_DECREASE_TIMER, {"minutes": {"value": 1}}, id="decrease_timer"
),
],
)
async def test_intents_require_the_entity_feature(
hass: HomeAssistant, init_components, intent_type: str, slots: dict[str, Any]
) -> None:
"""Test intents report unsupported when the list lacks the needed feature."""
device_id = _make_timer_device_id(hass)
await _register_timer_device(hass, device_id)
entity = _get_timer_entity(hass, device_id)
entity._attr_supported_features = TimerListEntityFeature(0)
assert not async_device_supports_timer_intent(hass, device_id, intent_type)
with pytest.raises(TimersNotSupportedError):
await intent.async_handle(hass, "test", intent_type, slots, device_id=device_id)
async def test_timer_status_needs_no_feature(
hass: HomeAssistant, init_components
) -> None:
"""Test reading timers works on a list that advertises nothing."""
device_id = _make_timer_device_id(hass)
await _register_timer_device(hass, device_id)
entity = _get_timer_entity(hass, device_id)
entity._attr_supported_features = TimerListEntityFeature(0)
assert async_device_supports_timer_intent(
hass, device_id, intent.INTENT_TIMER_STATUS
)
result = await intent.async_handle(
hass, "test", intent.INTENT_TIMER_STATUS, {}, device_id=device_id
)
assert result.speech_slots["timers"] == []
async def test_cancel_all_timers(hass: HomeAssistant, init_components) -> None:
"""Test cancelling all timers."""
device_id = _make_timer_device_id(hass)
+53 -8
View File
@@ -168,14 +168,59 @@ async def test_add_and_subtract_time(
@pytest.mark.usefixtures("test_entity")
async def test_subtract_time_finishes_timer(hass: HomeAssistant) -> None:
@pytest.mark.parametrize(
"setup_services",
[
pytest.param([], id="active"),
pytest.param(["pause_timer"], id="paused"),
],
)
async def test_subtract_time_finishes_timer(
hass: HomeAssistant, setup_services: list[str]
) -> None:
"""Test subtracting more time than remaining finishes the timer immediately."""
timer_id = await _create_timer(hass, duration=60)
for service in setup_services:
await _call(hass, service, timer_id=timer_id)
await _call(hass, "subtract_time", timer_id=timer_id, duration={"seconds": 120})
assert hass.states.get(TEST_ENTITY_ID).state == "0"
assert (await _get_timers(hass))[0]["status"] == "finished"
timers = await _get_timers(hass)
assert timers[0]["status"] == "finished"
assert timers[0]["ended_at"] is not None
@pytest.mark.usefixtures("test_entity")
@pytest.mark.parametrize(
"service",
[
pytest.param("add_time", id="add_time"),
pytest.param("subtract_time", id="subtract_time"),
],
)
async def test_zero_duration_is_noop(
hass: HomeAssistant, hass_ws_client: WebSocketGenerator, service: str
) -> None:
"""Test a zero duration changes nothing and emits no event."""
timer_id = await _create_timer(hass, duration=60)
before = (await _get_timers(hass))[0]
client = await hass_ws_client(hass)
await client.send_json_auto_id(
{"type": "timer_list/item/subscribe", "entity_id": TEST_ENTITY_ID}
)
assert (await client.receive_json())["success"]
assert (await client.receive_json())["event"]["type"] == "timers"
await _call(hass, service, timer_id=timer_id, duration={"seconds": 0})
assert (await _get_timers(hass))[0]["finishes_at"] == before["finishes_at"]
# A change event would arrive before the reply to this round trip.
await client.send_json_auto_id(
{"type": "timer_list/item/list", "entity_id": TEST_ENTITY_ID}
)
assert (await client.receive_json())["success"]
@pytest.mark.usefixtures("test_entity")
@@ -192,19 +237,19 @@ async def test_cancel_timer_archives_timer(hass: HomeAssistant) -> None:
@pytest.mark.usefixtures("test_entity")
@pytest.mark.parametrize(
"paused",
"setup_services",
[
pytest.param(False, id="active"),
pytest.param(True, id="paused"),
pytest.param([], id="active"),
pytest.param(["pause_timer"], id="paused"),
],
)
async def test_finish_timer_archives_as_finished(
hass: HomeAssistant, paused: bool
hass: HomeAssistant, setup_services: list[str]
) -> None:
"""Test finishing a timer early archives it as finished."""
timer_id = await _create_timer(hass, duration=3600)
if paused:
await _call(hass, "pause_timer", timer_id=timer_id)
for service in setup_services:
await _call(hass, service, timer_id=timer_id)
await _call(hass, "finish_timer", timer_id=timer_id)
+28 -2
View File
@@ -40,7 +40,10 @@ async def setup_entity(hass: HomeAssistant) -> None:
async def _setup_automation(
hass: HomeAssistant, trigger_type: str, extra_data: dict[str, str] | None = None
hass: HomeAssistant,
trigger_type: str,
extra_data: dict[str, str] | None = None,
entity_id: str = TEST_ENTITY_ID,
) -> None:
"""Set up an automation for the given timer list trigger."""
assert await async_setup_component(
@@ -51,7 +54,7 @@ async def _setup_automation(
"triggers": [
{
CONF_PLATFORM: f"{DOMAIN}.{trigger_type}",
CONF_TARGET: {CONF_ENTITY_ID: TEST_ENTITY_ID},
CONF_TARGET: {CONF_ENTITY_ID: entity_id},
}
],
"action": {
@@ -211,6 +214,29 @@ async def test_timer_time_changed_trigger_reports_delta(
assert service_calls[0].data["delta"] == expected_delta
async def test_trigger_picks_up_entity_added_after_setup(
hass: HomeAssistant, service_calls: list[ServiceCall]
) -> None:
"""Test a timer list created after the automation still fires the trigger."""
late_entity_id = "timer_list.late"
await _setup_automation(hass, "timer_created", entity_id=late_entity_id)
entity = MockTimerListEntity(name="Late")
entity.entity_id = late_entity_id
await create_mock_platform(hass, [entity])
await hass.services.async_call(
DOMAIN,
"create_timer",
{"duration": {"seconds": 60}},
target={ATTR_ENTITY_ID: late_entity_id},
blocking=True,
)
assert len(service_calls) == 1
assert service_calls[0].data["entity_id"] == late_entity_id
async def test_trigger_options_supported(hass: HomeAssistant) -> None:
"""Test the timer list triggers do not advertise behavior or duration."""
for trigger_type in (