Refactor Sonos alarms and favorites updating (#55529)

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(self.alarms)
-        return iter(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:
+        if update_id and self.alarms.last_id < update_id:
-            speaker = self.hass.data[DATA_SONOS].discovered[alarm.zone.uid]
+            # Skip updates if latest query result is outdated or lagging
-            async_dispatcher_send(
+            return False
-                self.hass, SONOS_CREATE_ALARM, speaker, [alarm.alarm_id]
+
-            )
+        if (
-        async_dispatcher_send(self.hass, f"{SONOS_ALARMS_UPDATED}-{self.household_id}")
+            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

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:
+        updated = await self.hass.async_add_executor_job(
-            await self.hass.async_add_executor_job(self.update_cache, soco)
+            self.update_cache, soco, update_id
-        except (OSError, SoCoException) as err:
+        )
-            _LOGGER.warning("Error requesting favorites from %s: %s", soco, err)
+        if not updated:
-            return False
+            return
 
         async_dispatcher_send(
             self.hass, f"{SONOS_FAVORITES_UPDATED}-{self.household_id}"
         )
-        return True
 
-    def update_cache(self, soco: SoCo) -> None:
+    @callback
-        """Request new Sonos favorites from a speaker."""
+    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

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:
+    async def _async_setup(self) -> None:
-        """Create a polling debouncer in async context.
+        """Finish setup in async context."""
-
+        self.cache_update_lock = asyncio.Lock()
-        Used to ensure redundant poll requests from all speakers are coalesced.
-        """
         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)
+            _LOGGER.debug("Polling %s using %s", self.class_type, speaker.soco)
-            success = await self.async_update_entities(speaker.soco)
+            try:
-
+                await self.async_update_entities(speaker.soco)
-            if success:
+            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
+    async def async_update_entities(
-    def async_handle_event(self, event_id: str, soco: SoCo) -> None:
+        self, soco: SoCo, update_id: int | None = None
-        """Create a task to update from an event callback."""
+    ) -> None:
-        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:
         """Update the cache and update entities."""
         raise NotImplementedError()
 
-    def update_cache(self, soco: SoCo) -> Any:
+    def update_cache(self, soco: SoCo, update_id: int | None = None) -> bool:
-        """Update the cache of the household-level feature."""
+        """Update the cache of the household-level feature and return if cache has changed."""
         raise NotImplementedError()

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."],

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:

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)
 

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

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

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 = MagicMock()
-    music_library.get_sonos_favorites.return_value = []
+    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": "<Alarms>"
         '<Alarm ID="14" StartTime="07:00:00" Duration="02:00:00" Recurrence="DAILY" '
         'Enabled="1" RoomUUID="RINCON_test" ProgramURI="x-rincon-buzzer:0" '
         'ProgramMetaData="" PlayMode="SHUFFLE_NOREPEAT" Volume="25" '
         'IncludeLinkedZones="0"/>'
-        "</Alarms> "
+        "</Alarms>",
     }
     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": "<Alarms>"
         '<Alarm ID="14" StartTime="07:00:00" Duration="02:00:00" Recurrence="DAILY" '
         'Enabled="1" RoomUUID="RINCON_test" ProgramURI="x-rincon-buzzer:0" '
@@ -150,7 +153,7 @@ def alarm_clock_fixture_extended():
         'Recurrence="DAILY" Enabled="1" RoomUUID="RINCON_test" '
         'ProgramURI="x-rincon-buzzer:0" ProgramMetaData="" PlayMode="SHUFFLE_NOREPEAT" '
         'Volume="25" IncludeLinkedZones="0"/>'
-        "</Alarms> "
+        "</Alarms>",
     }
     return alarm_clock
 

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