Centralize Sonos subscription logic (#51172)

* Centralize Sonos subscription logic

* Clean up mocked Sonos Service instances, use subscription callback

* Use existing mocked attributes

* Use event dispatcher dict, move methods together, make update_alarms sync

* Create dispatcher dict once

homeassistant/components/sonos/favorites.py

@@ -41,7 +41,9 @@ class SonosFavorites:
         Updated favorites are not always immediately available.
 
         """
-        event_id = event.variables["favorites_update_id"]
+        if not (event_id := event.variables.get("favorites_update_id")):
+            return
+
         if not self._event_version:
             self._event_version = event_id
             return

homeassistant/components/sonos/speaker.py

@@ -61,6 +61,14 @@ EVENT_CHARGING = {
     "CHARGING": True,
     "NOT_CHARGING": False,
 }
+SUBSCRIPTION_SERVICES = [
+    "alarmClock",
+    "avTransport",
+    "contentDirectory",
+    "deviceProperties",
+    "renderingControl",
+    "zoneGroupTopology",
+]
 UNAVAILABLE_VALUES = {"", "NOT_IMPLEMENTED", None}
 
 
@@ -140,8 +148,12 @@ class SonosSpeaker:
 
         self.is_first_poll: bool = True
         self._is_ready: bool = False
+
+        # Subscriptions and events
         self._subscriptions: list[SubscriptionBase] = []
         self._resubscription_lock: asyncio.Lock | None = None
+        self._event_dispatchers: dict[str, Callable] = {}
+
         self._poll_timer: Callable | None = None
         self._seen_timer: Callable | None = None
         self._platforms_ready: set[str] = set()
@@ -209,6 +221,15 @@ class SonosSpeaker:
         else:
             self._platforms_ready.add(SWITCH_DOMAIN)
 
+        self._event_dispatchers = {
+            "AlarmClock": self.async_dispatch_alarms,
+            "AVTransport": self.async_dispatch_media_update,
+            "ContentDirectory": self.favorites.async_delayed_update,
+            "DeviceProperties": self.async_dispatch_device_properties,
+            "RenderingControl": self.async_update_volume,
+            "ZoneGroupTopology": self.async_update_groups,
+        }
+
         dispatcher_send(self.hass, SONOS_CREATE_MEDIA_PLAYER, self)
 
     async def async_handle_new_entity(self, entity_type: str) -> None:
@@ -238,6 +259,9 @@ class SonosSpeaker:
         """Return whether this speaker is available."""
         return self._seen_timer is not None
 
+    #
+    # Subscription handling and event dispatchers
+    #
     async def async_subscribe(self) -> bool:
         """Initiate event subscriptions."""
         _LOGGER.debug("Creating subscriptions for %s", self.zone_name)
@@ -250,18 +274,11 @@ class SonosSpeaker:
                     f"when existing subscriptions exist: {self._subscriptions}"
                 )
 
-            await asyncio.gather(
+            subscriptions = [
-                self._subscribe(self.soco.avTransport, self.async_update_media),
+                self._subscribe(getattr(self.soco, service), self.async_dispatch_event)
-                self._subscribe(self.soco.renderingControl, self.async_update_volume),
+                for service in SUBSCRIPTION_SERVICES
-                self._subscribe(self.soco.contentDirectory, self.async_update_content),
+            ]
-                self._subscribe(
+            await asyncio.gather(*subscriptions)
-                    self.soco.zoneGroupTopology, self.async_dispatch_groups
-                ),
-                self._subscribe(
-                    self.soco.deviceProperties, self.async_dispatch_properties
-                ),
-                self._subscribe(self.soco.alarmClock, self.async_dispatch_alarms),
-            )
             return True
         except SoCoException as ex:
             _LOGGER.warning("Could not connect %s: %s", self.zone_name, ex)
@@ -279,26 +296,58 @@ class SonosSpeaker:
         self._subscriptions.append(subscription)
 
     @callback
-    def async_dispatch_properties(self, event: SonosEvent | None = None) -> None:
+    def async_dispatch_event(self, event: SonosEvent) -> None:
-        """Update properties from event."""
+        """Handle callback event and route as needed."""
-        self.hass.async_create_task(self.async_update_device_properties(event))
+        if self._poll_timer:
-
-    @callback
-    def async_dispatch_alarms(self, event: SonosEvent | None = None) -> None:
-        """Update alarms from event."""
-        self.hass.async_create_task(self.async_update_alarms(event))
-
-    @callback
-    def async_dispatch_groups(self, event: SonosEvent | None = None) -> None:
-        """Update groups from event."""
-        if event and self._poll_timer:
             _LOGGER.debug(
                 "Received event, cancelling poll timer for %s", self.zone_name
             )
             self._poll_timer()
             self._poll_timer = None
 
-        self.async_update_groups(event)
+        dispatcher = self._event_dispatchers[event.service.service_type]
+        dispatcher(event)
+
+    @callback
+    def async_dispatch_alarms(self, event: SonosEvent) -> None:
+        """Create a task to update alarms from an event."""
+        self.hass.async_add_executor_job(self.update_alarms)
+
+    @callback
+    def async_dispatch_device_properties(self, event: SonosEvent) -> None:
+        """Update device properties from an event."""
+        self.hass.async_create_task(self.async_update_device_properties(event))
+
+    async def async_update_device_properties(self, event: SonosEvent) -> None:
+        """Update device properties from an event."""
+        if (more_info := event.variables.get("more_info")) is not None:
+            battery_dict = dict(x.split(":") for x in more_info.split(","))
+            await self.async_update_battery_info(battery_dict)
+        self.async_write_entity_states()
+
+    @callback
+    def async_dispatch_media_update(self, event: SonosEvent) -> None:
+        """Update information about currently playing media from an event."""
+        self.hass.async_add_executor_job(self.update_media, event)
+
+    @callback
+    def async_update_volume(self, event: SonosEvent) -> None:
+        """Update information about currently volume settings."""
+        variables = event.variables
+
+        if "volume" in variables:
+            self.volume = int(variables["volume"]["Master"])
+
+        if "mute" in variables:
+            self.muted = variables["mute"]["Master"] == "1"
+
+        if "night_mode" in variables:
+            self.night_mode = variables["night_mode"] == "1"
+
+        if "dialog_level" in variables:
+            self.dialog_mode = variables["dialog_level"] == "1"
+
+        self.async_write_entity_states()
 
     async def async_seen(self, soco: SoCo | None = None) -> None:
         """Record that this speaker was seen right now."""
@@ -376,17 +425,6 @@ class SonosSpeaker:
 
         self._subscriptions = []
 
-    async def async_update_device_properties(self, event: SonosEvent = None) -> None:
-        """Update device properties using the provided SonosEvent."""
-        if event is None:
-            return
-
-        if (more_info := event.variables.get("more_info")) is not None:
-            battery_dict = dict(x.split(":") for x in more_info.split(","))
-            await self.async_update_battery_info(battery_dict)
-
-        self.async_write_entity_states()
-
     def update_alarms_for_speaker(self) -> set[str]:
         """Update current alarm instances.
 
@@ -409,17 +447,11 @@ class SonosSpeaker:
 
         return new_alarms
 
-    async def async_update_alarms(self, event: SonosEvent | None = None) -> None:
+    def update_alarms(self) -> None:
-        """Update device properties using the provided SonosEvent."""
+        """Update alarms from an event."""
-        if event is None:
+        if new_alarms := self.update_alarms_for_speaker():
-            return
+            dispatcher_send(self.hass, SONOS_CREATE_ALARM, self, new_alarms)
-
+        dispatcher_send(self.hass, SONOS_ALARM_UPDATE)
-        if new_alarms := await self.hass.async_add_executor_job(
-            self.update_alarms_for_speaker
-        ):
-            async_dispatcher_send(self.hass, SONOS_CREATE_ALARM, self, new_alarms)
-
-        async_dispatcher_send(self.hass, SONOS_ALARM_UPDATE)
 
     async def async_update_battery_info(self, battery_dict: dict[str, Any]) -> None:
         """Update battery info using the decoded SonosEvent."""
@@ -759,12 +791,6 @@ class SonosSpeaker:
         """Return the SonosFavorites instance for this household."""
         return self.hass.data[DATA_SONOS].favorites[self.household_id]
 
-    @callback
-    def async_update_content(self, event: SonosEvent | None = None) -> None:
-        """Update information about available content."""
-        if event and "favorites_update_id" in event.variables:
-            self.favorites.async_delayed_update(event)
-
     def update_volume(self) -> None:
         """Update information about current volume settings."""
         self.volume = self.soco.volume
@@ -772,30 +798,6 @@ class SonosSpeaker:
         self.night_mode = self.soco.night_mode
         self.dialog_mode = self.soco.dialog_mode
 
-    @callback
-    def async_update_volume(self, event: SonosEvent) -> None:
-        """Update information about currently volume settings."""
-        variables = event.variables
-
-        if "volume" in variables:
-            self.volume = int(variables["volume"]["Master"])
-
-        if "mute" in variables:
-            self.muted = variables["mute"]["Master"] == "1"
-
-        if "night_mode" in variables:
-            self.night_mode = variables["night_mode"] == "1"
-
-        if "dialog_level" in variables:
-            self.dialog_mode = variables["dialog_level"] == "1"
-
-        self.async_write_entity_states()
-
-    @callback
-    def async_update_media(self, event: SonosEvent | None = None) -> None:
-        """Update information about currently playing media."""
-        self.hass.async_add_executor_job(self.update_media, event)
-
     def update_media(self, event: SonosEvent | None = None) -> None:
         """Update information about currently playing media."""
         variables = event and event.variables

tests/components/sonos/conftest.py

@@ -10,15 +10,24 @@ from homeassistant.const import CONF_HOSTS
 from tests.common import MockConfigEntry
 
 
+class SonosMockService:
+    """Mock a Sonos Service used in callbacks."""
+
+    def __init__(self, service_type):
+        """Initialize the instance."""
+        self.service_type = service_type
+        self.subscribe = AsyncMock()
+
+
 class SonosMockEvent:
     """Mock a sonos Event used in callbacks."""
 
-    def __init__(self, soco, variables):
+    def __init__(self, soco, service, variables):
         """Initialize the instance."""
         self.sid = f"{soco.uid}_sub0000000001"
         self.seq = "0"
         self.timestamp = 1621000000.0
-        self.service = dummy_soco_service_fixture
+        self.service = service
         self.variables = variables
 
 
@@ -29,9 +38,7 @@ def config_entry_fixture():
 
 
 @pytest.fixture(name="soco")
-def soco_fixture(
+def soco_fixture(music_library, speaker_info, battery_info, alarm_clock):
-    music_library, speaker_info, battery_info, dummy_soco_service, alarm_clock
-):
     """Create a mock pysonos SoCo fixture."""
     with patch("pysonos.SoCo", autospec=True) as mock, patch(
         "socket.gethostbyname", return_value="192.168.42.2"
@@ -41,11 +48,11 @@ def soco_fixture(
         mock_soco.play_mode = "NORMAL"
         mock_soco.music_library = music_library
         mock_soco.get_speaker_info.return_value = speaker_info
-        mock_soco.avTransport = dummy_soco_service
+        mock_soco.avTransport = SonosMockService("AVTransport")
-        mock_soco.renderingControl = dummy_soco_service
+        mock_soco.renderingControl = SonosMockService("RenderingControl")
-        mock_soco.zoneGroupTopology = dummy_soco_service
+        mock_soco.zoneGroupTopology = SonosMockService("ZoneGroupTopology")
-        mock_soco.contentDirectory = dummy_soco_service
+        mock_soco.contentDirectory = SonosMockService("ContentDirectory")
-        mock_soco.deviceProperties = dummy_soco_service
+        mock_soco.deviceProperties = SonosMockService("DeviceProperties")
         mock_soco.alarmClock = alarm_clock
         mock_soco.mute = False
         mock_soco.night_mode = True
@@ -74,14 +81,6 @@ def config_fixture():
     return {DOMAIN: {MP_DOMAIN: {CONF_HOSTS: ["192.168.42.1"]}}}
 
 
-@pytest.fixture(name="dummy_soco_service")
-def dummy_soco_service_fixture():
-    """Create dummy_soco_service fixture."""
-    service = Mock()
-    service.subscribe = AsyncMock()
-    return service
-
-
 @pytest.fixture(name="music_library")
 def music_library_fixture():
     """Create music_library fixture."""
@@ -93,8 +92,8 @@ def music_library_fixture():
 @pytest.fixture(name="alarm_clock")
 def alarm_clock_fixture():
     """Create alarmClock fixture."""
-    alarm_clock = Mock()
+    alarm_clock = SonosMockService("AlarmClock")
-    alarm_clock.subscribe = AsyncMock()
+    alarm_clock.ListAlarms = Mock()
     alarm_clock.ListAlarms.return_value = {
         "CurrentAlarmList": "<Alarms>"
         '<Alarm ID="14" StartTime="07:00:00" Duration="02:00:00" Recurrence="DAILY" '
@@ -109,8 +108,8 @@ def alarm_clock_fixture():
 @pytest.fixture(name="alarm_clock_extended")
 def alarm_clock_fixture_extended():
     """Create alarmClock fixture."""
-    alarm_clock = Mock()
+    alarm_clock = SonosMockService("AlarmClock")
-    alarm_clock.subscribe = AsyncMock()
+    alarm_clock.ListAlarms = Mock()
     alarm_clock.ListAlarms.return_value = {
         "CurrentAlarmList": "<Alarms>"
         '<Alarm ID="14" StartTime="07:00:00" Duration="02:00:00" Recurrence="DAILY" '
@@ -156,7 +155,7 @@ def battery_event_fixture(soco):
         "zone_name": "Zone A",
         "more_info": "BattChg:NOT_CHARGING,RawBattPct:100,BattPct:100,BattTmp:25",
     }
-    return SonosMockEvent(soco, variables)
+    return SonosMockEvent(soco, soco.deviceProperties, variables)
 
 
 @pytest.fixture(name="alarm_event")
@@ -172,4 +171,4 @@ def alarm_event_fixture(soco):
         "daily_index_refresh_time": None,
     }
 
-    return SonosMockEvent(soco, variables)
+    return SonosMockEvent(soco, soco.alarmClock, variables)

tests/components/sonos/test_sensor.py

@@ -1,7 +1,7 @@
 """Tests for the Sonos battery sensor platform."""
 from pysonos.exceptions import NotSupportedException
 
-from homeassistant.components.sonos import DATA_SONOS, DOMAIN
+from homeassistant.components.sonos import DOMAIN
 from homeassistant.components.sonos.binary_sensor import ATTR_BATTERY_POWER_SOURCE
 from homeassistant.const import STATE_OFF, STATE_ON, STATE_UNAVAILABLE
 from homeassistant.setup import async_setup_component
@@ -63,6 +63,9 @@ async def test_battery_on_S1(hass, config_entry, config, soco, battery_event):
 
     await setup_platform(hass, config_entry, config)
 
+    subscription = soco.deviceProperties.subscribe.return_value
+    sub_callback = subscription.callback
+
     entity_registry = await hass.helpers.entity_registry.async_get_registry()
 
     battery = entity_registry.entities["sensor.zone_a_battery"]
@@ -74,8 +77,7 @@ async def test_battery_on_S1(hass, config_entry, config, soco, battery_event):
     assert power_state.state == STATE_UNAVAILABLE
 
     # Update the speaker with a callback event
-    speaker = hass.data[DATA_SONOS].discovered[soco.uid]
+    sub_callback(battery_event)
-    speaker.async_dispatch_properties(battery_event)
     await hass.async_block_till_done()
 
     battery_state = hass.states.get(battery.entity_id)