From 19f4d7346089cdb1ae4f299c4eb6c64a8a5f36f3 Mon Sep 17 00:00:00 2001 From: Marcel van der Veldt Date: Sun, 22 Feb 2026 01:44:55 +0100 Subject: [PATCH] Simplify can_group_with logic --- music_assistant/models/player.py | 53 +++++++++++++++--------------- tests/core/test_player_grouping.py | 39 ---------------------- 2 files changed, 27 insertions(+), 65 deletions(-) diff --git a/music_assistant/models/player.py b/music_assistant/models/player.py index 9575dd2b..23379492 100644 --- a/music_assistant/models/player.py +++ b/music_assistant/models/player.py @@ -1640,10 +1640,14 @@ class Player(ABC): # 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 self.type == PlayerType.PROTOCOL: + # protocol players use their own group members without translation + members.extend(self.group_members) + else: + 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": @@ -1730,7 +1734,6 @@ class Player(ABC): All protocol player IDs are translated to their visible parent player IDs. """ - result: set[str] = set() def _should_include_player(player: Player) -> bool: """Check if a player should be included in the can-group-with set.""" @@ -1739,40 +1742,38 @@ class Player(ABC): if player.player_id == self.player_id: return False # Don't include self # Don't include (playing) players that have group members (they are group leaders) - if ( + if ( # noqa: SIM103 player.state.playback_state in (PlaybackState.PLAYING, PlaybackState.PAUSED) and player.group_members - and player.type != PlayerType.PROTOCOL ): - return False # Regular native group leader - exclude - # Don't include players that are currently grouped/synced to OTHER players - # But DO include players grouped to THIS player (so they can be ungrouped) - grouped_to = player.state.synced_to or player.state.active_group - return grouped_to is None or grouped_to == self.player_id + return False + return True if self.__final_synced_to: # player is already synced/grouped, cannot group with others - return result + return set() - # always start with the native can_group_with options (expanded for provider instance IDs) - for player in self._expand_can_group_with(): + expanded_can_group_with = self._expand_can_group_with() + # Scenario 1: Player is a protocol player - just return the (expanded) result + if self.type == PlayerType.PROTOCOL: + return {x.player_id for x in expanded_can_group_with} + + result: set[str] = set() + # always start with the native can_group_with options (expanded from provider instance IDs) + # NOTE we need to translate protocol player IDs to visible player IDs here as well, + # to cover cases where a native player (e.g., native AirPlay) has grouped protocol players + # (e.g., Sonos AirPlay protocol players) + for player in expanded_can_group_with: if player.type == PlayerType.PROTOCOL: - # Protocol player is hidden - translate to its visible parent player if not player.protocol_parent_id: continue - visible_parent = self.mass.players.get_player(player.protocol_parent_id) - if not visible_parent or not _should_include_player(visible_parent): - continue - result.add(visible_parent.player_id) - else: - if not _should_include_player(player): + parent_player = self.mass.players.get_player(player.protocol_parent_id) + if not parent_player or not _should_include_player(parent_player): continue + result.add(parent_player.player_id) + elif _should_include_player(player): result.add(player.player_id) - # Scenario 1: Player is a protocol player - just return the (expanded) result - if self.type == PlayerType.PROTOCOL: - return result - # Scenario 2: External source is active - don't include protocol-based grouping # When an external source (e.g., Spotify Connect, TV) is active, grouping via # protocols (AirPlay, Sendspin, etc.) wouldn't work - only native grouping is available. diff --git a/tests/core/test_player_grouping.py b/tests/core/test_player_grouping.py index 59e0d5c8..eb7920eb 100644 --- a/tests/core/test_player_grouping.py +++ b/tests/core/test_player_grouping.py @@ -124,45 +124,6 @@ class TestCanGroupWithBasics: class TestSyncedPlayers: """Test behavior with synced/grouped players.""" - def test_synced_player_excluded_from_others(self, mock_mass: MagicMock) -> None: - """ - Test that a player synced to another is excluded from other players' can_group_with. - - Regression test for: Player synced to another showing up in third player's can_group_with. - """ - controller = PlayerController(mock_mass) - provider = MockProvider("test_provider", instance_id="test", mass=mock_mass) - - # Sync leader - leader = MockPlayer(provider, "leader", "Leader") - leader._attr_supported_features.add(PlayerFeature.SET_MEMBERS) - leader._attr_can_group_with = {"synced", "other"} - leader._attr_group_members = ["leader", "synced"] - leader._attr_playback_state = PlaybackState.PLAYING # Make it playing so it gets excluded - - # Synced player - synced = MockPlayer(provider, "synced", "Synced") - - # Third player - other = MockPlayer(provider, "other", "Other") - other._attr_supported_features.add(PlayerFeature.SET_MEMBERS) - other._attr_can_group_with = {"leader", "synced"} - - controller._players = {"leader": leader, "synced": synced, "other": other} - mock_mass.players = controller - - # Trigger synced_to calculation - leader.update_state(signal_event=False) - synced.update_state(signal_event=False) - other.update_state(signal_event=False) - - # The synced player should NOT appear in other's can_group_with - assert "synced" not in other.state.can_group_with - # The leader should also NOT appear (has group members) - assert "leader" not in other.state.can_group_with - # Other should only see itself as ungrouped - assert other.state.can_group_with == set() - def test_sync_leader_excludes_itself_from_members_can_group_with( self, mock_mass: MagicMock ) -> None: -- 2.34.1