From b93ce3c582b13735e193e2866db31999046523a4 Mon Sep 17 00:00:00 2001 From: Marcel van der Veldt Date: Wed, 16 Oct 2024 00:54:09 +0200 Subject: [PATCH] Some more bugfixes and tweaks for playergroups support (#1716) --- .../server/controllers/player_queues.py | 2 ++ music_assistant/server/controllers/players.py | 10 ++---- .../server/providers/chromecast/__init__.py | 2 +- .../server/providers/player_group/__init__.py | 36 +++++++++++-------- 4 files changed, 26 insertions(+), 24 deletions(-) diff --git a/music_assistant/server/controllers/player_queues.py b/music_assistant/server/controllers/player_queues.py index a64a1f20..276384f4 100644 --- a/music_assistant/server/controllers/player_queues.py +++ b/music_assistant/server/controllers/player_queues.py @@ -1155,6 +1155,8 @@ class PlayerQueuesController(CoreController): if next_index is None: raise QueueEmpty("No more tracks left in the queue.") queue_item = self.get_item(queue_id, next_index) + if queue_item is None: + raise QueueEmpty("No more tracks left in the queue.") # work out if we are playing an album and if we should prefer album loudness if ( diff --git a/music_assistant/server/controllers/players.py b/music_assistant/server/controllers/players.py index b8862745..24da52ab 100644 --- a/music_assistant/server/controllers/players.py +++ b/music_assistant/server/controllers/players.py @@ -587,12 +587,6 @@ 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) @@ -600,7 +594,7 @@ class PlayerController(CoreController): # 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. " + "Detected a (un)sync command to player %s which is part of a (active) group. " "This command will be redirected by turning off the entire group!", player.name, ) @@ -664,7 +658,7 @@ class PlayerController(CoreController): if child_player.synced_to and child_player.synced_to == target_player: continue # already synced to this target - if child_player.group_childs: + if child_player.group_childs and child_player.state != PlayerState.IDLE: # 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, " diff --git a/music_assistant/server/providers/chromecast/__init__.py b/music_assistant/server/providers/chromecast/__init__.py index 84d7dd43..b9ac7113 100644 --- a/music_assistant/server/providers/chromecast/__init__.py +++ b/music_assistant/server/providers/chromecast/__init__.py @@ -559,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 castplayer.player.type != PlayerType.GROUP: + if new_available and castplayer.player.type == PlayerType.PLAYER: # 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 421c2c3d..5e36fdf0 100644 --- a/music_assistant/server/providers/player_group/__init__.py +++ b/music_assistant/server/providers/player_group/__init__.py @@ -222,8 +222,8 @@ class PlayerGroupProvider(PlayerProvider): group_members.options = tuple( ConfigValueOption(x.display_name, x.player_id) for x in self.mass.players.all(True, False) - if x.provider != self.instance_id - and (player_prov := self.mass.get_provider(x.provider)) + if (player_prov := self.mass.get_provider(x.provider)) + and group_type == player_prov.lookup_key and ProviderFeature.SYNC_PLAYERS in player_prov.supported_features ) @@ -395,13 +395,6 @@ class PlayerGroupProvider(PlayerProvider): else: await self.cmd_power(player_id, True) - # set the state optimistically - group_player.current_media = media - group_player.elapsed_time = 0 - group_player.elapsed_time_last_updated = time() - 1 - group_player.state = PlayerState.PLAYING - self.mass.players.update(player_id) - # handle play_media for sync group if player_id.startswith(SYNCGROUP_PREFIX): # simply forward the command to the sync leader @@ -446,6 +439,13 @@ class PlayerGroupProvider(PlayerProvider): self.ugp_streams[player_id] = UGPStream(audio_source=audio_source, audio_format=UGP_FORMAT) base_url = f"{self.mass.streams.base_url}/ugp/{player_id}.aac" + # set the state optimistically + group_player.current_media = media + group_player.elapsed_time = 0 + group_player.elapsed_time_last_updated = time() - 1 + group_player.state = PlayerState.PLAYING + self.mass.players.update(player_id) + # forward to downstream play_media commands async with TaskManager(self.mass) as tg: for member in self.mass.players.iter_group_members( @@ -505,7 +505,7 @@ class PlayerGroupProvider(PlayerProvider): # create default config with the user chosen name self.mass.config.create_default_player_config( new_group_id, - player_prov.instance_id, + self.instance_id, name=name, enabled=True, values={CONF_GROUP_MEMBERS: members, CONF_GROUP_TYPE: group_type}, @@ -614,7 +614,7 @@ class PlayerGroupProvider(PlayerProvider): ): return child_player # this really should not be possible - raise RuntimeError("Impossible to select sync leader for syncgroup") + raise RuntimeError("No players available to form syncgroup") async def _sync_syncgroup(self, group_player: Player) -> None: """Sync all (possible) players of a syncgroup.""" @@ -642,8 +642,12 @@ class PlayerGroupProvider(PlayerProvider): """Update attributes of a player.""" for child_player in self.mass.players.iter_group_members(player, active_only=True): # just grab the first active player + if child_player.state not in (PlayerState.PLAYING, PlayerState.PAUSED): + continue + if child_player.synced_to: + continue player.state = child_player.state - if player.current_media: + if child_player.current_media: player.current_media = child_player.current_media player.elapsed_time = child_player.elapsed_time player.elapsed_time_last_updated = child_player.elapsed_time_last_updated @@ -701,13 +705,15 @@ class PlayerGroupProvider(PlayerProvider): return resp - def _filter_members(self, provider: str, members: list[str]) -> list[str]: + def _filter_members(self, group_type: str, members: list[str]) -> list[str]: """Filter out members that are not valid players.""" - if provider != GROUP_TYPE_UNIVERSAL: + if group_type != GROUP_TYPE_UNIVERSAL: + player_provider = self.mass.get_provider(group_type) return [ x for x in members - if (player := self.mass.players.get(x)) and player.provider == provider + if (player := self.mass.players.get(x)) + and player.provider in (player_provider.instance_id, self.instance_id) ] # cleanup members - filter out impossible choices syncgroup_childs: list[str] = [] -- 2.34.1