From f1084a57df5f38c470667e24b38983d26de1b8fa Mon Sep 17 00:00:00 2001 From: Pete Sage <76050312+PeteRager@users.noreply.github.com> Date: Wed, 31 Jul 2024 14:36:59 -0400 Subject: [PATCH] Fix Sonos media_player control may fail when grouping speakers (#121853) --- homeassistant/components/sonos/speaker.py | 21 ++- tests/components/sonos/conftest.py | 24 +++ .../sonos/fixtures/av_transport.json | 38 +++++ tests/components/sonos/fixtures/zgs_group.xml | 8 + .../sonos/fixtures/zgs_two_single.xml | 10 ++ tests/components/sonos/test_speaker.py | 146 +++++++++++++++++- 6 files changed, 241 insertions(+), 6 deletions(-) create mode 100644 tests/components/sonos/fixtures/av_transport.json create mode 100644 tests/components/sonos/fixtures/zgs_group.xml create mode 100644 tests/components/sonos/fixtures/zgs_two_single.xml diff --git a/homeassistant/components/sonos/speaker.py b/homeassistant/components/sonos/speaker.py index d77100a2236..d339e861a13 100644 --- a/homeassistant/components/sonos/speaker.py +++ b/homeassistant/components/sonos/speaker.py @@ -826,9 +826,6 @@ class SonosSpeaker: f"{SONOS_VANISHED}-{uid}", reason, ) - - if "zone_player_uui_ds_in_group" not in event.variables: - return self.event_stats.process(event) self.hass.async_create_background_task( self.create_update_groups_coro(event), @@ -857,8 +854,7 @@ class SonosSpeaker: async def _async_extract_group(event: SonosEvent | None) -> list[str]: """Extract group layout from a topology event.""" - group = event and event.zone_player_uui_ds_in_group - if group: + if group := (event and getattr(event, "zone_player_uui_ds_in_group", None)): assert isinstance(group, str) return group.split(",") @@ -867,11 +863,21 @@ class SonosSpeaker: @callback def _async_regroup(group: list[str]) -> None: """Rebuild internal group layout.""" + _LOGGER.debug("async_regroup %s %s", self.zone_name, group) if ( group == [self.soco.uid] and self.sonos_group == [self] and self.sonos_group_entities ): + # Single speakers do not have a coodinator, check and clear + if self.coordinator is not None: + _LOGGER.debug( + "Zone %s Cleared coordinator [%s]", + self.zone_name, + self.coordinator.zone_name, + ) + self.coordinator = None + self.async_write_entity_states() # Skip updating existing single speakers in polling mode return @@ -912,6 +918,11 @@ class SonosSpeaker: joined_speaker.coordinator = self joined_speaker.sonos_group = sonos_group joined_speaker.sonos_group_entities = sonos_group_entities + _LOGGER.debug( + "Zone %s Set coordinator [%s]", + joined_speaker.zone_name, + self.zone_name, + ) joined_speaker.async_write_entity_states() _LOGGER.debug("Regrouped %s: %s", self.zone_name, self.sonos_group_entities) diff --git a/tests/components/sonos/conftest.py b/tests/components/sonos/conftest.py index 4e5f704d322..bbec7a2308c 100644 --- a/tests/components/sonos/conftest.py +++ b/tests/components/sonos/conftest.py @@ -17,6 +17,7 @@ from homeassistant.components.media_player import DOMAIN as MP_DOMAIN from homeassistant.components.sonos import DOMAIN from homeassistant.const import CONF_HOSTS from homeassistant.core import HomeAssistant +from homeassistant.setup import async_setup_component from tests.common import MockConfigEntry, load_fixture, load_json_value_fixture @@ -661,3 +662,26 @@ def zgs_event_fixture(hass: HomeAssistant, soco: SoCo, zgs_discovery: str): await hass.async_block_till_done(wait_background_tasks=True) return _wrapper + + +@pytest.fixture(name="sonos_setup_two_speakers") +async def sonos_setup_two_speakers( + hass: HomeAssistant, soco_factory: SoCoMockFactory +) -> list[MockSoCo]: + """Set up home assistant with two Sonos Speakers.""" + soco_lr = soco_factory.cache_mock(MockSoCo(), "10.10.10.1", "Living Room") + soco_br = soco_factory.cache_mock(MockSoCo(), "10.10.10.2", "Bedroom") + await async_setup_component( + hass, + DOMAIN, + { + DOMAIN: { + "media_player": { + "interface_addr": "127.0.0.1", + "hosts": ["10.10.10.1", "10.10.10.2"], + } + } + }, + ) + await hass.async_block_till_done() + return [soco_lr, soco_br] diff --git a/tests/components/sonos/fixtures/av_transport.json b/tests/components/sonos/fixtures/av_transport.json new file mode 100644 index 00000000000..743ac61e3ff --- /dev/null +++ b/tests/components/sonos/fixtures/av_transport.json @@ -0,0 +1,38 @@ +{ + "transport_state": "PLAYING", + "current_play_mode": "NORMAL", + "current_crossfade_mode": "0", + "number_of_tracks": "1", + "current_track": "1", + "current_section": "0", + "current_track_uri": "x-rincon:RINCON_test_10.10.10.2", + "current_track_duration": "", + "current_track_meta_data": "", + "next_track_uri": "", + "next_track_meta_data": "", + "enqueued_transport_uri": "", + "enqueued_transport_uri_meta_data": "", + "playback_storage_medium": "NETWORK", + "av_transport_uri": "x-rincon:RINCON_test_10.10.10.2", + "av_transport_uri_meta_data": "", + "next_av_transport_uri": "", + "next_av_transport_uri_meta_data": "", + "current_transport_actions": "Stop, Play", + "current_valid_play_modes": "CROSSFADE", + "direct_control_client_id": "", + "direct_control_is_suspended": "0", + "direct_control_account_id": "", + "transport_status": "OK", + "sleep_timer_generation": "0", + "alarm_running": "0", + "snooze_running": "0", + "restart_pending": "0", + "transport_play_speed": "NOT_IMPLEMENTED", + "current_media_duration": "NOT_IMPLEMENTED", + "record_storage_medium": "NOT_IMPLEMENTED", + "possible_playback_storage_media": "NONE, NETWORK", + "possible_record_storage_media": "NOT_IMPLEMENTED", + "record_medium_write_status": "NOT_IMPLEMENTED", + "current_record_quality_mode": "NOT_IMPLEMENTED", + "possible_record_quality_modes": "NOT_IMPLEMENTED" +} diff --git a/tests/components/sonos/fixtures/zgs_group.xml b/tests/components/sonos/fixtures/zgs_group.xml new file mode 100644 index 00000000000..58f40be0049 --- /dev/null +++ b/tests/components/sonos/fixtures/zgs_group.xml @@ -0,0 +1,8 @@ + + + + + + + + diff --git a/tests/components/sonos/fixtures/zgs_two_single.xml b/tests/components/sonos/fixtures/zgs_two_single.xml new file mode 100644 index 00000000000..18c3c9231c6 --- /dev/null +++ b/tests/components/sonos/fixtures/zgs_two_single.xml @@ -0,0 +1,10 @@ + + + + + + + + + + diff --git a/tests/components/sonos/test_speaker.py b/tests/components/sonos/test_speaker.py index 2c4357060be..40d126c64f2 100644 --- a/tests/components/sonos/test_speaker.py +++ b/tests/components/sonos/test_speaker.py @@ -4,11 +4,18 @@ from unittest.mock import patch import pytest +from homeassistant.components.media_player import ( + DOMAIN as MP_DOMAIN, + SERVICE_MEDIA_PLAY, +) +from homeassistant.components.sonos import DOMAIN from homeassistant.components.sonos.const import DATA_SONOS, SCAN_INTERVAL from homeassistant.core import HomeAssistant from homeassistant.util import dt as dt_util -from tests.common import async_fire_time_changed +from .conftest import MockSoCo, SonosMockEvent + +from tests.common import async_fire_time_changed, load_fixture, load_json_value_fixture async def test_fallback_to_polling( @@ -67,3 +74,140 @@ async def test_subscription_creation_fails( await hass.async_block_till_done() assert speaker._subscriptions + + +def _create_zgs_sonos_event( + fixture_file: str, soco_1: MockSoCo, soco_2: MockSoCo, create_uui_ds: bool = True +) -> SonosMockEvent: + """Create a Sonos Event for zone group state, with the option of creating the uui_ds_in_group.""" + zgs = load_fixture(fixture_file, DOMAIN) + variables = {} + variables["ZoneGroupState"] = zgs + # Sonos does not always send this variable with zgs events + if create_uui_ds: + variables["zone_player_uui_ds_in_group"] = f"{soco_1.uid},{soco_2.uid}" + event = SonosMockEvent(soco_1, soco_1.zoneGroupTopology, variables) + if create_uui_ds: + event.zone_player_uui_ds_in_group = f"{soco_1.uid},{soco_2.uid}" + return event + + +def _create_avtransport_sonos_event( + fixture_file: str, soco: MockSoCo +) -> SonosMockEvent: + """Create a Sonos Event for an AVTransport update.""" + variables = load_json_value_fixture(fixture_file, DOMAIN) + return SonosMockEvent(soco, soco.avTransport, variables) + + +async def _media_play(hass: HomeAssistant, entity: str) -> None: + """Call media play service.""" + await hass.services.async_call( + MP_DOMAIN, + SERVICE_MEDIA_PLAY, + { + "entity_id": entity, + }, + blocking=True, + ) + + +async def test_zgs_event_group_speakers( + hass: HomeAssistant, sonos_setup_two_speakers: list[MockSoCo] +) -> None: + """Tests grouping and ungrouping two speakers.""" + # When Sonos speakers are grouped; one of the speakers is the coordinator and is in charge + # of playback across both speakers. Hence, service calls to play or pause on media_players + # that are part of the group are routed to the coordinator. + soco_lr = sonos_setup_two_speakers[0] + soco_br = sonos_setup_two_speakers[1] + + # Test 1 - Initial state - speakers are not grouped + state = hass.states.get("media_player.living_room") + assert state.attributes["group_members"] == ["media_player.living_room"] + state = hass.states.get("media_player.bedroom") + assert state.attributes["group_members"] == ["media_player.bedroom"] + # Each speaker is its own coordinator and calls should route to their SoCos + await _media_play(hass, "media_player.living_room") + assert soco_lr.play.call_count == 1 + await _media_play(hass, "media_player.bedroom") + assert soco_br.play.call_count == 1 + + soco_lr.play.reset_mock() + soco_br.play.reset_mock() + + # Test 2 - Group the speakers, living room is the coordinator + event = _create_zgs_sonos_event( + "zgs_group.xml", soco_lr, soco_br, create_uui_ds=True + ) + soco_lr.zoneGroupTopology.subscribe.return_value._callback(event) + soco_br.zoneGroupTopology.subscribe.return_value._callback(event) + await hass.async_block_till_done(wait_background_tasks=True) + state = hass.states.get("media_player.living_room") + assert state.attributes["group_members"] == [ + "media_player.living_room", + "media_player.bedroom", + ] + state = hass.states.get("media_player.bedroom") + assert state.attributes["group_members"] == [ + "media_player.living_room", + "media_player.bedroom", + ] + # Play calls should route to the living room SoCo + await _media_play(hass, "media_player.living_room") + await _media_play(hass, "media_player.bedroom") + assert soco_lr.play.call_count == 2 + assert soco_br.play.call_count == 0 + + soco_lr.play.reset_mock() + soco_br.play.reset_mock() + + # Test 3 - Ungroup the speakers + event = _create_zgs_sonos_event( + "zgs_two_single.xml", soco_lr, soco_br, create_uui_ds=False + ) + soco_lr.zoneGroupTopology.subscribe.return_value._callback(event) + soco_br.zoneGroupTopology.subscribe.return_value._callback(event) + await hass.async_block_till_done(wait_background_tasks=True) + state = hass.states.get("media_player.living_room") + assert state.attributes["group_members"] == ["media_player.living_room"] + state = hass.states.get("media_player.bedroom") + assert state.attributes["group_members"] == ["media_player.bedroom"] + # Calls should route to each speakers Soco + await _media_play(hass, "media_player.living_room") + assert soco_lr.play.call_count == 1 + await _media_play(hass, "media_player.bedroom") + assert soco_br.play.call_count == 1 + + +async def test_zgs_avtransport_group_speakers( + hass: HomeAssistant, sonos_setup_two_speakers: list[MockSoCo] +) -> None: + """Test processing avtransport and zgs events to change group membership.""" + soco_lr = sonos_setup_two_speakers[0] + soco_br = sonos_setup_two_speakers[1] + + # Test 1 - Send a transport event changing the coordinator + # for the living room speaker to the bedroom speaker. + event = _create_avtransport_sonos_event("av_transport.json", soco_lr) + soco_lr.avTransport.subscribe.return_value._callback(event) + await hass.async_block_till_done(wait_background_tasks=True) + # Call should route to the new coodinator which is the bedroom + await _media_play(hass, "media_player.living_room") + assert soco_lr.play.call_count == 0 + assert soco_br.play.call_count == 1 + + soco_lr.play.reset_mock() + soco_br.play.reset_mock() + + # Test 2- Send a zgs event to return living room to its own coordinator + event = _create_zgs_sonos_event( + "zgs_two_single.xml", soco_lr, soco_br, create_uui_ds=False + ) + soco_lr.zoneGroupTopology.subscribe.return_value._callback(event) + soco_br.zoneGroupTopology.subscribe.return_value._callback(event) + await hass.async_block_till_done(wait_background_tasks=True) + # Call should route to the living room + await _media_play(hass, "media_player.living_room") + assert soco_lr.play.call_count == 1 + assert soco_br.play.call_count == 0