Clean up all the things (#36)

Author: balloob

sendspin/cli.py

@@ -4,13 +4,17 @@
 
 import argparse
 import asyncio
+import logging
 import sys
 from collections.abc import Sequence
+import socket
 from typing import TYPE_CHECKING
 
 if TYPE_CHECKING:
     from sendspin.audio import AudioDevice
 
+LOGGER = logging.getLogger(__name__)
+
 PORTAUDIO_NOT_FOUND_MESSAGE = """Error: PortAudio library not found.
 
 Please install PortAudio for your system:
@@ -197,8 +201,6 @@ async def list_servers() -> None:
             print(f"  {server.name}")
             print(f"    URL:  {server.url}")
             print(f"    Host: {server.host}:{server.port}")
-        if servers:
-            print(f"\nTo connect to a server:\n  sendspin --url {servers[0].url}")
     except Exception as e:  # noqa: BLE001
         print(f"Error discovering servers: {e}")
         sys.exit(1)
@@ -241,23 +243,47 @@ def _resolve_audio_device(device_arg: str | None) -> AudioDevice:
         dev_type = "Default" if device_arg is None else "Specified"
         raise CLIError(f"{dev_type} audio device not found.")
 
+    LOGGER.info(
+        "Using audio device %d: %s",
+        device.index,
+        device.name,
+    )
+
     return device
 
 
-def _run_daemon_mode(args: argparse.Namespace) -> int:
+def _resolve_client_info(client_id: str | None, client_name: str | None) -> tuple[str, str]:
+    """Determine client ID and name."""
+    # Get hostname for defaults if needed
+    if client_id is None or client_name is None:
+        hostname = socket.gethostname()
+        if not hostname:
+            raise CLIError("Unable to determine hostname. Please specify --id and/or --name", 1)
+        # Auto-generate client ID and name from hostname
+        if client_id is None:
+            client_id = f"sendspin-cli-{hostname}"
+        if client_name is None:
+            client_name = hostname
+
+    return client_id, client_name
+
+
+async def _run_daemon_mode(args: argparse.Namespace) -> int:
     """Run the client in daemon mode (no UI)."""
     from sendspin.daemon.daemon import DaemonConfig, SendspinDaemon
 
+    client_id, client_name = _resolve_client_info(args.id, args.name)
+
     daemon_config = DaemonConfig(
         audio_device=_resolve_audio_device(args.audio_device),
         url=args.url,
-        client_id=args.id,
-        client_name=args.name,
+        client_id=client_id,
+        client_name=client_name,
         static_delay_ms=args.static_delay_ms,
     )
 
     daemon = SendspinDaemon(daemon_config)
-    return asyncio.run(daemon.run())
+    return await daemon.run()
 
 
 def main() -> int:
@@ -308,7 +334,7 @@ def main() -> int:
         return 0
 
     try:
-        return _run_client_mode(args)
+        return asyncio.run(_run_client_mode(args))
     except CLIError as e:
         print(f"Error: {e}")
         return e.exit_code
@@ -319,30 +345,32 @@ def main() -> int:
         raise
 
 
-def _run_client_mode(args: argparse.Namespace) -> int:
+async def _run_client_mode(args: argparse.Namespace) -> int:
     """Run the client in TUI or daemon mode."""
     # Handle daemon subcommand
     if args.command == "daemon":
-        return _run_daemon_mode(args)
+        return await _run_daemon_mode(args)
 
     # Handle deprecated --headless flag
     if args.headless:
         print("Warning: --headless is deprecated. Use 'sendspin daemon' instead.")
         print("Routing to daemon mode...\n")
-        return _run_daemon_mode(args)
+        return await _run_daemon_mode(args)
 
     from sendspin.tui.app import AppConfig, SendspinApp
 
+    client_id, client_name = _resolve_client_info(args.id, args.name)
+
     app_config = AppConfig(
         audio_device=_resolve_audio_device(args.audio_device),
         url=args.url,
-        client_id=args.id,
-        client_name=args.name,
+        client_id=client_id,
+        client_name=client_name,
         static_delay_ms=args.static_delay_ms,
     )
 
     app = SendspinApp(app_config)
-    return asyncio.run(app.run())
+    return await app.run()
 
 
 if __name__ == "__main__":

sendspin/daemon/daemon.py

@@ -6,9 +6,7 @@
 import contextlib
 import logging
 import signal
-import socket
 from dataclasses import dataclass
-from functools import partial
 
 from aiohttp import ClientError
 from aiosendspin.client import SendspinClient
@@ -29,9 +27,9 @@ class DaemonConfig:
     """Configuration for the Sendspin daemon."""
 
     audio_device: AudioDevice
+    client_id: str
+    client_name: str
     url: str | None = None
-    client_id: str | None = None
-    client_name: str | None = None
     static_delay_ms: float = 0.0
 
 
@@ -41,34 +39,9 @@ class SendspinDaemon:
     def __init__(self, config: DaemonConfig) -> None:
         """Initialize the daemon."""
         self._config = config
-        self._client: SendspinClient | None = None
-        self._audio_handler: AudioStreamHandler | None = None
-        self._discovery: ServiceDiscovery | None = None
-        self._shutdown_event: asyncio.Event | None = None
-
-    async def run(self) -> int:
-        """Run the daemon."""
-        config = self._config
-
-        # Get hostname for defaults if needed
-        client_id = config.client_id
-        client_name = config.client_name
-        if client_id is None or client_name is None:
-            hostname = socket.gethostname()
-            if not hostname:
-                logger.error("Unable to determine hostname. Please specify --id and/or --name")
-                return 1
-            # Auto-generate client ID and name from hostname
-            if client_id is None:
-                client_id = f"sendspin-cli-{hostname}"
-            if client_name is None:
-                client_name = hostname
-
-        logger.info("Starting Sendspin daemon: %s", client_id)
-
         self._client = SendspinClient(
-            client_id=client_id,
-            client_name=client_name,
+            client_id=config.client_id,
+            client_name=config.client_name,
             roles=[Roles.PLAYER],
             device_info=get_device_info(),
             player_support=ClientHelloPlayerSupport(
@@ -85,90 +58,62 @@ async def run(self) -> int:
             ),
             static_delay_ms=config.static_delay_ms,
         )
-
+        self._audio_handler = AudioStreamHandler(audio_device=config.audio_device)
         self._discovery = ServiceDiscovery()
+        self._shutdown_event = asyncio.Event()
+
+    async def run(self) -> int:
+        """Run the daemon."""
+        logger.info("Starting Sendspin daemon: %s", self._client._client_id)
+        url = self._config.url
+        loop = asyncio.get_running_loop()
+
+        def signal_handler() -> None:
+            logger.debug("Received interrupt signal, shutting down...")
+            self._shutdown_event.set()
+
+        # Register signal handlers
+        with contextlib.suppress(NotImplementedError):
+            loop.add_signal_handler(signal.SIGINT, signal_handler)
+            loop.add_signal_handler(signal.SIGTERM, signal_handler)
+
         await self._discovery.start()
 
         try:
-            # Get initial server URL
-            url = config.url
             if url is None:
                 logger.info("Waiting for mDNS discovery of Sendspin server...")
-                try:
-                    url = await self._discovery.wait_for_first_server()
-                    logger.info("Discovered Sendspin server at %s", url)
-                except asyncio.CancelledError:
-                    return 1
-                except Exception:
-                    logger.exception("Failed to discover server")
-                    return 1
-
-            logger.info(
-                "Using audio device %d: %s",
-                config.audio_device.index,
-                config.audio_device.name,
-            )
+                url = await self._discover_server()
+                if self._shutdown_event.is_set():
+                    return 0
 
             listeners = ClientListenerManager()
-
-            self._audio_handler = AudioStreamHandler(audio_device=config.audio_device)
             self._audio_handler.attach_client(self._client, listeners)
-
             listeners.attach(self._client)
 
-            loop = asyncio.get_running_loop()
-
-            self._shutdown_event = asyncio.Event()
-
-            def signal_handler() -> None:
-                logger.debug("Received interrupt signal, shutting down...")
-                if self._shutdown_event is not None:
-                    self._shutdown_event.set()
-
-            # Register signal handlers
-            with contextlib.suppress(NotImplementedError):
-                loop.add_signal_handler(signal.SIGINT, signal_handler)
-                loop.add_signal_handler(signal.SIGTERM, signal_handler)
-
-            try:
-                await self._connection_loop(url, use_discovery=config.url is None)
-            finally:
-                # Remove signal handlers
-                with contextlib.suppress(NotImplementedError):
-                    loop.remove_signal_handler(signal.SIGINT)
-                    loop.remove_signal_handler(signal.SIGTERM)
-                await self._audio_handler.cleanup()
-                await self._client.disconnect()
-                logger.info("Daemon stopped")
+            await self._connection_loop(url, use_discovery=self._config.url is None)
 
         finally:
-            # Stop discovery
+            await self._audio_handler.cleanup()
+            await self._client.disconnect()
             await self._discovery.stop()
+            logger.info("Daemon stopped")
 
         return 0
 
     async def _connection_loop(self, initial_url: str, use_discovery: bool) -> None:
         """Run the connection loop with automatic reconnection."""
-        assert self._client is not None
-        assert self._discovery is not None
-        assert self._audio_handler is not None
-        assert self._shutdown_event is not None
-
         url = initial_url
         error_backoff = 1.0
         max_backoff = 300.0
+        disconnect_event = asyncio.Event()
+        self._client.set_disconnect_listener(disconnect_event.set)
 
         while not self._shutdown_event.is_set():
+            disconnect_event.clear()
+
             try:
-                logger.info("Connecting to %s", url)
                 await self._client.connect(url)
-                logger.info("Connected to %s", url)
                 error_backoff = 1.0
-
-                # Wait for disconnect or shutdown
-                disconnect_event = asyncio.Event()
-                self._client.set_disconnect_listener(partial(asyncio.Event.set, disconnect_event))
-
                 shutdown_task = create_task(self._shutdown_event.wait())
                 disconnect_task = create_task(disconnect_event.wait())
 
@@ -182,8 +127,6 @@ async def _connection_loop(self, initial_url: str, use_discovery: bool) -> None:
                     with contextlib.suppress(asyncio.CancelledError):
                         await task
 
-                self._client.set_disconnect_listener(None)
-
                 if shutdown_task in done:
                     break
 
@@ -192,21 +135,7 @@ async def _connection_loop(self, initial_url: str, use_discovery: bool) -> None:
                 await self._audio_handler.cleanup()
 
                 if use_discovery:
-                    # Try to get new URL from discovery, or use last known URL
-                    new_url = self._discovery.current_url()
-                    if new_url:
-                        url = new_url
-
-                    # Wait for server to reappear if discovery shows nothing
-                    if not self._discovery.current_url():
-                        logger.info("Server offline, waiting for rediscovery...")
-                        while not self._shutdown_event.is_set():
-                            new_url = self._discovery.current_url()
-                            if new_url:
-                                url = new_url
-                                break
-                            await asyncio.sleep(1.0)
-
+                    url = await self._discover_server()
                     if self._shutdown_event.is_set():
                         break
 
@@ -227,8 +156,8 @@ async def _connection_loop(self, initial_url: str, use_discovery: bool) -> None:
                     pass  # Sleep completed, continue loop
 
                 # Check if URL changed while sleeping (only when using discovery)
-                if use_discovery:
-                    new_url = self._discovery.current_url()
+                if use_discovery and (servers := self._discovery.get_servers()):
+                    new_url = servers[0].url
                     if new_url and new_url != url:
                         logger.info("Server URL changed to %s", new_url)
                         url = new_url
@@ -239,9 +168,24 @@ async def _connection_loop(self, initial_url: str, use_discovery: bool) -> None:
 
             except Exception:
                 logger.exception("Unexpected error during connection")
-                try:
-                    await asyncio.wait_for(self._shutdown_event.wait(), timeout=error_backoff)
-                    break
-                except TimeoutError:
-                    pass
-                error_backoff = min(error_backoff * 2, max_backoff)
+                break
+
+    async def _discover_server(self) -> str:
+        """Discover next server to use.
+
+        Returns empty string when shutdown is set.
+        """
+        next_server_task = create_task(self._discovery.wait_for_server())
+
+        if not next_server_task.done():
+            shutdown_task = create_task(self._shutdown_event.wait())
+            await asyncio.wait(
+                {shutdown_task, next_server_task},
+                return_when=asyncio.FIRST_COMPLETED,
+            )
+
+        if self._shutdown_event.is_set():
+            return ""
+
+        server = await next_server_task
+        return server.url