Chore: Several fixes and optimizations for the Sonos provider
authorMarcel van der Veldt <m.vanderveldt@outlook.com>
Fri, 24 Jan 2025 09:46:47 +0000 (10:46 +0100)
committerMarcel van der Veldt <m.vanderveldt@outlook.com>
Fri, 24 Jan 2025 11:23:15 +0000 (12:23 +0100)
music_assistant/controllers/players.py
music_assistant/providers/sonos/player.py
music_assistant/providers/sonos/provider.py

index 511259a4167f8aa841dcfd2116f6f947a4fd81bb..43773d7b313ed5cb0c794ea61341517087541484 100644 (file)
@@ -54,6 +54,7 @@ if TYPE_CHECKING:
     from collections.abc import Awaitable, Callable, Coroutine, Iterator
 
     from music_assistant_models.config_entries import CoreConfig, PlayerConfig
+    from music_assistant_models.player_queue import PlayerQueue
 
 
 _PlayerControllerT = TypeVar("_PlayerControllerT", bound="PlayerController")
@@ -1137,19 +1138,21 @@ class PlayerController(CoreController):
     async def on_player_config_change(self, config: PlayerConfig, changed_keys: set[str]) -> None:
         """Call (by config manager) when the configuration of a player changes."""
         player_disabled = "enabled" in changed_keys and not config.enabled
+        if not (player := self.get(config.player_id)):
+            return
+        resume_queue: PlayerQueue | None = self.mass.player_queues.get(player.active_source)
         # signal player provider that the config changed
         if player_provider := self.mass.get_provider(config.provider):
             with suppress(PlayerUnavailableError):
                 await player_provider.on_player_config_change(config, changed_keys)
-        if not (player := self.get(config.player_id)):
-            return
         if player_disabled:
             # edge case: ensure that the player is powered off if the player gets disabled
             await self.cmd_power(config.player_id, False)
             player.available = False
-        # if the player was playing, restart playback
-        elif not player_disabled and player.state == PlayerState.PLAYING:
-            self.mass.call_later(1, self.mass.player_queues.resume, player.active_source)
+        # if the PlayerQueue was playing, restart playback
+        # TODO: add property to ConfigEntry if it requires a restart of playback on change
+        elif not player_disabled and resume_queue.state == PlayerState.PLAYING:
+            self.mass.call_later(1, self.mass.player_queues.resume, resume_queue.queue_id)
         # check for group memberships that need to be updated
         if player_disabled and player.active_group and player_provider:
             # try to remove from the group
index 561781628de55d2bcfcc1d563f362dd391e2ee88..0b64211995c4a8332a40c61fa873a3575eedf939 100644 (file)
@@ -17,7 +17,6 @@ from typing import TYPE_CHECKING
 import shortuuid
 from aiohttp.client_exceptions import ClientConnectorError
 from aiosonos.api.models import ContainerType, MusicService, SonosCapability
-from aiosonos.api.models import PlayBackState as SonosPlayBackState
 from aiosonos.client import SonosLocalApiClient
 from aiosonos.const import EventType as SonosEventType
 from aiosonos.const import SonosEvent
@@ -86,6 +85,18 @@ class SonosPlayer:
             self.player_id, CONF_AIRPLAY_MODE, False
         )
 
+    @property
+    def airplay_mode_active(self) -> bool:
+        """Return if airplay mode is active for the player."""
+        return (
+            self.airplay_mode_enabled
+            and self.client.player.is_coordinator
+            and (active_group := self.client.player.group)
+            and active_group.container_type == ContainerType.AIRPLAY
+            and (airplay_player := self.get_linked_airplay_player(False))
+            and airplay_player.state in (PlayerState.PLAYING, PlayerState.PAUSED)
+        )
+
     def get_linked_airplay_player(self, enabled_only: bool = True) -> Player | None:
         """Return the linked airplay player if available/enabled."""
         if enabled_only and not self.airplay_mode_enabled:
@@ -118,13 +129,8 @@ class SonosPlayer:
             name=self.discovery_info["device"]["name"]
             or self.discovery_info["device"]["modelDisplayName"],
             available=True,
-            # treat as powered at start if the player is playing/paused
-            powered=self.client.player.group.playback_state
-            in (
-                SonosPlayBackState.PLAYBACK_STATE_PLAYING,
-                SonosPlayBackState.PLAYBACK_STATE_BUFFERING,
-                SonosPlayBackState.PLAYBACK_STATE_PAUSED,
-            ),
+            # sonos has no power support so we always assume its powered
+            powered=True,
             device_info=DeviceInfo(
                 model=self.discovery_info["device"]["modelDisplayName"],
                 manufacturer=self.prov.manifest.name,
@@ -157,11 +163,18 @@ class SonosPlayer:
             )
         )
         # register callback for playerqueue state changes
+        # note we don't filter on the player_id here because we also need to catch
+        # events from group players
         self._on_cleanup_callbacks.append(
             self.mass.subscribe(
                 self._on_mass_queue_items_event,
                 EventType.QUEUE_ITEMS_UPDATED,
-                self.player_id,
+            )
+        )
+        self._on_cleanup_callbacks.append(
+            self.mass.subscribe(
+                self._on_mass_queue_event,
+                EventType.QUEUE_UPDATED,
             )
         )
 
@@ -183,7 +196,7 @@ class SonosPlayer:
         if self.client.player.is_passive:
             self.logger.debug("Ignore STOP command: Player is synced to another player.")
             return
-        if (airplay := self.get_linked_airplay_player(True)) and airplay.state != PlayerState.IDLE:
+        if (airplay := self.get_linked_airplay_player(True)) and self.airplay_mode_active:
             # linked airplay player is active, redirect the command
             self.logger.debug("Redirecting STOP command to linked airplay player.")
             if player_provider := self.mass.get_provider(airplay.provider):
@@ -213,7 +226,7 @@ class SonosPlayer:
         if self.client.player.is_passive:
             self.logger.debug("Ignore STOP command: Player is synced to another player.")
             return
-        if (airplay := self.get_linked_airplay_player(True)) and airplay.state != PlayerState.IDLE:
+        if (airplay := self.get_linked_airplay_player(True)) and self.airplay_mode_active:
             # linked airplay player is active, redirect the command
             self.logger.debug("Redirecting PAUSE command to linked airplay player.")
             if player_provider := self.mass.get_provider(airplay.provider):
@@ -460,6 +473,8 @@ class SonosPlayer:
             return
         if not self.connected:
             return
+        if not self.client.player.is_coordinator:
+            return
         # sync crossfade and repeat modes
         queue = self.mass.player_queues.get(event.object_id)
         if not queue or queue.state not in (PlayerState.PLAYING, PlayerState.PAUSED):
index 986a4e578e1342adac6d2b53673e9d29cde0d810..ec937025ed93c841001d62a04cf94a17163b369b 100644 (file)
@@ -9,14 +9,14 @@ from __future__ import annotations
 
 import asyncio
 import time
-from typing import TYPE_CHECKING
+from typing import TYPE_CHECKING, Any
 
 import shortuuid
 from aiohttp import web
 from aiohttp.client_exceptions import ClientError
 from aiosonos.api.models import SonosCapability
 from aiosonos.utils import get_discovery_info
-from music_assistant_models.config_entries import ConfigEntry
+from music_assistant_models.config_entries import ConfigEntry, PlayerConfig
 from music_assistant_models.enums import ConfigEntryType, ContentType, ProviderFeature
 from music_assistant_models.errors import PlayerCommandFailed
 from music_assistant_models.player import DeviceInfo, PlayerMedia
@@ -38,6 +38,7 @@ from .helpers import get_primary_ip_address
 from .player import SonosPlayer
 
 if TYPE_CHECKING:
+    from music_assistant_models.queue_item import QueueItem
     from zeroconf.asyncio import AsyncServiceInfo
 
 CONF_IPS = "ips"
@@ -193,6 +194,22 @@ class SonosPlayerProvider(PlayerProvider):
             ),
         )
 
+    async def on_player_config_change(self, config: PlayerConfig, changed_keys: set[str]) -> None:
+        """Call (by config manager) when the configuration of a player changes."""
+        await super().on_player_config_change(config, changed_keys)
+        if "values/airplay_mode" in changed_keys and (
+            (sonos_player := self.sonos_players.get(config.player_id))
+            and (airplay_player := sonos_player.get_linked_airplay_player(False))
+            and airplay_player.active_source == sonos_player.mass_player.active_source
+        ):
+            # edge case: we switched from airplay mode to sonos mode (or vice versa)
+            # we need to make sure that playback gets stopped on the airplay player
+            if airplay_prov := self.mass.get_provider(airplay_player.provider):
+                await airplay_prov.cmd_stop(airplay_player.player_id)
+                airplay_player.active_source = None
+            if not sonos_player.airplay_mode_enabled:
+                await self.mass.players.cmd_power(config.player_id, False)
+
     async def cmd_stop(self, player_id: str) -> None:
         """Send STOP command to given player."""
         if sonos_player := self.sonos_players[player_id]:
@@ -234,6 +251,11 @@ class SonosPlayerProvider(PlayerProvider):
         if airplay_player := sonos_player.get_linked_airplay_player(False):
             # if airplay mode is enabled, we could possibly receive child player id's that are
             # not Sonos players, but Airplay players. We redirect those.
+            if sonos_player.mass_player.active_source and not self.mass.player_queues.get(
+                sonos_player.mass_player.active_source
+            ):
+                # edge case player is not playing a MA queue - fail this request
+                raise PlayerCommandFailed("Player is not playing a Music Assistant queue.")
             airplay_child_ids = [x for x in child_player_ids if x.startswith("ap")]
             child_player_ids = [x for x in child_player_ids if x not in airplay_child_ids]
             if airplay_child_ids:
@@ -416,51 +438,7 @@ class SonosPlayerProvider(PlayerProvider):
             sonos_player_id, CONF_ENTRY_ENFORCE_MP3.key, CONF_ENTRY_ENFORCE_MP3.default_value
         )
         sonos_queue_items = [
-            {
-                "id": item.queue_item_id,
-                "deleted": not item.media_item.available,
-                "policies": {},
-                "track": {
-                    "type": "track",
-                    "mediaUrl": self.mass.streams.resolve_stream_url(
-                        item, output_codec=ContentType.MP3 if enforce_mp3 else ContentType.FLAC
-                    ),
-                    "contentType": "audio/flac",
-                    "service": {
-                        "name": "Music Assistant",
-                        "id": "8",
-                        "accountId": "",
-                        "objectId": item.queue_item_id,
-                    },
-                    "name": item.name,
-                    "imageUrl": self.mass.metadata.get_image_url(
-                        item.image, prefer_proxy=False, image_format="jpeg"
-                    )
-                    if item.image
-                    else None,
-                    "durationMillis": item.duration * 1000 if item.duration else None,
-                    "artist": {
-                        "name": artist_str,
-                    }
-                    if item.media_item
-                    and (artist_str := getattr(item.media_item, "artist_str", None))
-                    else None,
-                    "album": {
-                        "name": album.name,
-                    }
-                    if item.media_item and (album := getattr(item.media_item, "album", None))
-                    else None,
-                    "quality": {
-                        "bitDepth": item.streamdetails.audio_format.bit_depth,
-                        "sampleRate": item.streamdetails.audio_format.sample_rate,
-                        "codec": item.streamdetails.audio_format.content_type.value,
-                        "lossless": item.streamdetails.audio_format.content_type.is_lossless(),
-                    }
-                    if item.streamdetails
-                    else None,
-                },
-            }
-            for item in queue_items
+            self._parse_sonos_queue_item(item, enforce_mp3) for item in queue_items
         ]
         result = {
             "includesBeginningOfQueue": offset == 0,
@@ -564,3 +542,52 @@ class SonosPlayerProvider(PlayerProvider):
             self.mass.players.update(sonos_player_id)
             break
         return web.Response(status=204)
+
+    def _parse_sonos_queue_item(self, queue_item: QueueItem, enforce_mp3: bool) -> dict[str, Any]:
+        """Parse a Sonos queue item to a PlayerMedia object."""
+        available = queue_item.media_item.available if queue_item.media_item else True
+        return {
+            "id": queue_item.queue_item_id,
+            "deleted": not available,
+            "policies": {},
+            "track": {
+                "type": "track",
+                "mediaUrl": self.mass.streams.resolve_stream_url(
+                    queue_item, output_codec=ContentType.MP3 if enforce_mp3 else ContentType.FLAC
+                ),
+                "contentType": "audio/flac",
+                "service": {
+                    "name": "Music Assistant",
+                    "id": "8",
+                    "accountId": "",
+                    "objectId": queue_item.queue_item_id,
+                },
+                "name": queue_item.media_item.name if queue_item.media_item else queue_item.name,
+                "imageUrl": self.mass.metadata.get_image_url(
+                    queue_item.image, prefer_proxy=False, image_format="jpeg"
+                )
+                if queue_item.image
+                else None,
+                "durationMillis": queue_item.duration * 1000 if queue_item.duration else None,
+                "artist": {
+                    "name": artist_str,
+                }
+                if queue_item.media_item
+                and (artist_str := getattr(queue_item.media_item, "artist_str", None))
+                else None,
+                "album": {
+                    "name": album.name,
+                }
+                if queue_item.media_item
+                and (album := getattr(queue_item.media_item, "album", None))
+                else None,
+                "quality": {
+                    "bitDepth": queue_item.streamdetails.audio_format.bit_depth,
+                    "sampleRate": queue_item.streamdetails.audio_format.sample_rate,
+                    "codec": queue_item.streamdetails.audio_format.content_type.value,
+                    "lossless": queue_item.streamdetails.audio_format.content_type.is_lossless(),
+                }
+                if queue_item.streamdetails
+                else None,
+            },
+        }