From f1f55adf6c007f9a39e2c64f52a1f71c7fae4e10 Mon Sep 17 00:00:00 2001 From: Marcel van der Veldt Date: Sat, 12 Oct 2024 03:23:56 +0200 Subject: [PATCH] Additional logic and fixes for new group handling (#1706) --- .../server/controllers/player_queues.py | 25 +++- music_assistant/server/controllers/players.py | 121 ++++++++++++++---- .../server/models/player_provider.py | 11 -- .../server/providers/chromecast/__init__.py | 58 +++++++-- .../server/providers/player_group/__init__.py | 50 ++++---- .../server/providers/sonos_s1/__init__.py | 1 - 6 files changed, 189 insertions(+), 77 deletions(-) diff --git a/music_assistant/server/controllers/player_queues.py b/music_assistant/server/controllers/player_queues.py index 017cd6b9..5c4e4177 100644 --- a/music_assistant/server/controllers/player_queues.py +++ b/music_assistant/server/controllers/player_queues.py @@ -1,4 +1,15 @@ -"""Logic to play music from MusicProviders to supported players.""" +""" +MusicAssistant Player Queues Controller. + +Handles all logic to PLAY Media Items, provided by Music Providers to supported players. + +It is loosely coupled to the MusicAssistant Music Controller and Player Controller. +A Music Assistant Player always has a PlayerQueue associated with it +which holds the queue items and state. + +The PlayerQueue is in that case the active source of the player, +but it can also be something else, hence the loose coupling. +""" from __future__ import annotations @@ -848,6 +859,17 @@ class PlayerQueuesController(CoreController): raise PlayerUnavailableError("Queue {target_queue_id} is not available") if auto_play is None: auto_play = source_queue.state == PlayerState.PLAYING + + target_player = self.mass.players.get(target_queue_id) + if target_player.active_group or target_player.synced_to: + # edge case: the user wants to move playback from the group as a whole, to a single + # player in the group or it is grouped and the command targeted at the single player. + # We need to dissolve the group first. + await self.mass.players.cmd_power( + target_player.active_group or target_player.synced_to, False + ) + await asyncio.sleep(3) + source_items = self._queue_items[source_queue_id] target_queue.repeat_mode = source_queue.repeat_mode target_queue.shuffle_enabled = source_queue.shuffle_enabled @@ -858,6 +880,7 @@ class PlayerQueuesController(CoreController): target_queue.current_item = source_queue.current_item target_queue.current_item.queue_id = target_queue_id self.clear(source_queue_id) + self.load(target_queue_id, source_items, keep_remaining=False, keep_played=False) for item in source_items: item.queue_id = target_queue_id diff --git a/music_assistant/server/controllers/players.py b/music_assistant/server/controllers/players.py index 7d21b820..b8862745 100644 --- a/music_assistant/server/controllers/players.py +++ b/music_assistant/server/controllers/players.py @@ -1,4 +1,10 @@ -"""Logic to play music from MusicProviders to supported players.""" +""" +MusicAssistant Players Controller. + +Handles all logic to control supported players, +which are provided by Player Providers. + +""" from __future__ import annotations @@ -253,7 +259,9 @@ class PlayerController(CoreController): @api_command("players/cmd/power") @handle_player_command - async def cmd_power(self, player_id: str, powered: bool, skip_redirect: bool = False) -> None: + async def cmd_power( + self, player_id: str, powered: bool, skip_redirect: bool = False, skip_update: bool = False + ) -> None: """Send POWER command to given player. - player_id: player_id of the player to handle the command. @@ -264,13 +272,17 @@ class PlayerController(CoreController): if player.powered == powered: return # nothing to do - # redirect to active group player if player is group child - if not skip_redirect and player.active_group: - if group_player_provider := self.get_player_provider(player.active_group): - async with self._player_throttlers[player.active_group]: - await group_player_provider.on_group_child_power( - player.active_group, player_id, powered - ) + if player.active_group and not powered and not skip_redirect: + # this is simply not possible (well, not without major headaches) + # the player is part of a permanent (sync)group and the user tries to power off + # one child player... we can't allow this, as it would break the group so we + # power off the whole group instead. + self.logger.info( + "Detected a power OFF command to player %s which is part of a (active) group. " + "This command will be redirected to the entire group.", + player.name, + ) + await self.cmd_power(player.active_group, False) return # always stop player at power off @@ -282,14 +294,13 @@ class PlayerController(CoreController): await self.cmd_stop(player_id) # unsync player at power off - if not powered and ( - player.synced_to or (player.type == PlayerType.PLAYER and player.group_childs) - ): + if not powered and (player.synced_to): await self.cmd_unsync(player_id) - # elif not powered and player.type == PlayerType.PLAYER and player.group_childs: - # async with TaskManager(self.mass) as tg: - # for member in self.iter_group_members(player, True): - # tg.create_task(self.cmd_power(member.player_id, False)) + # power off all synced childs when player is a sync leader + elif not powered and player.type == PlayerType.PLAYER and player.group_childs: + async with TaskManager(self.mass) as tg: + for member in self.iter_group_members(player, True): + tg.create_task(self.cmd_power(member.player_id, False)) # handle actual power command if PlayerFeature.POWER in player.supported_features: @@ -307,7 +318,9 @@ class PlayerController(CoreController): # reset active source on power off if not powered: player.active_source = None - self.update(player_id) + + if not skip_update: + self.update(player_id) # handle 'auto play on power on' feature if ( @@ -515,6 +528,7 @@ class PlayerController(CoreController): finally: player.announcement_in_progress = False + @handle_player_command async def play_media( self, player_id: str, media: PlayerMedia, skip_redirect: bool = False ) -> None: @@ -573,20 +587,68 @@ class PlayerController(CoreController): return if not (player.synced_to or player.group_childs): return # nothing to do + if player.active_group: + raise PlayerCommandFailed( + "Command denied: player %s is part of (active) group %s", + player.display_name, + player.active_group, + ) + + if player.active_group: + # this is simply not possible (well, not without major headaches) + # the player is part of a permanent (sync)group and the user tries to unsync + # one child player... we can't allow this, as it would break the group so we + # power unsync the whole group instead. + self.logger.info( + "Detected a power OFF command to player %s which is part of a (active) group. " + "This command will be redirected by turning off the entire group!", + player.name, + ) + await self.cmd_power(player.active_group, False) + return + + # handle (edge)case where un unsync command is sent to a sync leader; + # we dissolve the entire syncgroup in this case. + # while maybe not strictly needed to do this for all player providers, + # we do this to keep the functionality consistent across all providers + if player.group_childs: + self.logger.warning( + "Detected unsync command to player %s which is a sync(group) leader, " + "all sync members will be unsynced!", + player.name, + ) + async with TaskManager(self.mass) as tg: + for group_child_id in player.group_childs: + if group_child_id == player_id: + continue + tg.create_task(self.cmd_unsync(group_child_id)) + return - # reset active source player if it is unsynced + # (optimistically) reset active source player if it is unsynced player.active_source = None + # forward command to the player provider if player_provider := self.get_player_provider(player_id): await player_provider.cmd_unsync(player_id) + # if the command succeeded we optimistically reset the sync state + # this is to prevent race conditions and to update the UI as fast as possible + player.synced_to = None @api_command("players/cmd/sync_many") async def cmd_sync_many(self, target_player: str, child_player_ids: list[str]) -> None: """Create temporary sync group by joining given players to target player.""" parent_player: Player = self.get(target_player, True) if PlayerFeature.SYNC not in parent_player.supported_features: - msg = f"Player {parent_player.name} does not support (un)sync commands" + msg = f"Player {parent_player.name} does not support sync commands" raise UnsupportedFeaturedException(msg) + + if parent_player.synced_to: + # guard edge case: player already synced to another player + raise PlayerCommandFailed( + f"Player {parent_player.name} is already synced to another player on its own, " + "you need to unsync it first before you can join other players to it.", + ) + # filter all player ids on compatibility and availability final_player_ids: UniqueList[str] = UniqueList() for child_player_id in child_player_ids: @@ -596,25 +658,32 @@ class PlayerController(CoreController): self.logger.warning("Player %s is not available", child_player_id) continue if PlayerFeature.SYNC not in child_player.supported_features: - self.logger.warning( - "Player %s does not support (un)sync commands", child_player.name - ) + # this should not happen, but just in case bad things happen, guard it + self.logger.warning("Player %s does not support sync commands", child_player.name) continue if child_player.synced_to and child_player.synced_to == target_player: continue # already synced to this target - elif child_player.synced_to: + + if child_player.group_childs: + # guard edge case: childplayer is already a sync leader on its own + raise PlayerCommandFailed( + f"Player {child_player.name} is already synced with other players, " + "you need to unsync it first before you can join it to another player.", + ) + if child_player.synced_to: # player already synced to another player, unsync first self.logger.warning( - "Player %s is already synced, unsyncing first", child_player.name + "Player %s is already synced to another player, unsyncing first", + child_player.name, ) await self.cmd_unsync(child_player.player_id) # power on the player if needed if not child_player.powered: - await self.cmd_power(child_player.player_id, True) + await self.cmd_power(child_player.player_id, True, skip_update=True) # if we reach here, all checks passed final_player_ids.append(child_player_id) # set active source if player is synced - child_player.active_source = parent_player.active_source + child_player.active_source = parent_player.player_id # forward command to the player provider after all (base) sanity checks player_provider = self.get_player_provider(target_player) diff --git a/music_assistant/server/models/player_provider.py b/music_assistant/server/models/player_provider.py index e1afab7b..dbbead7f 100644 --- a/music_assistant/server/models/player_provider.py +++ b/music_assistant/server/models/player_provider.py @@ -166,17 +166,6 @@ class PlayerProvider(Provider): # default implementation, simply call the cmd_sync for all child players await self.cmd_sync(child_id, target_player) - async def on_group_child_power( - self, group_player_id: str, child_player_id: str, powered: bool - ) -> None: - """Call when a child player of a group player is powered on/off.""" - # default implementation, simply redirect the request to the group player - self.logger.warning( - "Detected a player power command to a player that is part of a group. " - "Redirecting to group player..." - ) - await self.mass.players.cmd_power(group_player_id, powered) - async def poll_player(self, player_id: str) -> None: """Poll player for state updates. diff --git a/music_assistant/server/providers/chromecast/__init__.py b/music_assistant/server/providers/chromecast/__init__.py index 5121f71e..84d7dd43 100644 --- a/music_assistant/server/providers/chromecast/__init__.py +++ b/music_assistant/server/providers/chromecast/__init__.py @@ -18,6 +18,7 @@ from pychromecast.discovery import CastBrowser, SimpleCastListener from pychromecast.socket_client import CONNECTION_STATUS_CONNECTED, CONNECTION_STATUS_DISCONNECTED from music_assistant.common.models.config_entries import ( + BASE_PLAYER_CONFIG_ENTRIES, CONF_ENTRY_CROSSFADE_DURATION, CONF_ENTRY_CROSSFADE_FLOW_MODE_REQUIRED, ConfigEntry, @@ -172,9 +173,13 @@ class ChromecastProvider(PlayerProvider): async def get_player_config_entries(self, player_id: str) -> tuple[ConfigEntry, ...]: """Return all (provider/player specific) Config Entries for the given player (if any).""" cast_player = self.castplayers.get(player_id) + if cast_player and cast_player.player.type == PlayerType.GROUP: + return ( + *BASE_PLAYER_CONFIG_ENTRIES, + *PLAYER_CONFIG_ENTRIES, + CONF_ENTRY_SAMPLE_RATES_CAST_GROUP, + ) base_entries = await super().get_player_config_entries(player_id) - if cast_player and cast_player.cast_info.is_audio_group: - return (*base_entries, *PLAYER_CONFIG_ENTRIES, CONF_ENTRY_SAMPLE_RATES_CAST_GROUP) return (*base_entries, *PLAYER_CONFIG_ENTRIES, CONF_ENTRY_SAMPLE_RATES_CAST) def on_player_config_changed( @@ -207,9 +212,17 @@ class ChromecastProvider(PlayerProvider): castplayer = self.castplayers[player_id] if powered: await self._launch_app(castplayer) - return - # handle power off - await asyncio.to_thread(castplayer.cc.quit_app) + else: + castplayer.player.active_group = None + castplayer.player.active_source = None + await asyncio.to_thread(castplayer.cc.quit_app) + # optimistically update the group childs + if castplayer.player.type == PlayerType.GROUP: + active_group = castplayer.player.active_group or castplayer.player.player_id + for child_id in castplayer.player.group_childs: + if child := self.castplayers.get(child_id): + child.player.powered = powered + child.player.active_group = active_group if powered else None async def cmd_volume_set(self, player_id: str, volume_level: int) -> None: """Send VOLUME_SET command to given player.""" @@ -382,7 +395,7 @@ class ChromecastProvider(PlayerProvider): self.castplayers[player_id] = castplayer castplayer.status_listener = CastStatusListener(self, castplayer, self.mz_mgr) - if cast_info.is_audio_group and not cast_info.is_multichannel_group: + if castplayer.player.type == PlayerType.GROUP: mz_controller = MultizoneController(cast_info.uuid) castplayer.cc.register_handler(mz_controller) castplayer.mz_controller = mz_controller @@ -474,16 +487,18 @@ class ChromecastProvider(PlayerProvider): # active source if group_player: - castplayer.player.active_source = group_player.player.active_source - castplayer.player.active_group = group_player.player.player_id + castplayer.player.active_source = ( + group_player.player.active_source or group_player.player.player_id + ) + castplayer.player.active_group = ( + group_player.player.active_group or group_player.player.player_id + ) elif castplayer.cc.app_id == MASS_APP_ID: castplayer.player.active_source = castplayer.player_id - castplayer.player.active_group = None else: castplayer.player.active_source = castplayer.cc.app_display_name - castplayer.player.active_group = None - if status.content_id: + if status.content_id and not status.player_is_idle: castplayer.player.current_media = PlayerMedia( uri=status.content_id, title=status.title, @@ -496,7 +511,24 @@ class ChromecastProvider(PlayerProvider): else: castplayer.player.current_media = None - # current media + # weird workaround which is needed for multichannel group childs + # (e.g. a stereo pair within a cast group) + # where it does not receive updates from the group, + # so we need to update the group child(s) manually + if castplayer.player.type == PlayerType.GROUP and castplayer.player.powered: + for child_id in castplayer.player.group_childs: + if child := self.castplayers.get(child_id): + if not child.cast_info.is_multichannel_group: + continue + child.player.state = castplayer.player.state + child.player.current_media = castplayer.player.current_media + child.player.elapsed_time = castplayer.player.elapsed_time + child.player.elapsed_time_last_updated = ( + castplayer.player.elapsed_time_last_updated + ) + child.player.active_source = castplayer.player.active_source + child.player.active_group = castplayer.player.active_group + self.mass.loop.call_soon_threadsafe(self.mass.players.update, castplayer.player_id) def on_new_connection_status(self, castplayer: CastPlayer, status: ConnectionStatus) -> None: @@ -527,7 +559,7 @@ class ChromecastProvider(PlayerProvider): manufacturer=castplayer.cast_info.manufacturer, ) self.mass.loop.call_soon_threadsafe(self.mass.players.update, castplayer.player_id) - if new_available and not castplayer.cast_info.is_audio_group: + if new_available and castplayer.player.type != PlayerType.GROUP: # Poll current group status for group_uuid in self.mz_mgr.get_multizone_memberships(castplayer.cast_info.uuid): group_media_controller = self.mz_mgr.get_multizone_mediacontroller(group_uuid) diff --git a/music_assistant/server/providers/player_group/__init__.py b/music_assistant/server/providers/player_group/__init__.py index 8d08bf1e..421c2c3d 100644 --- a/music_assistant/server/providers/player_group/__init__.py +++ b/music_assistant/server/providers/player_group/__init__.py @@ -8,6 +8,7 @@ allowing the user to create 'presets' of players to sync together (of the same t from __future__ import annotations from collections.abc import Callable +from contextlib import suppress from time import time from typing import TYPE_CHECKING, Final, cast @@ -37,6 +38,7 @@ from music_assistant.common.models.enums import ( ProviderFeature, ) from music_assistant.common.models.errors import ( + PlayerUnavailableError, ProviderUnavailableError, UnsupportedFeaturedException, ) @@ -132,11 +134,6 @@ async def get_config_entries( class PlayerGroupProvider(PlayerProvider): """Base/builtin provider for creating (permanent) player groups.""" - @property - def supported_features(self) -> tuple[ProviderFeature, ...]: - """Return the features supported by this Provider.""" - return () - def __init__( self, mass: MusicAssistant, manifest: ProviderManifest, config: ProviderConfig ) -> None: @@ -393,10 +390,10 @@ class PlayerGroupProvider(PlayerProvider): """Handle PLAY MEDIA on given player.""" group_player = self.mass.players.get(player_id) # power on (or resync) if needed - if not group_player.powered: - await self.cmd_power(player_id, True) - elif player_id.startswith(SYNCGROUP_PREFIX): + if group_player.powered and player_id.startswith(SYNCGROUP_PREFIX): await self._sync_syncgroup(group_player) + else: + await self.cmd_power(player_id, True) # set the state optimistically group_player.current_media = media @@ -463,6 +460,7 @@ class PlayerGroupProvider(PlayerProvider): title=group_player.display_name, queue_id=group_player.player_id, ), + skip_redirect=True, ) ) @@ -473,7 +471,7 @@ class PlayerGroupProvider(PlayerProvider): # this shouldn't happen, but just in case raise UnsupportedFeaturedException("Command is not supported for UGP players") if sync_leader := self._get_sync_leader(group_player): - await self.enqueue_next_media( + await self.mass.players.enqueue_next_media( sync_leader.player_id, media=media, ) @@ -526,18 +524,23 @@ class PlayerGroupProvider(PlayerProvider): continue # already registered members = player_config.get_value(CONF_GROUP_MEMBERS) group_type = player_config.get_value(CONF_GROUP_TYPE) - self._register_group_player( - player_config.player_id, - group_type, - player_config.name or player_config.default_name, - members, - ) + with suppress(PlayerUnavailableError): + self._register_group_player( + player_config.player_id, + group_type, + player_config.name or player_config.default_name, + members, + ) def _register_group_player( self, group_player_id: str, group_type: str, name: str, members: Iterable[str] ) -> Player: """Register a syncgroup player.""" player_features = {PlayerFeature.POWER, PlayerFeature.VOLUME_SET} + + if not (self.mass.players.get(x) for x in members): + raise PlayerUnavailableError("One or more members are not available!") + if group_type == GROUP_TYPE_UNIVERSAL: model_name = "Universal Group" manufacturer = self.name @@ -552,17 +555,14 @@ class PlayerGroupProvider(PlayerProvider): player_provider = cast(PlayerProvider, player_provider) model_name = "Sync Group" manufacturer = self.mass.get_provider(group_type).name - if child_player := next((x for x in player_provider.players), None): - for feature in ( - PlayerFeature.PAUSE, - PlayerFeature.VOLUME_MUTE, - ): - if feature in child_player.supported_features: - player_features.add(feature) + for feature in ( + PlayerFeature.PAUSE, + PlayerFeature.VOLUME_MUTE, + ): + if all(x for x in player_provider.players if feature in x.supported_features): + player_features.add(feature) else: - # this may happen if the provider is not available yet - model_name = "Sync Group" - manufacturer = self.name + raise PlayerUnavailableError(f"Provider for syncgroup {group_type} is not available!") player = Player( player_id=group_player_id, diff --git a/music_assistant/server/providers/sonos_s1/__init__.py b/music_assistant/server/providers/sonos_s1/__init__.py index 6d6d075c..fb49555b 100644 --- a/music_assistant/server/providers/sonos_s1/__init__.py +++ b/music_assistant/server/providers/sonos_s1/__init__.py @@ -76,7 +76,6 @@ async def setup( ) -> ProviderInstanceType: """Initialize provider(instance) with given configuration.""" soco_config.EVENTS_MODULE = events_asyncio - soco_config.REQUEST_TIMEOUT = 9.5 zonegroupstate.EVENT_CACHE_TIMEOUT = SUBSCRIPTION_TIMEOUT prov = SonosPlayerProvider(mass, manifest, config) # set-up soco logging -- 2.34.1