Fixes and enhancements to syncgroups and UGP groups (#1621)

music_assistant/common/models/player.py

@@ -134,6 +134,9 @@ class Player(DataClassDictMixin):
     # last_poll: when was the player last polled (used with needs_poll)
     last_poll: float = 0
 
+    # internal use only
+    _prev_volume_level: int = 0
+
     @property
     def corrected_elapsed_time(self) -> float:
         """Return the corrected/realtime elapsed time."""

music_assistant/constants.py

@@ -65,6 +65,7 @@
 CONF_LANGUAGE: Final[str] = "language"
 CONF_SAMPLE_RATES: Final[str] = "sample_rates"
 CONF_HTTP_PROFILE: Final[str] = "http_profile"
+CONF_SYNC_LEADER: Final[str] = "sync_leader"
 
 # config default values
 DEFAULT_HOST: Final[str] = "0.0.0.0"

music_assistant/server/controllers/players.py

@@ -44,6 +44,7 @@
     CONF_GROUP_MEMBERS,
     CONF_HIDE_PLAYER,
     CONF_PLAYERS,
+    CONF_SYNC_LEADER,
     CONF_TTS_PRE_ANNOUNCE,
     SYNCGROUP_PREFIX,
 )
@@ -294,49 +295,80 @@ async def cmd_power(self, player_id: str, powered: bool) -> None:
         if player.powered == powered:
             return  # nothing to do
 
+        # grab info about any groups this player is active in
+        # to handle actions on the group when a (sync)group child turns on/off
+        if active_group_player_id := self._get_active_player_group(player):
+            active_group_player = self.get(active_group_player_id)
+            group_player_state = active_group_player.state
+        else:
+            active_group_player = None
+
         # always stop player at power off
         if (
             not powered
+            and player.powered
             and player.state in (PlayerState.PLAYING, PlayerState.PAUSED)
             and not player.synced_to
-            and player.powered
         ):
             await self.cmd_stop(player_id)
 
         # unsync player at power off
         if not powered:
-            if player.synced_to is not None:
+            if player.synced_to or player.group_childs:
                 await self.cmd_unsync(player_id)
-            for child in self.iter_group_members(player):
-                if not child.synced_to:
-                    continue
-                await self.cmd_unsync(child.player_id)
+
         if PlayerFeature.POWER in player.supported_features:
-            # forward to player provider
+            # player supports power command: forward to player provider
             player_provider = self.get_player_provider(player_id)
             async with self._player_throttlers[player_id]:
                 await player_provider.cmd_power(player_id, powered)
         else:
             # allow the stop command to process and prevent race conditions
             await asyncio.sleep(0.2)
+
         # always optimistically set the power state to update the UI
         # as fast as possible and prevent race conditions
         player.powered = powered
-        # always MA as active source on power ON
-        player.active_source = player_id if powered else None
+        # reset active source
+        player.active_source = None
         self.update(player_id)
-        # handle actions when a (sync)group child turns on/off
-        if active_group_player := self._get_active_player_group(player):
-            player_prov = self.get_player_provider(active_group_player)
-            player_prov.on_child_power(active_group_player, player.player_id, powered)
+
         # handle 'auto play on power on'  feature
-        elif (
-            powered
+        if (
+            not active_group_player
+            and powered
             and self.mass.config.get_raw_player_config_value(player_id, CONF_AUTO_PLAY, False)
             and player.active_source in (None, player_id)
         ):
             await self.mass.player_queues.resume(player_id)
 
+        # handle group player actions
+        if not (active_group_player and active_group_player.powered):
+            return
+
+        # run actions suitable for every type of group player
+        powered_childs = list(self.mass.players.iter_group_members(active_group_player, True))
+        if not powered and player in powered_childs:
+            powered_childs.remove(player.player_id)
+        elif powered and player.player_id not in powered_childs:
+            powered_childs.append(player.player_id)
+        # if the last player of a group turned off, turn off the group
+        if len(powered_childs) == 0:
+            self.logger.debug(
+                "Group %s has no more powered members, turning off group player",
+                active_group_player.display_name,
+            )
+            self.mass.create_task(self.mass.players.cmd_power(active_group_player.player_id, False))
+            return
+        # forward to either syncgroup logic or group player logic
+        if active_group_player.type == PlayerType.SYNC_GROUP:
+            self._on_syncgroup_child_power(active_group_player, player, powered, group_player_state)
+        elif active_group_player.type == PlayerType.GROUP:
+            player_prov = self.mass.get_provider(active_group_player.provider)
+            player_prov.on_group_child_power(
+                active_group_player, player, powered, group_player_state
+            )
+
     @api_command("players/cmd/volume_set")
     @handle_player_command
     async def cmd_volume_set(self, player_id: str, volume_level: int) -> None:
@@ -417,13 +449,19 @@ async def cmd_group_power(self, player_id: str, power: bool) -> None:
             await self.cmd_power(player_id, power)
             return
 
+        if not (group_player.type == PlayerType.SYNC_GROUP or group_player.group_childs):
+            # this is not a (temporary) sync group - nothing to do
+            raise UnsupportedFeaturedException("Player is not a sync group")
+
         # make sure to update the group power state
         group_player.powered = power
 
         # always stop (group/master)player at power off
         if not power and group_player.state in (PlayerState.PLAYING, PlayerState.PAUSED):
             await self.cmd_stop(player_id)
 
+        # handle syncgroup - this will also work for temporary syncgroups
+        # where players are manually synced against a group leader
         any_member_powered = False
         async with TaskManager(self.mass) as tg:
             for member in self.iter_group_members(group_player, only_powered=True):
@@ -465,8 +503,17 @@ async def cmd_volume_mute(self, player_id: str, muted: bool) -> None:
         player = self.get(player_id, True)
         assert player
         if PlayerFeature.VOLUME_MUTE not in player.supported_features:
-            msg = f"Player {player.display_name} does not support muting"
-            raise UnsupportedFeaturedException(msg)
+            self.logger.info(
+                "Player %s does not support muting, using volume instead", player.display_name
+            )
+            if muted:
+                player._prev_volume_level = player.volume_level
+                player.volume_muted = True
+                await self.cmd_volume_set(player_id, 0)
+            else:
+                player.volume_muted = False
+                await self.cmd_volume_set(player_id, player._prev_volume_level)
+            return
         player_provider = self.get_player_provider(player_id)
         async with self._player_throttlers[player_id]:
             await player_provider.cmd_volume_mute(player_id, muted)
@@ -640,6 +687,10 @@ async def cmd_unsync(self, player_id: str) -> None:
 
             - player_id: player_id of the player to handle the command.
         """
+        if (player := self.get(player_id)) and player.group_childs:
+            # this player is a syncgroup leader, unsync all children
+            await self.cmd_unsync_many(player.group_childs)
+            return
         await self.cmd_unsync_many([player_id])
 
     @api_command("players/cmd/sync_many")
@@ -692,8 +743,7 @@ async def cmd_sync_many(self, target_player: str, child_player_ids: list[str]) -
     async def cmd_unsync_many(self, player_ids: list[str]) -> None:
         """Handle UNSYNC command for all the given players."""
         # filter all player ids on compatibility and availability
-        final_player_ids: UniqueList[str] = UniqueList()
-        for player_id in player_ids:
+        for player_id in list(player_ids):
             if not (child_player := self.get(player_id)):
                 self.logger.warning("Player %s is not available", player_id)
                 continue
@@ -702,16 +752,13 @@ async def cmd_unsync_many(self, player_ids: list[str]) -> None:
                     "Player %s does not support (un)sync commands", child_player.name
                 )
                 continue
-            final_player_ids.append(player_id)
-            # reset active source player if is unsynced
+            if not child_player.synced_to:
+                continue
+            # reset active source player if it is unsynced
             child_player.active_source = None
-
-        if not final_player_ids:
-            return
-
-        # forward command to the player provider after all (base) sanity checks
-        player_provider = self.get_player_provider(final_player_ids[0])
-        await player_provider.cmd_unsync_many(final_player_ids)
+            # forward command to the player provider
+            if player_provider := self.get_player_provider(player_id):
+                await player_provider.cmd_unsync(player_id)
 
     def set(self, player: Player) -> None:
         """Set/Update player details on the controller."""
@@ -1167,6 +1214,11 @@ def get_sync_leader(self, group_player: Player) -> Player | None:
         ):
             if child_player.group_childs:
                 return child_player
+        pref_sync_leader = self.mass.config.get_raw_player_config_value(
+            group_player.player_id, CONF_SYNC_LEADER, "auto"
+        )
+        if pref_sync_leader != "auto" and (player := self.get(pref_sync_leader)):
+            return player
         # select new sync leader: return the first playing player
         for child_player in self.iter_group_members(
             group_player, only_powered=True, only_playing=True
@@ -1196,6 +1248,41 @@ async def sync_syncgroup(self, player_id: str) -> None:
                 continue
             await self.cmd_sync(member.player_id, sync_leader.player_id)
 
+    def _on_syncgroup_child_power(
+        self, group_player: Player, child_player: Player, new_power: bool, group_state: PlayerState
+    ) -> None:
+        """
+        Call when a power command was executed on one of the child players of a SyncGroup.
+
+        This is used to handle special actions such as (re)syncing.
+        The group state is sent with the state BEFORE the power command was executed.
+        """
+        group_playing = group_state == PlayerState.PLAYING
+        sync_leader = self.mass.players.get_sync_leader(group_player)
+        is_sync_leader = child_player.player_id == sync_leader.player_id
+        if group_playing and not new_power and is_sync_leader:
+            # the current sync leader player turned OFF while the group player
+            # should still be playing - we need to select a new sync leader and resume
+            self.logger.warning(
+                "Syncleader %s turned off while syncgroup is playing, "
+                "a forced resync for syngroup %s will be attempted...",
+                child_player.display_name,
+                group_player.display_name,
+            )
+
+            async def full_resync() -> None:
+                await self.mass.players.sync_syncgroup(group_player.player_id)
+                await self.mass.player_queues.resume(group_player.player_id)
+
+            self.mass.call_later(2, full_resync, task_id=f"forced_resync_{group_player.player_id}")
+            return
+        elif new_power:
+            # if a child player turned ON while the group is already active, we need to resync
+            if sync_leader.player_id != child_player.player_id:
+                self.mass.create_task(
+                    self.cmd_sync(child_player.player_id, sync_leader.player_id),
+                )
+
     async def _register_syncgroups(self) -> None:
         """Register all (virtual/fake) syncgroup players."""
         player_configs = await self.mass.config.get_player_configs()

music_assistant/server/models/player_provider.py

@@ -23,9 +23,9 @@
     ConfigValueOption,
     PlayerConfig,
 )
-from music_assistant.common.models.enums import ConfigEntryType, PlayerState, ProviderFeature
+from music_assistant.common.models.enums import ConfigEntryType, PlayerState
 from music_assistant.common.models.player import Player, PlayerMedia
-from music_assistant.constants import CONF_GROUP_MEMBERS, SYNCGROUP_PREFIX
+from music_assistant.constants import CONF_GROUP_MEMBERS, CONF_SYNC_LEADER, SYNCGROUP_PREFIX
 
 from .provider import Provider
 
@@ -71,6 +71,28 @@ async def get_player_config_entries(self, player_id: str) -> tuple[ConfigEntry,
                     multi_value=True,
                     required=True,
                 ),
+                ConfigEntry(
+                    key=CONF_SYNC_LEADER,
+                    type=ConfigEntryType.STRING,
+                    label="Preferred sync leader",
+                    default_value="auto",
+                    options=(
+                        *tuple(
+                            ConfigValueOption(x.display_name, x.player_id)
+                            for x in self.mass.players.all(True, False)
+                            if x.player_id
+                            in self.mass.config.get_raw_player_config_value(
+                                player_id, CONF_GROUP_MEMBERS, []
+                            )
+                        ),
+                        ConfigValueOption("Select automatically", "auto"),
+                    ),
+                    description="By default Music Assistant will automatically assign a "
+                    "(random) player as sync leader, meaning the other players in the sync group "
+                    "will be synced to that player. If you want to force a specific player to be "
+                    "the sync leader, select it here.",
+                    required=True,
+                ),
                 CONF_ENTRY_PLAYER_ICON_GROUP,
             )
         if not player_id.startswith(SYNCGROUP_PREFIX):
@@ -212,84 +234,22 @@ async def cmd_sync_many(self, target_player: str, child_player_ids: list[str]) -
             # default implementation, simply call the cmd_sync for all child players
             await self.cmd_sync(child_id, target_player)
 
-    async def cmd_unsync_many(self, player_ids: str) -> None:
-        """Handle UNSYNC command for all the given players.
-
-        Remove the given player from any syncgroups it currently is synced to.
-
-            - player_id: player_id of the player to handle the command.
-        """
-        for player_id in player_ids:
-            # default implementation, simply call the cmd_sync for all player_ids
-            await self.cmd_unsync(player_id)
-
     async def poll_player(self, player_id: str) -> None:
         """Poll player for state updates.
 
         This is called by the Player Manager;
         if 'needs_poll' is set to True in the player object.
         """
 
-    def on_child_power(self, player_id: str, child_player_id: str, new_power: bool) -> None:
+    def on_group_child_power(
+        self, group_player: Player, child_player: Player, new_power: bool, group_state: PlayerState
+    ) -> None:
         """
-        Call when a power command was executed on one of the child players of a Sync/Player group.
+        Call when a power command was executed on one of the child players of a PlayerGroup.
 
         This is used to handle special actions such as (re)syncing.
+        The group state is sent with the state BEFORE the power command was executed.
         """
-        group_player = self.mass.players.get(player_id)
-        child_player = self.mass.players.get(child_player_id)
-
-        if not group_player.powered:
-            # guard, this should be caught in the player controller but just in case...
-            return
-
-        powered_childs = list(self.mass.players.iter_group_members(group_player, True))
-        if not new_power and child_player in powered_childs:
-            powered_childs.remove(child_player)
-        if new_power and child_player not in powered_childs:
-            powered_childs.append(child_player)
-
-        # if the last player of a group turned off, turn off the group
-        if len(powered_childs) == 0:
-            self.logger.debug(
-                "Group %s has no more powered members, turning off group player",
-                group_player.display_name,
-            )
-            self.mass.create_task(self.mass.players.cmd_power(player_id, False))
-            return
-
-        # the below actions are only suitable for syncgroups
-        if ProviderFeature.SYNC_PLAYERS not in self.supported_features:
-            return
-
-        group_playing = group_player.state == PlayerState.PLAYING
-        is_sync_leader = (
-            len(child_player.group_childs) > 0
-            and child_player.active_source == group_player.player_id
-        )
-        if group_playing and not new_power and is_sync_leader:
-            # the current sync leader player turned OFF while the group player
-            # should still be playing - we need to select a new sync leader and resume
-            self.logger.warning(
-                "Syncleader %s turned off while syncgroup is playing, "
-                "a forced resume for syngroup %s will be attempted in 5 seconds...",
-                child_player.display_name,
-                group_player.display_name,
-            )
-
-            async def full_resync() -> None:
-                await self.mass.players.sync_syncgroup(group_player.player_id)
-                await self.mass.player_queues.resume(group_player.player_id)
-
-            self.mass.call_later(5, full_resync, task_id=f"forced_resync_{player_id}")
-            return
-        elif new_power:
-            # if a child player turned ON while the group is already active, we need to resync
-            sync_leader = self.mass.players.get_sync_leader(group_player)
-            if sync_leader.player_id != child_player_id:
-                self.mass.create_task(
-                    self.cmd_sync(child_player_id, sync_leader.player_id),
-                )
 
     # DO NOT OVERRIDE BELOW