From 84f0d3f961abd72340922c9e8c8f7cb40d4c11f6 Mon Sep 17 00:00:00 2001 From: jjlawren Date: Fri, 28 May 2021 21:32:50 -0500 Subject: [PATCH] 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 | 4 +- homeassistant/components/sonos/speaker.py | 158 ++++++++++---------- tests/components/sonos/conftest.py | 47 +++--- tests/components/sonos/test_sensor.py | 8 +- 4 files changed, 111 insertions(+), 106 deletions(-) diff --git a/homeassistant/components/sonos/favorites.py b/homeassistant/components/sonos/favorites.py index 19dcb5184e5..2f5cab23be2 100644 --- a/homeassistant/components/sonos/favorites.py +++ b/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 diff --git a/homeassistant/components/sonos/speaker.py b/homeassistant/components/sonos/speaker.py index 81c95f6e33f..079b916a4bc 100644 --- a/homeassistant/components/sonos/speaker.py +++ b/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( - self._subscribe(self.soco.avTransport, self.async_update_media), - self._subscribe(self.soco.renderingControl, self.async_update_volume), - self._subscribe(self.soco.contentDirectory, self.async_update_content), - self._subscribe( - 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), - ) + subscriptions = [ + self._subscribe(getattr(self.soco, service), self.async_dispatch_event) + for service in SUBSCRIPTION_SERVICES + ] + await asyncio.gather(*subscriptions) 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: - """Update properties from event.""" - self.hass.async_create_task(self.async_update_device_properties(event)) - - @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: + def async_dispatch_event(self, event: SonosEvent) -> None: + """Handle callback event and route as needed.""" + if 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: - """Update device properties using the provided SonosEvent.""" - if event is None: - return - - 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) + def update_alarms(self) -> None: + """Update alarms from an event.""" + if new_alarms := self.update_alarms_for_speaker(): + dispatcher_send(self.hass, SONOS_CREATE_ALARM, self, new_alarms) + 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 diff --git a/tests/components/sonos/conftest.py b/tests/components/sonos/conftest.py index aa14dcaa5cf..fc5ef84c2d6 100644 --- a/tests/components/sonos/conftest.py +++ b/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( - music_library, speaker_info, battery_info, dummy_soco_service, alarm_clock -): +def soco_fixture(music_library, speaker_info, battery_info, 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.renderingControl = dummy_soco_service - mock_soco.zoneGroupTopology = dummy_soco_service - mock_soco.contentDirectory = dummy_soco_service - mock_soco.deviceProperties = dummy_soco_service + mock_soco.avTransport = SonosMockService("AVTransport") + mock_soco.renderingControl = SonosMockService("RenderingControl") + mock_soco.zoneGroupTopology = SonosMockService("ZoneGroupTopology") + mock_soco.contentDirectory = SonosMockService("ContentDirectory") + 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.subscribe = AsyncMock() + alarm_clock = SonosMockService("AlarmClock") + alarm_clock.ListAlarms = Mock() alarm_clock.ListAlarms.return_value = { "CurrentAlarmList": "" '" '