From 40ed7d8e472780d273a3e72fe007dcc3f5cc9e4e Mon Sep 17 00:00:00 2001 From: Marcel van der Veldt Date: Fri, 9 Jun 2023 12:44:27 +0200 Subject: [PATCH] Fix slimproto and Airplay startup (#706) * bump aioslimproto to 2.3.2 * add options to configure slimproto * finish * Delete config.xml --- music_assistant/constants.py | 6 +- music_assistant/server/controllers/config.py | 29 +++- .../server/providers/airplay/__init__.py | 16 ++- .../server/providers/slimproto/__init__.py | 130 ++++++++++++++---- .../server/providers/slimproto/cli.py | 32 +++-- .../server/providers/slimproto/manifest.json | 2 +- music_assistant/server/server.py | 8 +- requirements_all.txt | 2 +- 8 files changed, 167 insertions(+), 58 deletions(-) diff --git a/music_assistant/constants.py b/music_assistant/constants.py index 398f775e..ab5d6273 100755 --- a/music_assistant/constants.py +++ b/music_assistant/constants.py @@ -28,10 +28,8 @@ FALLBACK_DURATION: Final[int] = 172800 # config keys CONF_SERVER_ID: Final[str] = "server_id" -CONF_WEB_IP: Final[str] = "webserver.ip" -CONF_WEB_PORT: Final[str] = "webserver.port" -CONF_DB_LIBRARY: Final[str] = "database.library" -CONF_DB_CACHE: Final[str] = "database.cache" +CONF_IP_ADDRESS: Final[str] = "ip_address" +CONF_PORT: Final[str] = "port" CONF_PROVIDERS: Final[str] = "providers" CONF_PLAYERS: Final[str] = "players" CONF_PATH: Final[str] = "path" diff --git a/music_assistant/server/controllers/config.py b/music_assistant/server/controllers/config.py index 2d389fcb..c1aa09d7 100644 --- a/music_assistant/server/controllers/config.py +++ b/music_assistant/server/controllers/config.py @@ -272,10 +272,10 @@ class ConfigController: self.set(conf_key, value) @api_command("config/providers/reload") - async def reload_provider(self, instance_id: str) -> None: + async def reload_provider(self, instance_id: str, config: ProviderConfig | None) -> None: """Reload provider.""" config = await self.get_provider_config(instance_id) - await self.mass.load_provider(config) + await self._load_provider_config(config) @api_command("config/players") async def get_player_configs(self, provider: str | None = None) -> list[PlayerConfig]: @@ -514,13 +514,13 @@ class ConfigController: return config # try to load the provider first to catch errors before we save it. if config.enabled: - await self.mass.load_provider(config) + await self._load_provider_config(config) else: # disable provider - # check if there are no other providers dependent of this provider - for prov in self.mass.get_available_providers(): - if prov.depends_on == config.domain and self.mass.get_provider(prov.domain): - raise RuntimeError(f"Provider {prov.name} depends on {config.domain}.") + # also unload any other providers dependent of this provider + for dep_prov in self.mass.providers: + 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) # load succeeded, save new config config.last_error = None @@ -584,3 +584,18 @@ class ConfigController: conf_key = f"{CONF_PROVIDERS}/{config.instance_id}" self.set(conf_key, config.to_raw()) return config + + async def _load_provider_config(self, config: ProviderConfig) -> None: + """Load given provider config.""" + # check if there are no other providers dependent of this provider + deps = set() + for dep_prov in self.mass.providers: + if dep_prov.manifest.depends_on == config.domain: + deps.add(dep_prov.instance_id) + await self.mass.unload_provider(dep_prov.instance_id) + # (re)load the provider + await self.mass.load_provider(config) + # reload any dependants + for dep in deps: + conf = await self.get_provider_config(dep) + await self.mass.load_provider(conf) diff --git a/music_assistant/server/providers/airplay/__init__.py b/music_assistant/server/providers/airplay/__init__.py index 233e59cc..53ff7e04 100644 --- a/music_assistant/server/providers/airplay/__init__.py +++ b/music_assistant/server/providers/airplay/__init__.py @@ -131,11 +131,13 @@ class AirplayProvider(PlayerProvider): ) await self._check_config_xml() # start running the bridge - asyncio.create_task(self._bridge_process_runner()) + asyncio.create_task(self._bridge_process_runner(slimproto_prov)) async def unload(self) -> None: """Handle close/cleanup of the provider.""" self._closing = True + if slimproto_prov := self.mass.get_provider("slimproto"): + slimproto_prov.unregister_virtual_provider("RaopBridge") await self._stop_bridge() async def get_player_config_entries(self, player_id: str) -> tuple[ConfigEntry, ...]: @@ -304,7 +306,7 @@ class AirplayProvider(PlayerProvider): f"Unable to locate RaopBridge for {platform.system()} ({platform.machine()})" ) - async def _bridge_process_runner(self) -> None: + async def _bridge_process_runner(self, slimproto_prov: SlimprotoProvider) -> None: """Run the bridge binary in the background.""" self.logger.debug( "Starting Airplay bridge using config file %s", @@ -313,16 +315,18 @@ class AirplayProvider(PlayerProvider): args = [ self._bridge_bin, "-s", - "localhost", + f"localhost:{slimproto_prov.port}", "-x", self._config_file, "-I", "-Z", "-d", "all=warn", - # filter out macbooks and apple tv's + # filter out apple tv's for now until we fix auth "-m", - "macbook,apple-tv,appletv", + "apple-tv,appletv", + # enable terminate on exit otherwise exists are soooo slooooowwww + "-k", ] start_success = False while True: @@ -347,8 +351,10 @@ class AirplayProvider(PlayerProvider): """Stop the bridge process.""" if self._bridge_proc: try: + self.logger.debug("Stopping bridge process...") self._bridge_proc.terminate() await self._bridge_proc.wait() + self.logger.debug("Bridge process stopped.") except ProcessLookupError: pass diff --git a/music_assistant/server/providers/slimproto/__init__.py b/music_assistant/server/providers/slimproto/__init__.py index 8651ebe4..55f3e02b 100644 --- a/music_assistant/server/providers/slimproto/__init__.py +++ b/music_assistant/server/providers/slimproto/__init__.py @@ -29,10 +29,10 @@ from music_assistant.common.models.enums import ( PlayerState, PlayerType, ) -from music_assistant.common.models.errors import QueueEmpty +from music_assistant.common.models.errors import QueueEmpty, SetupFailedError from music_assistant.common.models.player import DeviceInfo, Player from music_assistant.common.models.queue_item import QueueItem -from music_assistant.constants import CONF_CROSSFADE_DURATION +from music_assistant.constants import CONF_CROSSFADE_DURATION, CONF_PORT from music_assistant.server.models.player_provider import PlayerProvider from .cli import LmsCli @@ -72,7 +72,11 @@ class SyncPlayPoint: CONF_SYNC_ADJUST = "sync_adjust" +CONF_CLI_TELNET = "cli_telnet" +CONF_CLI_JSON = "cli_json" +CONF_DISCOVERY = "discovery" DEFAULT_PLAYER_VOLUME = 20 +DEFAULT_SLIMPROTO_PORT = 3483 async def setup( @@ -98,42 +102,113 @@ async def get_config_entries( values: the (intermediate) raw values for config entries sent with the action. """ # ruff: noqa: ARG001 - return tuple() # we do not have any config entries (yet) + return ( + ConfigEntry( + key=CONF_PORT, + type=ConfigEntryType.INTEGER, + default_value=DEFAULT_SLIMPROTO_PORT, + label="Slimproto port", + description="The TCP/UDP port to run the slimproto sockets server. " + "The default is 3483 and using a different port is not supported by " + "hardware squeezebox players. Only adjust this port if you want to " + "use other slimproto based servers side by side with software players, " + "such as squeezelite.\n\n" + "NOTE that the Airplay provider in MA (which relies on slimproto), does not seem " + "to support a different slimproto port.", + advanced=True, + ), + ConfigEntry( + key=CONF_CLI_TELNET, + type=ConfigEntryType.BOOLEAN, + default_value=True, + label="Enable classic Squeezebox Telnet CLI", + description="Some slimproto based players require the presence of the telnet CLI " + " to request more information. For example the Airplay provider " + "(which relies on slimproto) uses this to fetch the album cover and other metadata." + "By default this Telnet CLI is hosted on port 9090 but another port will be chosen if " + "that port is already taken. \n\n" + "Commands allowed on this interface are very limited and just enough to satisfy " + "player compatibility, so security risks are minimized to practically zero." + "You may safely disable this option if you have no players that rely on this feature " + "or you dont care about the additional metadata.", + advanced=True, + ), + ConfigEntry( + key=CONF_CLI_JSON, + type=ConfigEntryType.BOOLEAN, + default_value=True, + label="Enable JSON-RPC API", + description="Some slimproto based players require the presence of the JSON-RPC " + "API from LMS to request more information. For example to fetch the album cover " + "and other metadata. " + "This JSON-RPC API is compatible with Logitech Media Server but not all commands " + "are implemented. Just enough to satisfy player compatibility. \n\n" + "This API is hosted on the webserver responsible for streaming to players and thus " + "accessible on your local network but security impact should be minimal. " + "You may safely disable this option if you have no players that rely on this feature " + "or you dont care about the additional metadata.", + advanced=True, + ), + ConfigEntry( + key=CONF_DISCOVERY, + type=ConfigEntryType.BOOLEAN, + default_value=True, + label="Enable Discovery server", + description="Broadcast discovery packets for slimproto clients to automatically " + "discover and connect to this server. \n\n" + "You may want to disable this feature if you are running multiple slimproto servers " + "on your network and/or you don't want clients to auto connect to this server.", + advanced=True, + ), + ) class SlimprotoProvider(PlayerProvider): """Base/builtin provider for players using the SLIM protocol (aka slimproto).""" - _socket_servers: tuple[asyncio.Server | asyncio.BaseTransport] + _socket_servers: list[asyncio.Server | asyncio.BaseTransport] _socket_clients: dict[str, SlimClient] _sync_playpoints: dict[str, deque[SyncPlayPoint]] _virtual_providers: dict[str, tuple[Callable, Callable]] _cli: LmsCli + port: int = DEFAULT_SLIMPROTO_PORT async def handle_setup(self) -> None: """Handle async initialization of the provider.""" self._socket_clients = {} self._sync_playpoints = {} self._virtual_providers = {} - self._cli = LmsCli(self) - await self._cli.setup() - # autodiscovery of the slimproto server does not work - # when the port is not the default (3483) so we hardcode it for now - slimproto_port = 3483 - self.logger.info("Starting SLIMProto server on port %s", slimproto_port) - self._socket_servers = ( - # start slimproto server - await asyncio.start_server(self._create_client, "0.0.0.0", slimproto_port), - # setup discovery - await start_discovery( - self.mass.base_ip, - slimproto_port, - self._cli.cli_port, - self.mass.webserver.port, - "Music Assistant", - self.mass.server_id, - ), - ) + self.port = self.config.get_value(CONF_PORT) + # start slimproto socket server + try: + self._socket_servers = [ + await asyncio.start_server(self._create_client, "0.0.0.0", self.port) + ] + self.logger.info("Started SLIMProto server on port %s", self.port) + except OSError: + raise SetupFailedError( + f"Unable to start the Slimproto server - is port {self.port} already taken ?" + ) + + # start CLI interface(s) + enable_telnet = self.config.get_value(CONF_CLI_TELNET) + enable_json = self.config.get_value(CONF_CLI_JSON) + if enable_json or enable_telnet: + self._cli = LmsCli(self, enable_telnet, enable_json) + await self._cli.setup() + + # start discovery + if self.config.get_value(CONF_DISCOVERY): + self._socket_servers.append( + await start_discovery( + self.mass.base_ip, + self.port, + self._cli.cli_port if enable_telnet else None, + self.mass.webserver.port if enable_json else None, + "Music Assistant", + self.mass.server_id, + ) + ) async def unload(self) -> None: """Handle close/cleanup of the provider.""" @@ -146,7 +221,7 @@ class SlimprotoProvider(PlayerProvider): _server.close() if hasattr(self, "_cli"): await self._cli.unload() - self._socket_servers = None + self._socket_servers = [] async def _create_client( self, reader: asyncio.StreamReader, writer: asyncio.StreamWriter @@ -433,6 +508,13 @@ class SlimprotoProvider(PlayerProvider): update_callback, ) + def unregister_virtual_provider( + self, + player_model: str, + ) -> None: + """Unregister a virtual provider.""" + self._virtual_providers.pop(player_model, None) + def _handle_player_update(self, client: SlimClient) -> None: """Process SlimClient update/add to Player controller.""" player_id = client.player_id diff --git a/music_assistant/server/providers/slimproto/cli.py b/music_assistant/server/providers/slimproto/cli.py index 257aa65e..ad5e8fa1 100644 --- a/music_assistant/server/providers/slimproto/cli.py +++ b/music_assistant/server/providers/slimproto/cli.py @@ -138,27 +138,32 @@ class LmsCli: _unsub_callback: Callable | None = None _periodic_task: asyncio.Task | None = None - def __init__(self, slimproto: SlimprotoProvider) -> None: + def __init__( + self, slimproto: SlimprotoProvider, enable_telnet: bool, enable_json: bool + ) -> None: """Initialize.""" self.slimproto = slimproto + self.enable_telnet = enable_telnet + self.enable_json = enable_json self.logger = self.slimproto.logger.getChild("cli") self.mass = self.slimproto.mass self._cometd_clients: dict[str, CometDClient] = {} async def setup(self) -> None: """Handle async initialization of the plugin.""" - self.logger.info("Registering jsonrpc endpoints on the webserver") - self.mass.webserver.register_route("/jsonrpc.js", self._handle_jsonrpc) - self.mass.webserver.register_route("/cometd", self._handle_cometd) - # setup (telnet) cli for players requesting basic info on that port - self.cli_port = await select_free_port(9090, 9190) - self.logger.info("Starting (telnet) CLI on port %s", self.cli_port) - await asyncio.start_server(self._handle_cli_client, "0.0.0.0", self.cli_port) - self._unsub_callback = self.mass.subscribe( - self._on_mass_event, - (EventType.PLAYER_UPDATED, EventType.QUEUE_UPDATED), - ) - self._periodic_task = self.mass.create_task(self._do_periodic()) + if self.enable_json: + self.logger.info("Registering jsonrpc endpoints on the webserver") + self.mass.webserver.register_route("/jsonrpc.js", self._handle_jsonrpc) + self.mass.webserver.register_route("/cometd", self._handle_cometd) + self._unsub_callback = self.mass.subscribe( + self._on_mass_event, + (EventType.PLAYER_UPDATED, EventType.QUEUE_UPDATED), + ) + self._periodic_task = self.mass.create_task(self._do_periodic()) + if self.enable_telnet: + self.cli_port = await select_free_port(9090, 9190) + self.logger.info("Starting (telnet) CLI on port %s", self.cli_port) + await asyncio.start_server(self._handle_cli_client, "0.0.0.0", self.cli_port) async def unload(self) -> None: """ @@ -167,6 +172,7 @@ class LmsCli: Called when provider is deregistered (e.g. MA exiting or config reloading). """ self.mass.webserver.unregister_route("/jsonrpc.js") + self.mass.webserver.unregister_route("/cometd") if self._unsub_callback: self._unsub_callback() self._unsub_callback = None diff --git a/music_assistant/server/providers/slimproto/manifest.json b/music_assistant/server/providers/slimproto/manifest.json index a35d7e1d..59d21c1d 100644 --- a/music_assistant/server/providers/slimproto/manifest.json +++ b/music_assistant/server/providers/slimproto/manifest.json @@ -4,7 +4,7 @@ "name": "Slimproto", "description": "Support for slimproto based players (e.g. squeezebox, squeezelite).", "codeowners": ["@music-assistant"], - "requirements": ["aioslimproto==2.3.1"], + "requirements": ["aioslimproto==2.3.2"], "documentation": "https://github.com/music-assistant/hass-music-assistant/discussions/1123", "multi_instance": false, "builtin": false, diff --git a/music_assistant/server/server.py b/music_assistant/server/server.py index 0a97df38..aae7f37d 100644 --- a/music_assistant/server/server.py +++ b/music_assistant/server/server.py @@ -22,7 +22,6 @@ from music_assistant.common.models.provider import ProviderManifest from music_assistant.constants import ( CONF_PROVIDERS, CONF_SERVER_ID, - CONF_WEB_IP, MIN_SCHEMA_VERSION, ROOT_LOGGER_NAME, SCHEMA_VERSION, @@ -106,7 +105,6 @@ class MusicAssistant: ) # setup config controller first and fetch important config values await self.config.setup() - self.base_ip = self.config.get(CONF_WEB_IP, self.base_ip) LOGGER.info( "Starting Music Assistant Server (%s) - autodetected IP-address: %s", self.server_id, @@ -392,8 +390,12 @@ class MusicAssistant: if sync_task.provider_instance == instance_id: sync_task.task.cancel() await sync_task.task + # check if there are no other providers dependent of this provider + for dep_prov in self.providers: + if dep_prov.manifest.depends_on == provider.domain: + await self.unload_provider(dep_prov.instance_id) await provider.unload() - self._providers.pop(instance_id) + self._providers.pop(instance_id, None) self.signal_event(EventType.PROVIDERS_UPDATED, data=self.get_providers()) def _register_api_commands(self) -> None: diff --git a/requirements_all.txt b/requirements_all.txt index 2d83b74e..bb207160 100644 --- a/requirements_all.txt +++ b/requirements_all.txt @@ -5,7 +5,7 @@ aiodns>=3.0.0 aiofiles==23.1.0 aiohttp==3.8.4 aiorun==2022.11.1 -aioslimproto==2.3.1 +aioslimproto==2.3.2 aiosqlite==0.19.0 async-upnp-client==0.33.2 asyncio-throttle==1.0.2 -- 2.34.1