More fixes for protocol linking

Author: marcelveldt

music_assistant/controllers/players/README.md

@@ -46,7 +46,7 @@ The `PlayerState` is a dataclass representing the final state of the player. It:
 
 ```
 ┌─────────────────────────────────────────────────────────────────┐
-│                     Player (Internal)                            │
+│                     Player (Internal)                           │
 │  - Provider-specific implementation                             │
 │  - Control methods (play, pause, volume_set, etc.)              │
 │  - Raw state (_attr_volume_level, _attr_playback_state, etc.)   │

music_assistant/controllers/players/controller.py

@@ -2456,10 +2456,14 @@ async def _handle_set_members_with_protocols(
         # Forward native members to parent player's set_members
         if native_members_to_add or native_members_to_remove:
             filtered_native_add = self._filter_native_members(native_members_to_add, parent_player)
+            # For removal, allow protocol players if they're actually in the parent's group_members
+            # This handles native protocol players (e.g., native AirPlay) where group_members
+            # contains protocol player IDs
             filtered_native_remove = [
                 pid
                 for pid in native_members_to_remove
-                if (p := self.get_player(pid)) and p.type != PlayerType.PROTOCOL
+                if (p := self.get_player(pid))
+                and (p.type != PlayerType.PROTOCOL or pid in parent_player.group_members)
             ]
             self.logger.debug(
                 "Native grouping on %s: filtered_add=%s, filtered_remove=%s",

music_assistant/controllers/players/protocol_linking.py

@@ -1066,7 +1066,23 @@ def _translate_members_to_remove_for_protocols(
                     protocol_members.append(child_protocol.output_protocol_id)
                     continue
 
-            native_members.append(child_player_id)
+            # Check if child's protocol player is in parent's native group_members
+            # This handles native protocol players (e.g., native AirPlay player like Apple TV)
+            # where the parent itself contains protocol player IDs in its group_members
+            translated = False
+            for linked in child_player.linked_output_protocols:
+                if linked.output_protocol_id in parent_player.group_members:
+                    self.logger.debug(
+                        "Translating removal (native parent): %s -> protocol %s",
+                        child_player_id,
+                        linked.output_protocol_id,
+                    )
+                    native_members.append(linked.output_protocol_id)
+                    translated = True
+                    break
+
+            if not translated:
+                native_members.append(child_player_id)
 
         return protocol_members, native_members
 
@@ -1119,13 +1135,15 @@ def _try_child_preferred_protocol(
         if not child_protocol or not child_protocol.available:
             return None, None
 
-        # Check if parent supports this protocol
-        parent_protocol = parent_player.get_linked_protocol(child_protocol.protocol_domain)
+        # Check if parent supports this protocol (including native protocol)
+        parent_protocol = parent_player.get_output_protocol_by_domain(
+            child_protocol.protocol_domain
+        )
         if not parent_protocol or not parent_protocol.available:
             return None, None
 
         # Check if this protocol supports set_members
-        protocol_player = self.get_player(parent_protocol.output_protocol_id)
+        protocol_player = parent_player.get_protocol_player(parent_protocol.output_protocol_id)
         if (
             not protocol_player
             or PlayerFeature.SET_MEMBERS not in protocol_player.state.supported_features
@@ -1165,7 +1183,9 @@ def _try_find_common_protocol(
             )
             if not child_protocol or not child_protocol.available:
                 continue
-            protocol_player = self.get_player(parent_output_protocol.output_protocol_id)
+            protocol_player = parent_player.get_protocol_player(
+                parent_output_protocol.output_protocol_id
+            )
             if (
                 protocol_player
                 and PlayerFeature.SET_MEMBERS in protocol_player.state.supported_features
@@ -1231,9 +1251,11 @@ def _translate_members_for_protocols(
                 and (not parent_protocol_domain or protocol_domain == parent_protocol_domain)
             ):
                 if not parent_protocol_player or parent_protocol_domain != protocol_domain:
-                    parent_protocol = parent_player.get_linked_protocol(protocol_domain)
+                    parent_protocol = parent_player.get_output_protocol_by_domain(protocol_domain)
                     if parent_protocol:
-                        parent_protocol_player = self.get_player(parent_protocol.output_protocol_id)
+                        parent_protocol_player = parent_player.get_protocol_player(
+                            parent_protocol.output_protocol_id
+                        )
                         parent_protocol_domain = protocol_domain
                 protocol_members.append(child_protocol_id)
                 self.logger.log(
@@ -1290,7 +1312,9 @@ def _translate_members_for_protocols(
                     not parent_protocol_player
                     or parent_protocol_domain != parent_protocol.protocol_domain
                 ):
-                    parent_protocol_player = self.get_player(parent_protocol.output_protocol_id)
+                    parent_protocol_player = parent_player.get_protocol_player(
+                        parent_protocol.output_protocol_id
+                    )
                     if parent_protocol_player:
                         parent_protocol_domain = parent_protocol_player.provider.domain
                 protocol_members.append(child_protocol.output_protocol_id)
@@ -1365,28 +1389,55 @@ async def _forward_protocol_set_members(
             player_ids_to_remove=filtered_protocol_remove or None,
         )
 
+        # Set active output protocol on added child players
+        if filtered_protocol_add:
+            for child_protocol_id in filtered_protocol_add:
+                if child_protocol := self.get_player(child_protocol_id):
+                    if child_protocol.protocol_parent_id:
+                        if child_player := self.get_player(child_protocol.protocol_parent_id):
+                            if child_player.active_output_protocol != child_protocol_id:
+                                self.logger.debug(
+                                    "Setting active output protocol on child %s to %s",
+                                    child_player.state.name,
+                                    child_protocol_id,
+                                )
+                                child_player.set_active_output_protocol(child_protocol_id)
+
         # If we added members via this protocol, set it as the active output protocol
-        # and restart playback if currently playing
-        if (
-            filtered_protocol_add
-            and parent_player.active_output_protocol != parent_protocol_player.player_id
-        ):
+        # and restart playback if currently playing AND we're switching protocols
+        if filtered_protocol_add:
             previous_protocol = parent_player.active_output_protocol
             was_playing = parent_player.state.playback_state == PlaybackState.PLAYING
 
+            # Determine if we're switching protocols (which requires restart)
+            # Native protocol: parent_protocol_player is the same as parent_player
+            is_native_protocol = parent_protocol_player.player_id == parent_player.player_id
+            already_using_native = previous_protocol in (None, "native")
+            already_using_this_protocol = previous_protocol == parent_protocol_player.player_id
+
+            # Only restart if we're actually switching to a different protocol
+            switching_protocols = not (
+                (is_native_protocol and already_using_native) or already_using_this_protocol
+            )
+
             self.logger.debug(
-                "Setting active output protocol to %s after grouping members "
-                "(previous: %s, was_playing: %s)",
-                parent_protocol_player.player_id,
-                previous_protocol,
+                "Protocol grouping: is_native=%s, already_native=%s, already_this=%s, "
+                "switching=%s, was_playing=%s",
+                is_native_protocol,
+                already_using_native,
+                already_using_this_protocol,
+                switching_protocols,
                 was_playing,
             )
-            parent_player.set_active_output_protocol(parent_protocol_player.player_id)
 
-            # Restart playback on the new protocol if we were playing
-            if was_playing:
+            # Update active output protocol if not already using native
+            if not (is_native_protocol and already_using_native):
+                parent_player.set_active_output_protocol(parent_protocol_player.player_id)
+
+            # Restart playback only if we're switching protocols
+            if was_playing and switching_protocols:
                 self.logger.info(
-                    "Restarting playback on %s via %s protocol after grouping members",
+                    "Restarting playback on %s via %s protocol after switching protocols",
                     parent_player.state.name,
                     parent_protocol_player.provider.domain,
                 )

music_assistant/models/player.py

@@ -1070,6 +1070,21 @@ def get_linked_protocol(self, protocol_domain: str) -> OutputProtocol | None:
                 )
         return None
 
+    @final
+    def get_output_protocol_by_domain(self, protocol_domain: str) -> OutputProtocol | None:
+        """
+        Get an output protocol by domain, including native protocol.
+
+        Unlike get_linked_protocol, this also checks if the player's native protocol
+        matches the requested domain.
+
+        :param protocol_domain: The protocol domain to search for (e.g., "airplay", "sonos").
+        """
+        for output_protocol in self.output_protocols:
+            if output_protocol.protocol_domain == protocol_domain:
+                return output_protocol
+        return None
+
     @final
     def get_protocol_player(self, player_id: str) -> Player | None:
         """Get the protocol Player for a given player_id."""
@@ -1613,7 +1628,15 @@ def __final_group_members(self) -> list[str]:
             # If player is synced to another player, it has no group members itself
             return []
 
-        members = self.group_members.copy()
+        # Start by translating native group_members to visible player IDs
+        # This handles cases where a native player (e.g., native AirPlay) has grouped
+        # protocol players (e.g., Sonos AirPlay protocol players) that need translation
+        members: list[str] = []
+        translated_members = self._translate_protocol_ids_to_visible(set(self.group_members))
+        for member in translated_members:
+            if member.player_id not in members:
+                members.append(member.player_id)
+
         # If there's an active linked protocol, include its group members (translated)
         if self.__attr_active_output_protocol and self.__attr_active_output_protocol != "native":
             if protocol_player := self.mass.players.get_player(self.__attr_active_output_protocol):
@@ -1653,9 +1676,13 @@ def __final_synced_to(self) -> str | None:
             if protocol_player.synced_to:
                 # Protocol player is synced, translate to visible player
                 if proto_sync_parent := self.mass.players.get_player(protocol_player.synced_to):
+                    if proto_sync_parent.type != PlayerType.PROTOCOL:
+                        # Sync parent is already a visible player (e.g., native AirPlay player)
+                        return proto_sync_parent.player_id
                     if proto_sync_parent.protocol_parent_id and (
                         parent := self.mass.players.get_player(proto_sync_parent.protocol_parent_id)
                     ):
+                        # Sync parent is a protocol player, return its visible parent
                         return parent.player_id
 
         return None