diff --git a/homeassistant/components/sonos/alarms.py b/homeassistant/components/sonos/alarms.py index 215e4fede32..73149c6a286 100644 --- a/homeassistant/components/sonos/alarms.py +++ b/homeassistant/components/sonos/alarms.py @@ -6,9 +6,10 @@ import logging from typing import Any from soco import SoCo -from soco.alarms import Alarm, get_alarms +from soco.alarms import Alarm, Alarms from soco.exceptions import SoCoException +from homeassistant.core import callback from homeassistant.helpers.dispatcher import async_dispatcher_send from .const import DATA_SONOS, SONOS_ALARMS_UPDATED, SONOS_CREATE_ALARM @@ -23,48 +24,76 @@ class SonosAlarms(SonosHouseholdCoordinator): def __init__(self, *args: Any) -> None: """Initialize the data.""" super().__init__(*args) - self._alarms: dict[str, Alarm] = {} + self.alarms: Alarms = Alarms() + self.created_alarm_ids: set[str] = set() def __iter__(self) -> Iterator: """Return an iterator for the known alarms.""" - alarms = list(self._alarms.values()) - return iter(alarms) + return iter(self.alarms) def get(self, alarm_id: str) -> Alarm | None: """Get an Alarm instance.""" - return self._alarms.get(alarm_id) + return self.alarms.get(alarm_id) - async def async_update_entities(self, soco: SoCo) -> bool: + async def async_update_entities( + self, soco: SoCo, update_id: int | None = None + ) -> None: """Create and update alarms entities, return success.""" + updated = await self.hass.async_add_executor_job( + self.update_cache, soco, update_id + ) + if not updated: + return + + for alarm_id, alarm in self.alarms.alarms.items(): + if alarm_id in self.created_alarm_ids: + continue + speaker = self.hass.data[DATA_SONOS].discovered.get(alarm.zone.uid) + if speaker: + async_dispatcher_send( + self.hass, SONOS_CREATE_ALARM, speaker, [alarm_id] + ) + async_dispatcher_send(self.hass, f"{SONOS_ALARMS_UPDATED}-{self.household_id}") + + @callback + def async_handle_event(self, event_id: str, soco: SoCo) -> None: + """Create a task to update from an event callback.""" + _, event_id = event_id.split(":") + event_id = int(event_id) + self.hass.async_create_task(self.async_process_event(event_id, soco)) + + async def async_process_event(self, event_id: str, soco: SoCo) -> None: + """Process the event payload in an async lock and update entities.""" + async with self.cache_update_lock: + if event_id <= self.last_processed_event_id: + # Skip updates if this event_id has already been seen + return + await self.async_update_entities(soco, event_id) + + def update_cache(self, soco: SoCo, update_id: int | None = None) -> bool: + """Update cache of known alarms and return if cache has changed.""" try: - new_alarms = await self.hass.async_add_executor_job(self.update_cache, soco) + self.alarms.update(soco) except (OSError, SoCoException) as err: - _LOGGER.error("Could not refresh alarms using %s: %s", soco, err) + _LOGGER.error("Could not update alarms using %s: %s", soco, err) return False - for alarm in new_alarms: - speaker = self.hass.data[DATA_SONOS].discovered[alarm.zone.uid] - async_dispatcher_send( - self.hass, SONOS_CREATE_ALARM, speaker, [alarm.alarm_id] - ) - async_dispatcher_send(self.hass, f"{SONOS_ALARMS_UPDATED}-{self.household_id}") + if update_id and self.alarms.last_id < update_id: + # Skip updates if latest query result is outdated or lagging + return False + + if ( + self.last_processed_event_id + and self.alarms.last_id <= self.last_processed_event_id + ): + # Skip updates already processed + return False + + _LOGGER.debug( + "Updating processed event %s from %s (was %s)", + self.alarms.last_id, + soco, + self.last_processed_event_id, + ) + self.last_processed_event_id = self.alarms.last_id return True - - def update_cache(self, soco: SoCo) -> set[Alarm]: - """Populate cache of known alarms. - - Prune deleted alarms and return new alarms. - """ - soco_alarms = get_alarms(soco) - new_alarms = set() - - for alarm in soco_alarms: - if alarm.alarm_id not in self._alarms: - new_alarms.add(alarm) - self._alarms[alarm.alarm_id] = alarm - - for alarm_id, alarm in list(self._alarms.items()): - if alarm not in soco_alarms: - self._alarms.pop(alarm_id) - - return new_alarms diff --git a/homeassistant/components/sonos/favorites.py b/homeassistant/components/sonos/favorites.py index 9695265b24d..adf31b0f507 100644 --- a/homeassistant/components/sonos/favorites.py +++ b/homeassistant/components/sonos/favorites.py @@ -3,12 +3,14 @@ from __future__ import annotations from collections.abc import Iterator import logging +import re from typing import Any from soco import SoCo from soco.data_structures import DidlFavorite from soco.exceptions import SoCoException +from homeassistant.core import callback from homeassistant.helpers.dispatcher import async_dispatcher_send from .const import SONOS_FAVORITES_UPDATED @@ -24,30 +26,87 @@ class SonosFavorites(SonosHouseholdCoordinator): """Initialize the data.""" super().__init__(*args) self._favorites: list[DidlFavorite] = [] + self.last_polled_ids: dict[str, int] = {} def __iter__(self) -> Iterator: """Return an iterator for the known favorites.""" favorites = self._favorites.copy() return iter(favorites) - async def async_update_entities(self, soco: SoCo) -> bool: + async def async_update_entities( + self, soco: SoCo, update_id: int | None = None + ) -> None: """Update the cache and update entities.""" - try: - await self.hass.async_add_executor_job(self.update_cache, soco) - except (OSError, SoCoException) as err: - _LOGGER.warning("Error requesting favorites from %s: %s", soco, err) - return False + updated = await self.hass.async_add_executor_job( + self.update_cache, soco, update_id + ) + if not updated: + return async_dispatcher_send( self.hass, f"{SONOS_FAVORITES_UPDATED}-{self.household_id}" ) - return True - def update_cache(self, soco: SoCo) -> None: - """Request new Sonos favorites from a speaker.""" + @callback + def async_handle_event(self, event_id: str, container_ids: str, soco: SoCo) -> None: + """Create a task to update from an event callback.""" + if not (match := re.search(r"FV:2,(\d+)", container_ids)): + return + + container_id = int(match.groups()[0]) + event_id = int(event_id.split(",")[-1]) + + self.hass.async_create_task( + self.async_process_event(event_id, container_id, soco) + ) + + async def async_process_event( + self, event_id: int, container_id: int, soco: SoCo + ) -> None: + """Process the event payload in an async lock and update entities.""" + async with self.cache_update_lock: + last_poll_id = self.last_polled_ids.get(soco.uid) + if ( + self.last_processed_event_id + and event_id <= self.last_processed_event_id + ): + # Skip updates if this event_id has already been seen + if not last_poll_id: + self.last_polled_ids[soco.uid] = container_id + return + + if last_poll_id and container_id <= last_poll_id: + return + + _LOGGER.debug( + "New favorites event %s from %s (was %s)", + event_id, + soco, + self.last_processed_event_id, + ) + self.last_processed_event_id = event_id + await self.async_update_entities(soco, container_id) + + def update_cache(self, soco: SoCo, update_id: int | None = None) -> bool: + """Update cache of known favorites and return if cache has changed.""" new_favorites = soco.music_library.get_sonos_favorites() - self._favorites = [] + # Polled update_id values do not match event_id values + # Each speaker can return a different polled update_id + last_poll_id = self.last_polled_ids.get(soco.uid) + if last_poll_id and new_favorites.update_id <= last_poll_id: + # Skip updates already processed + return False + self.last_polled_ids[soco.uid] = new_favorites.update_id + + _LOGGER.debug( + "Processing favorites update_id %s for %s (was: %s)", + new_favorites.update_id, + soco, + last_poll_id, + ) + + self._favorites = [] for fav in new_favorites: try: # exclude non-playable favorites with no linked resources @@ -58,7 +117,9 @@ class SonosFavorites(SonosHouseholdCoordinator): _LOGGER.error("Unhandled favorite '%s': %s", fav.title, ex) _LOGGER.debug( - "Cached %s favorites for household %s", + "Cached %s favorites for household %s using %s", len(self._favorites), self.household_id, + soco, ) + return True diff --git a/homeassistant/components/sonos/household_coordinator.py b/homeassistant/components/sonos/household_coordinator.py index da964e93984..f233b338279 100644 --- a/homeassistant/components/sonos/household_coordinator.py +++ b/homeassistant/components/sonos/household_coordinator.py @@ -1,14 +1,14 @@ """Class representing a Sonos household storage helper.""" from __future__ import annotations -from collections import deque +import asyncio from collections.abc import Callable, Coroutine import logging -from typing import Any from soco import SoCo +from soco.exceptions import SoCoException -from homeassistant.core import HomeAssistant, callback +from homeassistant.core import HomeAssistant from homeassistant.helpers.debounce import Debouncer from .const import DATA_SONOS @@ -23,19 +23,18 @@ class SonosHouseholdCoordinator: """Initialize the data.""" self.hass = hass self.household_id = household_id - self._processed_events = deque(maxlen=5) self.async_poll: Callable[[], Coroutine[None, None, None]] | None = None + self.last_processed_event_id: int | None = None + self.cache_update_lock: asyncio.Lock | None = None def setup(self, soco: SoCo) -> None: """Set up the SonosAlarm instance.""" self.update_cache(soco) - self.hass.add_job(self._async_create_polling_debouncer) + self.hass.add_job(self._async_setup) - async def _async_create_polling_debouncer(self) -> None: - """Create a polling debouncer in async context. - - Used to ensure redundant poll requests from all speakers are coalesced. - """ + async def _async_setup(self) -> None: + """Finish setup in async context.""" + self.cache_update_lock = asyncio.Lock() self.async_poll = Debouncer( self.hass, _LOGGER, @@ -44,31 +43,37 @@ class SonosHouseholdCoordinator: function=self._async_poll, ).async_call + @property + def class_type(self) -> str: + """Return the class type of this instance.""" + return type(self).__name__ + async def _async_poll(self) -> None: """Poll any known speaker.""" discovered = self.hass.data[DATA_SONOS].discovered for uid, speaker in discovered.items(): - _LOGGER.debug("Updating %s using %s", type(self).__name__, speaker.soco) - success = await self.async_update_entities(speaker.soco) - - if success: + _LOGGER.debug("Polling %s using %s", self.class_type, speaker.soco) + try: + await self.async_update_entities(speaker.soco) + except (OSError, SoCoException) as err: + _LOGGER.error( + "Could not refresh %s using %s: %s", + self.class_type, + speaker.soco, + err, + ) + else: # Prefer this SoCo instance next update discovered.move_to_end(uid, last=False) break - @callback - def async_handle_event(self, event_id: str, soco: SoCo) -> None: - """Create a task to update from an event callback.""" - if event_id in self._processed_events: - return - self._processed_events.append(event_id) - self.hass.async_create_task(self.async_update_entities(soco)) - - async def async_update_entities(self, soco: SoCo) -> bool: + async def async_update_entities( + self, soco: SoCo, update_id: int | None = None + ) -> None: """Update the cache and update entities.""" raise NotImplementedError() - def update_cache(self, soco: SoCo) -> Any: - """Update the cache of the household-level feature.""" + def update_cache(self, soco: SoCo, update_id: int | None = None) -> bool: + """Update the cache of the household-level feature and return if cache has changed.""" raise NotImplementedError() diff --git a/homeassistant/components/sonos/manifest.json b/homeassistant/components/sonos/manifest.json index d9c2a2cc6c9..249a6d4cc00 100644 --- a/homeassistant/components/sonos/manifest.json +++ b/homeassistant/components/sonos/manifest.json @@ -3,7 +3,7 @@ "name": "Sonos", "config_flow": true, "documentation": "https://www.home-assistant.io/integrations/sonos", - "requirements": ["soco==0.23.3"], + "requirements": ["soco==0.24.0"], "dependencies": ["ssdp"], "after_dependencies": ["plex", "zeroconf"], "zeroconf": ["_sonos._tcp.local."], diff --git a/homeassistant/components/sonos/speaker.py b/homeassistant/components/sonos/speaker.py index b47d1444384..ea49175b665 100644 --- a/homeassistant/components/sonos/speaker.py +++ b/homeassistant/components/sonos/speaker.py @@ -451,7 +451,9 @@ class SonosSpeaker: """Add the soco instance associated with the event to the callback.""" if not (event_id := event.variables.get("favorites_update_id")): return - self.favorites.async_handle_event(event_id, self.soco) + if not (container_ids := event.variables.get("container_update_i_ds")): + return + self.favorites.async_handle_event(event_id, container_ids, self.soco) @callback def async_dispatch_media_update(self, event: SonosEvent) -> None: diff --git a/homeassistant/components/sonos/switch.py b/homeassistant/components/sonos/switch.py index 482780453af..cee60cbbafa 100644 --- a/homeassistant/components/sonos/switch.py +++ b/homeassistant/components/sonos/switch.py @@ -37,8 +37,14 @@ async def async_setup_entry(hass, config_entry, async_add_entities): async def _async_create_entity(speaker: SonosSpeaker, alarm_ids: list[str]) -> None: entities = [] + created_alarms = ( + hass.data[DATA_SONOS].alarms[speaker.household_id].created_alarm_ids + ) for alarm_id in alarm_ids: + if alarm_id in created_alarms: + continue _LOGGER.debug("Creating alarm %s on %s", alarm_id, speaker.zone_name) + created_alarms.add(alarm_id) entities.append(SonosAlarmEntity(alarm_id, speaker)) async_add_entities(entities) diff --git a/requirements_all.txt b/requirements_all.txt index 2af2fcabfe0..b5942b7f63a 100644 --- a/requirements_all.txt +++ b/requirements_all.txt @@ -2187,7 +2187,7 @@ smhi-pkg==1.0.15 snapcast==2.1.3 # homeassistant.components.sonos -soco==0.23.3 +soco==0.24.0 # homeassistant.components.solaredge_local solaredge-local==0.2.0 diff --git a/requirements_test_all.txt b/requirements_test_all.txt index db31304efce..d5b2f9ddfc8 100644 --- a/requirements_test_all.txt +++ b/requirements_test_all.txt @@ -1239,7 +1239,7 @@ smarthab==0.21 smhi-pkg==1.0.15 # homeassistant.components.sonos -soco==0.23.3 +soco==0.24.0 # homeassistant.components.solaredge solaredge==0.0.2 diff --git a/tests/components/sonos/conftest.py b/tests/components/sonos/conftest.py index d970c8923ef..f650c6e8fef 100644 --- a/tests/components/sonos/conftest.py +++ b/tests/components/sonos/conftest.py @@ -39,6 +39,7 @@ class SonosMockEvent: base, count = self.variables[var_name].split(":") newcount = int(count) + 1 self.variables[var_name] = ":".join([base, str(newcount)]) + return self.variables[var_name] @pytest.fixture(name="config_entry") @@ -114,8 +115,8 @@ def config_fixture(): @pytest.fixture(name="music_library") def music_library_fixture(): """Create music_library fixture.""" - music_library = Mock() - music_library.get_sonos_favorites.return_value = [] + music_library = MagicMock() + music_library.get_sonos_favorites.return_value.update_id = 1 return music_library @@ -125,12 +126,13 @@ def alarm_clock_fixture(): alarm_clock = SonosMockService("AlarmClock") alarm_clock.ListAlarms = Mock() alarm_clock.ListAlarms.return_value = { + "CurrentAlarmListVersion": "RINCON_test:14", "CurrentAlarmList": "" '' - " " + "", } return alarm_clock @@ -141,6 +143,7 @@ def alarm_clock_fixture_extended(): alarm_clock = SonosMockService("AlarmClock") alarm_clock.ListAlarms = Mock() alarm_clock.ListAlarms.return_value = { + "CurrentAlarmListVersion": "RINCON_test:15", "CurrentAlarmList": "" '' - " " + "", } return alarm_clock diff --git a/tests/components/sonos/test_switch.py b/tests/components/sonos/test_switch.py index f684a8f351e..d71d403fd8a 100644 --- a/tests/components/sonos/test_switch.py +++ b/tests/components/sonos/test_switch.py @@ -69,13 +69,17 @@ async def test_alarm_create_delete( alarm_clock.ListAlarms.return_value = two_alarms + alarm_event.variables["alarm_list_version"] = two_alarms["CurrentAlarmListVersion"] + sub_callback(event=alarm_event) await hass.async_block_till_done() assert "switch.sonos_alarm_14" in entity_registry.entities assert "switch.sonos_alarm_15" in entity_registry.entities - alarm_event.increment_variable("alarm_list_version") + one_alarm["CurrentAlarmListVersion"] = alarm_event.increment_variable( + "alarm_list_version" + ) alarm_clock.ListAlarms.return_value = one_alarm