More fixes for protocol linking
authorMarcel van der Veldt <m.vanderveldt@outlook.com>
Fri, 20 Feb 2026 14:20:15 +0000 (15:20 +0100)
committerMarcel van der Veldt <m.vanderveldt@outlook.com>
Fri, 20 Feb 2026 14:20:15 +0000 (15:20 +0100)
music_assistant/controllers/players/README.md
music_assistant/controllers/players/controller.py
music_assistant/controllers/players/protocol_linking.py
music_assistant/models/player.py
tests/core/test_protocol_linking.py

index 6458e87590acab550aac6e44510eb1f34dec338b..f102781cee13ead81cc42eb0a3c582184e05e073 100644 (file)
@@ -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.)   │
index ed33de88f002cd5cb1a7ee1c70b6947dc01a931c..67102a94d08aee9aac7a4701e12c011ef6b306e1 100644 (file)
@@ -2456,10 +2456,14 @@ class PlayerController(ProtocolLinkingMixin, CoreController):
         # 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",
index 2cb4d808715ebb6ec8928af419ff5a08c7bebbbf..cb74b0606c2aa405e0a4adf33be512e1d025c1ff 100644 (file)
@@ -1066,7 +1066,23 @@ class ProtocolLinkingMixin:
                     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 @@ class ProtocolLinkingMixin:
         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 @@ class ProtocolLinkingMixin:
             )
             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 @@ class ProtocolLinkingMixin:
                 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 @@ class ProtocolLinkingMixin:
                     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 @@ class ProtocolLinkingMixin:
             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,
                 )
index bb44dad634f72c7f65add9e04a5dc44d2633c1d5..a4f25edb7fdabcda5c96b529023e309cf3334f91 100644 (file)
@@ -1070,6 +1070,21 @@ class Player(ABC):
                 )
         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 @@ class Player(ABC):
             # 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 @@ class Player(ABC):
             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
index 8d08890506bf6a224bfa9f872413baa64b4a149c..a38b3ea3e08151da1e12f27bb2b898986853cc8a 100644 (file)
@@ -1267,6 +1267,7 @@ class TestPlayerGrouping:
             ]
         )
 
+        mock_mass.players = controller
         controller._players = {
             "sonos_123": sonos_player,
             "wiim_456": wiim_player,
@@ -1285,6 +1286,8 @@ class TestPlayerGrouping:
         }
 
         # Update state after modifying attributes
+        sonos_dlna.update_state(signal_event=False)
+        wiim_dlna.update_state(signal_event=False)
         sonos_airplay.update_state(signal_event=False)
         wiim_airplay.update_state(signal_event=False)
 
@@ -1947,3 +1950,320 @@ class TestProtocolSwitchingDuringPlayback:
         assert selected_player == sonos_airplay
         assert output_protocol is not None
         assert output_protocol.output_protocol_id == "airplay_sonos"
+
+
+class TestNativeProtocolPlayerGrouping:
+    """Tests for grouping with native protocol players (e.g., native AirPlay like Apple TV)."""
+
+    def test_native_airplay_groups_with_protocol_linked_player(self, mock_mass: MagicMock) -> None:
+        """Test grouping a native AirPlay player (Apple TV) with a protocol-linked player (Sonos).
+
+        This tests the scenario where:
+        - Apple TV is a native AirPlay PLAYER (not PROTOCOL type)
+        - Sonos has AirPlay as a linked protocol
+        - Apple TV groups with Sonos via the common AirPlay protocol
+        """
+        controller = PlayerController(mock_mass)
+
+        airplay_provider = MockProvider("airplay", instance_id="airplay", mass=mock_mass)
+        sonos_provider = MockProvider("sonos", instance_id="sonos", mass=mock_mass)
+
+        # Apple TV: native AirPlay PLAYER (supports grouping via AirPlay)
+        apple_tv = MockPlayer(airplay_provider, "apple_tv_1", "Apple TV Slaapkamer")
+        apple_tv._attr_supported_features.add(PlayerFeature.PLAY_MEDIA)
+        apple_tv._attr_supported_features.add(PlayerFeature.SET_MEMBERS)
+        apple_tv._attr_can_group_with = {"airplay"}  # Provider instance ID
+        apple_tv._cache.clear()
+
+        # Sonos native player (visible)
+        sonos_player = MockPlayer(sonos_provider, "sonos_badkamer", "Badkamer")
+        sonos_player._attr_supported_features.add(PlayerFeature.PLAY_MEDIA)
+        sonos_player._attr_supported_features.add(PlayerFeature.SET_MEMBERS)
+        sonos_player._cache.clear()
+
+        # AirPlay protocol player for Sonos (hidden, linked to sonos)
+        sonos_airplay = MockPlayer(
+            airplay_provider,
+            "airplay_sonos",
+            "Badkamer (AirPlay)",
+            player_type=PlayerType.PROTOCOL,
+        )
+        sonos_airplay._attr_supported_features.add(PlayerFeature.SET_MEMBERS)
+        sonos_airplay._attr_can_group_with = {"airplay"}
+        sonos_airplay._cache.clear()
+        sonos_airplay.set_protocol_parent_id("sonos_badkamer")
+
+        sonos_player.set_linked_output_protocols(
+            [
+                OutputProtocol(
+                    output_protocol_id="airplay_sonos",
+                    name="AirPlay",
+                    protocol_domain="airplay",
+                    priority=10,
+                    available=True,
+                )
+            ]
+        )
+
+        mock_mass.players = controller
+        controller._players = {
+            "apple_tv_1": apple_tv,
+            "sonos_badkamer": sonos_player,
+            "airplay_sonos": sonos_airplay,
+        }
+        controller._player_throttlers = {
+            "apple_tv_1": Throttler(1, 0.05),
+            "sonos_badkamer": Throttler(1, 0.05),
+            "airplay_sonos": Throttler(1, 0.05),
+        }
+
+        # Update states
+        sonos_airplay.update_state(signal_event=False)
+        sonos_player.update_state(signal_event=False)
+        apple_tv.update_state(signal_event=False)
+
+        # Translate members for grouping Sonos to Apple TV
+        protocol_members, _native_members, protocol_player, protocol_domain = (
+            controller._translate_members_for_protocols(
+                parent_player=apple_tv,
+                player_ids=["sonos_badkamer"],
+                parent_protocol_player=None,
+                parent_protocol_domain=None,
+            )
+        )
+
+        # Should find common AirPlay protocol
+        assert len(protocol_members) == 1
+        assert "airplay_sonos" in protocol_members
+        assert protocol_domain == "airplay"
+        # For native AirPlay player, protocol_player should be the Apple TV itself
+        assert protocol_player == apple_tv
+
+    def test_get_output_protocol_by_domain_finds_native(self, mock_mass: MagicMock) -> None:
+        """Test that get_output_protocol_by_domain finds native protocol."""
+        controller = PlayerController(mock_mass)
+
+        airplay_provider = MockProvider("airplay", instance_id="airplay", mass=mock_mass)
+
+        # Native AirPlay player
+        apple_tv = MockPlayer(airplay_provider, "apple_tv_1", "Apple TV")
+        apple_tv._attr_supported_features.add(PlayerFeature.PLAY_MEDIA)
+        apple_tv._cache.clear()
+
+        mock_mass.players = controller
+        controller._players = {"apple_tv_1": apple_tv}
+
+        apple_tv.update_state(signal_event=False)
+
+        # Should find native AirPlay protocol
+        protocol = apple_tv.get_output_protocol_by_domain("airplay")
+        assert protocol is not None
+        assert protocol.output_protocol_id == "native"
+        assert protocol.protocol_domain == "airplay"
+        assert protocol.is_native is True
+
+
+class TestFinalGroupMembersTranslation:
+    """Tests for __final_group_members translation of protocol player IDs."""
+
+    def test_final_group_members_translates_protocol_ids(self, mock_mass: MagicMock) -> None:
+        """Test that __final_group_members translates protocol player IDs to visible IDs.
+
+        When a native AirPlay player (Apple TV) has protocol players in its group_members,
+        the final state should show the visible parent player IDs instead.
+        """
+        controller = PlayerController(mock_mass)
+
+        airplay_provider = MockProvider("airplay", instance_id="airplay", mass=mock_mass)
+        sonos_provider = MockProvider("sonos", instance_id="sonos", mass=mock_mass)
+
+        # Apple TV with group members containing a protocol player ID
+        apple_tv = MockPlayer(airplay_provider, "apple_tv_1", "Apple TV")
+        apple_tv._attr_supported_features.add(PlayerFeature.PLAY_MEDIA)
+        apple_tv._attr_supported_features.add(PlayerFeature.SET_MEMBERS)
+        apple_tv._attr_group_members = ["apple_tv_1", "airplay_sonos"]
+        apple_tv._cache.clear()
+
+        # Sonos visible player
+        sonos_player = MockPlayer(sonos_provider, "sonos_1", "Sonos")
+        sonos_player._attr_supported_features.add(PlayerFeature.PLAY_MEDIA)
+        sonos_player._cache.clear()
+
+        # AirPlay protocol player for Sonos
+        sonos_airplay = MockPlayer(
+            airplay_provider,
+            "airplay_sonos",
+            "Sonos (AirPlay)",
+            player_type=PlayerType.PROTOCOL,
+        )
+        sonos_airplay._cache.clear()
+        sonos_airplay.set_protocol_parent_id("sonos_1")
+
+        mock_mass.players = controller
+        controller._players = {
+            "apple_tv_1": apple_tv,
+            "sonos_1": sonos_player,
+            "airplay_sonos": sonos_airplay,
+        }
+        controller._player_throttlers = {
+            "apple_tv_1": Throttler(1, 0.05),
+            "sonos_1": Throttler(1, 0.05),
+            "airplay_sonos": Throttler(1, 0.05),
+        }
+
+        sonos_airplay.update_state(signal_event=False)
+        sonos_player.update_state(signal_event=False)
+        apple_tv.update_state(signal_event=False)
+
+        # Final group_members should show visible player IDs
+        final_members = apple_tv.state.group_members
+        assert "apple_tv_1" in final_members
+        assert "sonos_1" in final_members
+        # Protocol player ID should NOT appear in final state
+        assert "airplay_sonos" not in final_members
+
+
+class TestFinalSyncedToWithNativeProtocolParent:
+    """Tests for __final_synced_to when sync parent is a native protocol player."""
+
+    def test_synced_to_native_airplay_player(self, mock_mass: MagicMock) -> None:
+        """Test that synced_to correctly shows native AirPlay player as parent.
+
+        When a Sonos player's AirPlay protocol player is synced to a native AirPlay
+        player (Apple TV), the Sonos's final synced_to should show the Apple TV.
+        """
+        controller = PlayerController(mock_mass)
+
+        airplay_provider = MockProvider("airplay", instance_id="airplay", mass=mock_mass)
+        sonos_provider = MockProvider("sonos", instance_id="sonos", mass=mock_mass)
+
+        # Apple TV: native AirPlay PLAYER (the group leader)
+        apple_tv = MockPlayer(airplay_provider, "apple_tv_1", "Apple TV")
+        apple_tv._attr_supported_features.add(PlayerFeature.PLAY_MEDIA)
+        apple_tv._attr_supported_features.add(PlayerFeature.SET_MEMBERS)
+        apple_tv._cache.clear()
+
+        # Sonos visible player
+        sonos_player = MockPlayer(sonos_provider, "sonos_1", "Sonos")
+        sonos_player._attr_supported_features.add(PlayerFeature.PLAY_MEDIA)
+        sonos_player._cache.clear()
+
+        # AirPlay protocol player for Sonos - synced to Apple TV
+        sonos_airplay = MockPlayer(
+            airplay_provider,
+            "airplay_sonos",
+            "Sonos (AirPlay)",
+            player_type=PlayerType.PROTOCOL,
+        )
+        # Set group_members with Apple TV first to indicate synced_to Apple TV
+        sonos_airplay._attr_group_members = ["apple_tv_1", "airplay_sonos"]
+        sonos_airplay._cache.clear()
+        sonos_airplay.set_protocol_parent_id("sonos_1")
+
+        sonos_player.set_linked_output_protocols(
+            [
+                OutputProtocol(
+                    output_protocol_id="airplay_sonos",
+                    name="AirPlay",
+                    protocol_domain="airplay",
+                    priority=10,
+                    available=True,
+                )
+            ]
+        )
+
+        mock_mass.players = controller
+        controller._players = {
+            "apple_tv_1": apple_tv,
+            "sonos_1": sonos_player,
+            "airplay_sonos": sonos_airplay,
+        }
+        controller._player_throttlers = {
+            "apple_tv_1": Throttler(1, 0.05),
+            "sonos_1": Throttler(1, 0.05),
+            "airplay_sonos": Throttler(1, 0.05),
+        }
+
+        apple_tv.update_state(signal_event=False)
+        sonos_airplay.update_state(signal_event=False)
+        sonos_player.update_state(signal_event=False)
+
+        # Sonos's final synced_to should be Apple TV (visible player)
+        assert sonos_player.state.synced_to == "apple_tv_1"
+
+
+class TestUngroupTranslation:
+    """Tests for translation when ungrouping from native protocol players."""
+
+    def test_ungroup_translates_visible_to_protocol_id(self, mock_mass: MagicMock) -> None:
+        """Test that ungrouping correctly translates visible ID to protocol ID.
+
+        When ungrouping Sonos from Apple TV, the visible Sonos ID should be
+        translated to its AirPlay protocol player ID for the removal.
+        """
+        controller = PlayerController(mock_mass)
+
+        airplay_provider = MockProvider("airplay", instance_id="airplay", mass=mock_mass)
+        sonos_provider = MockProvider("sonos", instance_id="sonos", mass=mock_mass)
+
+        # Apple TV with Sonos's AirPlay protocol player in group_members
+        apple_tv = MockPlayer(airplay_provider, "apple_tv_1", "Apple TV")
+        apple_tv._attr_supported_features.add(PlayerFeature.PLAY_MEDIA)
+        apple_tv._attr_supported_features.add(PlayerFeature.SET_MEMBERS)
+        apple_tv._attr_group_members = ["apple_tv_1", "airplay_sonos"]
+        apple_tv._cache.clear()
+
+        # Sonos visible player
+        sonos_player = MockPlayer(sonos_provider, "sonos_1", "Sonos")
+        sonos_player._attr_supported_features.add(PlayerFeature.PLAY_MEDIA)
+        sonos_player._cache.clear()
+
+        # AirPlay protocol player for Sonos
+        sonos_airplay = MockPlayer(
+            airplay_provider,
+            "airplay_sonos",
+            "Sonos (AirPlay)",
+            player_type=PlayerType.PROTOCOL,
+        )
+        sonos_airplay._cache.clear()
+        sonos_airplay.set_protocol_parent_id("sonos_1")
+
+        sonos_player.set_linked_output_protocols(
+            [
+                OutputProtocol(
+                    output_protocol_id="airplay_sonos",
+                    name="AirPlay",
+                    protocol_domain="airplay",
+                    priority=10,
+                    available=True,
+                )
+            ]
+        )
+
+        mock_mass.players = controller
+        controller._players = {
+            "apple_tv_1": apple_tv,
+            "sonos_1": sonos_player,
+            "airplay_sonos": sonos_airplay,
+        }
+        controller._player_throttlers = {
+            "apple_tv_1": Throttler(1, 0.05),
+            "sonos_1": Throttler(1, 0.05),
+            "airplay_sonos": Throttler(1, 0.05),
+        }
+
+        sonos_airplay.update_state(signal_event=False)
+        sonos_player.update_state(signal_event=False)
+        apple_tv.update_state(signal_event=False)
+
+        # Translate members for removal - visible ID should become protocol ID
+        _protocol_members, native_members = controller._translate_members_to_remove_for_protocols(
+            parent_player=apple_tv,
+            player_ids=["sonos_1"],  # Visible player ID
+            parent_protocol_player=None,
+            parent_protocol_domain=None,
+        )
+
+        # Should translate to the protocol player ID for native removal
+        assert "airplay_sonos" in native_members
+        assert "sonos_1" not in native_members