More playergroups improvements (#1723)

music_assistant/common/models/enums.py

@@ -368,6 +368,7 @@ class ProviderFeature(StrEnum):
  # PLAYERPROVIDER FEATURES
  #
  SYNC_PLAYERS = "sync_players"
+ REMOVE_PLAYER = "remove_player"
 
  #
  # METADATAPROVIDER FEATURES

music_assistant/common/models/errors.py

@@ -127,3 +127,9 @@ class ProviderPermissionDenied(MusicAssistantError):
  """Error thrown when a provider action is denied because of permissions."""
 
  error_code = 18
+
+
+class ActionUnavailable(MusicAssistantError):
+ """Error thrown when a action is denied because is is (temporary) unavailable/not possible."""
+
+ error_code = 19

music_assistant/server/controllers/config.py

@@ -5,6 +5,7 @@
 import base64
 import logging
 import os
+from contextlib import suppress
 from typing import TYPE_CHECKING, Any
 from uuid import uuid4
 
@@ -25,8 +26,13 @@
  PlayerConfig,
  ProviderConfig,
 )
-from music_assistant.common.models.enums import EventType, ProviderType
-from music_assistant.common.models.errors import InvalidDataError
+from music_assistant.common.models.enums import EventType, ProviderFeature, ProviderType
+from music_assistant.common.models.errors import (
+ ActionUnavailable,
+ InvalidDataError,
+ PlayerCommandFailed,
+ UnsupportedFeaturedException,
+)
 from music_assistant.constants import (
  CONF_CORE,
  CONF_PLAYERS,
@@ -374,11 +380,12 @@ async def save_player_config(
  """Save/update PlayerConfig."""
  config = await self.get_player_config(player_id)
  changed_keys = config.update(values)
-
  if not changed_keys:
  # no changes
  return None
-
+ # validate/handle the update in the player manager
+ await self.mass.players.on_player_config_change(config, changed_keys)
+ # actually store changes (if the above did not raise)
  conf_key = f"{CONF_PLAYERS}/{player_id}"
  self.set(conf_key, config.to_raw())
  # send config updated event
@@ -387,8 +394,6 @@ async def save_player_config(
  object_id=config.player_id,
  data=config,
  )
- # signal update to the player manager
- self.mass.players.on_player_config_changed(config, changed_keys)
  # return full player config (just in case)
  return await self.get_player_config(player_id)
 
@@ -398,11 +403,32 @@ async def remove_player_config(self, player_id: str) -> None:
  conf_key = f"{CONF_PLAYERS}/{player_id}"
  existing = self.get(conf_key)
  if not existing:
- msg = f"Player {player_id} does not exist"
+ msg = f"Player configuration for {player_id} does not exist"
  raise KeyError(msg)
+ player = self.mass.players.get(player_id)
+ player_prov = player.provider if player else existing["provider"]
+ player_provider = self.mass.get_provider(player_prov)
+ if player_provider and ProviderFeature.REMOVE_PLAYER in player_provider.supported_features:
+ # provider supports removal of player (e.g. group player)
+ await player_provider.remove_player(player_id)
+ elif player and player_provider and player.available:
+ # removing a player config while it is active is not allowed
+ # unless the provider repoirts it has the remove_player feature (e.g. group player)
+ raise ActionUnavailable("Can not remove config for an active player!")
+ # check for group memberships that need to be updated
+ if player and player.active_group and player_provider:
+ # try to remove from the group
+ group_player = self.mass.players.get(player.active_group)
+ with suppress(UnsupportedFeaturedException, PlayerCommandFailed):
+ await player_provider.set_members(
+ player.active_group,
+ [x for x in group_player.group_childs if x != player.player_id],
+ )
+ # tell the player manager to remove the player if its lingering around
+ # set cleanup_flag to false otherwise we end up in an infinite loop
+ self.mass.players.remove(player_id, cleanup_config=False)
+ # remove the actual config if all of the above passed
  self.remove(conf_key)
- # signal update to the player manager
- self.mass.players.on_player_config_removed(player_id)
 
  def create_default_player_config(
  self,

music_assistant/server/controllers/player_queues.py

@@ -653,6 +653,7 @@ async def play(self, queue_id: str) -> None:
  return
  if (
  (queue := self._queues.get(queue_id))
+ and queue.active
  and queue_player.powered
  and queue.state == PlayerState.PAUSED
  ):

music_assistant/server/controllers/players.py

@@ -648,7 +648,7 @@ async def cmd_sync_many(self, target_player: str, child_player_ids: list[str]) -
  for child_player_id in child_player_ids:
  if child_player_id == target_player:
  continue
- if not (child_player := self.get(child_player_id)):
+ if not (child_player := self.get(child_player_id)) or not child_player.available:
  self.logger.warning("Player %s is not available", child_player_id)
  continue
  if PlayerFeature.SYNC not in child_player.supported_features:
@@ -756,7 +756,7 @@ def register_or_update(self, player: Player) -> None:
  self.register(player)
 
  def remove(self, player_id: str, cleanup_config: bool = True) -> None:
- """Remove a player from the registry."""
+ """Remove a player from the player manager."""
  player = self._players.pop(player_id, None)
  if player is None:
  return
@@ -821,6 +821,10 @@ def update(
  # ignore updates for disabled players
  return
 
+ # correct available state if needed
+ if not player.enabled:
+ player.available = False
+
  # always signal update to the playerqueue
  self.mass.player_queues.on_player_update(player, changed_values)
 
@@ -976,7 +980,7 @@ def iter_group_members(
  """Get (child) players attached to a group player or syncgroup."""
  for child_id in list(group_player.group_childs):
  if child_player := self.get(child_id, False):
- if not child_player.available:
+ if not child_player.available or not child_player.enabled:
  continue
  if not (not only_powered or child_player.powered):
  continue
@@ -1028,32 +1032,33 @@ async def _poll_players(self) -> None:
  self.mass.loop.call_soon(self.update, player_id)
  await asyncio.sleep(1)
 
- def on_player_config_changed(self, config: PlayerConfig, changed_keys: set[str]) -> None:
+ async def on_player_config_change(self, config: PlayerConfig, changed_keys: set[str]) -> None:
  """Call (by config manager) when the configuration of a player changes."""
+ player_disabled = "enabled" in changed_keys and not config.enabled
+ # signal player provider that the config changed
+ if player_provider := self.mass.get_provider(config.provider):
+ with suppress(PlayerUnavailableError):
+ await player_provider.on_player_config_change(config, changed_keys)
  if not (player := self.mass.players.get(config.player_id)):
  return
- if config.enabled:
- player_prov = self.mass.players.get_player_provider(config.player_id)
- self.mass.create_task(player_prov.poll_player(config.player_id))
+ if player_disabled:
+ # edge case: ensure that the player is powered off if the player gets disabled
+ await self.cmd_power(config.player_id, False)
+ player.available = False
+ # if the player was playing, restart playback
+ elif not player_disabled and player.state == PlayerState.PLAYING:
+ self.mass.call_later(1, self.mass.player_queues.resume(player.active_source))
+ # check for group memberships that need to be updated
+ if player_disabled and player.active_group and player_provider:
+ # try to remove from the group
+ group_player = self.mass.players.get(player.active_group)
+ with suppress(UnsupportedFeaturedException, PlayerCommandFailed):
+ await player_provider.set_members(
+ player.active_group,
+ [x for x in group_player.group_childs if x != player.player_id],
+ )
  player.enabled = config.enabled
- # signal player provider that the config changed
- with suppress(PlayerUnavailableError):
- if provider := self.mass.get_provider(config.provider):
- provider.on_player_config_changed(config, changed_keys)
  self.mass.players.update(config.player_id, force_update=True)
- # if the player was playing, restart playback
- if player and player.state == PlayerState.PLAYING:
- self.mass.create_task(self.mass.player_queues.resume(player.active_source))
-
- def on_player_config_removed(self, player_id: str) -> None:
- """Call (by config manager) when the configuration of a player is removed."""
- if (player := self.mass.players.get(player_id)) and player.available:
- self.mass.players.update(player_id, force_update=True)
- if player and (provider := self.mass.get_provider(player.provider)):
- provider = cast(PlayerProvider, provider)
- provider.on_player_config_removed(player_id)
- if not self.mass.players.get(player_id):
- self.mass.signal_event(EventType.PLAYER_REMOVED, player_id)
 
  async def _play_announcement(
  self,

music_assistant/server/models/music_provider.py

@@ -57,6 +57,10 @@ def lookup_key(self) -> str:
  return self.domain
  return self.instance_id
 
+ async def loaded_in_mass(self) -> None:
+ """Call after the provider has been loaded."""
+ self.mass.music.start_sync(providers=[self.instance_id])
+
  async def search(
  self,
  search_query: str,

music_assistant/server/models/player_provider.py

@@ -4,6 +4,9 @@
 
 from abc import abstractmethod
 
+from zeroconf import ServiceStateChange
+from zeroconf.asyncio import AsyncServiceInfo
+
 from music_assistant.common.models.config_entries import (
  BASE_PLAYER_CONFIG_ENTRIES,
  CONF_ENTRY_ANNOUNCE_VOLUME,
@@ -13,6 +16,7 @@
  ConfigEntry,
  PlayerConfig,
 )
+from music_assistant.common.models.errors import UnsupportedFeaturedException
 from music_assistant.common.models.player import Player, PlayerMedia
 
 from .provider import Provider
@@ -26,6 +30,10 @@ class PlayerProvider(Provider):
  Player Provider implementations should inherit from this base model.
  """
 
+ async def loaded_in_mass(self) -> None:
+ """Call after the provider has been loaded."""
+ await self.discover_players()
+
  async def get_player_config_entries(self, player_id: str) -> tuple[ConfigEntry, ...]:
  """Return all (provider/player specific) Config Entries for the given player (if any)."""
  return (
@@ -37,11 +45,19 @@ async def get_player_config_entries(self, player_id: str) -> tuple[ConfigEntry,
  CONF_ENTRY_ANNOUNCE_VOLUME_MAX,
  )
 
- def on_player_config_changed(self, config: PlayerConfig, changed_keys: set[str]) -> None:
+ async def on_player_config_change(self, config: PlayerConfig, changed_keys: set[str]) -> None:
  """Call (by config manager) when the configuration of a player changes."""
-
- def on_player_config_removed(self, player_id: str) -> None:
- """Call (by config manager) when the configuration of a player is removed."""
+ # default implementation: feel free to override
+ if (
+ "enabled" in changed_keys
+ and config.enabled
+ and not self.mass.players.get(config.player_id)
+ ):
+ # if a player gets enabled, trigger discovery
+ task_id = f"discover_players_{self.instance_id}"
+ self.mass.call_later(5, self.discover_players, task_id=task_id)
+ else:
+ await self.poll_player(config.player_id)
 
  @abstractmethod
  async def cmd_stop(self, player_id: str) -> None:
@@ -173,6 +189,34 @@ async def poll_player(self, player_id: str) -> None:
  if 'needs_poll' is set to True in the player object.
  """
 
+ async def remove_player(self, player_id: str) -> None:
+ """Remove a player."""
+ # will only be called for players with REMOVE_PLAYER feature set.
+ raise NotImplementedError
+
+ async def discover_players(self) -> None:
+ """Discover players for this provider."""
+ # This will be called (once) when the player provider is loaded into MA.
+ # Default implementation is mdns discovery, which will also automatically
+ # discovery players during runtime. If a provider overrides this method and
+ # doesn't use mdns, it is responsible for periodically searching for new players.
+ if not self.available:
+ return
+ for mdns_type in self.manifest.mdns_discovery or []:
+ for mdns_name in set(self.mass.aiozc.zeroconf.cache.cache):
+ if mdns_type not in mdns_name or mdns_type == mdns_name:
+ continue
+ info = AsyncServiceInfo(mdns_type, mdns_name)
+ if await info.async_request(self.mass.aiozc.zeroconf, 3000):
+ await self.on_mdns_service_state_change(
+ mdns_name, ServiceStateChange.Added, info
+ )
+
+ async def set_members(self, player_id: str, members: list[str]) -> None:
+ """Set members for a groupplayer."""
+ # will only be called for (group)players with SET_MEMBERS feature set.
+ raise UnsupportedFeaturedException
+
  # DO NOT OVERRIDE BELOW
 
  @property

music_assistant/server/providers/_template_player_provider/__init__.py

@@ -235,18 +235,10 @@ async def get_player_config_entries(self, player_id: str) -> tuple[ConfigEntry,
  # Please note that you need to call the super() method to get the default entries.
  return ()
 
- def on_player_config_changed(self, config: PlayerConfig, changed_keys: set[str]) -> None:
+ async def on_player_config_change(self, config: PlayerConfig, changed_keys: set[str]) -> None:
  """Call (by config manager) when the configuration of a player changes."""
  # OPTIONAL
- # this callback will be called whenever a player config changes
- # you can use this to react to changes in player configuration
- # but this is completely optional and you can leave it out if not needed.
-
- def on_player_config_removed(self, player_id: str) -> None:
- """Call (by config manager) when the configuration of a player is removed."""
- # OPTIONAL
- # ensure that any group players get removed
- # this callback will be called whenever a player config is removed
+ # this will be called whenever a player config changes
  # you can use this to react to changes in player configuration
  # but this is completely optional and you can leave it out if not needed.
 

music_assistant/server/providers/airplay/__init__.py

@@ -865,7 +865,6 @@ async def _setup_player(
  if address is None:
  return
  self.logger.debug("Discovered Airplay device %s on %s", display_name, address)
- self._players[player_id] = AirPlayPlayer(self, player_id, info, address)
  manufacturer, model = get_model_from_am(info.decoded_properties.get("am"))
  if "apple tv" in model.lower():
  # For now, we ignore the Apple TV until we implement the authentication.
@@ -878,6 +877,7 @@ async def _setup_player(
  if not self.mass.config.get_raw_player_config_value(player_id, "enabled", True):
  self.logger.debug("Ignoring %s in discovery as it is disabled.", display_name)
  return
+ self._players[player_id] = AirPlayPlayer(self, player_id, info, address)
  if not (volume := await self.mass.cache.get(player_id, base_key=CACHE_KEY_PREV_VOLUME)):
  volume = FALLBACK_VOLUME
  mass_player = Player(

music_assistant/server/providers/bluesound/__init__.py

@@ -269,7 +269,6 @@ async def on_mdns_service_state_change(
  if not mass_player.available:
  self.logger.debug("Player back online: %s", mass_player.display_name)
  bluos_player.client.sync()
- mass_player.available = True
  bluos_player.discovery_info = info
  self.mass.players.update(self.player_id)
  return

music_assistant/server/providers/builtin/__init__.py

@@ -141,6 +141,7 @@ async def loaded_in_mass(self) -> None:
  self._playlists_dir = os.path.join(self.mass.storage_path, "playlists")
  if not await asyncio.to_thread(os.path.exists, self._playlists_dir):
  await asyncio.to_thread(os.mkdir, self._playlists_dir)
+ await super().loaded_in_mass()
 
  @property
  def is_streaming_provider(self) -> bool:

music_assistant/server/providers/chromecast/__init__.py

@@ -39,7 +39,7 @@
  from pychromecast.models import CastInfo
  from pychromecast.socket_client import ConnectionStatus
 
- from music_assistant.common.models.config_entries import PlayerConfig, ProviderConfig
+ from music_assistant.common.models.config_entries import ProviderConfig
  from music_assistant.common.models.provider import ProviderManifest
  from music_assistant.server import MusicAssistant
  from music_assistant.server.models import ProviderInstanceType
@@ -145,8 +145,8 @@ def __init__(
  else:
  logging.getLogger("pychromecast").setLevel(self.logger.level + 10)
 
- async def loaded_in_mass(self) -> None:
- """Call after the provider has been loaded."""
+ async def discover_players(self) -> None:
+ """Discover Cast players on the network."""
  # start discovery in executor
  await self.mass.loop.run_in_executor(None, self.browser.start_discovery)
 
@@ -182,16 +182,6 @@ async def get_player_config_entries(self, player_id: str) -> tuple[ConfigEntry,
  base_entries = await super().get_player_config_entries(player_id)
  return (*base_entries, *PLAYER_CONFIG_ENTRIES, CONF_ENTRY_SAMPLE_RATES_CAST)
 
- def on_player_config_changed(
- self,
- config: PlayerConfig,
- changed_keys: set[str],
- ) -> None:
- """Call (by config manager) when the configuration of a player changes."""
- super().on_player_config_changed(config, changed_keys)
- if "enabled" in changed_keys and config.player_id not in self.castplayers:
- self.mass.create_task(self.mass.load_provider, self.instance_id)
-
  async def cmd_stop(self, player_id: str) -> None:
  """Send STOP command to given player."""
  castplayer = self.castplayers[player_id]