From b9822a32d989c42323a221f56023244f11d5d009 Mon Sep 17 00:00:00 2001 From: Marcel van der Veldt Date: Mon, 29 Jan 2024 13:06:35 +0100 Subject: [PATCH] Some stability and bigfixes (#1043) * fix reloading of sonos provider * fix thread safety issue * fix reload of sonos provider * handle changed IP from sonos player * fix some small typos * some more typos * Improve polling to detect players go offline * fix for unresumable radio streams * fix pause on slimproto * fix for some radio stations not playing due to parse error handover to ffmpeg * fix for replace next --- .../server/controllers/player_queues.py | 11 +++- music_assistant/server/controllers/players.py | 6 +-- music_assistant/server/helpers/audio.py | 15 +++--- .../server/providers/slimproto/__init__.py | 1 + .../server/providers/sonos/__init__.py | 44 +++++++++------- .../server/providers/sonos/helpers.py | 22 ++++---- .../server/providers/sonos/player.py | 52 +++++++++++++++---- 7 files changed, 101 insertions(+), 50 deletions(-) diff --git a/music_assistant/server/controllers/player_queues.py b/music_assistant/server/controllers/player_queues.py index 163df9e0..94daafff 100755 --- a/music_assistant/server/controllers/player_queues.py +++ b/music_assistant/server/controllers/player_queues.py @@ -302,6 +302,12 @@ class PlayerQueuesController(CoreController): self.domain, f"default_enqueue_action_{media_item.media_type.value}" ) ) + if option == QueueOption.REPLACE_NEXT and queue.state not in ( + PlayerState.PLAYING, + PlayerState.PAUSED, + ): + # replace next requested but nothing is playing + option = QueueOption.REPLACE # clear queue if needed if option == QueueOption.REPLACE: self.clear(queue_id) @@ -617,6 +623,9 @@ class PlayerQueuesController(CoreController): if resume_item is not None: resume_pos = resume_pos if resume_pos > 10 else 0 fade_in = fade_in if fade_in is not None else resume_pos > 0 + if resume_item.media_type == MediaType.RADIO: + # we're not able to skip in online radio so this is pointless + resume_pos = 0 await self.play_index(queue_id, resume_item.queue_item_id, resume_pos, fade_in) else: raise QueueEmpty(f"Resume queue requested but queue {queue_id} is empty") @@ -979,7 +988,7 @@ class PlayerQueuesController(CoreController): if PlayerFeature.ENQUEUE_NEXT in player.supported_features: # player supports enqueue next feature. # we enqueue the next track after a new track - # has started playing and before the current track ends + # has started playing and (repeat) before the current track ends new_track_started = new_state.get("state") == PlayerState.PLAYING and prev_state.get( "current_index" ) != new_state.get("current_index") diff --git a/music_assistant/server/controllers/players.py b/music_assistant/server/controllers/players.py index 1f36c346..aa30cb58 100755 --- a/music_assistant/server/controllers/players.py +++ b/music_assistant/server/controllers/players.py @@ -831,13 +831,13 @@ class PlayerController(CoreController): if player_playing: self.mass.loop.call_soon(self.update, player_id) # Poll player; - # - every 360 seconds if the player if not powered + # - every 120 seconds if the player if not powered # - every 30 seconds if the player is powered # - every 10 seconds if the player is playing if ( (player.powered and count % 30 == 0) or (player_playing and count % 10 == 0) - or count == 360 + or count % 120 == 0 ) and (player_prov := self.get_player_provider(player_id)): try: await player_prov.poll_player(player_id) @@ -855,7 +855,7 @@ class PlayerController(CoreController): finally: # always update player state self.mass.loop.call_soon(self.update, player_id) - if count >= 360: + if count >= 120: count = 0 await asyncio.sleep(1) diff --git a/music_assistant/server/helpers/audio.py b/music_assistant/server/helpers/audio.py index 09e0719d..0b8a9764 100644 --- a/music_assistant/server/helpers/audio.py +++ b/music_assistant/server/helpers/audio.py @@ -14,7 +14,7 @@ from time import time from typing import TYPE_CHECKING import aiofiles -from aiohttp import ClientTimeout +from aiohttp import ClientResponseError, ClientTimeout from music_assistant.common.models.errors import ( AudioError, @@ -539,11 +539,14 @@ async def resolve_radio_stream(mass: MusicAssistant, url: str) -> tuple[str, boo # determine ICY metadata support by looking at the http headers headers = {"Icy-MetaData": "1", "User-Agent": "VLC/3.0.2.LibVLC/3.0.2"} timeout = ClientTimeout(total=0, connect=10, sock_read=5) - async with mass.http_session.head( - url, headers=headers, allow_redirects=True, timeout=timeout - ) as resp: - headers = resp.headers - supports_icy = int(headers.get("icy-metaint", "0")) > 0 + try: + async with mass.http_session.head( + url, headers=headers, allow_redirects=True, timeout=timeout + ) as resp: + headers = resp.headers + supports_icy = int(headers.get("icy-metaint", "0")) > 0 + except ClientResponseError as err: + LOGGER.debug("Error while parsing radio URL %s: %s", url, err) result = (url, supports_icy) await mass.cache.set(cache_key, result) diff --git a/music_assistant/server/providers/slimproto/__init__.py b/music_assistant/server/providers/slimproto/__init__.py index 4817b48f..a0407d71 100644 --- a/music_assistant/server/providers/slimproto/__init__.py +++ b/music_assistant/server/providers/slimproto/__init__.py @@ -628,6 +628,7 @@ class SlimprotoProvider(PlayerProvider): PlayerFeature.POWER, PlayerFeature.SYNC, PlayerFeature.VOLUME_SET, + PlayerFeature.PAUSE, ), max_sample_rate=int(client.max_sample_rate), supports_24bit=int(client.max_sample_rate) > 44100, diff --git a/music_assistant/server/providers/sonos/__init__.py b/music_assistant/server/providers/sonos/__init__.py index e46616b2..5066ae37 100644 --- a/music_assistant/server/providers/sonos/__init__.py +++ b/music_assistant/server/providers/sonos/__init__.py @@ -16,8 +16,8 @@ from dataclasses import dataclass, field from typing import TYPE_CHECKING import soco.config as soco_config -from requests.exceptions import Timeout -from soco import SoCoException, events_asyncio, zonegroupstate +from requests.exceptions import RequestException +from soco import events_asyncio, zonegroupstate from soco.core import SoCo from soco.discovery import discover @@ -36,7 +36,7 @@ from music_assistant.common.models.enums import ( from music_assistant.common.models.errors import PlayerCommandFailed, 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_CROSSFADE, CONF_PLAYERS +from music_assistant.constants import CONF_CROSSFADE from music_assistant.server.helpers.didl_lite import create_didl_metadata from music_assistant.server.models.player_provider import PlayerProvider @@ -55,6 +55,7 @@ PLAYER_FEATURES = ( PlayerFeature.VOLUME_MUTE, PlayerFeature.VOLUME_SET, PlayerFeature.ENQUEUE_NEXT, + PlayerFeature.PAUSE, ) CONF_NETWORK_SCAN = "network_scan" @@ -454,9 +455,12 @@ class SonosPlayerProvider(PlayerProvider): for soco in discovered_devices: try: self._add_player(soco) - except (OSError, SoCoException, Timeout) as err: + except RequestException as err: + # player is offline + self.logger.debug("Failed to add SonosPlayer %s: %s", soco, err) + except Exception as err: self.logger.warning( - "Failed to add SonosPlayer using %s: %s", soco, err, exc_info=err + "Failed to add SonosPlayer %s: %s", soco, err, exc_info=err ) finally: self._discovery_running = False @@ -468,32 +472,30 @@ class SonosPlayerProvider(PlayerProvider): self.mass.create_task(self._run_discovery()) # reschedule self once finished - self._discovery_reschedule_timer = self.mass.loop.call_later(300, reschedule) + self._discovery_reschedule_timer = self.mass.loop.call_later(1800, reschedule) def _add_player(self, soco: SoCo) -> None: """Add discovered Sonos player.""" player_id = soco.uid - if player_id in self.sonosplayers: - return # already added + # check if existing player changed IP + if existing := self.sonosplayers.get(player_id): + if existing.soco.ip_address != soco.ip_address: + existing.update_ip(soco.ip_address) + return if not soco.is_visible: return - enabled = self.mass.config.get(f"{CONF_PLAYERS}/{player_id}/enabled", True) + enabled = self.mass.config.get_raw_player_config_value(player_id, "enabled", True) if not enabled: self.logger.debug("Ignoring disabled player: %s", player_id) return - if soco not in soco.visible_zones: - return - speaker_info = soco.get_speaker_info(True, timeout=7) if soco.uid not in self.boot_counts: self.boot_counts[soco.uid] = soco.boot_seqnum self.logger.debug("Adding new player: %s", speaker_info) support_hires = speaker_info["model_name"] in HIRES_MODELS - self.sonosplayers[player_id] = sonos_player = SonosPlayer( - self, - soco=soco, - mass_player=Player( + if not (mass_player := self.mass.players.get(soco.uid)): + mass_player = Player( player_id=soco.uid, provider=self.domain, type=PlayerType.PLAYER, @@ -508,10 +510,16 @@ class SonosPlayerProvider(PlayerProvider): ), max_sample_rate=48000 if support_hires else 44100, supports_24bit=support_hires, - ), + ) + self.sonosplayers[player_id] = sonos_player = SonosPlayer( + self, + soco=soco, + mass_player=mass_player, ) sonos_player.setup() - self.mass.loop.call_soon_threadsafe(self.mass.players.register, sonos_player.mass_player) + self.mass.loop.call_soon_threadsafe( + self.mass.players.register_or_update, sonos_player.mass_player + ) async def _enqueue_item( self, diff --git a/music_assistant/server/providers/sonos/helpers.py b/music_assistant/server/providers/sonos/helpers.py index e2895835..872b7682 100644 --- a/music_assistant/server/providers/sonos/helpers.py +++ b/music_assistant/server/providers/sonos/helpers.py @@ -6,7 +6,6 @@ import logging from collections.abc import Callable from typing import TYPE_CHECKING, Any, Concatenate, ParamSpec, TypeVar, overload -from requests.exceptions import Timeout from soco import SoCo from soco.exceptions import SoCoException, SoCoUPnPException @@ -14,15 +13,14 @@ from music_assistant.common.models.errors import PlayerCommandFailed if TYPE_CHECKING: from . import SonosPlayer - from .media import SonosMedia + UID_PREFIX = "RINCON_" UID_POSTFIX = "01400" -SONOS_SPEAKER_ACTIVITY = "sonos_speaker_activity" _LOGGER = logging.getLogger(__name__) -_T = TypeVar("_T", bound="SonosPlayer | SonosMedia") +_T = TypeVar("_T", bound="SonosPlayer") _R = TypeVar("_R") _P = ParamSpec("_P") @@ -30,6 +28,10 @@ _FuncType = Callable[Concatenate[_T, _P], _R] _ReturnFuncType = Callable[Concatenate[_T, _P], _R | None] +class SonosUpdateError(PlayerCommandFailed): + """Update failed.""" + + @overload def soco_error( errorcodes: None = ..., @@ -55,7 +57,7 @@ def soco_error( args_soco = next((arg for arg in args if isinstance(arg, SoCo)), None) try: result = funct(self, *args, **kwargs) - except (OSError, SoCoException, SoCoUPnPException, Timeout) as err: + except (OSError, SoCoException, SoCoUPnPException, TimeoutError) as err: error_code = getattr(err, "error_code", None) function = funct.__qualname__ if errorcodes and error_code in errorcodes: @@ -66,7 +68,7 @@ def soco_error( raise RuntimeError("Unexpected use of soco_error") from err message = f"Error calling {function} on {target}: {err}" - raise PlayerCommandFailed(message) from err + raise SonosUpdateError(message) from err return result @@ -77,15 +79,9 @@ def soco_error( def _find_target_identifier(instance: Any, fallback_soco: SoCo | None) -> str | None: """Extract the best available target identifier from the provided instance object.""" - if entity_id := getattr(instance, "entity_id", None): - # SonosEntity instance - return entity_id if zone_name := getattr(instance, "zone_name", None): - # SonosSpeaker instance + # SonosPlayer instance return zone_name - if speaker := getattr(instance, "speaker", None): - # Holds a SonosSpeaker instance attribute - return speaker.zone_name if soco := getattr(instance, "soco", fallback_soco): # Holds a SoCo instance attribute # Only use attributes with no I/O diff --git a/music_assistant/server/providers/sonos/player.py b/music_assistant/server/providers/sonos/player.py index 4f3161b4..70da75c5 100644 --- a/music_assistant/server/providers/sonos/player.py +++ b/music_assistant/server/providers/sonos/player.py @@ -33,8 +33,9 @@ from sonos_websocket import SonosWebsocket from music_assistant.common.helpers.datetime import utc from music_assistant.common.models.enums import PlayerFeature, PlayerState from music_assistant.common.models.errors import PlayerCommandFailed -from music_assistant.common.models.player import Player -from music_assistant.server.providers.sonos.helpers import soco_error +from music_assistant.common.models.player import DeviceInfo, Player + +from .helpers import SonosUpdateError, soco_error if TYPE_CHECKING: from . import SonosPlayerProvider @@ -100,10 +101,6 @@ class SonosSubscriptionsFailed(PlayerCommandFailed): """Subscription creation failed.""" -class SonosUpdateError(PlayerCommandFailed): - """Update failed.""" - - class SonosPlayer: """Wrapper around Sonos/SoCo with some additional attributes.""" @@ -171,6 +168,13 @@ class SonosPlayer: subscribed_services = {sub.service.service_type for sub in self._subscriptions} return SUBSCRIPTION_SERVICES - subscribed_services + @property + def should_poll(self) -> bool: + """Return if this player should be polled/pinged.""" + if not self.available: + return True + return (time.monotonic() - self._last_activity) > 120 + def setup(self) -> None: """Run initial setup of the speaker (NOT async friendly).""" if self.soco.is_coordinator: @@ -289,18 +293,41 @@ class SonosPlayer: async def check_poll(self) -> None: """Validate availability of the speaker based on recent activity.""" - if not (not self.available or (time.monotonic() - self._last_activity) > 600): + if not self.should_poll: return + self.logger.debug("Polling player for availability...") try: - await self.mass.create_task(self.ping) + await asyncio.to_thread(self.ping) self._speaker_activity("ping") except SonosUpdateError: + if not self.available: + return # already offline self.logger.warning( "No recent activity and cannot reach %s, marking unavailable", self.zone_name, ) await self.offline() + def update_ip(self, ip_address: str) -> None: + """Handle updated IP of a Sonos player (NOT async friendly).""" + if self.available: + return + self.logger.info( + "Player IP-address changed from %s to %s", self.soco.ip_address, ip_address + ) + try: + self.ping() + except SonosUpdateError: + return + self.soco.ip_address = ip_address + self.setup() + self.mass_player.device_info = DeviceInfo( + model=self.mass_player.device_info.model, + address=ip_address, + manufacturer=self.mass_player.device_info.manufacturer, + ) + self.update_player() + @soco_error() def ping(self) -> None: """Test device availability. Failure will raise SonosUpdateError.""" @@ -328,7 +355,7 @@ class SonosPlayer: # send update to the player manager right away only if we are triggered from an event # when we're just updating from a manual poll, the player manager # will detect changes to the player object itself - self.sonos_prov.mass.players.update(self.player_id) + self.mass.loop.call_soon_threadsafe(self.sonos_prov.mass.players.update, self.player_id) @soco_error() def poll_track_info(self) -> dict[str, Any]: @@ -668,6 +695,13 @@ class SonosPlayer: # generic attributes (player_info) self.mass_player.available = self.available + if not self.available: + self.mass_player.powered = False + self.mass_player.state = PlayerState.IDLE + self.mass_player.synced_to = None + self.mass_player.group_childs = set() + return + # transport info (playback state) self.mass_player.state = current_state = _convert_state(self.playback_status) -- 2.34.1