From bfc390ec5664e0d10020df7161385a8e4a5de700 Mon Sep 17 00:00:00 2001 From: Michael Gerbush <405526+msgerbush@users.noreply.github.com> Date: Tue, 12 May 2026 11:33:45 -0400 Subject: [PATCH 1/7] Recover Sonos player after stale host failures --- .../outbound/players/sonos_player_adapter.py | 214 +++++++++++++++--- .../players/test_sonos_player_adapters.py | 85 ++++++- 2 files changed, 269 insertions(+), 30 deletions(-) diff --git a/jukebox/adapters/outbound/players/sonos_player_adapter.py b/jukebox/adapters/outbound/players/sonos_player_adapter.py index 4a59eea4..76869276 100644 --- a/jukebox/adapters/outbound/players/sonos_player_adapter.py +++ b/jukebox/adapters/outbound/players/sonos_player_adapter.py @@ -1,4 +1,5 @@ import logging +from collections.abc import Callable import soco from requests.exceptions import RequestException @@ -7,12 +8,32 @@ from soco.plugins.sharelink import ShareLinkPlugin from urllib3.exceptions import HTTPError +from jukebox.adapters.outbound.sonos_discovery_adapter import SoCoSonosDiscoveryAdapter from jukebox.domain.errors import PlaybackError from jukebox.domain.ports import PlayerPort -from jukebox.settings.entities import ResolvedSonosGroupRuntime +from jukebox.settings.entities import ( + ResolvedSonosGroupRuntime, + SelectedSonosGroupSettings, + SelectedSonosSpeakerSettings, +) from jukebox.settings.errors import InvalidSettingsError +from jukebox.sonos.service import DefaultSonosService, SonosService LOGGER = logging.getLogger("jukebox") +_SONOS_TRANSPORT_ERRORS = (HTTPError, OSError, RequestException, SoCoException) + + +def _log_upnp_failure(command_name: str, err: SoCoUPnPException) -> None: + if "UPnP Error 804" in str(err.message): + LOGGER.warning("%s failed, probably a bad uri: %s", command_name, err.message) + elif "UPnP Error 701" in str(err.message): + LOGGER.warning( + "%s failed, probably a not available transition: %s", + command_name, + err.message, + ) + else: + LOGGER.exception("%s failed: %s", command_name, str(err)) def catch_soco_upnp_exception(func): @@ -20,17 +41,7 @@ def wrapper(*args, **kwargs): try: return func(*args, **kwargs) except SoCoUPnPException as err: - if "UPnP Error 804" in str(err.message): - LOGGER.warning("%s with `%s` failed, probably a bad uri: %s", func.__name__, args, err.message) - elif "UPnP Error 701" in str(err.message): - LOGGER.warning( - "%s with `%s` failed, probably a not available transition: %s", - func.__name__, - args, - err.message, - ) - else: - LOGGER.exception("%s with `%s` failed: %s", func.__name__, args, str(err)) + _log_upnp_failure(func.__name__, err) raise PlaybackError(str(err)) from err return wrapper @@ -44,7 +55,14 @@ def __init__( host: str | None = None, name: str | None = None, group: ResolvedSonosGroupRuntime | None = None, + sonos_service: SonosService | None = None, ): + self.manual_name = name + self.group = group + self.selected_group = _selected_group_from_runtime_group(group) + self.sonos_service = sonos_service or DefaultSonosService(SoCoSonosDiscoveryAdapter()) + self.speaker_name = "unknown Sonos player" + try: if group is not None: coordinator_host = host or group.coordinator.host @@ -57,13 +75,13 @@ def __init__( else: self.speaker = self._discover(name) - speaker_info = self.speaker.get_speaker_info() + speaker_info = self._refresh_speaker_metadata() except (HTTPError, OSError, RequestException, RuntimeError, SoCoException, SoCoUPnPException) as err: raise InvalidSettingsError(f"Failed to initialize Sonos player: {err}") from err LOGGER.info( "Found `%s` with software version: %s", - self.speaker.player_name, + self.speaker_name, speaker_info.get("software_version", None), ) self.sharelink = ShareLinkPlugin(self.speaker) @@ -185,30 +203,168 @@ def _get_rollback_coordinator_for_join(speaker: SoCo) -> SoCo | None: return current_coordinator - @catch_soco_upnp_exception + def _refresh_speaker_metadata(self) -> dict: + speaker_info = self.speaker.get_speaker_info() + self.speaker_name = self._speaker_name_from_info(speaker_info) + if self.selected_group is None: + self.selected_group = self._selected_group_from_current_speaker() + return speaker_info + + def _speaker_name_from_info(self, speaker_info: dict) -> str: + zone_name = speaker_info.get("zone_name") + if isinstance(zone_name, str) and zone_name: + return zone_name + + player_name = getattr(self.speaker, "player_name", None) + if isinstance(player_name, str) and player_name: + return player_name + + return "unknown Sonos player" + + def _selected_group_from_current_speaker(self) -> SelectedSonosGroupSettings | None: + uid = getattr(self.speaker, "uid", None) + household_id = getattr(self.speaker, "household_id", None) + if not isinstance(uid, str) or not uid: + return None + if not isinstance(household_id, str) or not household_id: + return None + + return SelectedSonosGroupSettings( + household_id=household_id, + coordinator_uid=uid, + members=[SelectedSonosSpeakerSettings(uid=uid)], + ) + + def _execute_with_recovery(self, command_name: str, command: Callable[[], None]) -> None: + try: + command() + return + except SoCoUPnPException as err: + _log_upnp_failure(command_name, err) + raise PlaybackError(str(err)) from err + except _SONOS_TRANSPORT_ERRORS as err: + LOGGER.warning("%s failed for Sonos player `%s`: %s", command_name, self.speaker_name, err) + original_error = err + + if not self._recover_speaker(command_name): + raise PlaybackError(str(original_error)) from original_error + + try: + command() + except SoCoUPnPException as err: + _log_upnp_failure(command_name, err) + raise PlaybackError(str(err)) from err + except _SONOS_TRANSPORT_ERRORS as err: + LOGGER.warning("%s failed after Sonos recovery for `%s`: %s", command_name, self.speaker_name, err) + raise PlaybackError(str(err)) from err + + def _recover_speaker(self, command_name: str) -> bool: + if self.selected_group is not None: + return self._recover_selected_group(command_name) + + if self.manual_name is not None: + return self._recover_by_name(command_name) + + LOGGER.warning("%s could not recover Sonos player because no rediscoverable target is available", command_name) + return False + + def _recover_selected_group(self, command_name: str) -> bool: + assert self.selected_group is not None + try: + resolved_group = self.sonos_service.resolve_selected_group(self.selected_group) + self._switch_to_resolved_group(resolved_group) + except ( + HTTPError, + OSError, + RequestException, + RuntimeError, + ValueError, + SoCoException, + SoCoUPnPException, + ) as err: + LOGGER.warning("%s could not re-resolve Sonos player `%s`: %s", command_name, self.speaker_name, err) + return False + + LOGGER.info( + "%s recovered Sonos player `%s` at `%s`", + command_name, + self.speaker_name, + resolved_group.coordinator.host, + ) + return True + + def _switch_to_resolved_group(self, resolved_group: ResolvedSonosGroupRuntime) -> None: + enforce_group = self.group is not None + self.speaker = SoCo(resolved_group.coordinator.host) + if enforce_group: + self._enforce_group(resolved_group) + self.group = resolved_group + self.selected_group = _selected_group_from_runtime_group(resolved_group) + self._refresh_speaker_metadata() + self.sharelink = ShareLinkPlugin(self.speaker) + + def _recover_by_name(self, command_name: str) -> bool: + try: + self.speaker = self._discover(self.manual_name) + self._refresh_speaker_metadata() + self.sharelink = ShareLinkPlugin(self.speaker) + except (HTTPError, OSError, RequestException, RuntimeError, SoCoException, SoCoUPnPException) as err: + LOGGER.warning("%s could not rediscover Sonos player named `%s`: %s", command_name, self.manual_name, err) + return False + + LOGGER.info("%s rediscovered Sonos player `%s`", command_name, self.speaker_name) + return True + def play(self, uri: str, shuffle: bool = False) -> None: - LOGGER.info("Playing `%s` on the player `%s`", uri, self.speaker.player_name) - self.speaker.clear_queue() - _ = self.handle_uri(uri) - self.speaker.play_mode = "SHUFFLE_NOREPEAT" if shuffle else "NORMAL" - self.speaker.play_from_queue(index=0, start=True) + def command() -> None: + LOGGER.info("Playing `%s` on the player `%s`", uri, self.speaker_name) + self.speaker.clear_queue() + _ = self.handle_uri(uri) + self.speaker.play_mode = "SHUFFLE_NOREPEAT" if shuffle else "NORMAL" + self.speaker.play_from_queue(index=0, start=True) + + self._execute_with_recovery("play", command) - @catch_soco_upnp_exception def pause(self) -> None: - LOGGER.info("Pausing player `%s`", self.speaker.player_name) - self.speaker.pause() + def command() -> None: + LOGGER.info("Pausing player `%s`", self.speaker_name) + self.speaker.pause() + + self._execute_with_recovery("pause", command) - @catch_soco_upnp_exception def resume(self) -> None: - LOGGER.info("Resuming player `%s`", self.speaker.player_name) - self.speaker.play() + def command() -> None: + LOGGER.info("Resuming player `%s`", self.speaker_name) + self.speaker.play() + + self._execute_with_recovery("resume", command) - @catch_soco_upnp_exception def stop(self) -> None: - LOGGER.info("Stopping player `%s` and clearing its queue", self.speaker.player_name) - self.speaker.clear_queue() + def command() -> None: + LOGGER.info("Stopping player `%s` and clearing its queue", self.speaker_name) + self.speaker.clear_queue() + + self._execute_with_recovery("stop", command) def handle_uri(self, uri): if self.sharelink.is_share_link(uri): return self.sharelink.add_share_link_to_queue(uri, position=1) return self.speaker.add_uri_to_queue(uri, position=1) + + +def _selected_group_from_runtime_group( + group: ResolvedSonosGroupRuntime | None, +) -> SelectedSonosGroupSettings | None: + if group is None: + return None + + member_uids = [member.uid for member in group.members] + for uid in group.missing_member_uids: + if uid not in member_uids: + member_uids.append(uid) + + return SelectedSonosGroupSettings( + household_id=group.household_id, + coordinator_uid=group.coordinator.uid, + members=[SelectedSonosSpeakerSettings(uid=uid) for uid in member_uids], + ) diff --git a/tests/jukebox/adapters/outbound/players/test_sonos_player_adapters.py b/tests/jukebox/adapters/outbound/players/test_sonos_player_adapters.py index 483a40a0..7edd89de 100644 --- a/tests/jukebox/adapters/outbound/players/test_sonos_player_adapters.py +++ b/tests/jukebox/adapters/outbound/players/test_sonos_player_adapters.py @@ -1,12 +1,13 @@ from unittest.mock import MagicMock, patch import pytest +from requests.exceptions import ConnectionError as RequestConnectionError from soco.exceptions import SoCoUPnPException from jukebox.adapters.outbound.players.sonos_player_adapter import SonosPlayerAdapter, catch_soco_upnp_exception from jukebox.domain.errors import PlaybackError from jukebox.settings.errors import InvalidSettingsError -from tests.jukebox.settings._helpers import build_resolved_sonos_group_runtime +from tests.jukebox.settings._helpers import StubSonosService, build_resolved_sonos_group_runtime def make_exception(code: str): @@ -594,6 +595,88 @@ def test_pause_calls_underlying_sonos_player(mock_sharelink, mock_soco): mock_speaker.pause.assert_called_once() +@patch("jukebox.adapters.outbound.players.sonos_player_adapter.SoCo") +@patch("jukebox.adapters.outbound.players.sonos_player_adapter.ShareLinkPlugin") +def test_pause_uses_cached_player_name(mock_sharelink, mock_soco): + """Should not poll Sonos for player_name while pausing.""" + + class SpeakerWithNetworkedName: + def __init__(self): + self.get_speaker_info = MagicMock(return_value={"software_version": "1.0", "zone_name": "Living Room"}) + self.pause = MagicMock() + + @property + def player_name(self): + raise RequestConnectionError("No route to host") + + speaker = SpeakerWithNetworkedName() + mock_soco.return_value = speaker + + adapter = SonosPlayerAdapter(host="192.168.1.100") + adapter.pause() + + speaker.pause.assert_called_once() + + +@patch("jukebox.adapters.outbound.players.sonos_player_adapter.SoCo") +@patch("jukebox.adapters.outbound.players.sonos_player_adapter.ShareLinkPlugin") +def test_pause_recovers_selected_group_after_ip_changes(mock_sharelink, mock_soco): + """Should re-resolve the saved Sonos group by UID and retry the command once.""" + old_group = build_resolved_sonos_group_runtime( + coordinator_uid="RINCON_949F3E8DD34001400", + speakers=[("RINCON_949F3E8DD34001400", "Living Room", "192.168.1.24", "household-1")], + ) + new_group = build_resolved_sonos_group_runtime( + coordinator_uid="RINCON_949F3E8DD34001400", + speakers=[("RINCON_949F3E8DD34001400", "Living Room", "192.168.1.25", "household-1")], + ) + old_speaker = MagicMock() + old_speaker.uid = "RINCON_949F3E8DD34001400" + old_speaker.household_id = "household-1" + old_speaker.group = None + old_speaker.get_speaker_info.return_value = {"software_version": "1.0", "zone_name": "Living Room"} + old_speaker.pause.side_effect = RequestConnectionError("No route to host") + new_speaker = MagicMock() + new_speaker.uid = "RINCON_949F3E8DD34001400" + new_speaker.household_id = "household-1" + new_speaker.group = None + new_speaker.get_speaker_info.return_value = {"software_version": "1.0", "zone_name": "Living Room"} + mock_soco.side_effect = lambda host: { + "192.168.1.24": old_speaker, + "192.168.1.25": new_speaker, + }[host] + sonos_service = StubSonosService(resolved_group=new_group) + + adapter = SonosPlayerAdapter(group=old_group, sonos_service=sonos_service) + adapter.pause() + + old_speaker.pause.assert_called_once() + new_speaker.pause.assert_called_once() + assert adapter.speaker is new_speaker + assert len(sonos_service.calls) == 1 + assert sonos_service.calls[0].coordinator_uid == "RINCON_949F3E8DD34001400" + + +@patch("jukebox.adapters.outbound.players.sonos_player_adapter.SoCo") +@patch("jukebox.adapters.outbound.players.sonos_player_adapter.ShareLinkPlugin") +def test_pause_raises_playback_error_when_recovery_fails(mock_sharelink, mock_soco, caplog): + """Should report command failure after recovery cannot find the selected speaker.""" + mock_speaker = MagicMock() + mock_soco.return_value = mock_speaker + mock_speaker.uid = "RINCON_949F3E8DD34001400" + mock_speaker.household_id = "household-1" + mock_speaker.get_speaker_info.return_value = {"software_version": "1.0", "zone_name": "Living Room"} + mock_speaker.pause.side_effect = RequestConnectionError("No route to host") + sonos_service = StubSonosService(error=ValueError("not found on network")) + + adapter = SonosPlayerAdapter(host="192.168.1.24", sonos_service=sonos_service) + with pytest.raises(PlaybackError, match="No route to host"): + adapter.pause() + + mock_speaker.pause.assert_called_once() + assert "pause could not re-resolve Sonos player `Living Room`: not found on network" in caplog.text + + @patch("jukebox.adapters.outbound.players.sonos_player_adapter.SoCo") @patch("jukebox.adapters.outbound.players.sonos_player_adapter.ShareLinkPlugin") def test_resume_calls_underlying_sonos_player(mock_sharelink, mock_soco): From 48f0e9dd798c3245c2d1c33e8ecbf80a12aee4d1 Mon Sep 17 00:00:00 2001 From: Michael Gerbush <405526+msgerbush@users.noreply.github.com> Date: Tue, 12 May 2026 22:46:26 -0400 Subject: [PATCH 2/7] Wire Sonos recovery through DI --- .../outbound/players/sonos_player_adapter.py | 17 +++++++--- jukebox/di_container.py | 20 +++++++++-- jukebox/sonos/__init__.py | 3 +- jukebox/sonos/service.py | 14 ++++---- .../players/test_sonos_player_adapters.py | 12 +++---- tests/jukebox/test_jukebox_di_container.py | 33 ++++++++++++++++--- 6 files changed, 73 insertions(+), 26 deletions(-) diff --git a/jukebox/adapters/outbound/players/sonos_player_adapter.py b/jukebox/adapters/outbound/players/sonos_player_adapter.py index 76869276..6b420c37 100644 --- a/jukebox/adapters/outbound/players/sonos_player_adapter.py +++ b/jukebox/adapters/outbound/players/sonos_player_adapter.py @@ -8,7 +8,6 @@ from soco.plugins.sharelink import ShareLinkPlugin from urllib3.exceptions import HTTPError -from jukebox.adapters.outbound.sonos_discovery_adapter import SoCoSonosDiscoveryAdapter from jukebox.domain.errors import PlaybackError from jukebox.domain.ports import PlayerPort from jukebox.settings.entities import ( @@ -17,7 +16,7 @@ SelectedSonosSpeakerSettings, ) from jukebox.settings.errors import InvalidSettingsError -from jukebox.sonos.service import DefaultSonosService, SonosService +from jukebox.sonos.service import SonosGroupResolver LOGGER = logging.getLogger("jukebox") _SONOS_TRANSPORT_ERRORS = (HTTPError, OSError, RequestException, SoCoException) @@ -55,12 +54,12 @@ def __init__( host: str | None = None, name: str | None = None, group: ResolvedSonosGroupRuntime | None = None, - sonos_service: SonosService | None = None, + sonos_group_resolver: SonosGroupResolver | None = None, ): self.manual_name = name self.group = group self.selected_group = _selected_group_from_runtime_group(group) - self.sonos_service = sonos_service or DefaultSonosService(SoCoSonosDiscoveryAdapter()) + self.sonos_group_resolver = sonos_group_resolver self.speaker_name = "unknown Sonos player" try: @@ -270,8 +269,16 @@ def _recover_speaker(self, command_name: str) -> bool: def _recover_selected_group(self, command_name: str) -> bool: assert self.selected_group is not None + if self.sonos_group_resolver is None: + LOGGER.warning( + "%s could not re-resolve Sonos player `%s` because no Sonos group resolver is configured", + command_name, + self.speaker_name, + ) + return False + try: - resolved_group = self.sonos_service.resolve_selected_group(self.selected_group) + resolved_group = self.sonos_group_resolver.resolve_selected_group(self.selected_group) self._switch_to_resolved_group(resolved_group) except ( HTTPError, diff --git a/jukebox/di_container.py b/jukebox/di_container.py index c3a97492..d623e94c 100644 --- a/jukebox/di_container.py +++ b/jukebox/di_container.py @@ -16,7 +16,7 @@ from jukebox.settings.runtime_resolver import JukeboxRuntimeResolver from jukebox.settings.service_protocols import SettingsService from jukebox.shared.config_utils import get_current_tag_path -from jukebox.sonos.service import DefaultSonosService +from jukebox.sonos.service import DefaultSonosService, SonosGroupResolver def build_settings_service( @@ -88,7 +88,10 @@ def build_runtime_resolver(settings_service: SettingsService) -> JukeboxRuntimeR return JukeboxRuntimeResolver(settings_service, DefaultSonosService(SoCoSonosDiscoveryAdapter())) -def build_jukebox(config: ResolvedJukeboxRuntimeConfig): +def build_jukebox( + config: ResolvedJukeboxRuntimeConfig, + sonos_group_resolver: SonosGroupResolver | None = None, +): """Build and wire all dependencies for Jukebox.""" library = JsonLibraryAdapter(config.library_path) @@ -96,7 +99,14 @@ def build_jukebox(config: ResolvedJukeboxRuntimeConfig): match config.player_type: case "sonos": - player = SonosPlayerAdapter(host=config.sonos_host, name=config.sonos_name, group=config.sonos_group) + if sonos_group_resolver is None: + sonos_group_resolver = build_sonos_group_resolver() + player = SonosPlayerAdapter( + host=config.sonos_host, + name=config.sonos_name, + group=config.sonos_group, + sonos_group_resolver=sonos_group_resolver, + ) case "dryrun": player = DryrunPlayerAdapter() case _: @@ -139,3 +149,7 @@ def build_jukebox(config: ResolvedJukeboxRuntimeConfig): ) return reader, handle_tag_event + + +def build_sonos_group_resolver() -> SonosGroupResolver: + return DefaultSonosService(SoCoSonosDiscoveryAdapter()) diff --git a/jukebox/sonos/__init__.py b/jukebox/sonos/__init__.py index 263cd714..23f05a54 100644 --- a/jukebox/sonos/__init__.py +++ b/jukebox/sonos/__init__.py @@ -11,7 +11,7 @@ SonosSelectionResult, SonosSelectionStatus, ) -from .service import DefaultSonosService, SonosService +from .service import DefaultSonosService, SonosGroupResolver, SonosService __all__ = [ "DefaultSonosService", @@ -20,6 +20,7 @@ "SaveSonosSelection", "SonosDiscoveryError", "SonosDiscoveryPort", + "SonosGroupResolver", "SonosSelectionAvailability", "SonosSelectionResult", "SonosSelectionStatus", diff --git a/jukebox/sonos/service.py b/jukebox/sonos/service.py index 06abbaa2..6fb2282e 100644 --- a/jukebox/sonos/service.py +++ b/jukebox/sonos/service.py @@ -14,7 +14,14 @@ ) -class SonosService(Protocol): +class SonosGroupResolver(Protocol): + def resolve_selected_group( + self, + selected_group: SelectedSonosGroupSettings, + ) -> ResolvedSonosGroupRuntime: ... + + +class SonosService(SonosGroupResolver, Protocol): def list_network_speakers(self) -> list[DiscoveredSonosSpeaker]: ... def inspect_selected_group( @@ -22,11 +29,6 @@ def inspect_selected_group( selected_group: SelectedSonosGroupSettings, ) -> "InspectedSelectedSonosGroup": ... - def resolve_selected_group( - self, - selected_group: SelectedSonosGroupSettings, - ) -> ResolvedSonosGroupRuntime: ... - @dataclass(frozen=True) class InspectedSelectedSonosGroup: diff --git a/tests/jukebox/adapters/outbound/players/test_sonos_player_adapters.py b/tests/jukebox/adapters/outbound/players/test_sonos_player_adapters.py index 7edd89de..f042d4f9 100644 --- a/tests/jukebox/adapters/outbound/players/test_sonos_player_adapters.py +++ b/tests/jukebox/adapters/outbound/players/test_sonos_player_adapters.py @@ -645,16 +645,16 @@ def test_pause_recovers_selected_group_after_ip_changes(mock_sharelink, mock_soc "192.168.1.24": old_speaker, "192.168.1.25": new_speaker, }[host] - sonos_service = StubSonosService(resolved_group=new_group) + sonos_group_resolver = StubSonosService(resolved_group=new_group) - adapter = SonosPlayerAdapter(group=old_group, sonos_service=sonos_service) + adapter = SonosPlayerAdapter(group=old_group, sonos_group_resolver=sonos_group_resolver) adapter.pause() old_speaker.pause.assert_called_once() new_speaker.pause.assert_called_once() assert adapter.speaker is new_speaker - assert len(sonos_service.calls) == 1 - assert sonos_service.calls[0].coordinator_uid == "RINCON_949F3E8DD34001400" + assert len(sonos_group_resolver.calls) == 1 + assert sonos_group_resolver.calls[0].coordinator_uid == "RINCON_949F3E8DD34001400" @patch("jukebox.adapters.outbound.players.sonos_player_adapter.SoCo") @@ -667,9 +667,9 @@ def test_pause_raises_playback_error_when_recovery_fails(mock_sharelink, mock_so mock_speaker.household_id = "household-1" mock_speaker.get_speaker_info.return_value = {"software_version": "1.0", "zone_name": "Living Room"} mock_speaker.pause.side_effect = RequestConnectionError("No route to host") - sonos_service = StubSonosService(error=ValueError("not found on network")) + sonos_group_resolver = StubSonosService(error=ValueError("not found on network")) - adapter = SonosPlayerAdapter(host="192.168.1.24", sonos_service=sonos_service) + adapter = SonosPlayerAdapter(host="192.168.1.24", sonos_group_resolver=sonos_group_resolver) with pytest.raises(PlaybackError, match="No route to host"): adapter.pause() diff --git a/tests/jukebox/test_jukebox_di_container.py b/tests/jukebox/test_jukebox_di_container.py index 549811f8..2a4103b7 100644 --- a/tests/jukebox/test_jukebox_di_container.py +++ b/tests/jukebox/test_jukebox_di_container.py @@ -29,6 +29,11 @@ def test_build_jukebox_with_sonos_and_pn532(self, mock_library, mock_current_tag "sys.modules", {"jukebox.adapters.outbound.readers.pn532_reader_adapter": MagicMock(Pn532ReaderAdapter=mock_pn532_class)}, ) + sonos_group_resolver = MagicMock() + build_sonos_group_resolver = mocker.patch( + "jukebox.di_container.build_sonos_group_resolver", + return_value=sonos_group_resolver, + ) config = ResolvedJukeboxRuntimeConfig( library_path="/test/library.json", @@ -52,7 +57,13 @@ def test_build_jukebox_with_sonos_and_pn532(self, mock_library, mock_current_tag mock_library.assert_called_once_with("/test/library.json") mock_current_tag.assert_called_once_with("/test/current-tag.txt") - mock_player.assert_called_once_with(host="192.168.1.100", name=None, group=config.sonos_group) + mock_player.assert_called_once_with( + host="192.168.1.100", + name=None, + group=config.sonos_group, + sonos_group_resolver=sonos_group_resolver, + ) + build_sonos_group_resolver.assert_called_once_with() mock_pn532_class.assert_called_once_with( read_timeout_seconds=0.25, spi_reset=20, @@ -82,12 +93,18 @@ def test_build_jukebox_with_sonos_name(self, mock_library, mock_current_tag, moc pn532_connection=SpiConnectionParams(reset=20, cs=4, irq=None), verbose=False, ) + sonos_group_resolver = MagicMock() - reader, handle_tag_event = build_jukebox(config) + reader, handle_tag_event = build_jukebox(config, sonos_group_resolver=sonos_group_resolver) mock_library.assert_called_once_with("/test/library.json") mock_current_tag.assert_called_once_with("/test/current-tag.txt") - mock_player.assert_called_once_with(host=None, name="Living Room", group=None) + mock_player.assert_called_once_with( + host=None, + name="Living Room", + group=None, + sonos_group_resolver=sonos_group_resolver, + ) mock_reader.assert_called_once_with() assert reader == mock_reader.return_value assert handle_tag_event is not None @@ -112,12 +129,18 @@ def test_build_jukebox_with_sonos_autodiscovery(self, mock_library, mock_current pn532_connection=SpiConnectionParams(reset=20, cs=4, irq=None), verbose=False, ) + sonos_group_resolver = MagicMock() - reader, handle_tag_event = build_jukebox(config) + reader, handle_tag_event = build_jukebox(config, sonos_group_resolver=sonos_group_resolver) mock_library.assert_called_once_with("/test/library.json") mock_current_tag.assert_called_once_with("/test/current-tag.txt") - mock_player.assert_called_once_with(host=None, name=None, group=None) + mock_player.assert_called_once_with( + host=None, + name=None, + group=None, + sonos_group_resolver=sonos_group_resolver, + ) mock_reader.assert_called_once_with() assert reader == mock_reader.return_value assert handle_tag_event is not None From 15f1d5bf418aaf77fc7549b0469e34b760bc7c73 Mon Sep 17 00:00:00 2001 From: Michael Gerbush <405526+msgerbush@users.noreply.github.com> Date: Tue, 12 May 2026 22:50:41 -0400 Subject: [PATCH 3/7] Remove stale Sonos UPnP decorator --- .../outbound/players/sonos_player_adapter.py | 11 ----- .../players/test_sonos_player_adapters.py | 44 +++++++++---------- 2 files changed, 22 insertions(+), 33 deletions(-) diff --git a/jukebox/adapters/outbound/players/sonos_player_adapter.py b/jukebox/adapters/outbound/players/sonos_player_adapter.py index 6b420c37..2c3852c5 100644 --- a/jukebox/adapters/outbound/players/sonos_player_adapter.py +++ b/jukebox/adapters/outbound/players/sonos_player_adapter.py @@ -35,17 +35,6 @@ def _log_upnp_failure(command_name: str, err: SoCoUPnPException) -> None: LOGGER.exception("%s failed: %s", command_name, str(err)) -def catch_soco_upnp_exception(func): - def wrapper(*args, **kwargs): - try: - return func(*args, **kwargs) - except SoCoUPnPException as err: - _log_upnp_failure(func.__name__, err) - raise PlaybackError(str(err)) from err - - return wrapper - - class SonosPlayerAdapter(PlayerPort): """Adapter for Sonos player implementing PlayerPort.""" diff --git a/tests/jukebox/adapters/outbound/players/test_sonos_player_adapters.py b/tests/jukebox/adapters/outbound/players/test_sonos_player_adapters.py index f042d4f9..606d79a4 100644 --- a/tests/jukebox/adapters/outbound/players/test_sonos_player_adapters.py +++ b/tests/jukebox/adapters/outbound/players/test_sonos_player_adapters.py @@ -4,7 +4,7 @@ from requests.exceptions import ConnectionError as RequestConnectionError from soco.exceptions import SoCoUPnPException -from jukebox.adapters.outbound.players.sonos_player_adapter import SonosPlayerAdapter, catch_soco_upnp_exception +from jukebox.adapters.outbound.players.sonos_player_adapter import SonosPlayerAdapter from jukebox.domain.errors import PlaybackError from jukebox.settings.errors import InvalidSettingsError from tests.jukebox.settings._helpers import StubSonosService, build_resolved_sonos_group_runtime @@ -732,42 +732,42 @@ def test_init_with_duplicate_speaker_names_logs_warning(mock_sharelink, mock_soc ("stop", "clear_queue", ()), ], ) +@pytest.mark.parametrize("error_code, expected_message", (("804", "bad uri"), ("701", "not available transition"))) @patch("jukebox.adapters.outbound.players.sonos_player_adapter.SoCo") -def test_methods_log_and_raise_on_upnp_error(mock_soco, caplog, adapter_method, soco_method, args): +def test_methods_log_and_raise_on_known_upnp_error( + mock_soco, + caplog, + adapter_method, + soco_method, + args, + error_code, + expected_message, +): mock_speaker = MagicMock() mock_soco.return_value = mock_speaker mock_speaker.get_speaker_info.return_value = {"software_version": "1.0"} - getattr(mock_speaker, soco_method).side_effect = make_exception("804") + getattr(mock_speaker, soco_method).side_effect = make_exception(error_code) adapter = SonosPlayerAdapter(host="192.168.1.100") with pytest.raises(PlaybackError): getattr(adapter, adapter_method)(*args) - assert "bad uri" in caplog.text + assert expected_message in caplog.text getattr(mock_speaker, soco_method).assert_called() -@pytest.mark.parametrize("error_code, expected_message", (("804", "bad uri"), ("701", "not available transition"))) -def test_decorator_logs_warning_for_know_codes(caplog, error_code, expected_message): - @catch_soco_upnp_exception - def failing(): - raise make_exception(error_code) - - with pytest.raises(PlaybackError), caplog.at_level("WARNING"): - failing() - - assert expected_message in caplog.text - assert any(r.levelname == "WARNING" for r in caplog.records) - +@patch("jukebox.adapters.outbound.players.sonos_player_adapter.SoCo") +def test_methods_log_exception_for_unknown_upnp_error(mock_soco, caplog): + mock_speaker = MagicMock() + mock_soco.return_value = mock_speaker + mock_speaker.get_speaker_info.return_value = {"software_version": "1.0"} + mock_speaker.pause.side_effect = make_exception("999") -def test_decorator_logs_exception_for_other_codes(caplog): - @catch_soco_upnp_exception - def failing(): - raise make_exception("999") + adapter = SonosPlayerAdapter(host="192.168.1.100") with pytest.raises(PlaybackError), caplog.at_level("ERROR"): - failing() + adapter.pause() - assert any(r.levelname == "ERROR" for r in caplog.records) + assert any(record.levelname == "ERROR" for record in caplog.records) From 5eaf22a3cffe4b5c3564552f57079bb01d43a38f Mon Sep 17 00:00:00 2001 From: Michael Gerbush <405526+msgerbush@users.noreply.github.com> Date: Tue, 12 May 2026 23:08:36 -0400 Subject: [PATCH 4/7] Clarify Sonos playback target recovery boundary --- .../outbound/players/sonos_player_adapter.py | 62 +++++++------------ jukebox/di_container.py | 12 ++-- jukebox/sonos/__init__.py | 12 +++- jukebox/sonos/service.py | 61 ++++++++++++++++-- .../players/test_sonos_player_adapters.py | 18 ++++-- tests/jukebox/settings/_helpers.py | 7 +++ tests/jukebox/settings/test_sonos_runtime.py | 23 ++++++- tests/jukebox/test_jukebox_di_container.py | 30 +++++---- 8 files changed, 154 insertions(+), 71 deletions(-) diff --git a/jukebox/adapters/outbound/players/sonos_player_adapter.py b/jukebox/adapters/outbound/players/sonos_player_adapter.py index 2c3852c5..c8074142 100644 --- a/jukebox/adapters/outbound/players/sonos_player_adapter.py +++ b/jukebox/adapters/outbound/players/sonos_player_adapter.py @@ -10,13 +10,13 @@ from jukebox.domain.errors import PlaybackError from jukebox.domain.ports import PlayerPort -from jukebox.settings.entities import ( - ResolvedSonosGroupRuntime, - SelectedSonosGroupSettings, - SelectedSonosSpeakerSettings, -) +from jukebox.settings.entities import ResolvedSonosGroupRuntime from jukebox.settings.errors import InvalidSettingsError -from jukebox.sonos.service import SonosGroupResolver +from jukebox.sonos.service import ( + SonosPlaybackTarget, + SonosPlaybackTargetResolver, + playback_target_from_runtime_group, +) LOGGER = logging.getLogger("jukebox") _SONOS_TRANSPORT_ERRORS = (HTTPError, OSError, RequestException, SoCoException) @@ -43,12 +43,12 @@ def __init__( host: str | None = None, name: str | None = None, group: ResolvedSonosGroupRuntime | None = None, - sonos_group_resolver: SonosGroupResolver | None = None, + sonos_playback_target_resolver: SonosPlaybackTargetResolver | None = None, ): self.manual_name = name self.group = group - self.selected_group = _selected_group_from_runtime_group(group) - self.sonos_group_resolver = sonos_group_resolver + self.playback_target = playback_target_from_runtime_group(group) + self.sonos_playback_target_resolver = sonos_playback_target_resolver self.speaker_name = "unknown Sonos player" try: @@ -194,8 +194,8 @@ def _get_rollback_coordinator_for_join(speaker: SoCo) -> SoCo | None: def _refresh_speaker_metadata(self) -> dict: speaker_info = self.speaker.get_speaker_info() self.speaker_name = self._speaker_name_from_info(speaker_info) - if self.selected_group is None: - self.selected_group = self._selected_group_from_current_speaker() + if self.playback_target is None: + self.playback_target = self._playback_target_from_current_speaker() return speaker_info def _speaker_name_from_info(self, speaker_info: dict) -> str: @@ -209,7 +209,7 @@ def _speaker_name_from_info(self, speaker_info: dict) -> str: return "unknown Sonos player" - def _selected_group_from_current_speaker(self) -> SelectedSonosGroupSettings | None: + def _playback_target_from_current_speaker(self) -> SonosPlaybackTarget | None: uid = getattr(self.speaker, "uid", None) household_id = getattr(self.speaker, "household_id", None) if not isinstance(uid, str) or not uid: @@ -217,10 +217,10 @@ def _selected_group_from_current_speaker(self) -> SelectedSonosGroupSettings | N if not isinstance(household_id, str) or not household_id: return None - return SelectedSonosGroupSettings( + return SonosPlaybackTarget( household_id=household_id, coordinator_uid=uid, - members=[SelectedSonosSpeakerSettings(uid=uid)], + member_uids=(uid,), ) def _execute_with_recovery(self, command_name: str, command: Callable[[], None]) -> None: @@ -247,8 +247,8 @@ def _execute_with_recovery(self, command_name: str, command: Callable[[], None]) raise PlaybackError(str(err)) from err def _recover_speaker(self, command_name: str) -> bool: - if self.selected_group is not None: - return self._recover_selected_group(command_name) + if self.playback_target is not None: + return self._recover_playback_target(command_name) if self.manual_name is not None: return self._recover_by_name(command_name) @@ -256,18 +256,18 @@ def _recover_speaker(self, command_name: str) -> bool: LOGGER.warning("%s could not recover Sonos player because no rediscoverable target is available", command_name) return False - def _recover_selected_group(self, command_name: str) -> bool: - assert self.selected_group is not None - if self.sonos_group_resolver is None: + def _recover_playback_target(self, command_name: str) -> bool: + assert self.playback_target is not None + if self.sonos_playback_target_resolver is None: LOGGER.warning( - "%s could not re-resolve Sonos player `%s` because no Sonos group resolver is configured", + "%s could not re-resolve Sonos player `%s` because no Sonos playback target resolver is configured", command_name, self.speaker_name, ) return False try: - resolved_group = self.sonos_group_resolver.resolve_selected_group(self.selected_group) + resolved_group = self.sonos_playback_target_resolver.resolve_playback_target(self.playback_target) self._switch_to_resolved_group(resolved_group) except ( HTTPError, @@ -295,7 +295,7 @@ def _switch_to_resolved_group(self, resolved_group: ResolvedSonosGroupRuntime) - if enforce_group: self._enforce_group(resolved_group) self.group = resolved_group - self.selected_group = _selected_group_from_runtime_group(resolved_group) + self.playback_target = playback_target_from_runtime_group(resolved_group) self._refresh_speaker_metadata() self.sharelink = ShareLinkPlugin(self.speaker) @@ -346,21 +346,3 @@ def handle_uri(self, uri): if self.sharelink.is_share_link(uri): return self.sharelink.add_share_link_to_queue(uri, position=1) return self.speaker.add_uri_to_queue(uri, position=1) - - -def _selected_group_from_runtime_group( - group: ResolvedSonosGroupRuntime | None, -) -> SelectedSonosGroupSettings | None: - if group is None: - return None - - member_uids = [member.uid for member in group.members] - for uid in group.missing_member_uids: - if uid not in member_uids: - member_uids.append(uid) - - return SelectedSonosGroupSettings( - household_id=group.household_id, - coordinator_uid=group.coordinator.uid, - members=[SelectedSonosSpeakerSettings(uid=uid) for uid in member_uids], - ) diff --git a/jukebox/di_container.py b/jukebox/di_container.py index d623e94c..1474053c 100644 --- a/jukebox/di_container.py +++ b/jukebox/di_container.py @@ -16,7 +16,7 @@ from jukebox.settings.runtime_resolver import JukeboxRuntimeResolver from jukebox.settings.service_protocols import SettingsService from jukebox.shared.config_utils import get_current_tag_path -from jukebox.sonos.service import DefaultSonosService, SonosGroupResolver +from jukebox.sonos.service import DefaultSonosService, SonosPlaybackTargetResolver def build_settings_service( @@ -90,7 +90,7 @@ def build_runtime_resolver(settings_service: SettingsService) -> JukeboxRuntimeR def build_jukebox( config: ResolvedJukeboxRuntimeConfig, - sonos_group_resolver: SonosGroupResolver | None = None, + sonos_playback_target_resolver: SonosPlaybackTargetResolver | None = None, ): """Build and wire all dependencies for Jukebox.""" @@ -99,13 +99,13 @@ def build_jukebox( match config.player_type: case "sonos": - if sonos_group_resolver is None: - sonos_group_resolver = build_sonos_group_resolver() + if sonos_playback_target_resolver is None: + sonos_playback_target_resolver = build_sonos_playback_target_resolver() player = SonosPlayerAdapter( host=config.sonos_host, name=config.sonos_name, group=config.sonos_group, - sonos_group_resolver=sonos_group_resolver, + sonos_playback_target_resolver=sonos_playback_target_resolver, ) case "dryrun": player = DryrunPlayerAdapter() @@ -151,5 +151,5 @@ def build_jukebox( return reader, handle_tag_event -def build_sonos_group_resolver() -> SonosGroupResolver: +def build_sonos_playback_target_resolver() -> SonosPlaybackTargetResolver: return DefaultSonosService(SoCoSonosDiscoveryAdapter()) diff --git a/jukebox/sonos/__init__.py b/jukebox/sonos/__init__.py index 23f05a54..0b51b456 100644 --- a/jukebox/sonos/__init__.py +++ b/jukebox/sonos/__init__.py @@ -11,7 +11,13 @@ SonosSelectionResult, SonosSelectionStatus, ) -from .service import DefaultSonosService, SonosGroupResolver, SonosService +from .service import ( + DefaultSonosService, + SonosPlaybackTarget, + SonosPlaybackTargetResolver, + SonosService, + playback_target_from_runtime_group, +) __all__ = [ "DefaultSonosService", @@ -20,10 +26,12 @@ "SaveSonosSelection", "SonosDiscoveryError", "SonosDiscoveryPort", - "SonosGroupResolver", + "SonosPlaybackTarget", + "SonosPlaybackTargetResolver", "SonosSelectionAvailability", "SonosSelectionResult", "SonosSelectionStatus", "SonosService", + "playback_target_from_runtime_group", "sort_sonos_speakers", ] diff --git a/jukebox/sonos/service.py b/jukebox/sonos/service.py index 6fb2282e..a6664cad 100644 --- a/jukebox/sonos/service.py +++ b/jukebox/sonos/service.py @@ -5,6 +5,7 @@ ResolvedSonosGroupRuntime, ResolvedSonosSpeakerRuntime, SelectedSonosGroupSettings, + SelectedSonosSpeakerSettings, ) from .discovery import ( @@ -14,14 +15,29 @@ ) -class SonosGroupResolver(Protocol): - def resolve_selected_group( +@dataclass(frozen=True) +class SonosPlaybackTarget: + household_id: str | None + coordinator_uid: str + member_uids: tuple[str, ...] + + def __post_init__(self) -> None: + if not self.member_uids: + raise ValueError("Sonos playback target must include at least one member") + if len(set(self.member_uids)) != len(self.member_uids): + raise ValueError("Sonos playback target members must not contain duplicate uids") + if self.coordinator_uid not in self.member_uids: + raise ValueError("Sonos playback target coordinator must match a member uid") + + +class SonosPlaybackTargetResolver(Protocol): + def resolve_playback_target( self, - selected_group: SelectedSonosGroupSettings, + target: SonosPlaybackTarget, ) -> ResolvedSonosGroupRuntime: ... -class SonosService(SonosGroupResolver, Protocol): +class SonosService(SonosPlaybackTargetResolver, Protocol): def list_network_speakers(self) -> list[DiscoveredSonosSpeaker]: ... def inspect_selected_group( @@ -29,6 +45,11 @@ def inspect_selected_group( selected_group: SelectedSonosGroupSettings, ) -> "InspectedSelectedSonosGroup": ... + def resolve_selected_group( + self, + selected_group: SelectedSonosGroupSettings, + ) -> ResolvedSonosGroupRuntime: ... + @dataclass(frozen=True) class InspectedSelectedSonosGroup: @@ -88,6 +109,12 @@ def resolve_selected_group( missing_member_uids=inspection.missing_member_uids, ) + def resolve_playback_target( + self, + target: SonosPlaybackTarget, + ) -> ResolvedSonosGroupRuntime: + return self.resolve_selected_group(_selected_group_from_playback_target(target)) + @staticmethod def _build_runtime_speaker(speaker: DiscoveredSonosSpeaker) -> ResolvedSonosSpeakerRuntime: return ResolvedSonosSpeakerRuntime( @@ -155,3 +182,29 @@ def _inspection_needs_network_fallback( if inspection.error_message is not None: return True return len(inspection.resolved_members) != len(selected_group.members) + + +def playback_target_from_runtime_group( + group: ResolvedSonosGroupRuntime | None, +) -> SonosPlaybackTarget | None: + if group is None: + return None + + member_uids = [member.uid for member in group.members] + for uid in group.missing_member_uids: + if uid not in member_uids: + member_uids.append(uid) + + return SonosPlaybackTarget( + household_id=group.household_id, + coordinator_uid=group.coordinator.uid, + member_uids=tuple(member_uids), + ) + + +def _selected_group_from_playback_target(target: SonosPlaybackTarget) -> SelectedSonosGroupSettings: + return SelectedSonosGroupSettings( + household_id=target.household_id, + coordinator_uid=target.coordinator_uid, + members=[SelectedSonosSpeakerSettings(uid=uid) for uid in target.member_uids], + ) diff --git a/tests/jukebox/adapters/outbound/players/test_sonos_player_adapters.py b/tests/jukebox/adapters/outbound/players/test_sonos_player_adapters.py index 606d79a4..d95b86a0 100644 --- a/tests/jukebox/adapters/outbound/players/test_sonos_player_adapters.py +++ b/tests/jukebox/adapters/outbound/players/test_sonos_player_adapters.py @@ -645,16 +645,19 @@ def test_pause_recovers_selected_group_after_ip_changes(mock_sharelink, mock_soc "192.168.1.24": old_speaker, "192.168.1.25": new_speaker, }[host] - sonos_group_resolver = StubSonosService(resolved_group=new_group) + sonos_playback_target_resolver = StubSonosService(resolved_group=new_group) - adapter = SonosPlayerAdapter(group=old_group, sonos_group_resolver=sonos_group_resolver) + adapter = SonosPlayerAdapter( + group=old_group, + sonos_playback_target_resolver=sonos_playback_target_resolver, + ) adapter.pause() old_speaker.pause.assert_called_once() new_speaker.pause.assert_called_once() assert adapter.speaker is new_speaker - assert len(sonos_group_resolver.calls) == 1 - assert sonos_group_resolver.calls[0].coordinator_uid == "RINCON_949F3E8DD34001400" + assert len(sonos_playback_target_resolver.calls) == 1 + assert sonos_playback_target_resolver.calls[0].coordinator_uid == "RINCON_949F3E8DD34001400" @patch("jukebox.adapters.outbound.players.sonos_player_adapter.SoCo") @@ -667,9 +670,12 @@ def test_pause_raises_playback_error_when_recovery_fails(mock_sharelink, mock_so mock_speaker.household_id = "household-1" mock_speaker.get_speaker_info.return_value = {"software_version": "1.0", "zone_name": "Living Room"} mock_speaker.pause.side_effect = RequestConnectionError("No route to host") - sonos_group_resolver = StubSonosService(error=ValueError("not found on network")) + sonos_playback_target_resolver = StubSonosService(error=ValueError("not found on network")) - adapter = SonosPlayerAdapter(host="192.168.1.24", sonos_group_resolver=sonos_group_resolver) + adapter = SonosPlayerAdapter( + host="192.168.1.24", + sonos_playback_target_resolver=sonos_playback_target_resolver, + ) with pytest.raises(PlaybackError, match="No route to host"): adapter.pause() diff --git a/tests/jukebox/settings/_helpers.py b/tests/jukebox/settings/_helpers.py index f7a778c6..0e78346b 100644 --- a/tests/jukebox/settings/_helpers.py +++ b/tests/jukebox/settings/_helpers.py @@ -66,6 +66,13 @@ def resolve_selected_group(self, selected_group): assert self.resolved_group is not None return self.resolved_group + def resolve_playback_target(self, target): + self.calls.append(target) + if self.error is not None: + raise self.error + assert self.resolved_group is not None + return self.resolved_group + def inspect_selected_group(self, selected_group): self.calls.append(selected_group) if self.error is not None: diff --git a/tests/jukebox/settings/test_sonos_runtime.py b/tests/jukebox/settings/test_sonos_runtime.py index ada1c604..89c68dec 100644 --- a/tests/jukebox/settings/test_sonos_runtime.py +++ b/tests/jukebox/settings/test_sonos_runtime.py @@ -2,7 +2,7 @@ from jukebox.settings.entities import SelectedSonosGroupSettings, SelectedSonosSpeakerSettings from jukebox.sonos.discovery import DiscoveredSonosSpeaker -from jukebox.sonos.service import DefaultSonosService +from jukebox.sonos.service import DefaultSonosService, SonosPlaybackTarget class StubDiscovery: @@ -55,6 +55,27 @@ def test_default_sonos_service_resolves_multi_member_group_from_uids(): assert discovery.requests == [("household", "household-1")] +def test_default_sonos_service_resolves_playback_target_from_uids(): + discovery = StubDiscovery( + [ + build_discovered_speaker("speaker-1", "Kitchen", "192.168.1.30", "household-1"), + build_discovered_speaker("speaker-2", "Living Room", "192.168.1.40", "household-1"), + ] + ) + service = DefaultSonosService(discovery) + target = SonosPlaybackTarget( + household_id="household-1", + coordinator_uid="speaker-2", + member_uids=("speaker-1", "speaker-2"), + ) + + resolved_group = service.resolve_playback_target(target) + + assert resolved_group.coordinator.uid == "speaker-2" + assert [member.uid for member in resolved_group.members] == ["speaker-1", "speaker-2"] + assert discovery.requests == [("household", "household-1")] + + def test_default_sonos_service_lists_network_speakers(): discovery = StubDiscovery( [ diff --git a/tests/jukebox/test_jukebox_di_container.py b/tests/jukebox/test_jukebox_di_container.py index 2a4103b7..39366ea6 100644 --- a/tests/jukebox/test_jukebox_di_container.py +++ b/tests/jukebox/test_jukebox_di_container.py @@ -29,10 +29,10 @@ def test_build_jukebox_with_sonos_and_pn532(self, mock_library, mock_current_tag "sys.modules", {"jukebox.adapters.outbound.readers.pn532_reader_adapter": MagicMock(Pn532ReaderAdapter=mock_pn532_class)}, ) - sonos_group_resolver = MagicMock() - build_sonos_group_resolver = mocker.patch( - "jukebox.di_container.build_sonos_group_resolver", - return_value=sonos_group_resolver, + sonos_playback_target_resolver = MagicMock() + build_sonos_playback_target_resolver = mocker.patch( + "jukebox.di_container.build_sonos_playback_target_resolver", + return_value=sonos_playback_target_resolver, ) config = ResolvedJukeboxRuntimeConfig( @@ -61,9 +61,9 @@ def test_build_jukebox_with_sonos_and_pn532(self, mock_library, mock_current_tag host="192.168.1.100", name=None, group=config.sonos_group, - sonos_group_resolver=sonos_group_resolver, + sonos_playback_target_resolver=sonos_playback_target_resolver, ) - build_sonos_group_resolver.assert_called_once_with() + build_sonos_playback_target_resolver.assert_called_once_with() mock_pn532_class.assert_called_once_with( read_timeout_seconds=0.25, spi_reset=20, @@ -93,9 +93,12 @@ def test_build_jukebox_with_sonos_name(self, mock_library, mock_current_tag, moc pn532_connection=SpiConnectionParams(reset=20, cs=4, irq=None), verbose=False, ) - sonos_group_resolver = MagicMock() + sonos_playback_target_resolver = MagicMock() - reader, handle_tag_event = build_jukebox(config, sonos_group_resolver=sonos_group_resolver) + reader, handle_tag_event = build_jukebox( + config, + sonos_playback_target_resolver=sonos_playback_target_resolver, + ) mock_library.assert_called_once_with("/test/library.json") mock_current_tag.assert_called_once_with("/test/current-tag.txt") @@ -103,7 +106,7 @@ def test_build_jukebox_with_sonos_name(self, mock_library, mock_current_tag, moc host=None, name="Living Room", group=None, - sonos_group_resolver=sonos_group_resolver, + sonos_playback_target_resolver=sonos_playback_target_resolver, ) mock_reader.assert_called_once_with() assert reader == mock_reader.return_value @@ -129,9 +132,12 @@ def test_build_jukebox_with_sonos_autodiscovery(self, mock_library, mock_current pn532_connection=SpiConnectionParams(reset=20, cs=4, irq=None), verbose=False, ) - sonos_group_resolver = MagicMock() + sonos_playback_target_resolver = MagicMock() - reader, handle_tag_event = build_jukebox(config, sonos_group_resolver=sonos_group_resolver) + reader, handle_tag_event = build_jukebox( + config, + sonos_playback_target_resolver=sonos_playback_target_resolver, + ) mock_library.assert_called_once_with("/test/library.json") mock_current_tag.assert_called_once_with("/test/current-tag.txt") @@ -139,7 +145,7 @@ def test_build_jukebox_with_sonos_autodiscovery(self, mock_library, mock_current host=None, name=None, group=None, - sonos_group_resolver=sonos_group_resolver, + sonos_playback_target_resolver=sonos_playback_target_resolver, ) mock_reader.assert_called_once_with() assert reader == mock_reader.return_value From bc06059b25ccb81a6f2d55642109afd304fd5cbe Mon Sep 17 00:00:00 2001 From: Michael Gerbush <405526+msgerbush@users.noreply.github.com> Date: Tue, 12 May 2026 23:27:22 -0400 Subject: [PATCH 5/7] Require Sonos playback target resolver --- .../outbound/players/sonos_player_adapter.py | 21 ++---- .../players/test_sonos_player_adapters.py | 69 +++++++++++-------- 2 files changed, 47 insertions(+), 43 deletions(-) diff --git a/jukebox/adapters/outbound/players/sonos_player_adapter.py b/jukebox/adapters/outbound/players/sonos_player_adapter.py index c8074142..2a01d351 100644 --- a/jukebox/adapters/outbound/players/sonos_player_adapter.py +++ b/jukebox/adapters/outbound/players/sonos_player_adapter.py @@ -43,7 +43,8 @@ def __init__( host: str | None = None, name: str | None = None, group: ResolvedSonosGroupRuntime | None = None, - sonos_playback_target_resolver: SonosPlaybackTargetResolver | None = None, + *, + sonos_playback_target_resolver: SonosPlaybackTargetResolver, ): self.manual_name = name self.group = group @@ -247,8 +248,9 @@ def _execute_with_recovery(self, command_name: str, command: Callable[[], None]) raise PlaybackError(str(err)) from err def _recover_speaker(self, command_name: str) -> bool: - if self.playback_target is not None: - return self._recover_playback_target(command_name) + playback_target = self.playback_target + if playback_target is not None: + return self._recover_playback_target(command_name, playback_target) if self.manual_name is not None: return self._recover_by_name(command_name) @@ -256,18 +258,9 @@ def _recover_speaker(self, command_name: str) -> bool: LOGGER.warning("%s could not recover Sonos player because no rediscoverable target is available", command_name) return False - def _recover_playback_target(self, command_name: str) -> bool: - assert self.playback_target is not None - if self.sonos_playback_target_resolver is None: - LOGGER.warning( - "%s could not re-resolve Sonos player `%s` because no Sonos playback target resolver is configured", - command_name, - self.speaker_name, - ) - return False - + def _recover_playback_target(self, command_name: str, playback_target: SonosPlaybackTarget) -> bool: try: - resolved_group = self.sonos_playback_target_resolver.resolve_playback_target(self.playback_target) + resolved_group = self.sonos_playback_target_resolver.resolve_playback_target(playback_target) self._switch_to_resolved_group(resolved_group) except ( HTTPError, diff --git a/tests/jukebox/adapters/outbound/players/test_sonos_player_adapters.py b/tests/jukebox/adapters/outbound/players/test_sonos_player_adapters.py index d95b86a0..56435d36 100644 --- a/tests/jukebox/adapters/outbound/players/test_sonos_player_adapters.py +++ b/tests/jukebox/adapters/outbound/players/test_sonos_player_adapters.py @@ -1,3 +1,4 @@ +from typing import Any from unittest.mock import MagicMock, patch import pytest @@ -14,10 +15,20 @@ def make_exception(code: str): return SoCoUPnPException(f"UPnP Error {code} received", code, f"{code}") +def build_adapter(**kwargs: Any) -> SonosPlayerAdapter: + kwargs.setdefault("sonos_playback_target_resolver", MagicMock()) + return SonosPlayerAdapter(**kwargs) + + +def test_init_requires_playback_target_resolver(): + with pytest.raises(TypeError, match="sonos_playback_target_resolver"): + SonosPlayerAdapter(host="192.168.1.100") + + @patch("jukebox.adapters.outbound.players.sonos_player_adapter.SoCo") @patch("jukebox.adapters.outbound.players.sonos_player_adapter.ShareLinkPlugin") def test_init_with_host(mock_sharelink, mock_soco): - SonosPlayerAdapter(host="192.168.1.100") + build_adapter(host="192.168.1.100") mock_soco.assert_called_once_with("192.168.1.100") mock_sharelink.assert_called_once_with(mock_soco.return_value) @@ -30,7 +41,7 @@ def test_init_without_host_triggers_discovery(mock_sharelink, mock_soco_module): mock_speaker.player_name = "Living Room" mock_soco_module.discover.return_value = {mock_speaker} - adapter = SonosPlayerAdapter() + adapter = build_adapter() mock_soco_module.discover.assert_called_once() mock_sharelink.assert_called_once_with(mock_speaker) @@ -44,7 +55,7 @@ def test_init_without_host_raises_when_no_speakers_found(mock_sharelink, mock_so mock_soco_module.discover.return_value = None with pytest.raises(InvalidSettingsError, match="No Sonos speakers found on the network"): - SonosPlayerAdapter() + build_adapter() mock_sharelink.assert_not_called() @@ -60,7 +71,7 @@ def test_init_discovery_picks_first_speaker_alphabetically(mock_sharelink, mock_ mock_soco_module.discover.return_value = {speaker_b, speaker_a} - adapter = SonosPlayerAdapter() + adapter = build_adapter() assert adapter.speaker is speaker_a @@ -75,7 +86,7 @@ def test_init_with_name_selects_matching_speaker(mock_sharelink, mock_soco_modul speaker_b.player_name = "Living Room" mock_soco_module.discover.return_value = {speaker_a, speaker_b} - adapter = SonosPlayerAdapter(name="Living Room") + adapter = build_adapter(name="Living Room") assert adapter.speaker is speaker_b @@ -90,7 +101,7 @@ def test_init_with_name_raises_when_speaker_not_found(mock_sharelink, mock_soco_ mock_soco_module.discover.return_value = {mock_speaker} with pytest.raises(InvalidSettingsError, match="No Sonos speaker named 'Bedroom' found on the network"): - SonosPlayerAdapter(name="Bedroom") + build_adapter(name="Bedroom") mock_sharelink.assert_not_called() @@ -129,7 +140,7 @@ def test_init_with_resolved_group_enforces_membership_before_playback(mock_share ], ) - adapter = SonosPlayerAdapter(group=group) + adapter = build_adapter(group=group) kitchen.join.assert_called_once_with(coordinator) extra.unjoin.assert_called_once_with() @@ -184,7 +195,7 @@ def test_init_with_resolved_group_preserves_nonvisible_members(mock_sharelink, m ], ) - SonosPlayerAdapter(group=group) + build_adapter(group=group) kitchen.join.assert_called_once_with(coordinator) extra.unjoin.assert_called_once_with() @@ -236,7 +247,7 @@ def test_init_with_partial_group_prunes_extras_but_keeps_missing_desired_members missing_member_uids=["speaker-3"], ) - SonosPlayerAdapter(group=group) + build_adapter(group=group) kitchen.join.assert_called_once_with(coordinator) extra.unjoin.assert_called_once_with() @@ -256,7 +267,7 @@ def test_init_with_one_member_resolved_group_preserves_single_speaker_behavior(m group = build_resolved_sonos_group_runtime() - adapter = SonosPlayerAdapter(group=group) + adapter = build_adapter(group=group) speaker.join.assert_not_called() speaker.unjoin.assert_not_called() @@ -270,7 +281,7 @@ def test_init_with_host_wraps_network_errors(mock_sharelink, mock_soco): mock_soco.side_effect = TimeoutError("timed out") with pytest.raises(InvalidSettingsError, match="Failed to initialize Sonos player: timed out"): - SonosPlayerAdapter(host="192.168.1.100") + build_adapter(host="192.168.1.100") mock_sharelink.assert_not_called() @@ -305,7 +316,7 @@ def test_init_with_group_wraps_group_enforcement_errors(mock_sharelink, mock_soc ) with pytest.raises(InvalidSettingsError, match="Failed to initialize Sonos player: join timed out"): - SonosPlayerAdapter(group=group) + build_adapter(group=group) mock_sharelink.assert_not_called() @@ -343,7 +354,7 @@ def test_init_with_group_join_failure_does_not_remove_existing_members(mock_shar ) with pytest.raises(InvalidSettingsError, match="Failed to initialize Sonos player: join timed out"): - SonosPlayerAdapter(group=group) + build_adapter(group=group) extra.unjoin.assert_not_called() mock_sharelink.assert_not_called() @@ -386,7 +397,7 @@ def test_init_with_group_failure_rolls_back_earlier_joins(mock_sharelink, mock_s ) with pytest.raises(InvalidSettingsError, match="Failed to initialize Sonos player: join timed out"): - SonosPlayerAdapter(group=group) + build_adapter(group=group) kitchen.join.assert_called_once_with(coordinator) kitchen.unjoin.assert_called_once_with() @@ -434,7 +445,7 @@ def test_init_with_group_failure_restores_joined_member_to_original_group(mock_s ) with pytest.raises(InvalidSettingsError, match="Failed to initialize Sonos player: join timed out"): - SonosPlayerAdapter(group=group) + build_adapter(group=group) assert kitchen.join.call_args_list == [((coordinator,),), ((old_coordinator,),)] kitchen.unjoin.assert_not_called() @@ -483,7 +494,7 @@ def test_init_with_group_failure_rolls_back_earlier_removals(mock_sharelink, moc ) with pytest.raises(InvalidSettingsError, match="Failed to initialize Sonos player: unjoin timed out"): - SonosPlayerAdapter(group=group) + build_adapter(group=group) extra_one.unjoin.assert_called_once_with() extra_one.join.assert_called_once_with(coordinator) @@ -518,7 +529,7 @@ def test_play_does_not_reenforce_group_after_startup(mock_sharelink, mock_soco): ], ) - adapter = SonosPlayerAdapter(group=group) + adapter = build_adapter(group=group) kitchen.join.reset_mock() coordinator.unjoin.reset_mock() mock_soco.reset_mock() @@ -538,7 +549,7 @@ def test_play_calls_underlying_sonos_player(mock_sharelink, mock_soco): mock_soco.return_value = mock_speaker mock_speaker.get_speaker_info.return_value = {"software_version": "1.0"} - adapter = SonosPlayerAdapter(host="192.168.1.100") + adapter = build_adapter(host="192.168.1.100") adapter.play("uri:123", shuffle=False) mock_speaker.clear_queue.assert_called_once_with() @@ -558,7 +569,7 @@ def test_play_calls_underlying_sonos_player_for_non_share_link(mock_sharelink, m mock_sharelink.return_value = mock_sharelink_value mock_sharelink_value.is_share_link = lambda x: False - adapter = SonosPlayerAdapter(host="192.168.1.100") + adapter = build_adapter(host="192.168.1.100") adapter.play("non-share-link", shuffle=False) mock_speaker.clear_queue.assert_called_once_with() @@ -575,7 +586,7 @@ def test_play_with_shuffle(mock_sharelink, mock_soco): mock_soco.return_value = mock_speaker mock_speaker.get_speaker_info.return_value = {"software_version": "1.0"} - adapter = SonosPlayerAdapter(host="192.168.1.100") + adapter = build_adapter(host="192.168.1.100") adapter.play("uri:456", shuffle=True) assert mock_speaker.play_mode == "SHUFFLE_NOREPEAT" @@ -589,7 +600,7 @@ def test_pause_calls_underlying_sonos_player(mock_sharelink, mock_soco): mock_soco.return_value = mock_speaker mock_speaker.get_speaker_info.return_value = {"software_version": "1.0"} - adapter = SonosPlayerAdapter(host="192.168.1.100") + adapter = build_adapter(host="192.168.1.100") adapter.pause() mock_speaker.pause.assert_called_once() @@ -612,7 +623,7 @@ def player_name(self): speaker = SpeakerWithNetworkedName() mock_soco.return_value = speaker - adapter = SonosPlayerAdapter(host="192.168.1.100") + adapter = build_adapter(host="192.168.1.100") adapter.pause() speaker.pause.assert_called_once() @@ -647,7 +658,7 @@ def test_pause_recovers_selected_group_after_ip_changes(mock_sharelink, mock_soc }[host] sonos_playback_target_resolver = StubSonosService(resolved_group=new_group) - adapter = SonosPlayerAdapter( + adapter = build_adapter( group=old_group, sonos_playback_target_resolver=sonos_playback_target_resolver, ) @@ -672,7 +683,7 @@ def test_pause_raises_playback_error_when_recovery_fails(mock_sharelink, mock_so mock_speaker.pause.side_effect = RequestConnectionError("No route to host") sonos_playback_target_resolver = StubSonosService(error=ValueError("not found on network")) - adapter = SonosPlayerAdapter( + adapter = build_adapter( host="192.168.1.24", sonos_playback_target_resolver=sonos_playback_target_resolver, ) @@ -691,7 +702,7 @@ def test_resume_calls_underlying_sonos_player(mock_sharelink, mock_soco): mock_soco.return_value = mock_speaker mock_speaker.get_speaker_info.return_value = {"software_version": "1.0"} - adapter = SonosPlayerAdapter(host="192.168.1.100") + adapter = build_adapter(host="192.168.1.100") adapter.resume() mock_speaker.play.assert_called_once() @@ -705,7 +716,7 @@ def test_stop_calls_underlying_sonos_player(mock_sharelink, mock_soco): mock_soco.return_value = mock_speaker mock_speaker.get_speaker_info.return_value = {"software_version": "1.0"} - adapter = SonosPlayerAdapter(host="192.168.1.100") + adapter = build_adapter(host="192.168.1.100") adapter.stop() mock_speaker.clear_queue.assert_called_once() @@ -723,7 +734,7 @@ def test_init_with_duplicate_speaker_names_logs_warning(mock_sharelink, mock_soc speaker_c.player_name = "Kitchen" mock_soco_module.discover.return_value = [speaker_a, speaker_b, speaker_c] - adapter = SonosPlayerAdapter(name="Kitchen") + adapter = build_adapter(name="Kitchen") assert adapter.speaker.player_name == "Kitchen" assert "Multiple Sonos speakers with name 'Kitchen' found. Using first match." in caplog.text @@ -755,7 +766,7 @@ def test_methods_log_and_raise_on_known_upnp_error( getattr(mock_speaker, soco_method).side_effect = make_exception(error_code) - adapter = SonosPlayerAdapter(host="192.168.1.100") + adapter = build_adapter(host="192.168.1.100") with pytest.raises(PlaybackError): getattr(adapter, adapter_method)(*args) @@ -771,7 +782,7 @@ def test_methods_log_exception_for_unknown_upnp_error(mock_soco, caplog): mock_speaker.get_speaker_info.return_value = {"software_version": "1.0"} mock_speaker.pause.side_effect = make_exception("999") - adapter = SonosPlayerAdapter(host="192.168.1.100") + adapter = build_adapter(host="192.168.1.100") with pytest.raises(PlaybackError), caplog.at_level("ERROR"): adapter.pause() From 03be8b3e2d205b9ccd6a3d6f292a42e7268900aa Mon Sep 17 00:00:00 2001 From: Michael Gerbush <405526+msgerbush@users.noreply.github.com> Date: Tue, 12 May 2026 23:35:43 -0400 Subject: [PATCH 6/7] Remove redundant Sonos constructor test --- .../adapters/outbound/players/test_sonos_player_adapters.py | 5 ----- 1 file changed, 5 deletions(-) diff --git a/tests/jukebox/adapters/outbound/players/test_sonos_player_adapters.py b/tests/jukebox/adapters/outbound/players/test_sonos_player_adapters.py index 56435d36..2bb5b786 100644 --- a/tests/jukebox/adapters/outbound/players/test_sonos_player_adapters.py +++ b/tests/jukebox/adapters/outbound/players/test_sonos_player_adapters.py @@ -20,11 +20,6 @@ def build_adapter(**kwargs: Any) -> SonosPlayerAdapter: return SonosPlayerAdapter(**kwargs) -def test_init_requires_playback_target_resolver(): - with pytest.raises(TypeError, match="sonos_playback_target_resolver"): - SonosPlayerAdapter(host="192.168.1.100") - - @patch("jukebox.adapters.outbound.players.sonos_player_adapter.SoCo") @patch("jukebox.adapters.outbound.players.sonos_player_adapter.ShareLinkPlugin") def test_init_with_host(mock_sharelink, mock_soco): From c8044f1039ee3d3609625362d22b8697790192cb Mon Sep 17 00:00:00 2001 From: Michael Gerbush <405526+msgerbush@users.noreply.github.com> Date: Wed, 20 May 2026 22:41:42 -0400 Subject: [PATCH 7/7] Separate Sonos recovery switch failures --- .../outbound/players/sonos_player_adapter.py | 12 ++- .../players/test_sonos_player_adapters.py | 85 +++++++++++++++++++ 2 files changed, 96 insertions(+), 1 deletion(-) diff --git a/jukebox/adapters/outbound/players/sonos_player_adapter.py b/jukebox/adapters/outbound/players/sonos_player_adapter.py index 2a01d351..eb6a9cb4 100644 --- a/jukebox/adapters/outbound/players/sonos_player_adapter.py +++ b/jukebox/adapters/outbound/players/sonos_player_adapter.py @@ -261,7 +261,6 @@ def _recover_speaker(self, command_name: str) -> bool: def _recover_playback_target(self, command_name: str, playback_target: SonosPlaybackTarget) -> bool: try: resolved_group = self.sonos_playback_target_resolver.resolve_playback_target(playback_target) - self._switch_to_resolved_group(resolved_group) except ( HTTPError, OSError, @@ -274,6 +273,17 @@ def _recover_playback_target(self, command_name: str, playback_target: SonosPlay LOGGER.warning("%s could not re-resolve Sonos player `%s`: %s", command_name, self.speaker_name, err) return False + try: + self._switch_to_resolved_group(resolved_group) + except (HTTPError, OSError, RequestException, RuntimeError, SoCoException, SoCoUPnPException) as err: + LOGGER.warning( + "%s recovered Sonos player `%s` but failed during group switch: %s", + command_name, + self.speaker_name, + err, + ) + return False + LOGGER.info( "%s recovered Sonos player `%s` at `%s`", command_name, diff --git a/tests/jukebox/adapters/outbound/players/test_sonos_player_adapters.py b/tests/jukebox/adapters/outbound/players/test_sonos_player_adapters.py index 2bb5b786..8a012b71 100644 --- a/tests/jukebox/adapters/outbound/players/test_sonos_player_adapters.py +++ b/tests/jukebox/adapters/outbound/players/test_sonos_player_adapters.py @@ -689,6 +689,91 @@ def test_pause_raises_playback_error_when_recovery_fails(mock_sharelink, mock_so assert "pause could not re-resolve Sonos player `Living Room`: not found on network" in caplog.text +@patch("jukebox.adapters.outbound.players.sonos_player_adapter.SoCo") +@patch("jukebox.adapters.outbound.players.sonos_player_adapter.ShareLinkPlugin") +def test_pause_reports_group_switch_failure_separately(mock_sharelink, mock_soco, caplog): + """Should distinguish successful target resolution from a later failed group switch.""" + old_group = build_resolved_sonos_group_runtime( + coordinator_uid="RINCON_949F3E8DD34001400", + speakers=[("RINCON_949F3E8DD34001400", "Living Room", "192.168.1.24", "household-1")], + ) + new_group = build_resolved_sonos_group_runtime( + coordinator_uid="RINCON_949F3E8DD34001400", + speakers=[("RINCON_949F3E8DD34001400", "Living Room", "192.168.1.25", "household-1")], + ) + old_speaker = MagicMock() + old_speaker.uid = "RINCON_949F3E8DD34001400" + old_speaker.household_id = "household-1" + old_speaker.group = None + old_speaker.get_speaker_info.return_value = {"software_version": "1.0", "zone_name": "Living Room"} + old_speaker.pause.side_effect = RequestConnectionError("No route to host") + + def build_soco(host: str): + if host == "192.168.1.24": + return old_speaker + if host == "192.168.1.25": + raise TimeoutError("switch timed out") + raise AssertionError(f"unexpected Sonos host: {host}") + + mock_soco.side_effect = build_soco + sonos_playback_target_resolver = StubSonosService(resolved_group=new_group) + + adapter = build_adapter( + group=old_group, + sonos_playback_target_resolver=sonos_playback_target_resolver, + ) + with pytest.raises(PlaybackError, match="No route to host"): + adapter.pause() + + old_speaker.pause.assert_called_once() + assert "pause recovered Sonos player `Living Room` but failed during group switch: switch timed out" in caplog.text + assert "pause could not re-resolve Sonos player `Living Room`" not in caplog.text + + +@patch("jukebox.adapters.outbound.players.sonos_player_adapter.SoCo") +@patch("jukebox.adapters.outbound.players.sonos_player_adapter.ShareLinkPlugin") +def test_pause_does_not_swallow_group_switch_invariant_failures(mock_sharelink, mock_soco, caplog): + """Should not treat adapter invariant failures as rediscovery misses.""" + old_group = build_resolved_sonos_group_runtime( + coordinator_uid="RINCON_949F3E8DD34001400", + speakers=[("RINCON_949F3E8DD34001400", "Living Room", "192.168.1.24", "household-1")], + ) + new_group = build_resolved_sonos_group_runtime( + coordinator_uid="RINCON_949F3E8DD34001400", + speakers=[("RINCON_949F3E8DD34001400", "Living Room", "192.168.1.25", "household-1")], + ) + old_speaker = MagicMock() + old_speaker.uid = "RINCON_949F3E8DD34001400" + old_speaker.household_id = "household-1" + old_speaker.group = None + old_speaker.get_speaker_info.return_value = {"software_version": "1.0", "zone_name": "Living Room"} + old_speaker.pause.side_effect = RequestConnectionError("No route to host") + new_speaker = MagicMock() + new_speaker.uid = "RINCON_949F3E8DD34001400" + new_speaker.household_id = "household-1" + new_speaker.group = None + mock_soco.side_effect = lambda host: { + "192.168.1.24": old_speaker, + "192.168.1.25": new_speaker, + }[host] + sonos_playback_target_resolver = StubSonosService(resolved_group=new_group) + + adapter = build_adapter( + group=old_group, + sonos_playback_target_resolver=sonos_playback_target_resolver, + ) + with ( + patch( + "jukebox.adapters.outbound.players.sonos_player_adapter.playback_target_from_runtime_group", + side_effect=ValueError("invalid playback target"), + ), + pytest.raises(ValueError, match="invalid playback target"), + ): + adapter.pause() + + assert "pause could not re-resolve Sonos player `Living Room`" not in caplog.text + + @patch("jukebox.adapters.outbound.players.sonos_player_adapter.SoCo") @patch("jukebox.adapters.outbound.players.sonos_player_adapter.ShareLinkPlugin") def test_resume_calls_underlying_sonos_player(mock_sharelink, mock_soco):