jeans/core
jeans/core
1
0
Fork 0
mirror of https://github.com/home-assistant/core.git synced 2025-07-23 05:07:41 +00:00
Code Issues Packages Projects Releases Wiki Activity

Use non-persistent connection for MPD (#94507)

Browse Source 
* Do not use a persistent connection to MPD

In other words, don't rely on the connection management provided by
"python-mpd2". Instead of keeping the connection to MPD open, we
explicitly connect before and disconnect after each command.

There is probably a bit of overhead to this, but as the integration
uses a local-polling approach to begin with, no functionality is lost or
degraded.

This change greatly hardens the MPD integration against both network
issues and problems with the daemon itself. All connection-related
failure modes have effectively been removed.

* Update state retrieval methods

Only "async_get_media_image" attempts to connect, all others are either
called from there, or from the main "async_update" method (see previous
commit) which also attempts to connect.

So, this commit mainly revolves around gracefully handling situations
where no connection is available when trying to retrieve MPD state.

Finally, note the removal of "self._commands". This property is only
used at the start of "_async_get_file_image_response" and was thus
changed into a local variable.

* Update media-player control methods

These all need to explicitly connect to MPD as part of their flow.

* Correct ruff failure (auto-fixed)

* Use "async_timeout.timeout" context manager

* Minor changes

* Replace "async_timeout" with "asyncio.timeout"

* Initialise "self._status" to empty dictionary

Used to be initialised as None, which caused "NoneType is not iterable"
type of issues in case of an unexpected disconnect (at which point
status gets set to None again). Instead of guarding against None
everywhere, using an empty dictionary seemed more prudent...

Furthermore, more cautiously access its members to prevent potential
KeyError-s in similar cases.

* Fix livelock in "async_mute_volume()"

This method doesn't need a connection; it calls into two other methods
that actually connect to MPD – attempting to connect from here resulted
in a livelock.
This commit is contained in:
Thijs Putman 2023-11-27 18:35:46 +01:00 committed by GitHub
parent 2a4ab3d53d
commit 360ef894a7
No known key found for this signature in database
GPG Key ID: 4AEE18F83AFDEB23
1 changed files with 191 additions and 137 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

328
homeassistant/components/mpd/media_player.py
View File

@ -1,11 +1,13 @@
"""Support to interact with a Music Player Daemon.""" """Support to interact with a Music Player Daemon."""
from __future__ import annotations from __future__ import annotations
from contextlib import suppress import asyncio
from contextlib import asynccontextmanager, suppress
from datetime import timedelta from datetime import timedelta
import hashlib import hashlib
import logging import logging
import os import os
from socket import gaierror
from typing import Any from typing import Any
import mpd import mpd
@ -92,11 +94,11 @@ class MpdDevice(MediaPlayerEntity):
self._name = name self._name = name
self.password = password self.password = password
self._status = None self._status = {}
self._currentsong = None self._currentsong = None
self._playlists = None self._playlists = None
self._currentplaylist = None self._currentplaylist = None
self._is_connected = False self._is_available = None
self._muted = False self._muted = False
self._muted_volume = None self._muted_volume = None
self._media_position_updated_at = None self._media_position_updated_at = None
@ -104,67 +106,88 @@ class MpdDevice(MediaPlayerEntity):
self._media_image_hash = None self._media_image_hash = None
# Track if the song changed so image doesn't have to be loaded every update. # Track if the song changed so image doesn't have to be loaded every update.
self._media_image_file = None self._media_image_file = None
self._commands = None
# set up MPD client # set up MPD client
self._client = MPDClient() self._client = MPDClient()
self._client.timeout = 30 self._client.timeout = 30
self._client.idletimeout = None self._client.idletimeout = 10
self._client_lock = asyncio.Lock()
async def _connect(self): # Instead of relying on python-mpd2 to maintain a (persistent) connection to
"""Connect to MPD.""" # MPD, the below explicitly sets up a *non*-persistent connection. This is
try: # done to workaround the issue as described in:
await self._client.connect(self.server, self.port) # <https://github.com/Mic92/python-mpd2/issues/31>
@asynccontextmanager
if self.password is not None: async def connection(self):
await self._client.password(self.password) """Handle MPD connect and disconnect."""
except mpd.ConnectionError: async with self._client_lock:
return try:
# MPDClient.connect() doesn't always respect its timeout. To
self._is_connected = True # prevent a deadlock, enforce an additional (slightly longer)
# timeout on the coroutine itself.
def _disconnect(self): try:
"""Disconnect from MPD.""" async with asyncio.timeout(self._client.timeout + 5):
with suppress(mpd.ConnectionError): await self._client.connect(self.server, self.port)
self._client.disconnect() except asyncio.TimeoutError as error:
self._is_connected = False # TimeoutError has no message (which hinders logging further
self._status = None # down the line), so provide one.
raise asyncio.TimeoutError(
async def _fetch_status(self): "Connection attempt timed out"
"""Fetch status from MPD.""" ) from error
self._status = await self._client.status() if self.password is not None:
self._currentsong = await self._client.currentsong() await self._client.password(self.password)
await self._async_update_media_image_hash() self._is_available = True
yield
if (position := self._status.get("elapsed")) is None: except (
position = self._status.get("time") asyncio.TimeoutError,
gaierror,
if isinstance(position, str) and ":" in position: mpd.ConnectionError,
position = position.split(":")[0] OSError,
) as error:
if position is not None and self._media_position != position: # Log a warning during startup or when previously connected; for
self._media_position_updated_at = dt_util.utcnow() # subsequent errors a debug message is sufficient.
self._media_position = int(float(position)) log_level = logging.DEBUG
if self._is_available is not False:
await self._update_playlists() log_level = logging.WARNING
_LOGGER.log(
@property log_level, "Error connecting to '%s': %s", self.server, error
def available(self): )
"""Return true if MPD is available and connected.""" self._is_available = False
return self._is_connected self._status = {}
# Also yield on failure. Handling mpd.ConnectionErrors caused by
# attempting to control a disconnected client is the
# responsibility of the caller.
yield
finally:
with suppress(mpd.ConnectionError):
self._client.disconnect()
async def async_update(self) -> None: async def async_update(self) -> None:
"""Get the latest data and update the state.""" """Get the latest data from MPD and update the state."""
try: async with self.connection():
if not self._is_connected: try:
await self._connect() self._status = await self._client.status()
self._commands = list(await self._client.commands()) self._currentsong = await self._client.currentsong()
await self._async_update_media_image_hash()
await self._fetch_status() if (position := self._status.get("elapsed")) is None:
except (mpd.ConnectionError, OSError, ValueError) as error: position = self._status.get("time")
# Cleanly disconnect in case connection is not in valid state
_LOGGER.debug("Error updating status: %s", error) if isinstance(position, str) and ":" in position:
self._disconnect() position = position.split(":")[0]
if position is not None and self._media_position != position:
self._media_position_updated_at = dt_util.utcnow()
self._media_position = int(float(position))
await self._update_playlists()
except (mpd.ConnectionError, ValueError) as error:
_LOGGER.debug("Error updating status: %s", error)
@property
def available(self) -> bool:
"""Return true if MPD is available and connected."""
return self._is_available is True
@property @property
def name(self): def name(self):
@ -174,13 +197,13 @@ class MpdDevice(MediaPlayerEntity):
@property @property
def state(self) -> MediaPlayerState: def state(self) -> MediaPlayerState:
"""Return the media state.""" """Return the media state."""
if self._status is None: if not self._status:
return MediaPlayerState.OFF return MediaPlayerState.OFF
if self._status["state"] == "play": if self._status.get("state") == "play":
return MediaPlayerState.PLAYING return MediaPlayerState.PLAYING
if self._status["state"] == "pause": if self._status.get("state") == "pause":
return MediaPlayerState.PAUSED return MediaPlayerState.PAUSED
if self._status["state"] == "stop": if self._status.get("state") == "stop":
return MediaPlayerState.OFF return MediaPlayerState.OFF
return MediaPlayerState.OFF return MediaPlayerState.OFF
@ -259,20 +282,26 @@ class MpdDevice(MediaPlayerEntity):
async def async_get_media_image(self) -> tuple[bytes | None, str | None]: async def async_get_media_image(self) -> tuple[bytes | None, str | None]:
"""Fetch media image of current playing track.""" """Fetch media image of current playing track."""
if not (file := self._currentsong.get("file")): async with self.connection():
return None, None if self._currentsong is None or not (file := self._currentsong.get("file")):
response = await self._async_get_file_image_response(file) return None, None
if response is None:
return None, None
image = bytes(response["binary"]) with suppress(mpd.ConnectionError):
mime = response.get( response = await self._async_get_file_image_response(file)
"type", "image/png" if response is None:
) # readpicture has type, albumart does not return None, None
return (image, mime)
image = bytes(response["binary"])
mime = response.get(
"type", "image/png"
) # readpicture has type, albumart does not
return (image, mime)
async def _async_update_media_image_hash(self): async def _async_update_media_image_hash(self):
"""Update the hash value for the media image.""" """Update the hash value for the media image."""
if self._currentsong is None:
return
file = self._currentsong.get("file") file = self._currentsong.get("file")
if file == self._media_image_file: if file == self._media_image_file:
@ -295,16 +324,21 @@ class MpdDevice(MediaPlayerEntity):
self._media_image_file = file self._media_image_file = file
async def _async_get_file_image_response(self, file): async def _async_get_file_image_response(self, file):
# not all MPD implementations and versions support the `albumart` and `fetchpicture` commands # not all MPD implementations and versions support the `albumart` and
can_albumart = "albumart" in self._commands # `fetchpicture` commands.
can_readpicture = "readpicture" in self._commands commands = []
with suppress(mpd.ConnectionError):
commands = list(await self._client.commands())
can_albumart = "albumart" in commands
can_readpicture = "readpicture" in commands
response = None response = None
# read artwork embedded into the media file # read artwork embedded into the media file
if can_readpicture: if can_readpicture:
try: try:
response = await self._client.readpicture(file) with suppress(mpd.ConnectionError):
response = await self._client.readpicture(file)
except mpd.CommandError as error: except mpd.CommandError as error:
if error.errno is not mpd.FailureResponseCode.NO_EXIST: if error.errno is not mpd.FailureResponseCode.NO_EXIST:
_LOGGER.warning( _LOGGER.warning(
@ -315,7 +349,8 @@ class MpdDevice(MediaPlayerEntity):
# read artwork contained in the media directory (cover.{jpg,png,tiff,bmp}) if none is embedded # read artwork contained in the media directory (cover.{jpg,png,tiff,bmp}) if none is embedded
if can_albumart and not response: if can_albumart and not response:
try: try:
response = await self._client.albumart(file) with suppress(mpd.ConnectionError):
response = await self._client.albumart(file)
except mpd.CommandError as error: except mpd.CommandError as error:
if error.errno is not mpd.FailureResponseCode.NO_EXIST: if error.errno is not mpd.FailureResponseCode.NO_EXIST:
_LOGGER.warning( _LOGGER.warning(
@ -339,7 +374,7 @@ class MpdDevice(MediaPlayerEntity):
@property @property
def supported_features(self) -> MediaPlayerEntityFeature: def supported_features(self) -> MediaPlayerEntityFeature:
"""Flag media player features that are supported.""" """Flag media player features that are supported."""
if self._status is None: if not self._status:
return MediaPlayerEntityFeature(0) return MediaPlayerEntityFeature(0)
supported = SUPPORT_MPD supported = SUPPORT_MPD
@ -373,55 +408,64 @@ class MpdDevice(MediaPlayerEntity):
"""Update available MPD playlists.""" """Update available MPD playlists."""
try: try:
self._playlists = [] self._playlists = []
for playlist_data in await self._client.listplaylists(): with suppress(mpd.ConnectionError):
self._playlists.append(playlist_data["playlist"]) for playlist_data in await self._client.listplaylists():
self._playlists.append(playlist_data["playlist"])
except mpd.CommandError as error: except mpd.CommandError as error:
self._playlists = None self._playlists = None
_LOGGER.warning("Playlists could not be updated: %s:", error) _LOGGER.warning("Playlists could not be updated: %s:", error)
async def async_set_volume_level(self, volume: float) -> None: async def async_set_volume_level(self, volume: float) -> None:
"""Set volume of media player.""" """Set volume of media player."""
if "volume" in self._status: async with self.connection():
await self._client.setvol(int(volume * 100)) if "volume" in self._status:
await self._client.setvol(int(volume * 100))
async def async_volume_up(self) -> None: async def async_volume_up(self) -> None:
"""Service to send the MPD the command for volume up.""" """Service to send the MPD the command for volume up."""
if "volume" in self._status: async with self.connection():
current_volume = int(self._status["volume"]) if "volume" in self._status:
current_volume = int(self._status["volume"])
if current_volume <= 100: if current_volume <= 100:
self._client.setvol(current_volume + 5) self._client.setvol(current_volume + 5)
async def async_volume_down(self) -> None: async def async_volume_down(self) -> None:
"""Service to send the MPD the command for volume down.""" """Service to send the MPD the command for volume down."""
if "volume" in self._status: async with self.connection():
current_volume = int(self._status["volume"]) if "volume" in self._status:
current_volume = int(self._status["volume"])
if current_volume >= 0: if current_volume >= 0:
await self._client.setvol(current_volume - 5) await self._client.setvol(current_volume - 5)
async def async_media_play(self) -> None: async def async_media_play(self) -> None:
"""Service to send the MPD the command for play/pause.""" """Service to send the MPD the command for play/pause."""
if self._status["state"] == "pause": async with self.connection():
await self._client.pause(0) if self._status.get("state") == "pause":
else: await self._client.pause(0)
await self._client.play() else:
await self._client.play()
async def async_media_pause(self) -> None: async def async_media_pause(self) -> None:
"""Service to send the MPD the command for play/pause.""" """Service to send the MPD the command for play/pause."""
await self._client.pause(1) async with self.connection():
await self._client.pause(1)
async def async_media_stop(self) -> None: async def async_media_stop(self) -> None:
"""Service to send the MPD the command for stop.""" """Service to send the MPD the command for stop."""
await self._client.stop() async with self.connection():
await self._client.stop()
async def async_media_next_track(self) -> None: async def async_media_next_track(self) -> None:
"""Service to send the MPD the command for next track.""" """Service to send the MPD the command for next track."""
await self._client.next() async with self.connection():
await self._client.next()
async def async_media_previous_track(self) -> None: async def async_media_previous_track(self) -> None:
"""Service to send the MPD the command for previous track.""" """Service to send the MPD the command for previous track."""
await self._client.previous() async with self.connection():
await self._client.previous()
async def async_mute_volume(self, mute: bool) -> None: async def async_mute_volume(self, mute: bool) -> None:
"""Mute. Emulated with set_volume_level.""" """Mute. Emulated with set_volume_level."""
@ -437,75 +481,82 @@ class MpdDevice(MediaPlayerEntity):
self, media_type: MediaType | str, media_id: str, **kwargs: Any self, media_type: MediaType | str, media_id: str, **kwargs: Any
) -> None: ) -> None:
"""Send the media player the command for playing a playlist.""" """Send the media player the command for playing a playlist."""
if media_source.is_media_source_id(media_id): async with self.connection():
media_type = MediaType.MUSIC if media_source.is_media_source_id(media_id):
play_item = await media_source.async_resolve_media( media_type = MediaType.MUSIC
self.hass, media_id, self.entity_id play_item = await media_source.async_resolve_media(
) self.hass, media_id, self.entity_id
media_id = async_process_play_media_url(self.hass, play_item.url) )
media_id = async_process_play_media_url(self.hass, play_item.url)
if media_type == MediaType.PLAYLIST: if media_type == MediaType.PLAYLIST:
_LOGGER.debug("Playing playlist: %s", media_id) _LOGGER.debug("Playing playlist: %s", media_id)
if media_id in self._playlists: if media_id in self._playlists:
self._currentplaylist = media_id self._currentplaylist = media_id
else:
self._currentplaylist = None
_LOGGER.warning("Unknown playlist name %s", media_id)
await self._client.clear()
await self._client.load(media_id)
await self._client.play()
else: else:
await self._client.clear()
self._currentplaylist = None self._currentplaylist = None
_LOGGER.warning("Unknown playlist name %s", media_id) await self._client.add(media_id)
await self._client.clear() await self._client.play()
await self._client.load(media_id)
await self._client.play()
else:
await self._client.clear()
self._currentplaylist = None
await self._client.add(media_id)
await self._client.play()
@property @property
def repeat(self) -> RepeatMode: def repeat(self) -> RepeatMode:
"""Return current repeat mode.""" """Return current repeat mode."""
if self._status["repeat"] == "1": if self._status.get("repeat") == "1":
if self._status["single"] == "1": if self._status.get("single") == "1":
return RepeatMode.ONE return RepeatMode.ONE
return RepeatMode.ALL return RepeatMode.ALL
return RepeatMode.OFF return RepeatMode.OFF
async def async_set_repeat(self, repeat: RepeatMode) -> None: async def async_set_repeat(self, repeat: RepeatMode) -> None:
"""Set repeat mode.""" """Set repeat mode."""
if repeat == RepeatMode.OFF: async with self.connection():
await self._client.repeat(0) if repeat == RepeatMode.OFF:
await self._client.single(0) await self._client.repeat(0)
else:
await self._client.repeat(1)
if repeat == RepeatMode.ONE:
await self._client.single(1)
else:
await self._client.single(0) await self._client.single(0)
else:
await self._client.repeat(1)
if repeat == RepeatMode.ONE:
await self._client.single(1)
else:
await self._client.single(0)
@property @property
def shuffle(self): def shuffle(self):
"""Boolean if shuffle is enabled.""" """Boolean if shuffle is enabled."""
return bool(int(self._status["random"])) return bool(int(self._status.get("random")))
async def async_set_shuffle(self, shuffle: bool) -> None: async def async_set_shuffle(self, shuffle: bool) -> None:
"""Enable/disable shuffle mode.""" """Enable/disable shuffle mode."""
await self._client.random(int(shuffle)) async with self.connection():
await self._client.random(int(shuffle))
async def async_turn_off(self) -> None: async def async_turn_off(self) -> None:
"""Service to send the MPD the command to stop playing.""" """Service to send the MPD the command to stop playing."""
await self._client.stop() async with self.connection():
await self._client.stop()
async def async_turn_on(self) -> None: async def async_turn_on(self) -> None:
"""Service to send the MPD the command to start playing.""" """Service to send the MPD the command to start playing."""
await self._client.play() async with self.connection():
await self._update_playlists(no_throttle=True) await self._client.play()
await self._update_playlists(no_throttle=True)
async def async_clear_playlist(self) -> None: async def async_clear_playlist(self) -> None:
"""Clear players playlist.""" """Clear players playlist."""
await self._client.clear() async with self.connection():
await self._client.clear()
async def async_media_seek(self, position: float) -> None: async def async_media_seek(self, position: float) -> None:
"""Send seek command.""" """Send seek command."""
await self._client.seekcur(position) async with self.connection():
await self._client.seekcur(position)
async def async_browse_media( async def async_browse_media(
self, self,
@ -513,8 +564,11 @@ class MpdDevice(MediaPlayerEntity):
media_content_id: str | None = None, media_content_id: str | None = None,
) -> BrowseMedia: ) -> BrowseMedia:
"""Implement the websocket media browsing helper.""" """Implement the websocket media browsing helper."""
return await media_source.async_browse_media( async with self.connection():
self.hass, return await media_source.async_browse_media(
media_content_id, self.hass,
content_filter=lambda item: item.media_content_type.startswith("audio/"), media_content_id,
) content_filter=lambda item: item.media_content_type.startswith(
"audio/"
),
)