From: Marcel van der Veldt Date: Thu, 29 Jan 2026 11:15:04 +0000 (+0100) Subject: Improvements and bugfixes for player and provider config entry handling (#3049) X-Git-Url: https://git.kitaultman.com/?a=commitdiff_plain;h=f1829e88515262cf570cf518f21e3c5039218279;p=music-assistant-server.git Improvements and bugfixes for player and provider config entry handling (#3049) --- diff --git a/music_assistant/constants.py b/music_assistant/constants.py index 88b57820..0dff508f 100644 --- a/music_assistant/constants.py +++ b/music_assistant/constants.py @@ -1,6 +1,7 @@ """All constants for Music Assistant.""" import pathlib +from copy import deepcopy from typing import Final, cast from music_assistant_models.config_entries import ( @@ -821,7 +822,7 @@ def create_sample_rates_config_entry( supported_bit_depths = [16] final_supported_sample_rates = supported_sample_rates or [] final_supported_bit_depths = supported_bit_depths or [] - conf_entry = ConfigEntry.from_dict(CONF_ENTRY_SAMPLE_RATES.to_dict()) + conf_entry = deepcopy(CONF_ENTRY_SAMPLE_RATES) conf_entry.hidden = hidden options: list[ConfigValueOption] = [] default_value: list[str] = [] diff --git a/music_assistant/controllers/config.py b/music_assistant/controllers/config.py index 71677082..4607da61 100644 --- a/music_assistant/controllers/config.py +++ b/music_assistant/controllers/config.py @@ -101,8 +101,8 @@ from music_assistant.constants import ( DEFAULT_PROVIDER_CONFIG_ENTRIES, ENCRYPT_SUFFIX, NON_HTTP_PROVIDERS, + SYNCGROUP_PREFIX, ) -from music_assistant.controllers.players.sync_groups import SyncGroupPlayer from music_assistant.helpers.api import api_command from music_assistant.helpers.json import JSON_DECODE_EXCEPTIONS, async_json_dumps, async_json_loads from music_assistant.helpers.util import load_provider_module, validate_announcement_chime_url @@ -139,7 +139,6 @@ class ConfigController: self._data: dict[str, Any] = {} self.filename = os.path.join(self.mass.storage_path, "settings.json") self._timer_handle: asyncio.TimerHandle | None = None - self._value_cache: dict[str, ConfigValueType] = {} async def setup(self) -> None: """Async initialize of controller.""" @@ -341,23 +340,20 @@ class ConfigController: perform runtime type validation. Callers are responsible for ensuring the specified type matches the actual config value type. """ - cache_key = f"prov_conf_value_{instance_id}.{key}" - if (cached_value := self._value_cache.get(cache_key)) is not None: - return cached_value + # prefer stored value so we don't have to retrieve all config entries every time + if (raw_value := self.get_raw_provider_config_value(instance_id, key)) is not None: + return raw_value conf = await self.get_provider_config(instance_id) if key not in conf.values: if default is not None: return default msg = f"Config key {key} not found for provider {instance_id}" raise KeyError(msg) - val = ( + return ( conf.values[key].value if conf.values[key].value is not None else conf.values[key].default_value ) - # store value in cache because this method can potentially be called very often - self._value_cache[cache_key] = val - return val @api_command("config/providers/get_entries") async def get_provider_config_entries( # noqa: PLR0915 @@ -391,9 +387,6 @@ class ConfigController: LOGGER.exception(msg) return [] - if values is None: - values = self.get(f"{CONF_PROVIDERS}/{instance_id}/values", {}) if instance_id else {} - # add dynamic optional config entries that depend on features if instance_id and (provider := self.mass.get_provider(instance_id)): supported_features = provider.supported_features @@ -465,10 +458,14 @@ class ConfigController: self.mass, instance_id=instance_id, action=action, values=values ), ] - # set current value from stored values - for entry in all_entries: - if entry.value is None: - entry.value = values.get(entry.key, None) + if action and values is not None: + # set current value from passed values for config entries + # only do this if we're passed values (e.g. during an action) + # deepcopy here to avoid modifying original entries + all_entries = [deepcopy(entry) for entry in all_entries] + for entry in all_entries: + if entry.value is None: + entry.value = values.get(entry.key, entry.default_value) return all_entries @api_command("config/providers/save", required_role="admin") @@ -528,6 +525,11 @@ class ConfigController: return self.remove(conf_key) + def set_provider_default_name(self, instance_id: str, default_name: str) -> None: + """Set (or update) the default name for a provider.""" + conf_key = f"{CONF_PROVIDERS}/{instance_id}/default_name" + self.set(conf_key, default_name) + @api_command("config/players") async def get_player_configs( self, @@ -609,22 +611,24 @@ class ConfigController: if not (player := self.mass.players.get(player_id, False)): msg = f"Player {player_id} not found" raise KeyError(msg) - - if values is None: - values = self.get(f"{CONF_PLAYERS}/{player_id}/values", {}) - - player_entries = await player.get_config_entries(action=action, values=values) + # get player(protocol) specific entries + player_entries = await self._get_player_config_entries(player, action=action, values=values) + # get default entries which are common for all players default_entries = self._get_default_player_config_entries(player) player_entries_keys = {entry.key for entry in player_entries} all_entries = [ - *player_entries, # ignore default entries that were overridden by the player specific ones *[x for x in default_entries if x.key not in player_entries_keys], + *player_entries, ] - # set current value from stored values - for entry in all_entries: - if entry.value is None: - entry.value = values.get(entry.key, None) + if action and values is not None: + # set current value from passed values for config entries + # only do this if we're passed values (e.g. during an action) + # deepcopy here to avoid modifying original entries + all_entries = [deepcopy(entry) for entry in all_entries] + for entry in all_entries: + if entry.value is None: + entry.value = values.get(entry.key, entry.default_value) return all_entries @overload @@ -693,6 +697,10 @@ class ConfigController: perform runtime type validation. Callers are responsible for ensuring the specified type matches the actual config value type. """ + # prefer stored value so we don't have to retrieve all config entries every time + if (raw_value := self.get_raw_player_config_value(player_id, key)) is not None: + if not unpack_splitted_values: + return raw_value conf = await self.get_player_config(player_id) if key not in conf.values: if default is not None: @@ -809,6 +817,54 @@ class ConfigController: conf_key = f"{CONF_PLAYERS}/{player_id}/default_name" self.set(conf_key, default_name) + def set_player_type(self, player_id: str, player_type: PlayerType) -> None: + """Set (or update) the type for a player.""" + conf_key = f"{CONF_PLAYERS}/{player_id}/player_type" + self.set(conf_key, player_type) + + def create_default_player_config( + self, + player_id: str, + provider: str, + player_type: PlayerType, + name: str | None = None, + enabled: bool = True, + values: dict[str, ConfigValueType] | None = None, + ) -> None: + """ + Create default/empty PlayerConfig. + + This is meant as helper to create default configs when a player is registered. + Called by the player manager on player register. + """ + # return early if the config already exists + if existing_conf := self.get(f"{CONF_PLAYERS}/{player_id}"): + # update default name if needed + if name and name != existing_conf.get("default_name"): + self.set(f"{CONF_PLAYERS}/{player_id}/default_name", name) + # update player_type if needed + if existing_conf.get("player_type") != player_type: + self.set(f"{CONF_PLAYERS}/{player_id}/player_type", player_type.value) + return + # config does not yet exist, create a default one + conf_key = f"{CONF_PLAYERS}/{player_id}" + default_conf = PlayerConfig( + values={}, + provider=provider, + player_id=player_id, + enabled=enabled, + name=name, + default_name=name, + player_type=player_type, + ) + default_conf_raw = default_conf.to_raw() + if values is not None: + default_conf_raw["values"] = values + self.set( + conf_key, + default_conf_raw, + ) + @api_command("config/players/dsp/get") def get_player_dsp_config(self, player_id: str) -> DSPConfig: """ @@ -888,44 +944,6 @@ class ConfigController: data=all_presets, ) - def create_default_player_config( - self, - player_id: str, - provider: str, - name: str | None = None, - enabled: bool = True, - values: dict[str, ConfigValueType] | None = None, - ) -> None: - """ - Create default/empty PlayerConfig. - - This is meant as helper to create default configs when a player is registered. - Called by the player manager on player register. - """ - # return early if the config already exists - if self.get(f"{CONF_PLAYERS}/{player_id}"): - # update default name if needed - if name: - self.set(f"{CONF_PLAYERS}/{player_id}/default_name", name) - return - # config does not yet exist, create a default one - conf_key = f"{CONF_PLAYERS}/{player_id}" - default_conf = PlayerConfig( - values={}, - provider=provider, - player_id=player_id, - enabled=enabled, - name=name, - default_name=name, - ) - default_conf_raw = default_conf.to_raw() - if values is not None: - default_conf_raw["values"] = values - self.set( - conf_key, - default_conf_raw, - ) - async def create_builtin_provider_config(self, provider_domain: str) -> None: """ Create builtin ProviderConfig. @@ -1040,6 +1058,9 @@ class ConfigController: perform runtime type validation. Callers are responsible for ensuring the specified type matches the actual config value type. """ + # prefer stored value so we don't have to retrieve all config entries every time + if (raw_value := self.get_raw_core_config_value(domain, key)) is not None: + return raw_value conf = await self.get_core_config(domain) if key not in conf.values: if default is not None: @@ -1066,17 +1087,19 @@ class ConfigController: action: [optional] action key called from config entries UI. values: the (intermediate) raw values for config entries sent with the action. """ - if values is None: - values = self.get(f"{CONF_CORE}/{domain}/values", {}) controller: CoreController = getattr(self.mass, domain) all_entries = list( await controller.get_config_entries(action=action, values=values) + DEFAULT_CORE_CONFIG_ENTRIES ) - # set current value from stored values - for entry in all_entries: - if entry.value is None: - entry.value = values.get(entry.key, None) + if action and values is not None: + # set current value from passed values for config entries + # only do this if we're passed values (e.g. during an action) + # deepcopy here to avoid modifying original entries + all_entries = [deepcopy(entry) for entry in all_entries] + for entry in all_entries: + if entry.value is None: + entry.value = values.get(entry.key, entry.default_value) return all_entries @api_command("config/core/save", required_role="admin") @@ -1226,7 +1249,6 @@ class ConfigController: def save(self, immediate: bool = False) -> None: """Schedule save of data to disk.""" - self._value_cache = {} if self._timer_handle is not None: self._timer_handle.cancel() self._timer_handle = None @@ -1463,6 +1485,11 @@ class ConfigController: if config.enabled and prov_instance and available: # update config for existing/loaded provider instance await prov_instance.update_config(config, changed_keys) + # push instance name to config (to persist it if it was autogenerated) + if prov_instance.default_name != config.default_name: + self.set_provider_default_name( + prov_instance.instance_id, prov_instance.default_name + ) elif config.enabled: # provider is enabled but not available, try to load it await self.mass.load_provider_config(config) @@ -1555,30 +1582,74 @@ class ConfigController: self.mass.create_task(self.mass.music.correct_multi_instance_provider_mappings()) return config - def _get_default_player_config_entries(self, player: Player) -> list[ConfigEntry]: - """Return the default player config entries.""" - entries: list[ConfigEntry] = [] - # default protocol-player config entries - if player.type == PlayerType.PROTOCOL: - # bare minimum: only playback related entries - entries += [ - CONF_ENTRY_OUTPUT_CHANNELS, + async def _get_player_config_entries( + self, + player: Player, + action: str | None = None, + values: dict[str, ConfigValueType] | None = None, + ) -> list[ConfigEntry]: + """ + Return Player(protocol) specific config entries, without any default entries. + + In general this returns entries that are specific to this provider/player type only, + and includes audio related entries that are not part of the default set. + + player: the player instance + action: [optional] action key called from config entries UI. + values: the (intermediate) raw values for config entries sent with the action. + """ + default_entries: list[ConfigEntry] + is_dedicated_group_player = player.type in ( + PlayerType.GROUP, + PlayerType.STEREO_PAIR, + ) and not player.player_id.startswith(("universal_", SYNCGROUP_PREFIX)) + is_http_based_player_protocol = player.provider.domain not in NON_HTTP_PROVIDERS + if player.type == PlayerType.GROUP and not is_dedicated_group_player: + # no audio related entries for universal group players or sync group players + default_entries = [] + else: + # default output/audio related entries + default_entries = [ + # output channel is always configurable per player(protocol) + CONF_ENTRY_OUTPUT_CHANNELS ] - if not player.requires_flow_mode: - entries.append(CONF_ENTRY_FLOW_MODE) - if player.provider.domain not in NON_HTTP_PROVIDERS: - entries += [ + if is_http_based_player_protocol: + # for http based players we can add the http streaming related entries + default_entries += [ CONF_ENTRY_SAMPLE_RATES, CONF_ENTRY_OUTPUT_CODEC, CONF_ENTRY_HTTP_PROFILE, CONF_ENTRY_ENABLE_ICY_METADATA, ] - return entries + # add flow mode entry for http-based players that do not already enforce it + if not player.requires_flow_mode: + default_entries.append(CONF_ENTRY_FLOW_MODE) + # request player specific entries + player_entries = await player.get_config_entries(action=action, values=values) + players_keys = {entry.key for entry in player_entries} + # filter out any default entries that are already provided by the player + default_entries = [entry for entry in default_entries if entry.key not in players_keys] + return [*player_entries, *default_entries] + + def _get_default_player_config_entries(self, player: Player) -> list[ConfigEntry]: + """ + Return the default (generic) player config entries. + + This does not return audio/protocol specific entries, those are handled elsewhere. + """ + entries: list[ConfigEntry] = [] + # default protocol-player config entries + if player.type == PlayerType.PROTOCOL: + # protocol players have no generic config entries + # only audio/protocol specific ones + return [] # some base entries for all player types + # note that these may NOT be playback/audio related entries += [ CONF_ENTRY_SMART_FADES_MODE, CONF_ENTRY_CROSSFADE_DURATION, + # we allow volume normalization/output limiter here as it is a per-queue(player) setting CONF_ENTRY_VOLUME_NORMALIZATION, CONF_ENTRY_OUTPUT_LIMITER, CONF_ENTRY_VOLUME_NORMALIZATION_TARGET, @@ -1619,49 +1690,21 @@ class ConfigController: default_value=player.expose_to_ha_by_default, ), ] - # group-player config entries if player.type == PlayerType.GROUP: - is_dedicated_group_player = ( - not isinstance(player, SyncGroupPlayer) - and player.provider.domain != "universal_group" - ) entries += [ CONF_ENTRY_PLAYER_ICON_GROUP, ] - if is_dedicated_group_player and not player.requires_flow_mode: - entries.append(CONF_ENTRY_FLOW_MODE) - if is_dedicated_group_player and player.provider.domain not in NON_HTTP_PROVIDERS: - entries += [ - CONF_ENTRY_SAMPLE_RATES, - CONF_ENTRY_OUTPUT_CODEC, - CONF_ENTRY_HTTP_PROFILE, - CONF_ENTRY_ENABLE_ICY_METADATA, - ] return entries - # normal player (or stereo pair) config entries entries += [ CONF_ENTRY_PLAYER_ICON, - CONF_ENTRY_OUTPUT_CHANNELS, # add default entries for announce feature CONF_ENTRY_ANNOUNCE_VOLUME_STRATEGY, CONF_ENTRY_ANNOUNCE_VOLUME, CONF_ENTRY_ANNOUNCE_VOLUME_MIN, CONF_ENTRY_ANNOUNCE_VOLUME_MAX, ] - # add flow mode config entry for players that not already explicitly enable it - if not player.requires_flow_mode: - entries.append(CONF_ENTRY_FLOW_MODE) - # add HTTP streaming config entries for non-http players - if player.provider.domain not in NON_HTTP_PROVIDERS: - entries += [ - CONF_ENTRY_SAMPLE_RATES, - CONF_ENTRY_OUTPUT_CODEC, - CONF_ENTRY_HTTP_PROFILE, - CONF_ENTRY_ENABLE_ICY_METADATA, - ] - return entries def _create_player_control_config_entries(self, player: Player) -> list[ConfigEntry]: diff --git a/music_assistant/controllers/player_queues.py b/music_assistant/controllers/player_queues.py index 1323812f..4cd386e7 100644 --- a/music_assistant/controllers/player_queues.py +++ b/music_assistant/controllers/player_queues.py @@ -64,7 +64,6 @@ from music_assistant_models.queue_item import QueueItem from music_assistant.constants import ( ATTR_ANNOUNCEMENT_IN_PROGRESS, - CONF_FLOW_MODE, MASS_LOGO_ONLINE, VERBOSE_LOG_LEVEL, ) @@ -932,7 +931,6 @@ class PlayerQueuesController(CoreController): target_player = self.mass.players.get(queue_id) if target_player is None: raise PlayerUnavailableError(f"Player {queue_id} is not available") - enqueue_supported = PlayerFeature.ENQUEUE in target_player.supported_features queue.next_item_id_enqueued = None # always update session id when we start a new playback session queue.session_id = shortuuid.random(length=8) @@ -982,12 +980,7 @@ class PlayerQueuesController(CoreController): raise MediaNotFoundError("No playable item found to start playback") # work out if we need to use flow mode - prefer_flow_mode = await self.mass.config.get_player_config_value( - queue_id, CONF_FLOW_MODE, default=False - ) - flow_mode = ( - prefer_flow_mode or not enqueue_supported - ) and queue_item.media_type not in ( + flow_mode = target_player.flow_mode and queue_item.media_type not in ( # don't use flow mode for duration-less streams MediaType.RADIO, MediaType.PLUGIN_SOURCE, diff --git a/music_assistant/controllers/players/sync_groups.py b/music_assistant/controllers/players/sync_groups.py index b66fb9dd..9da87d0e 100644 --- a/music_assistant/controllers/players/sync_groups.py +++ b/music_assistant/controllers/players/sync_groups.py @@ -567,6 +567,7 @@ class SyncGroupController: self.mass.config.create_default_player_config( player_id=player_id, provider=provider.instance_id, + player_type=PlayerType.GROUP, name=name, enabled=True, values={ diff --git a/music_assistant/mass.py b/music_assistant/mass.py index c39e0980..4c7ebe7c 100644 --- a/music_assistant/mass.py +++ b/music_assistant/mass.py @@ -841,6 +841,9 @@ class MusicAssistant: async def _on_provider_loaded() -> None: await provider.loaded_in_mass() await self.run_provider_discovery(provider.instance_id) + # push instance name to config (to persist it if it was autogenerated) + if provider.default_name != conf.default_name: + self.config.set_provider_default_name(provider.instance_id, provider.default_name) self.create_task(_on_provider_loaded()) diff --git a/music_assistant/models/player.py b/music_assistant/models/player.py index 98c0b712..c4f4d2f0 100644 --- a/music_assistant/models/player.py +++ b/music_assistant/models/player.py @@ -97,7 +97,7 @@ class Player(ABC): self._player_id = player_id self._provider = provider self.mass.config.create_default_player_config( - player_id, self.provider_id, self.name, self.enabled_by_default + player_id, self.provider_id, self.type, self.name, self.enabled_by_default ) self._config = self.mass.config.get_base_player_config(player_id, self.provider_id) self._extra_data: dict[str, Any] = {} @@ -921,18 +921,18 @@ class Player(ABC): """ return bool(self.mass.players.get_active_queue(self)) - @cached_property + @property @final def flow_mode(self) -> bool: """ Return if the player needs flow mode. - Will by default be set to True if the player does not support PlayerFeature.ENQUEUE - or has a flow mode config entry set to True. + Will use 'requires_flow_mode' unless overridden by flow_mode config. """ if bool(self._config.get_value(CONF_FLOW_MODE)) is True: + # flow mode explicitly enabled in config return True - return PlayerFeature.ENQUEUE not in self.supported_features + return self.requires_flow_mode @property @final @@ -969,6 +969,9 @@ class Player(ABC): # persist the default name if it changed if self.name and self.config.default_name != self.name: self.mass.config.set_player_default_name(self.player_id, self.name) + # persist the player type if it changed + if self.type != self._config.player_type: + self.mass.config.set_player_type(self.player_id, self.type) # return early if nothing changed (unless force_update is True) if len(changed_values) == 0 and not force_update: return diff --git a/music_assistant/providers/dlna/manifest.json b/music_assistant/providers/dlna/manifest.json index 570bc2d1..8e921d61 100644 --- a/music_assistant/providers/dlna/manifest.json +++ b/music_assistant/providers/dlna/manifest.json @@ -2,8 +2,8 @@ "type": "player", "domain": "dlna", "stage": "stable", - "name": "UPnP/DLNA Player provider", - "description": "Stream music to supported DLNA-compatible devices.", + "name": "DLNA", + "description": "Stream music to supported UPnP/DLNA-compatible devices.", "codeowners": ["@music-assistant"], "credits": [ "[Asyncio UPnP Client library by Steven Looman](https://github.com/StevenLooman/async_upnp_client)" diff --git a/music_assistant/providers/musiccast/constants.py b/music_assistant/providers/musiccast/constants.py index b9aeaede..02d4121d 100644 --- a/music_assistant/providers/musiccast/constants.py +++ b/music_assistant/providers/musiccast/constants.py @@ -1,9 +1,6 @@ """Constants for the MusicCast provider.""" -from music_assistant_models.config_entries import ConfigEntry - from music_assistant.constants import ( - CONF_ENTRY_FLOW_MODE, CONF_ENTRY_HTTP_PROFILE_DEFAULT_2, CONF_ENTRY_ICY_METADATA_HIDDEN_DISABLED, create_sample_rates_config_entry, @@ -11,18 +8,9 @@ from music_assistant.constants import ( # Constants for players # both the http profile and icy didn't matter for me testing it. -CONF_ENTRY_FLOW_MODE_HIDDEN_DISABLED = ConfigEntry.from_dict( - { - **CONF_ENTRY_FLOW_MODE.to_dict(), - "default_value": False, - "value": False, - "hidden": True, - } -) PLAYER_CONFIG_ENTRIES = [ CONF_ENTRY_HTTP_PROFILE_DEFAULT_2, CONF_ENTRY_ICY_METADATA_HIDDEN_DISABLED, - CONF_ENTRY_FLOW_MODE_HIDDEN_DISABLED, create_sample_rates_config_entry(max_sample_rate=192000, max_bit_depth=24), ] # player id is {device_id}{ZONE_SPLITTER}{zone_name} diff --git a/music_assistant/providers/squeezelite/manifest.json b/music_assistant/providers/squeezelite/manifest.json index 431fd125..ce42f3d3 100644 --- a/music_assistant/providers/squeezelite/manifest.json +++ b/music_assistant/providers/squeezelite/manifest.json @@ -2,8 +2,8 @@ "type": "player", "domain": "squeezelite", "stage": "stable", - "name": "Squeezelite (slimproto players)", - "description": "Stream music to Squeezelite and some legacy Squeezebox players on your local network.", + "name": "Squeezelite", + "description": "Stream music to Squeezelite and some legacy Squeezebox players on your local network using the slimproto protocol.", "codeowners": ["@music-assistant"], "credits": [ "[aioslimproto](https://github.com/music-assistant/aioslimproto)" diff --git a/music_assistant/providers/universal_group/provider.py b/music_assistant/providers/universal_group/provider.py index b742a0a4..c7a28ee2 100644 --- a/music_assistant/providers/universal_group/provider.py +++ b/music_assistant/providers/universal_group/provider.py @@ -5,6 +5,7 @@ from __future__ import annotations from typing import TYPE_CHECKING import shortuuid +from music_assistant_models.enums import PlayerType from music_assistant.constants import CONF_DYNAMIC_GROUP_MEMBERS, CONF_GROUP_MEMBERS from music_assistant.models.player_provider import PlayerProvider @@ -31,6 +32,7 @@ class UniversalGroupProvider(PlayerProvider): self.mass.config.create_default_player_config( player_id=player_id, provider=self.instance_id, + player_type=PlayerType.GROUP, name=name, enabled=True, values={