From: Marcel van der Veldt Date: Fri, 20 Feb 2026 14:20:15 +0000 (+0100) Subject: More fixes for protocol linking X-Git-Url: https://git.kitaultman.com/?a=commitdiff_plain;h=67cd5f92737f63e73d1ab6f050b7af9d426e5682;p=music-assistant-server.git More fixes for protocol linking --- diff --git a/music_assistant/controllers/players/README.md b/music_assistant/controllers/players/README.md index 6458e875..f102781c 100644 --- a/music_assistant/controllers/players/README.md +++ b/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.) │ diff --git a/music_assistant/controllers/players/controller.py b/music_assistant/controllers/players/controller.py index ed33de88..67102a94 100644 --- a/music_assistant/controllers/players/controller.py +++ b/music_assistant/controllers/players/controller.py @@ -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", diff --git a/music_assistant/controllers/players/protocol_linking.py b/music_assistant/controllers/players/protocol_linking.py index 2cb4d808..cb74b060 100644 --- a/music_assistant/controllers/players/protocol_linking.py +++ b/music_assistant/controllers/players/protocol_linking.py @@ -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, ) diff --git a/music_assistant/models/player.py b/music_assistant/models/player.py index bb44dad6..a4f25edb 100644 --- a/music_assistant/models/player.py +++ b/music_assistant/models/player.py @@ -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 diff --git a/tests/core/test_protocol_linking.py b/tests/core/test_protocol_linking.py index 8d088905..a38b3ea3 100644 --- a/tests/core/test_protocol_linking.py +++ b/tests/core/test_protocol_linking.py @@ -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