Fix configuration flows (#545)
authorMarcel van der Veldt <m.vanderveldt@outlook.com>
Fri, 17 Mar 2023 14:35:21 +0000 (15:35 +0100)
committerGitHub <noreply@github.com>
Fri, 17 Mar 2023 14:35:21 +0000 (15:35 +0100)
- Validate config before saving
- Ensure all players and providers have a default config
- Various config related fixes
- Improve add provider flow

music_assistant/common/models/config_entries.py
music_assistant/common/models/provider.py
music_assistant/server/controllers/config.py
music_assistant/server/controllers/players.py
music_assistant/server/models/provider.py
music_assistant/server/providers/airplay/__init__.py
music_assistant/server/providers/frontend/__init__.py
music_assistant/server/providers/frontend/manifest.json
music_assistant/server/providers/slimproto/__init__.py
music_assistant/server/server.py
requirements_all.txt

index b5e9f54344686bf77cc4f09f2da1bdff0f0b85cf..433d14fa5985d0b23d8ee83710d3070a5ed86549 100644 (file)
@@ -206,6 +206,13 @@ class Config(DataClassDictMixin):
 
         return changed_keys
 
+    def validate(self) -> None:
+        """Validate if configuration is valid."""
+        # For now we just use the parse method to check for not allowed None values
+        # this can be extended later
+        for value in self.values.values():
+            value.parse(value, value.value, allow_none=False)
+
 
 @dataclass
 class ProviderConfig(Config):
@@ -218,6 +225,8 @@ class ProviderConfig(Config):
     enabled: bool = True
     # name: an (optional) custom name for this provider instance/config
     name: str | None = None
+    # last_error: an optional error message if the provider could not be setup with this config
+    last_error: str | None = None
 
 
 @dataclass
@@ -230,6 +239,10 @@ class PlayerConfig(Config):
     enabled: bool = True
     # name: an (optional) custom name for this player
     name: str | None = None
+    # available: boolean to indicate if the player is available
+    available: bool = True
+    # default_name: default name to use when there is name available
+    default_name: str | None = None
 
 
 @dataclass
index 80ab604f4b273250f526b8afb23e55a6fb9d46b3..58a166ed2f7a7f6fb68dee325fb3d83b19fe8dec 100644 (file)
@@ -56,7 +56,6 @@ class ProviderInstance(TypedDict):
     instance_id: str
     supported_features: list[ProviderFeature]
     available: bool
-    last_error: str | None
 
 
 @dataclass
index f74b66749afc5959cc948857a914a08e8dee9a63..83c7ab13a72d0080402241d227254cefe9104eb5 100644 (file)
@@ -177,71 +177,40 @@ class ConfigController:
         raise KeyError(f"No config found for provider id {instance_id}")
 
     @api_command("config/providers/update")
-    def update_provider_config(self, instance_id: str, update: ConfigUpdate) -> None:
+    async def update_provider_config(self, instance_id: str, update: ConfigUpdate) -> None:
         """Update ProviderConfig."""
         config = self.get_provider_config(instance_id)
         changed_keys = config.update(update)
-
-        if not changed_keys:
+        try:
+            prov = self.mass.get_provider(instance_id)
+            available = prov.available
+        except ProviderUnavailableError:
+            available = False
+        if not changed_keys and (config.enabled == available):
             # no changes
             return
-
+        # try to load the provider first to catch errors before we save it.
+        if config.enabled:
+            await self.mass.load_provider(config)
+        # load succeeded, save new config
         conf_key = f"{CONF_PROVIDERS}/{instance_id}"
         self.set(conf_key, config.to_raw())
-        updated_config = self.get_provider_config(config.instance_id)
-        self.mass.create_task(self.mass.load_provider(updated_config))
 
-    @api_command("config/providers/create")
-    def create_provider_config(
-        self, provider_domain: str, default_enabled: bool = False
+    @api_command("config/providers/add")
+    async def add_provider_config(
+        self, provider_domain: str, config: ProviderConfig | None = None
     ) -> ProviderConfig:
-        """Create default/empty ProviderConfig.
-
-        This is intended to be used as helper method to add a new provider,
-        and it performs some quick sanity checks as well as handling the
-        instance_id generation.
-        """
-        # lookup provider manifest
-        for prov in self.mass.get_available_providers():
-            if prov.domain == provider_domain:
-                manifest = prov
-                break
-        else:
-            raise KeyError(f"Unknown provider domain: {provider_domain}")
-
-        # determine instance id based on previous configs
-        existing = {
-            x.instance_id for x in self.get_provider_configs(provider_domain=provider_domain)
-        }
-
-        if existing and not manifest.multi_instance:
-            raise ValueError(f"Provider {manifest.name} does not support multiple instances")
-
-        if len(existing) == 0:
-            instance_id = provider_domain
-            name = manifest.name
-        else:
-            instance_id = f"{provider_domain}{len(existing)+1}"
-            name = f"{manifest.name} {len(existing)+1}"
-
-        # all checks passed, return a default config
-        default_config = ProviderConfig.parse(
-            prov.config_entries,
-            {
-                "type": manifest.type.value,
-                "domain": manifest.domain,
-                "instance_id": instance_id,
-                "name": name,
-                "enabled": default_enabled,
-                "values": {},
-            },
-        )
-
-        # config provided and checks passed, storeconfig
-        conf_key = f"{CONF_PROVIDERS}/{instance_id}"
-        self.set(conf_key, default_config.to_raw())
-
-        return default_config
+        """Add new Provider (instance) Config Flow."""
+        if not config:
+            return self._get_default_provider_config(provider_domain)
+        # if provider config is provided, the frontend wants to submit a new provider instance
+        # based on the earlier created template config.
+        # try to load the provider first to catch errors before we save it.
+        await self.mass.load_provider(config)
+        # config provided and load success, storeconfig
+        conf_key = f"{CONF_PROVIDERS}/{config.instance_id}"
+        self.set(conf_key, config.to_raw())
+        return config
 
     @api_command("config/providers/remove")
     async def remove_provider_config(self, instance_id: str) -> None:
@@ -256,58 +225,38 @@ class ConfigController:
             # cleanup entries in library
             await self.mass.music.cleanup_provider(instance_id)
 
+    @api_command("config/providers/reload")
+    async def reload_provider(self, instance_id: str) -> None:
+        """Reload provider."""
+        config = self.get_provider_config(instance_id)
+        await self.mass.load_provider(config)
+
     @api_command("config/players")
     def get_player_configs(self, provider: str | None = None) -> list[PlayerConfig]:
         """Return all known player configurations, optionally filtered by provider domain."""
-        result: dict[str, PlayerConfig] = {}
-        # we use an intermediate dict to cover both edge cases:
-        # - player does not yet have a config stored persistently
-        # - player is disabled in config and not available
-
-        # do all existing players first
-        for player in self.mass.players:
-            if provider is not None and player.provider != provider:
-                continue
-            result[player.player_id] = self.get_player_config(player.player_id)
-
-        # add remaining configs that do have a config stored but are not (yet) available now
-        raw_configs = self.get(CONF_PLAYERS, {})
-        for player_id, raw_conf in raw_configs.items():
-            if player_id in result:
-                continue
-            if provider is not None and raw_conf["provider"] != provider:
-                continue
+        return [
+            self.get_player_config(player_id)
+            for player_id, raw_conf in self.get(CONF_PLAYERS).items()
+            if (provider in (None, raw_conf["provider"]))
+        ]
+
+    @api_command("config/players/get")
+    def get_player_config(self, player_id: str) -> PlayerConfig:
+        """Return configuration for a single player."""
+        if raw_conf := self.get(f"{CONF_PLAYERS}/{player_id}"):
             try:
                 prov = self.mass.get_provider(raw_conf["provider"])
                 prov_entries = prov.get_player_config_entries(player_id)
             except (ProviderUnavailableError, PlayerUnavailableError):
                 prov_entries = tuple()
+                raw_conf["available"] = False
+                raw_conf["name"] = (
+                    raw_conf.get("name") or raw_conf.get("default_name") or raw_conf["player_id"]
+                )
 
             entries = DEFAULT_PLAYER_CONFIG_ENTRIES + prov_entries
-            result[player.player_id] = PlayerConfig.parse(entries, raw_conf)
-
-        return list(result.values())
-
-    @api_command("config/players/get")
-    def get_player_config(self, player_id: str) -> PlayerConfig:
-        """Return configuration for a single player."""
-        conf = self.get(f"{CONF_PLAYERS}/{player_id}")
-        if not conf:
-            player = self.mass.players.get(player_id, raise_unavailable=False)
-            conf = {
-                "provider": player.provider,
-                "player_id": player_id,
-                "enabled": player.enabled_by_default,
-            }
-
-        try:
-            prov = self.mass.get_provider(conf["provider"])
-            prov_entries = prov.get_player_config_entries(player_id)
-        except (ProviderUnavailableError, PlayerUnavailableError):
-            prov_entries = tuple()
-
-        entries = DEFAULT_PLAYER_CONFIG_ENTRIES + prov_entries
-        return PlayerConfig.parse(entries, conf)
+            return PlayerConfig.parse(entries, raw_conf)
+        raise KeyError(f"No config found for player id {player_id}")
 
     @api_command("config/players/get_value")
     def get_player_config_value(self, player_id: str, key: str) -> ConfigEntryValue:
@@ -348,10 +297,6 @@ class ConfigController:
             player = self.mass.players.get(config.player_id)
             player.enabled = config.enabled
             self.mass.players.update(config.player_id)
-            # copy playername to find back the playername if its disabled
-            if not config.enabled and not config.name:
-                config.name = player.display_name
-                self.set(conf_key, config.to_raw())
         except PlayerUnavailableError:
             pass
 
@@ -383,6 +328,86 @@ class ConfigController:
             assert isinstance(provider, PlayerProvider)
             provider.on_player_config_removed(player_id)
 
+    def create_default_player_config(self, player_id: str, provider: str, name: str) -> 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
+            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, default_name=name
+        )
+        self.set(
+            conf_key,
+            default_conf.to_raw(),
+        )
+
+    def create_default_provider_config(self, provider_domain: str) -> None:
+        """
+        Create default ProviderConfig.
+
+        This is meant as helper to create default configs for default enabled providers.
+        Called by the server initialization code which load all providers at startup.
+        """
+        for conf in self.get_provider_configs(provider_domain=provider_domain):
+            # return if there is already a config
+            return
+        # config does not yet exist, create a default one
+        default_config = self._get_default_provider_config(provider_domain)
+        conf_key = f"{CONF_PROVIDERS}/{default_config.instance_id}"
+        self.set(conf_key, default_config.to_raw())
+
+    def _get_default_provider_config(self, provider_domain: str) -> ProviderConfig:
+        """
+        Return default/empty ProviderConfig.
+
+        This is intended to be used as helper method to add a new provider,
+        and it performs some quick sanity checks as well as handling the
+        instance_id generation.
+        """
+        # lookup provider manifest
+        for prov in self.mass.get_available_providers():
+            if prov.domain == provider_domain:
+                manifest = prov
+                break
+        else:
+            raise KeyError(f"Unknown provider domain: {provider_domain}")
+
+        # determine instance id based on previous configs
+        existing = {
+            x.instance_id for x in self.get_provider_configs(provider_domain=provider_domain)
+        }
+
+        if existing and not manifest.multi_instance:
+            raise ValueError(f"Provider {manifest.name} does not support multiple instances")
+
+        if len(existing) == 0:
+            instance_id = provider_domain
+            name = manifest.name
+        else:
+            instance_id = f"{provider_domain}{len(existing)+1}"
+            name = f"{manifest.name} {len(existing)+1}"
+
+        # all checks passed, return a default config
+        return ProviderConfig.parse(
+            prov.config_entries,
+            {
+                "type": manifest.type.value,
+                "domain": manifest.domain,
+                "instance_id": instance_id,
+                "name": name,
+                "values": {},
+            },
+        )
+
     async def _load(self) -> None:
         """Load data from persistent storage."""
         assert not self._data, "Already loaded"
index 7ab6bcf184f1fa76e2337d7046b7562c70da53e5..ee9e700376e8df3cb83db601f7812c6405151e06 100755 (executable)
@@ -114,6 +114,9 @@ class PlayerController:
         if player_id in self._players:
             raise AlreadyRegisteredError(f"Player {player_id} is already registered")
 
+        # make sure a default config exists
+        self.mass.config.create_default_player_config(player_id, player.provider, player.name)
+
         player.enabled = self.mass.config.get(f"{CONF_PLAYERS}/{player_id}/enabled", True)
 
         # register playerqueue for this player
@@ -516,7 +519,8 @@ class PlayerController:
         count = 0
         while True:
             count += 1
-            for player_id, player in self._players.items():
+            for player in list(self._players.values()):
+                player_id = player.player_id
                 # if the player is playing, update elapsed time every tick
                 # to ensure the queue has accurate details
                 player_playing = (
index c37ac91de7114d6e5d30429e5cf49b2f07cba23d..41ee67437d19b6d993266b965b068833a7164c22 100644 (file)
@@ -28,7 +28,6 @@ class Provider:
         self.logger = logging.getLogger(f"{ROOT_LOGGER_NAME}.providers.{self.domain}")
         self.cache = mass.cache
         self.available = False
-        self.last_error = None
 
     @property
     def supported_features(self) -> tuple[ProviderFeature, ...]:
@@ -90,5 +89,4 @@ class Provider:
             "instance_id": self.instance_id,
             "supported_features": [x.value for x in self.supported_features],
             "available": self.available,
-            "last_error": self.last_error,
         }
index 3d43440358d124391eac990ad4976c5ff4e5d6d4..d565f5751a3a5d335b26a41a93c81e31ba7a9158 100644 (file)
@@ -16,9 +16,9 @@ import aiofiles
 
 from music_assistant.common.models.config_entries import ConfigEntry
 from music_assistant.common.models.enums import ConfigEntryType
-from music_assistant.common.models.errors import PlayerUnavailableError
 from music_assistant.common.models.player import DeviceInfo, Player
 from music_assistant.common.models.queue_item import QueueItem
+from music_assistant.constants import CONF_PLAYERS
 from music_assistant.server.models.player_provider import PlayerProvider
 
 if TYPE_CHECKING:
@@ -284,6 +284,9 @@ class AirplayProvider(PlayerProvider):
             "-Z",
             "-d",
             "all=warn",
+            # filter out macbooks and apple tv's
+            "-m",
+            "macbook,apple-tv,appletv",
         ]
         start_success = False
         while True:
@@ -315,6 +318,7 @@ class AirplayProvider(PlayerProvider):
 
     async def _check_config_xml(self, recreate: bool = False) -> None:
         """Check the bridge config XML file."""
+        # ruff: noqa: PLR0915
         if recreate or not os.path.isfile(self._config_file):
             if os.path.isfile(self._config_file):
                 os.remove(self._config_file)
@@ -349,28 +353,38 @@ class AirplayProvider(PlayerProvider):
         common_elem.find("sample_rate").text = "44100"
         common_elem.find("resample").text = "0"
         common_elem.find("player_volume").text = "20"
+
+        # default values for players
+        for conf_entry in PLAYER_CONFIG_ENTRIES:
+            if conf_entry.type == ConfigEntryType.LABEL:
+                continue
+            conf_val = conf_entry.default_value
+            xml_elem = common_elem.find(conf_entry.key)
+            if xml_elem is None:
+                xml_elem = ET.SubElement(common_elem, conf_entry.key)
+            if conf_entry.type == ConfigEntryType.BOOLEAN:
+                xml_elem.text = "1" if conf_val else "0"
+            else:
+                xml_elem.text = str(conf_val)
+
         # get/set all device configs
         for device_elem in xml_root.findall("device"):
             player_id = device_elem.find("mac").text
-            try:
-                player_conf = self.mass.config.get_player_config(player_id)
-            except PlayerUnavailableError:
-                player_conf = None
+            # use raw config values because players are not
+            # yet available at startup/init (race condition)
+            raw_player_conf = self.mass.config.get(f"{CONF_PLAYERS}/{player_id}")
+            if not raw_player_conf:
+                continue
             # prefer name from UDN because default name is often wrong
             udn = device_elem.find("udn").text
             udn_name = udn.split("@")[1].split("._")[0]
             device_elem.find("name").text = udn_name
-            device_elem.find("enabled").text = (
-                "1" if (not player_conf or player_conf.enabled) else "0"
-            )
+            device_elem.find("enabled").text = "1" if raw_player_conf["enabled"] else "0"
 
             for conf_entry in PLAYER_CONFIG_ENTRIES:
                 if conf_entry.type == ConfigEntryType.LABEL:
                     continue
-                if player_conf:
-                    conf_val = player_conf.get_value(conf_entry.key)
-                else:
-                    conf_val = conf_entry.default_value
+                conf_val = raw_player_conf["values"].get(conf_entry.key, conf_entry.default_value)
                 xml_elem = device_elem.find(conf_entry.key)
                 if xml_elem is None:
                     xml_elem = ET.SubElement(device_elem, conf_entry.key)
index 7a78a46bbe7ddfe4404d18d81527732bb307d388..3269e1127a63bb79014eed3145a32391023d3a69 100644 (file)
@@ -34,4 +34,5 @@ class Frontend(PluginProvider):
 
     async def serve_static(self, file_path: str, _request: web.Request) -> web.FileResponse:
         """Serve file response."""
-        return web.FileResponse(file_path)
+        headers = {"Cache-Control": "no-cache"}
+        return web.FileResponse(file_path, headers=headers)
index e96aba53a640369a6a0b33163fb5bac67a9d5ffb..a97194fb54dbdecb2fcd587737dda7df161c4262 100644 (file)
@@ -7,7 +7,7 @@
   "config_entries": [
   ],
 
-  "requirements": ["music-assistant-frontend==20230313.0"],
+  "requirements": ["music-assistant-frontend==20230317.0"],
   "documentation": "",
   "multi_instance": false,
   "builtin": true,
index 09ae5698ebeb9dc1f10270dc3dedfc2b8d7f97bc..a969bcd2c5c7b3283e010970d6e9dbee86230872 100644 (file)
@@ -27,11 +27,11 @@ from music_assistant.common.models.enums import (
 from music_assistant.common.models.errors import PlayerUnavailableError, QueueEmpty
 from music_assistant.common.models.player import DeviceInfo, Player
 from music_assistant.common.models.queue_item import QueueItem
+from music_assistant.constants import CONF_PLAYERS
 from music_assistant.server.models.player_provider import PlayerProvider
 from music_assistant.server.providers.json_rpc import parse_args
 
 if TYPE_CHECKING:
-    from music_assistant.common.models.config_entries import PlayerConfig
     from music_assistant.server.providers.json_rpc import JSONRPCApi
 
 # sync constants
@@ -59,6 +59,8 @@ class SyncPlayPoint:
 
 
 CONF_SYNC_ADJUST = "sync_adjust"
+CONF_PLAYER_VOLUME = "player_volume"
+DEFAULT_PLAYER_VOLUME = 20
 
 SLIM_PLAYER_CONFIG_ENTRIES = (
     ConfigEntry(
@@ -71,6 +73,14 @@ SLIM_PLAYER_CONFIG_ENTRIES = (
         "and you always hear the audio too late on this player, you can shift the audio a bit.",
         advanced=True,
     ),
+    ConfigEntry(
+        key=CONF_PLAYER_VOLUME,
+        type=ConfigEntryType.INTEGER,
+        default_value=DEFAULT_PLAYER_VOLUME,
+        label="Default startup volume",
+        description="Default volume level to set/use when initializing the player.",
+        advanced=True,
+    ),
 )
 
 
@@ -80,14 +90,12 @@ class SlimprotoProvider(PlayerProvider):
     _socket_servers: tuple[asyncio.Server | asyncio.BaseTransport]
     _socket_clients: dict[str, SlimClient]
     _sync_playpoints: dict[str, deque[SyncPlayPoint]]
-    _sync_adjusts: dict[str, int]
     _virtual_providers: dict[str, tuple[Callable, Callable]]
 
     async def setup(self) -> None:
         """Handle async initialization of the provider."""
         self._socket_clients = {}
         self._sync_playpoints = {}
-        self._sync_adjusts = {}
         self._virtual_providers = {}
         # autodiscovery of the slimproto server does not work
         # when the port is not the default (3483) so we hardcode it for now
@@ -158,14 +166,6 @@ class SlimprotoProvider(PlayerProvider):
         """Return all (provider/player specific) Config Entries for the given player (if any)."""
         return SLIM_PLAYER_CONFIG_ENTRIES
 
-    def on_player_config_changed(
-        self, config: PlayerConfig, changed_keys: set[str]  # noqa: ARG002
-    ) -> None:
-        """Call (by config manager) when the configuration of a player changes."""
-        # during synced playback this value is requested multiple times a second,
-        # so we cache it in a quick lookup dict
-        self._sync_adjusts[config.player_id] = config.get_value(CONF_SYNC_ADJUST)
-
     async def cmd_stop(self, player_id: str) -> None:
         """Send STOP command to given player."""
         # forward command to player and any connected sync child's
@@ -493,8 +493,11 @@ class SlimprotoProvider(PlayerProvider):
             # update existing players so they can update their `can_sync_with` field
             for client in self._socket_clients.values():
                 self._handle_player_update(client)
-            # precache player config
-            self.on_player_config_changed(self.mass.config.get_player_config(player_id), set())
+        # handle init/startup volume
+        init_volume = self.mass.config.get(
+            f"{CONF_PLAYERS}/{player_id}/{CONF_PLAYER_VOLUME}", DEFAULT_PLAYER_VOLUME
+        )
+        self.mass.create_task(client.volume_set(init_volume))
 
     def _handle_disconnected(self, client: SlimClient) -> None:
         """Handle a client disconnected event."""
@@ -531,7 +534,9 @@ class SlimprotoProvider(PlayerProvider):
 
     def _get_corrected_elapsed_milliseconds(self, client: SlimClient) -> int:
         """Return corrected elapsed milliseconds."""
-        sync_delay = self._sync_adjusts[client.player_id]
+        sync_delay = self.mass.config.get(
+            f"{CONF_PLAYERS}/{client.player_id}/{CONF_SYNC_ADJUST}", 0
+        )
         if sync_delay != 0:
             return client.elapsed_milliseconds - sync_delay
         return client.elapsed_milliseconds
index dad85ed9189272425ca4751364d898bef3944277..7f527d5c51c93b013e277e7c236079ebd6134851 100644 (file)
@@ -19,7 +19,7 @@ from music_assistant.common.models.enums import EventType, ProviderType
 from music_assistant.common.models.errors import ProviderUnavailableError, SetupFailedError
 from music_assistant.common.models.event import MassEvent
 from music_assistant.common.models.provider import ProviderManifest
-from music_assistant.constants import CONF_SERVER_ID, CONF_WEB_IP, ROOT_LOGGER_NAME
+from music_assistant.constants import CONF_PROVIDERS, CONF_SERVER_ID, CONF_WEB_IP, ROOT_LOGGER_NAME
 from music_assistant.server.controllers.cache import CacheController
 from music_assistant.server.controllers.config import ConfigController
 from music_assistant.server.controllers.metadata import MetaDataController
@@ -298,15 +298,15 @@ class MusicAssistant:
         """Load (or reload) a provider."""
         # if provider is already loaded, stop and unload it first
         await self.unload_provider(conf.instance_id)
-
         LOGGER.debug("Loading provider %s", conf.name or conf.domain)
-        # abort if provider is disabled
         if not conf.enabled:
-            LOGGER.debug(
-                "Not loading provider %s because it is disabled",
-                conf.name or conf.instance_id,
-            )
-            return
+            raise SetupFailedError("Provider is disabled")
+
+        # validate config
+        try:
+            conf.validate()
+        except (KeyError, ValueError, AttributeError, TypeError) as err:
+            raise SetupFailedError("Configuration is invalid") from err
 
         domain = conf.domain
         prov_manifest = self._available_providers.get(domain)
@@ -316,61 +316,63 @@ class MusicAssistant:
             raise SetupFailedError(
                 f"Provider {domain} already loaded and only one instance allowed."
             )
-
+        # check valid manifest (just in case)
         if not prov_manifest:
             raise SetupFailedError(f"Provider {domain} manifest not found")
 
-        # try to load the module
-        try:
-            prov_mod = importlib.import_module(f".{domain}", "music_assistant.server.providers")
-            for name, obj in inspect.getmembers(prov_mod):
-                if not inspect.isclass(obj):
-                    continue
-                # lookup class to initialize
-                if name == prov_manifest.init_class or (
-                    not prov_manifest.init_class
-                    and issubclass(
-                        obj, MusicProvider | PlayerProvider | MetadataProvider | PluginProvider
-                    )
-                    and obj != MusicProvider
-                    and obj != PlayerProvider
-                    and obj != MetadataProvider
-                    and obj != PluginProvider
-                ):
-                    prov_cls = obj
+        # handle dependency on other provider
+        if prov_manifest.depends_on:
+            for _ in range(30):
+                try:
+                    self.get_provider(prov_manifest.depends_on)
                     break
+                except ProviderUnavailableError:
+                    await asyncio.sleep(1)
             else:
-                raise AttributeError("Unable to locate Provider class")
-            provider: ProviderInstanceType = prov_cls(self, prov_manifest, conf)
-            self._providers[provider.instance_id] = provider
-            try:
-                await provider.setup()
-            except Exception as err:
-                provider.last_error = str(err)
-                provider.available = False
-                raise err
-
-            # mark provider as available once setup succeeded
-            provider.available = True
-            provider.last_error = None
-            # if this is a music provider, start sync
-            if provider.type == ProviderType.MUSIC:
-                await self.music.start_sync(providers=[provider.instance_id])
-        # pylint: disable=broad-except
-        except Exception as exc:
-            LOGGER.exception(
-                "Error loading provider(instance) %s: %s",
-                conf.name or conf.domain,
-                str(exc),
-            )
+                raise SetupFailedError(
+                    f"Provider {domain} depends on {prov_manifest.depends_on} "
+                    "which is not available."
+                )
+
+        # try to load the module
+        prov_mod = importlib.import_module(f".{domain}", "music_assistant.server.providers")
+        for name, obj in inspect.getmembers(prov_mod):
+            if not inspect.isclass(obj):
+                continue
+            # lookup class to initialize
+            if name == prov_manifest.init_class or (
+                not prov_manifest.init_class
+                and issubclass(
+                    obj, MusicProvider | PlayerProvider | MetadataProvider | PluginProvider
+                )
+                and obj != MusicProvider
+                and obj != PlayerProvider
+                and obj != MetadataProvider
+                and obj != PluginProvider
+            ):
+                prov_cls = obj
+                break
         else:
-            LOGGER.info(
-                "Loaded %s provider %s",
-                provider.type.value,
-                conf.name or conf.domain,
-            )
-        # always signal event, regardless if the loading succeeded or not
+            raise AttributeError("Unable to locate Provider class")
+        provider: ProviderInstanceType = prov_cls(self, prov_manifest, conf)
+
+        try:
+            await asyncio.wait_for(provider.setup(), 30)
+        except TimeoutError as err:
+            raise SetupFailedError(f"Provider {domain} did not load within 30 seconds") from err
+        # if we reach this point, the provider loaded successfully
+        LOGGER.info(
+            "Loaded %s provider %s",
+            provider.type.value,
+            conf.name or conf.domain,
+        )
+        provider.available = True
+        self._providers[provider.instance_id] = provider
+        self.config.set(f"{CONF_PROVIDERS}/{conf.instance_id}/last_error", None)
         self.signal_event(EventType.PROVIDERS_UPDATED, data=self.get_providers())
+        # if this is a music provider, start sync
+        if provider.type == ProviderType.MUSIC:
+            await self.music.start_sync(providers=[provider.instance_id])
 
     async def unload_provider(self, instance_id: str) -> None:
         """Unload a provider."""
@@ -408,26 +410,33 @@ class MusicAssistant:
         await self.__load_available_providers()
 
         # create default config for any 'load_by_default' providers (e.g. URL provider)
-        # we must do this first to resolve any dependencies
-        # NOTE: this will auto load any not yet existing providers
-        provider_configs = self.config.get_provider_configs()
         for prov_manifest in self._available_providers.values():
             if not prov_manifest.load_by_default:
                 continue
-            existing = any(x for x in provider_configs if x.domain == prov_manifest.domain)
-            if existing:
-                continue
-            self.config.create_provider_config(prov_manifest.domain, True)
+            self.config.create_default_provider_config(prov_manifest.domain)
+
+        async def load_provider(prov_conf: ProviderConfig) -> None:
+            """Try to load a provider and catch errors."""
+            try:
+                await self.load_provider(prov_conf)
+            # pylint: disable=broad-except
+            except Exception as exc:
+                LOGGER.exception(
+                    "Error loading provider(instance) %s: %s",
+                    prov_conf.name or prov_conf.domain,
+                    str(exc),
+                )
+                # if loading failed, we store the error in the config object
+                # so we can show something useful to the user
+                prov_conf.last_error = str(exc)
+                self.config.set(f"{CONF_PROVIDERS}/{prov_conf.instance_id}/last_error", str(exc))
 
         # load all configured (and enabled) providers
-        for allow_depends_on in (False, True):
+        async with asyncio.TaskGroup() as tg:
             for prov_conf in self.config.get_provider_configs():
-                prov_manifest = self._available_providers[prov_conf.domain]
-                if prov_manifest.depends_on and not allow_depends_on:
+                if not prov_conf.enabled:
                     continue
-                if prov_conf.instance_id in self._providers:
-                    continue
-                await self.load_provider(prov_conf)
+                tg.create_task(load_provider(prov_conf))
 
     async def __load_available_providers(self) -> None:
         """Preload all available provider manifest files."""
index 7469fc5cf26e6c2fc652caae9326a7830658c2f4..0cf44d481ad23b50dddaf85092cc502b17fd6ede 100644 (file)
@@ -14,7 +14,7 @@ getmac==0.8.2
 git+https://github.com/pytube/pytube.git@refs/pull/1501/head
 mashumaro==3.5.0
 memory-tempfile==2.2.3
-music-assistant-frontend==20230313.0
+music-assistant-frontend==20230317.0
 orjson==3.8.7
 pillow==9.4.0
 PyChromecast==13.0.4