From ab812a004c9a4dd43f53490c4c96dc3e68c75abe Mon Sep 17 00:00:00 2001 From: Maxim Raznatovski Date: Tue, 9 Sep 2025 12:00:42 +0200 Subject: [PATCH] Fix issues when disabling a player provider (#2388) After the player refactor some parts of the disabling and deleting of player providers weren't updated yet to use the new APIs. Also `lookup_key` is now used for `provider_filters` since it was already expected by callers. --- music_assistant/controllers/config.py | 13 ++++-------- music_assistant/controllers/players.py | 29 ++++++++++++++++++-------- 2 files changed, 24 insertions(+), 18 deletions(-) diff --git a/music_assistant/controllers/config.py b/music_assistant/controllers/config.py index 250f3c8d..2d49e91a 100644 --- a/music_assistant/controllers/config.py +++ b/music_assistant/controllers/config.py @@ -300,11 +300,11 @@ class ConfigController: # cleanup entries in library await self.mass.music.cleanup_provider(instance_id) if existing["type"] == "player": - # cleanup entries in player manager + # all players should already be removed by now through unload_provider for player in list(self.mass.players): if player.provider.instance_id != instance_id: continue - self.mass.players.remove(player.player_id, cleanup_config=True) + self.mass.players.delete_player_config(player.player_id) # cleanup remaining player configs for player_conf in list(self.get(CONF_PLAYERS, {}).values()): if player_conf["provider"] == instance_id: @@ -442,7 +442,7 @@ class ConfigController: raise ActionUnavailable("Can not remove config for an active player!") # tell the player manager to remove the player if its lingering around # set permanent to false otherwise we end up in an infinite loop - self.mass.players.unregister(player_id, permanent=False) + await self.mass.players.unregister(player_id, permanent=False) # remove the actual config if all of the above passed self.remove(conf_key) # Also remove the DSP config if it exists @@ -979,12 +979,7 @@ class ConfigController: if dep_prov.manifest.depends_on == config.domain: await self.mass.unload_provider(dep_prov.instance_id) await self.mass.unload_provider(config.instance_id) - if config.type == ProviderType.PLAYER: - # cleanup entries in player manager - for player in self.mass.players.all(return_unavailable=True, return_disabled=True): - if player.provider.instance_id != instance_id: - continue - self.mass.players.remove(player.player_id, cleanup_config=False) + # For player providers, unload_provider should have removed all its players by now return config async def _add_provider_config( diff --git a/music_assistant/controllers/players.py b/music_assistant/controllers/players.py index f9aea235..7ac0451f 100644 --- a/music_assistant/controllers/players.py +++ b/music_assistant/controllers/players.py @@ -161,7 +161,7 @@ class PlayerController(CoreController): :param return_unavailable [bool]: Include unavailable players. :param return_disabled [bool]: Include disabled players. - :param provider_filter [str]: Optional filter by provider ID. + :param provider_filter [str]: Optional filter by provider lookup key. :return: List of Player objects. """ @@ -170,7 +170,7 @@ class PlayerController(CoreController): for player in self._players.values() if (player.available or return_unavailable) and (player.enabled or return_disabled) - and (provider_filter is None or player.provider.instance_id == provider_filter) + and (provider_filter is None or player.provider.lookup_key == provider_filter) ] @api_command("players/all") @@ -185,7 +185,7 @@ class PlayerController(CoreController): :param return_unavailable [bool]: Include unavailable players. :param return_disabled [bool]: Include disabled players. - :param provider_filter [str]: Optional filter by provider ID. + :param provider_filter [str]: Optional filter by provider lookup key. :return: List of PlayerState objects. """ @@ -1242,7 +1242,7 @@ class PlayerController(CoreController): self.mass.player_queues.on_player_remove(player_id, permanent=permanent) await player.on_unload() if permanent: - self.mass.config.remove(f"players/{player_id}") + self.delete_player_config(player_id) self.mass.signal_event(EventType.PLAYER_REMOVED, player_id) @api_command("players/remove") @@ -1254,11 +1254,8 @@ class PlayerController(CoreController): """ player = self.get(player_id) if player is None: - # we simply permanently delete the player by wiping its config - conf_key = f"{CONF_PLAYERS}/{player_id}" - dsp_conf_key = f"{CONF_PLAYER_DSP}/{player_id}" - for key in (conf_key, dsp_conf_key): - self.mass.config.remove(key) + # we simply permanently delete the player config since it is not registered + self.delete_player_config(player_id) return if player.type == PlayerType.GROUP: # Handle group player removal @@ -1273,6 +1270,20 @@ class PlayerController(CoreController): await group_player.set_members( player_ids_to_remove=[player_id], ) + # We removed the player and can now clean up its config + self.delete_player_config(player_id) + + def delete_player_config(self, player_id: str) -> None: + """ + Permanently delete a player's configuration. + + Should only be called for players that are not registered by the player controller. + """ + # we simply permanently delete the player by wiping its config + conf_key = f"{CONF_PLAYERS}/{player_id}" + dsp_conf_key = f"{CONF_PLAYER_DSP}/{player_id}" + for key in (conf_key, dsp_conf_key): + self.mass.config.remove(key) def signal_player_state_update( self, -- 2.34.1