diff --git a/TIMER_LIST_POC_TRADEOFFS.md b/TIMER_LIST_POC_TRADEOFFS.md deleted file mode 100644 index 3c6fd8746b87..000000000000 --- a/TIMER_LIST_POC_TRADEOFFS.md +++ /dev/null @@ -1,68 +0,0 @@ -# Timer list POC — known trade-offs - -This branch is a proof of concept that replaces `intent`'s hand-rolled, -in-memory voice-timer storage with real `timer_list` entities (one -auto-created per `assist_satellite` device). It intentionally cuts scope in -several places so the PR stays reviewable. These are called out here so -reviewers and whoever picks up follow-on work don't have to rediscover them. - -## Dropped / deferred features - -- **`conversation_command` timers are gone.** The old system supported - device-less timers that ran a delayed conversation command instead of - notifying a device. This doesn't fit the entity-per-device model and needs - its own design; it was removed rather than shimmed in. -- **`mobile_app` timer support is broken.** `mobile_app` registers a device - for timer push-notifications but has no `AssistSatelliteEntity`, so it - never gets an auto-created `timer_list` entity. Starting a timer for a - mobile_app device now raises `TimersNotSupportedError`. Its tests are - marked `xfail` with the reason recorded inline - (`tests/components/mobile_app/test_timers.py`). Fix: give mobile_app - devices a `timer_list` entity the same way `assist_satellite` does. -- **No `cancel_all_timers` / `clear_finished_timers` services.** Removed from - `timer_list`/`local_timer_list` earlier in this branch. Archived timers can - only be removed individually or via the automatic retention limit below. - -## Behavior changes from the old voice-timer system - -- **Finish action is always "archive."** Timers no longer support - remove-on-finish or auto-restart; every finished/cancelled timer is kept - (as `finished`/`cancelled`) until removed. -- **Archived timers are capped at 10 per entity**, oldest evicted first, with - no way to configure the limit. -- **`created_seconds` no longer grows.** In the old system, adding time to a - timer past its original length grew the value reported to satellites as - the timer's nominal duration. The new `TimerItem.duration` is fixed at - creation, so `TimerInfo.created_seconds` (and the `total_seconds` field - sent to esphome/wyoming satellites) reflects the *original* duration even - after time is added. -- **Removing more time than remains skips the "updated" event.** The timer - transitions straight to `finished`, matching `LocalTimerListEntity`'s - existing behavior, rather than emitting an intermediate zero-duration - update before finishing (as the old system did). - -## Design shortcuts - -- **Auto-created timer list entities aren't linked to their device.** They - show up as standalone entities named `"{device name} Timers"` rather than - nested under the satellite's device in the UI (no `DeviceInfo` set). Purely - cosmetic; the lookup logic doesn't depend on device linkage and this can be - added later. -- **Auto-created vs. user-created entities are told apart implicitly.** - Both live in the `timer_list` domain; the code distinguishes "this is a - satellite's auto-created list" from "this is a user's `local_timer_list` - helper" purely by the entity registry's `platform` field (`"timer_list"` - vs. `"local_timer_list"`). There's no explicit flag — if that convention - ever changes, voice matching (`_all_timer_infos` in - `homeassistant/components/intent/timers.py`) silently stops seeing the - right entities. -- **No cleanup of orphaned timer list entities.** If a satellite device is - removed and re-added with a new device ID, its old `timer_list` entity - (keyed by the old device ID) is never cleaned up. -- **New always-on dependency edges.** `intent` (a `system`-type integration, - always loaded) now depends on `timer_list`, so `timer_list.async_setup` - (services + websocket commands, no entities) runs on every Home Assistant - install regardless of whether voice timers are used. `assist_satellite` - now depends on `local_timer_list`. -- **No user-facing control over the auto-created lists** — no config entry, - options flow, or way to opt a satellite out of getting one. diff --git a/homeassistant/components/timer_list/__init__.py b/homeassistant/components/timer_list/__init__.py index 9753b9bfeebb..eaa4a0cefbeb 100644 --- a/homeassistant/components/timer_list/__init__.py +++ b/homeassistant/components/timer_list/__init__.py @@ -34,9 +34,14 @@ from homeassistant.helpers.typing import ConfigType from homeassistant.util import dt as dt_util from .const import ( + ATTR_CREATED_AT, ATTR_DURATION, + ATTR_FINISHED_AT, + ATTR_FINISHES_AT, + ATTR_REMAINING, ATTR_STATUS, ATTR_TIMER_ID, + ATTR_TIMERS, DATA_COMPONENT, DOMAIN, TimerListEntityFeature, @@ -104,11 +109,11 @@ def timer_to_dict(item: TimerItem, now: datetime) -> dict[str, Any]: ATTR_TIMER_ID: item.timer_id, ATTR_NAME: item.name, ATTR_STATUS: item.status.value, - "duration": item.duration.total_seconds(), - "created_at": item.created_at.isoformat(), - "finishes_at": item.finishes_at.isoformat() if item.finishes_at else None, - "finished_at": item.finished_at.isoformat() if item.finished_at else None, - "remaining": item.remaining_at(now).total_seconds(), + ATTR_DURATION: item.duration.total_seconds(), + ATTR_CREATED_AT: item.created_at.isoformat(), + ATTR_FINISHES_AT: item.finishes_at.isoformat() if item.finishes_at else None, + ATTR_FINISHED_AT: item.finished_at.isoformat() if item.finished_at else None, + ATTR_REMAINING: item.remaining_at(now).total_seconds(), } @@ -332,7 +337,7 @@ async def _async_get_timers( now = dt_util.utcnow() statuses: list[TimerStatus] | None = call.data.get(ATTR_STATUS) return { - "timers": [ + ATTR_TIMERS: [ timer_to_dict(timer, now) for timer in entity.timers if not statuses or timer.status in statuses @@ -383,7 +388,7 @@ async def websocket_handle_subscribe( msg["id"], { "type": "timers", - "timers": [timer_to_dict(timer, now) for timer in entity.timers], + ATTR_TIMERS: [timer_to_dict(timer, now) for timer in entity.timers], }, ) ) @@ -412,5 +417,5 @@ async def websocket_handle_list( now = dt_util.utcnow() connection.send_result( msg["id"], - {"timers": [timer_to_dict(timer, now) for timer in entity.timers]}, + {ATTR_TIMERS: [timer_to_dict(timer, now) for timer in entity.timers]}, ) diff --git a/script/hassfest/manifest.py b/script/hassfest/manifest.py index 7ab3d7786a93..fb23e8b3c957 100644 --- a/script/hassfest/manifest.py +++ b/script/hassfest/manifest.py @@ -125,7 +125,6 @@ NO_IOT_CLASS = [ "tag", "temperature", "timer", - "timer_list", "trace", "vibration", "web_rtc", diff --git a/script/hassfest/quality_scale.py b/script/hassfest/quality_scale.py index 420d1c366d97..aa765800df02 100644 --- a/script/hassfest/quality_scale.py +++ b/script/hassfest/quality_scale.py @@ -2074,7 +2074,6 @@ NO_QUALITY_SCALE = [ "tag", "temperature", "timer", - "timer_list", "local_timer_list", "trace", "usage_prediction", diff --git a/tests/components/timer_list/__init__.py b/tests/components/timer_list/__init__.py index 35ce9929e4c9..a4149ab6dd30 100644 --- a/tests/components/timer_list/__init__.py +++ b/tests/components/timer_list/__init__.py @@ -1,212 +1,36 @@ """Tests for the Timer list integration.""" -from datetime import datetime, timedelta -from functools import partial -from typing import override - from homeassistant.components.timer_list import ( DOMAIN, - TimerItem, + InMemoryTimerListEntity, TimerListEntity, - TimerListEntityFeature, - TimerListEventType, - TimerStatus, ) from homeassistant.config_entries import ConfigEntry, ConfigFlow -from homeassistant.core import CALLBACK_TYPE, HomeAssistant, callback -from homeassistant.exceptions import ServiceValidationError +from homeassistant.core import HomeAssistant from homeassistant.helpers.entity_platform import AddConfigEntryEntitiesCallback -from homeassistant.helpers.event import async_track_point_in_utc_time -from homeassistant.util import dt as dt_util, ulid as ulid_util +from homeassistant.util import ulid as ulid_util from tests.common import MockConfigEntry, MockPlatform, mock_platform TEST_DOMAIN = "test" -ALL_FEATURES = ( - TimerListEntityFeature.START_TIMER - | TimerListEntityFeature.PAUSE_TIMER - | TimerListEntityFeature.CANCEL_TIMER - | TimerListEntityFeature.ADD_TIME -) - -_FINISHED_STATUSES = (TimerStatus.FINISHED, TimerStatus.CANCELLED) -MAX_ARCHIVED_TIMERS = 10 - class MockFlow(ConfigFlow): """Test flow.""" -class MockTimerListEntity(TimerListEntity): +class MockTimerListEntity(InMemoryTimerListEntity): """Test timer list entity. - Reimplements the same in-memory storage and scheduling as the - ``local_timer_list`` platform, so the generic services, websocket API, - and triggers can be exercised without depending on that integration. + Subclasses the reference ``InMemoryTimerListEntity`` so the generic + services, websocket API, and triggers are exercised against the real + storage/scheduling logic without depending on the ``local_timer_list`` + integration. """ - _attr_supported_features = ALL_FEATURES - def __init__(self, name: str = "Timers") -> None: """Initialize entity.""" - super().__init__() - self._attr_name = name - self._timers: dict[str, TimerItem] = {} - self._cancel_callbacks: dict[str, CALLBACK_TYPE] = {} - - @property - @override - def timers(self) -> list[TimerItem]: - """Return the timers in the list.""" - return list(self._timers.values()) - - @override - async def async_start_timer(self, *, name: str | None, duration: timedelta) -> str: - """Create and start a new timer, returning its id.""" - now = dt_util.utcnow() - timer_id = ulid_util.ulid_now() - timer = TimerItem( - timer_id=timer_id, - name=name, - status=TimerStatus.ACTIVE, - duration=duration, - created_at=now, - finishes_at=now + duration, - ) - self._timers[timer_id] = timer - self._schedule(timer) - self._notify(TimerListEventType.STARTED, timer) - return timer_id - - @override - async def async_pause_timer(self, timer_id: str) -> None: - """Pause an active timer.""" - timer = self._get_timer(timer_id) - if timer.status != TimerStatus.ACTIVE or timer.finishes_at is None: - return - timer.remaining = max(timedelta(0), timer.finishes_at - dt_util.utcnow()) - timer.finishes_at = None - timer.status = TimerStatus.PAUSED - self._unschedule(timer_id) - self._notify(TimerListEventType.UPDATED, timer) - - @override - async def async_unpause_timer(self, timer_id: str) -> None: - """Resume a paused timer.""" - timer = self._get_timer(timer_id) - if timer.status != TimerStatus.PAUSED or timer.remaining is None: - return - timer.finishes_at = dt_util.utcnow() + timer.remaining - timer.remaining = None - timer.status = TimerStatus.ACTIVE - self._schedule(timer) - self._notify(TimerListEventType.UPDATED, timer) - - @override - async def async_cancel_timer(self, timer_id: str) -> None: - """Cancel a timer, archiving it in the ``cancelled`` state.""" - timer = self._get_timer(timer_id) - self._unschedule(timer_id) - timer.status = TimerStatus.CANCELLED - timer.finishes_at = None - timer.remaining = None - timer.finished_at = dt_util.utcnow() - self._notify(TimerListEventType.CANCELLED, timer) - self._enforce_archive_limit() - - @override - async def async_add_time(self, timer_id: str, duration: timedelta) -> None: - """Add (or, with a negative duration, remove) time on a timer.""" - timer = self._get_timer(timer_id) - if timer.status == TimerStatus.ACTIVE and timer.finishes_at is not None: - now = dt_util.utcnow() - finishes_at = timer.finishes_at + duration - if finishes_at <= now: - self._unschedule(timer_id) - self._async_timer_finished(timer_id, now) - return - timer.finishes_at = finishes_at - self._schedule(timer) - elif timer.status == TimerStatus.PAUSED and timer.remaining is not None: - timer.remaining = max(timedelta(0), timer.remaining + duration) - else: - return - self._notify(TimerListEventType.UPDATED, timer) - - @override - async def async_remove_timer(self, timer_id: str) -> None: - """Remove a timer from the list regardless of its status.""" - timer = self._get_timer(timer_id) - self._unschedule(timer_id) - del self._timers[timer_id] - self._notify(TimerListEventType.REMOVED, timer) - - def _get_timer(self, timer_id: str) -> TimerItem: - """Return a timer by id or raise if it does not exist.""" - if (timer := self._timers.get(timer_id)) is None: - raise ServiceValidationError( - translation_domain=DOMAIN, - translation_key="timer_not_found", - translation_placeholders={"timer_id": timer_id}, - ) - return timer - - @callback - def _schedule(self, timer: TimerItem) -> None: - """Schedule (or reschedule) the finish callback for a timer.""" - self._unschedule(timer.timer_id) - assert timer.finishes_at is not None - self._cancel_callbacks[timer.timer_id] = async_track_point_in_utc_time( - self.hass, - partial(self._async_timer_finished, timer.timer_id), - timer.finishes_at, - ) - - @callback - def _unschedule(self, timer_id: str) -> None: - """Cancel a pending finish callback, if any.""" - if cancel := self._cancel_callbacks.pop(timer_id, None): - cancel() - - @callback - def _async_timer_finished(self, timer_id: str, now: datetime) -> None: - """Handle a timer reaching its finish time, archiving it.""" - self._cancel_callbacks.pop(timer_id, None) - if (timer := self._timers.get(timer_id)) is None: - return - - timer.status = TimerStatus.FINISHED - timer.finishes_at = None - timer.remaining = None - timer.finished_at = dt_util.utcnow() - self._notify(TimerListEventType.FINISHED, timer) - self._enforce_archive_limit() - - @callback - def _enforce_archive_limit(self) -> None: - """Evict the oldest archived timers beyond ``MAX_ARCHIVED_TIMERS``.""" - archived = sorted( - ( - timer - for timer in self._timers.values() - if timer.status in _FINISHED_STATUSES - ), - key=lambda timer: timer.finished_at or dt_util.utcnow(), - ) - excess = len(archived) - MAX_ARCHIVED_TIMERS - if excess <= 0: - return - for timer in archived[:excess]: - del self._timers[timer.timer_id] - self._notify(TimerListEventType.REMOVED, timer) - - @override - async def async_will_remove_from_hass(self) -> None: - """Cancel all pending finish callbacks.""" - for cancel in self._cancel_callbacks.values(): - cancel() - self._cancel_callbacks.clear() + super().__init__(name=name, unique_id=ulid_util.ulid_now()) async def create_mock_platform( diff --git a/tests/components/timer_list/test_trigger.py b/tests/components/timer_list/test_trigger.py index c5b1b67df6a0..dbf9776f3293 100644 --- a/tests/components/timer_list/test_trigger.py +++ b/tests/components/timer_list/test_trigger.py @@ -136,6 +136,28 @@ async def test_timer_cancelled_trigger( assert service_calls[0].data["status"] == "cancelled" +async def test_timer_updated_trigger( + hass: HomeAssistant, service_calls: list[ServiceCall] +) -> None: + """Test the timer_updated trigger fires when a timer is paused.""" + await _setup_automation(hass, "timer_updated") + timer_id = await _start_timer(hass) + await hass.async_block_till_done() + assert len(service_calls) == 0 + + await hass.services.async_call( + DOMAIN, + "pause_timer", + {"timer_id": timer_id}, + target={ATTR_ENTITY_ID: TEST_ENTITY_ID}, + blocking=True, + ) + + assert len(service_calls) == 1 + assert service_calls[0].data["timer_id"] == timer_id + assert service_calls[0].data["status"] == "paused" + + async def test_trigger_options_supported(hass: HomeAssistant) -> None: """Test the timer list triggers do not advertise behavior or duration.""" for trigger_type in (