Refactor squeezebox integration media_player to use coordinator (#127695)

homeassistant/components/squeezebox/__init__.py

@@ -2,9 +2,10 @@
 
 from asyncio import timeout
 from dataclasses import dataclass
+from datetime import datetime
 import logging
 
-from pysqueezebox import Server
+from pysqueezebox import Player, Server
 
 from homeassistant.config_entries import ConfigEntry
 from homeassistant.const import (
@@ -23,20 +24,30 @@ from homeassistant.helpers.device_registry import (
     DeviceEntryType,
     format_mac,
 )
+from homeassistant.helpers.dispatcher import async_dispatcher_send
+from homeassistant.helpers.event import async_call_later
 
 from .const import (
     CONF_HTTPS,
+    DISCOVERY_INTERVAL,
     DISCOVERY_TASK,
     DOMAIN,
+    KNOWN_PLAYERS,
+    KNOWN_SERVERS,
     MANUFACTURER,
     SERVER_MODEL,
+    SIGNAL_PLAYER_DISCOVERED,
+    SIGNAL_PLAYER_REDISCOVERED,
     STATUS_API_TIMEOUT,
     STATUS_QUERY_LIBRARYNAME,
     STATUS_QUERY_MAC,
     STATUS_QUERY_UUID,
     STATUS_QUERY_VERSION,
 )
-from .coordinator import LMSStatusDataUpdateCoordinator
+from .coordinator import (
+    LMSStatusDataUpdateCoordinator,
+    SqueezeBoxPlayerUpdateCoordinator,
+)
 
 _LOGGER = logging.getLogger(__name__)
 
@@ -117,15 +128,55 @@ async def async_setup_entry(hass: HomeAssistant, entry: SqueezeboxConfigEntry) -
     )
     _LOGGER.debug("LMS Device %s", device)
 
-    coordinator = LMSStatusDataUpdateCoordinator(hass, lms)
+    server_coordinator = LMSStatusDataUpdateCoordinator(hass, lms)
 
     entry.runtime_data = SqueezeboxData(
-        coordinator=coordinator,
+        coordinator=server_coordinator,
         server=lms,
     )
 
-    await coordinator.async_config_entry_first_refresh()
+    # set up player discovery
+    known_servers = hass.data.setdefault(DOMAIN, {}).setdefault(KNOWN_SERVERS, {})
+    known_players = known_servers.setdefault(lms.uuid, {}).setdefault(KNOWN_PLAYERS, [])
+
+    async def _player_discovery(now: datetime | None = None) -> None:
+        """Discover squeezebox players by polling server."""
+
+        async def _discovered_player(player: Player) -> None:
+            """Handle a (re)discovered player."""
+            if player.player_id in known_players:
+                await player.async_update()
+                async_dispatcher_send(
+                    hass, SIGNAL_PLAYER_REDISCOVERED, player.player_id, player.connected
+                )
+            else:
+                _LOGGER.debug("Adding new entity: %s", player)
+                player_coordinator = SqueezeBoxPlayerUpdateCoordinator(
+                    hass, player, lms.uuid
+                )
+                known_players.append(player.player_id)
+                async_dispatcher_send(
+                    hass, SIGNAL_PLAYER_DISCOVERED, player_coordinator
+                )
+
+        if players := await lms.async_get_players():
+            for player in players:
+                hass.async_create_task(_discovered_player(player))
+
+        entry.async_on_unload(
+            async_call_later(hass, DISCOVERY_INTERVAL, _player_discovery)
+        )
+
+    await server_coordinator.async_config_entry_first_refresh()
     await hass.config_entries.async_forward_entry_setups(entry, PLATFORMS)
+
+    _LOGGER.debug(
+        "Adding player discovery job for LMS server: %s", entry.data[CONF_HOST]
+    )
+    entry.async_create_background_task(
+        hass, _player_discovery(), "squeezebox.media_player.player_discovery"
+    )
+
     return True
 
 

homeassistant/components/squeezebox/const.py

@@ -5,6 +5,7 @@ DISCOVERY_TASK = "discovery_task"
 DOMAIN = "squeezebox"
 DEFAULT_PORT = 9000
 KNOWN_PLAYERS = "known_players"
+KNOWN_SERVERS = "known_servers"
 MANUFACTURER = "https://lyrion.org/"
 PLAYER_DISCOVERY_UNSUB = "player_discovery_unsub"
 SENSOR_UPDATE_INTERVAL = 60
@@ -27,3 +28,7 @@ STATUS_QUERY_MAC = "mac"
 STATUS_QUERY_UUID = "uuid"
 STATUS_QUERY_VERSION = "version"
 SQUEEZEBOX_SOURCE_STRINGS = ("source:", "wavin:", "spotify:")
+SIGNAL_PLAYER_DISCOVERED = "squeezebox_player_discovered"
+SIGNAL_PLAYER_REDISCOVERED = "squeezebox_player_rediscovered"
+DISCOVERY_INTERVAL = 60
+PLAYER_UPDATE_INTERVAL = 5

homeassistant/components/squeezebox/coordinator.py

@@ -1,18 +1,23 @@
 """DataUpdateCoordinator for the Squeezebox integration."""
 
 from asyncio import timeout
+from collections.abc import Callable
 from datetime import timedelta
 import logging
 import re
+from typing import Any
 
-from pysqueezebox import Server
+from pysqueezebox import Player, Server
 
-from homeassistant.core import HomeAssistant
+from homeassistant.core import HomeAssistant, callback
+from homeassistant.helpers.dispatcher import async_dispatcher_connect
 from homeassistant.helpers.update_coordinator import DataUpdateCoordinator, UpdateFailed
 from homeassistant.util import dt as dt_util
 
 from .const import (
+    PLAYER_UPDATE_INTERVAL,
     SENSOR_UPDATE_INTERVAL,
+    SIGNAL_PLAYER_REDISCOVERED,
     STATUS_API_TIMEOUT,
     STATUS_SENSOR_LASTSCAN,
     STATUS_SENSOR_NEEDSRESTART,
@@ -38,7 +43,7 @@ class LMSStatusDataUpdateCoordinator(DataUpdateCoordinator):
         self.newversion_regex = re.compile("<.*$")
 
     async def _async_update_data(self) -> dict:
-        """Fetch data fromn LMS status call.
+        """Fetch data from LMS status call.
 
         Then we process only a subset to make then nice for HA
         """
@@ -70,3 +75,46 @@ class LMSStatusDataUpdateCoordinator(DataUpdateCoordinator):
 
         _LOGGER.debug("Processed serverstatus %s=%s", self.lms.name, data)
         return data
+
+
+class SqueezeBoxPlayerUpdateCoordinator(DataUpdateCoordinator[dict[str, Any]]):
+    """Coordinator for Squeezebox players."""
+
+    def __init__(self, hass: HomeAssistant, player: Player, server_uuid: str) -> None:
+        """Initialize the coordinator."""
+        super().__init__(
+            hass,
+            _LOGGER,
+            name=player.name,
+            update_interval=timedelta(seconds=PLAYER_UPDATE_INTERVAL),
+            always_update=True,
+        )
+        self.player = player
+        self.available = True
+        self._remove_dispatcher: Callable | None = None
+        self.server_uuid = server_uuid
+
+    async def _async_update_data(self) -> dict[str, Any]:
+        """Update Player if available, or listen for rediscovery if not."""
+        if self.available:
+            # Only update players available at last update, unavailable players are rediscovered instead
+            await self.player.async_update()
+
+            if self.player.connected is False:
+                _LOGGER.debug("Player %s is not available", self.name)
+                self.available = False
+
+                # start listening for restored players
+                self._remove_dispatcher = async_dispatcher_connect(
+                    self.hass, SIGNAL_PLAYER_REDISCOVERED, self.rediscovered
+                )
+        return {}
+
+    @callback
+    def rediscovered(self, unique_id: str, connected: bool) -> None:
+        """Make a player available again."""
+        if unique_id == self.player.player_id and connected:
+            self.available = True
+            _LOGGER.debug("Player %s is available again", self.name)
+            if self._remove_dispatcher:
+                self._remove_dispatcher()

homeassistant/components/squeezebox/media_player.py

@@ -6,9 +6,9 @@ from collections.abc import Callable
 from datetime import datetime
 import json
 import logging
-from typing import Any
+from typing import TYPE_CHECKING, Any
 
-from pysqueezebox import Player, Server, async_discover
+from pysqueezebox import Server, async_discover
 import voluptuous as vol
 
 from homeassistant.components import media_source
@@ -25,50 +25,53 @@ from homeassistant.components.media_player import (
     async_process_play_media_url,
 )
 from homeassistant.config_entries import SOURCE_INTEGRATION_DISCOVERY
-from homeassistant.const import ATTR_COMMAND, CONF_HOST, CONF_PORT
+from homeassistant.const import ATTR_COMMAND, CONF_HOST, CONF_PORT, Platform
 from homeassistant.core import HomeAssistant, callback
 from homeassistant.exceptions import ServiceValidationError
 from homeassistant.helpers import (
     config_validation as cv,
     discovery_flow,
     entity_platform,
+    entity_registry as er,
 )
 from homeassistant.helpers.device_registry import (
     CONNECTION_NETWORK_MAC,
     DeviceInfo,
     format_mac,
 )
-from homeassistant.helpers.dispatcher import (
+from homeassistant.helpers.dispatcher import async_dispatcher_connect
-    async_dispatcher_connect,
-    async_dispatcher_send,
-)
 from homeassistant.helpers.entity_platform import AddEntitiesCallback
-from homeassistant.helpers.event import async_call_later
 from homeassistant.helpers.start import async_at_start
+from homeassistant.helpers.update_coordinator import CoordinatorEntity
 from homeassistant.util.dt import utcnow
 
-from . import SqueezeboxConfigEntry
 from .browse_media import (
     build_item_response,
     generate_playlist,
     library_payload,
     media_source_content_filter,
 )
-from .const import DISCOVERY_TASK, DOMAIN, KNOWN_PLAYERS, SQUEEZEBOX_SOURCE_STRINGS
+from .const import (
+    DISCOVERY_TASK,
+    DOMAIN,
+    KNOWN_PLAYERS,
+    KNOWN_SERVERS,
+    SIGNAL_PLAYER_DISCOVERED,
+    SQUEEZEBOX_SOURCE_STRINGS,
+)
+from .coordinator import SqueezeBoxPlayerUpdateCoordinator
+
+if TYPE_CHECKING:
+    from . import SqueezeboxConfigEntry
 
 SERVICE_CALL_METHOD = "call_method"
 SERVICE_CALL_QUERY = "call_query"
 
 ATTR_QUERY_RESULT = "query_result"
 
-SIGNAL_PLAYER_REDISCOVERED = "squeezebox_player_rediscovered"
-
 _LOGGER = logging.getLogger(__name__)
 
-DISCOVERY_INTERVAL = 60
 
-
-KNOWN_SERVERS = "known_servers"
 ATTR_PARAMETERS = "parameters"
 ATTR_OTHER_PLAYER = "other_player"
 
@@ -112,49 +115,15 @@ async def async_setup_entry(
     entry: SqueezeboxConfigEntry,
     async_add_entities: AddEntitiesCallback,
 ) -> None:
-    """Set up an player discovery from a config entry."""
+    """Set up the Squeezebox media_player platform from a server config entry."""
-    hass.data.setdefault(DOMAIN, {})
-    known_players = hass.data[DOMAIN].setdefault(KNOWN_PLAYERS, [])
-    lms = entry.runtime_data.server
 
-    async def _player_discovery(now: datetime | None = None) -> None:
+    # Add media player entities when discovered
-        """Discover squeezebox players by polling server."""
+    async def _player_discovered(player: SqueezeBoxPlayerUpdateCoordinator) -> None:
-
+        _LOGGER.debug("Setting up media_player entity for player %s", player)
-        async def _discovered_player(player: Player) -> None:
+        async_add_entities([SqueezeBoxMediaPlayerEntity(player)])
-            """Handle a (re)discovered player."""
-            entity = next(
-                (
-                    known
-                    for known in known_players
-                    if known.unique_id == player.player_id
-                ),
-                None,
-            )
-            if entity:
-                await player.async_update()
-                async_dispatcher_send(
-                    hass, SIGNAL_PLAYER_REDISCOVERED, player.player_id, player.connected
-                )
-
-            if not entity:
-                _LOGGER.debug("Adding new entity: %s", player)
-                entity = SqueezeBoxEntity(player, lms)
-                known_players.append(entity)
-                async_add_entities([entity], True)
-
-        if players := await lms.async_get_players():
-            for player in players:
-                hass.async_create_task(_discovered_player(player))
 
     entry.async_on_unload(
-            async_call_later(hass, DISCOVERY_INTERVAL, _player_discovery)
+        async_dispatcher_connect(hass, SIGNAL_PLAYER_DISCOVERED, _player_discovered)
-        )
-
-    _LOGGER.debug(
-        "Adding player discovery job for LMS server: %s", entry.data[CONF_HOST]
-    )
-    entry.async_create_background_task(
-        hass, _player_discovery(), "squeezebox.media_player.player_discovery"
     )
 
     # Register entity services
@@ -184,8 +153,10 @@ async def async_setup_entry(
     entry.async_on_unload(async_at_start(hass, start_server_discovery))
 
 
-class SqueezeBoxEntity(MediaPlayerEntity):
+class SqueezeBoxMediaPlayerEntity(
-    """Representation of a SqueezeBox device.
+    CoordinatorEntity[SqueezeBoxPlayerUpdateCoordinator], MediaPlayerEntity
+):
+    """Representation of the media player features of a SqueezeBox device.
 
     Wraps a pysqueezebox.Player() object.
     """
@@ -212,13 +183,18 @@ class SqueezeBoxEntity(MediaPlayerEntity):
     _attr_has_entity_name = True
     _attr_name = None
     _last_update: datetime | None = None
-    _attr_available = True
 
-    def __init__(self, player: Player, server: Server) -> None:
+    def __init__(
+        self,
+        coordinator: SqueezeBoxPlayerUpdateCoordinator,
+    ) -> None:
         """Initialize the SqueezeBox device."""
+        super().__init__(coordinator)
+        player = coordinator.player
         self._player = player
         self._query_result: bool | dict = {}
         self._remove_dispatcher: Callable | None = None
+        self._previous_media_position = 0
         self._attr_unique_id = format_mac(player.player_id)
         _manufacturer = None
         if player.model == "SqueezeLite" or "SqueezePlay" in player.model:
@@ -234,11 +210,24 @@ class SqueezeBoxEntity(MediaPlayerEntity):
             identifiers={(DOMAIN, self._attr_unique_id)},
             name=player.name,
             connections={(CONNECTION_NETWORK_MAC, self._attr_unique_id)},
-            via_device=(DOMAIN, server.uuid),
+            via_device=(DOMAIN, coordinator.server_uuid),
             model=player.model,
             manufacturer=_manufacturer,
         )
 
+    @callback
+    def _handle_coordinator_update(self) -> None:
+        """Handle updated data from the coordinator."""
+        if self._previous_media_position != self.media_position:
+            self._previous_media_position = self.media_position
+            self._last_update = utcnow()
+        self.async_write_ha_state()
+
+    @property
+    def available(self) -> bool:
+        """Return True if entity is available."""
+        return self.coordinator.available and super().available
+
     @property
     def extra_state_attributes(self) -> dict[str, Any]:
         """Return device-specific attributes."""
@@ -248,15 +237,6 @@ class SqueezeBoxEntity(MediaPlayerEntity):
             if getattr(self, attr) is not None
         }
 
-    @callback
-    def rediscovered(self, unique_id: str, connected: bool) -> None:
-        """Make a player available again."""
-        if unique_id == self.unique_id and connected:
-            self._attr_available = True
-            _LOGGER.debug("Player %s is available again", self.name)
-            if self._remove_dispatcher:
-                self._remove_dispatcher()
-
     @property
     def state(self) -> MediaPlayerState | None:
         """Return the state of the device."""
@@ -269,26 +249,11 @@ class SqueezeBoxEntity(MediaPlayerEntity):
         )
         return None
 
-    async def async_update(self) -> None:
-        """Update the Player() object."""
-        # only update available players, newly available players will be rediscovered and marked available
-        if self._attr_available:
-            last_media_position = self.media_position
-            await self._player.async_update()
-            if self.media_position != last_media_position:
-                self._last_update = utcnow()
-            if self._player.connected is False:
-                _LOGGER.debug("Player %s is not available", self.name)
-                self._attr_available = False
-
-                # start listening for restored players
-                self._remove_dispatcher = async_dispatcher_connect(
-                    self.hass, SIGNAL_PLAYER_REDISCOVERED, self.rediscovered
-                )
-
     async def async_will_remove_from_hass(self) -> None:
         """Remove from list of known players when removed from hass."""
-        self.hass.data[DOMAIN][KNOWN_PLAYERS].remove(self)
+        known_servers = self.hass.data[DOMAIN][KNOWN_SERVERS]
+        known_players = known_servers[self.coordinator.server_uuid][KNOWN_PLAYERS]
+        known_players.remove(self.coordinator.player.player_id)
 
     @property
     def volume_level(self) -> float | None:
@@ -380,13 +345,15 @@ class SqueezeBoxEntity(MediaPlayerEntity):
     @property
     def group_members(self) -> list[str]:
         """List players we are synced with."""
-        player_ids = {
+        ent_reg = er.async_get(self.hass)
-            p.unique_id: p.entity_id for p in self.hass.data[DOMAIN][KNOWN_PLAYERS]
-        }
         return [
-            player_ids[player]
+            entity_id
             for player in self._player.sync_group
-            if player in player_ids
+            if (
+                entity_id := ent_reg.async_get_entity_id(
+                    Platform.MEDIA_PLAYER, DOMAIN, player
+                )
+            )
         ]
 
     @property
@@ -397,55 +364,68 @@ class SqueezeBoxEntity(MediaPlayerEntity):
     async def async_turn_off(self) -> None:
         """Turn off media player."""
         await self._player.async_set_power(False)
+        await self.coordinator.async_refresh()
 
     async def async_volume_up(self) -> None:
         """Volume up media player."""
         await self._player.async_set_volume("+5")
+        await self.coordinator.async_refresh()
 
     async def async_volume_down(self) -> None:
         """Volume down media player."""
         await self._player.async_set_volume("-5")
+        await self.coordinator.async_refresh()
 
     async def async_set_volume_level(self, volume: float) -> None:
         """Set volume level, range 0..1."""
         volume_percent = str(int(volume * 100))
         await self._player.async_set_volume(volume_percent)
+        await self.coordinator.async_refresh()
 
     async def async_mute_volume(self, mute: bool) -> None:
         """Mute (true) or unmute (false) media player."""
         await self._player.async_set_muting(mute)
+        await self.coordinator.async_refresh()
 
     async def async_media_stop(self) -> None:
         """Send stop command to media player."""
         await self._player.async_stop()
+        await self.coordinator.async_refresh()
 
     async def async_media_play_pause(self) -> None:
         """Send pause command to media player."""
         await self._player.async_toggle_pause()
+        await self.coordinator.async_refresh()
 
     async def async_media_play(self) -> None:
         """Send play command to media player."""
         await self._player.async_play()
+        await self.coordinator.async_refresh()
 
     async def async_media_pause(self) -> None:
         """Send pause command to media player."""
         await self._player.async_pause()
+        await self.coordinator.async_refresh()
 
     async def async_media_next_track(self) -> None:
         """Send next track command."""
         await self._player.async_index("+1")
+        await self.coordinator.async_refresh()
 
     async def async_media_previous_track(self) -> None:
         """Send next track command."""
         await self._player.async_index("-1")
+        await self.coordinator.async_refresh()
 
     async def async_media_seek(self, position: float) -> None:
         """Send seek command."""
         await self._player.async_time(position)
+        await self.coordinator.async_refresh()
 
     async def async_turn_on(self) -> None:
         """Turn the media player on."""
         await self._player.async_set_power(True)
+        await self.coordinator.async_refresh()
 
     async def async_play_media(
         self, media_type: MediaType | str, media_id: str, **kwargs: Any
@@ -504,6 +484,7 @@ class SqueezeBoxEntity(MediaPlayerEntity):
         await self._player.async_load_playlist(playlist, cmd)
         if index is not None:
             await self._player.async_index(index)
+        await self.coordinator.async_refresh()
 
     async def async_set_repeat(self, repeat: RepeatMode) -> None:
         """Set the repeat mode."""
@@ -515,15 +496,18 @@ class SqueezeBoxEntity(MediaPlayerEntity):
             repeat_mode = "none"
 
         await self._player.async_set_repeat(repeat_mode)
+        await self.coordinator.async_refresh()
 
     async def async_set_shuffle(self, shuffle: bool) -> None:
         """Enable/disable shuffle mode."""
         shuffle_mode = "song" if shuffle else "none"
         await self._player.async_set_shuffle(shuffle_mode)
+        await self.coordinator.async_refresh()
 
     async def async_clear_playlist(self) -> None:
         """Send the media player the command for clear playlist."""
         await self._player.async_clear_playlist()
+        await self.coordinator.async_refresh()
 
     async def async_call_method(
         self, command: str, parameters: list[str] | None = None
@@ -558,21 +542,24 @@ class SqueezeBoxEntity(MediaPlayerEntity):
         If the other player is a member of a sync group, it will leave the current sync group
         without asking.
         """
-        player_ids = {
+        ent_reg = er.async_get(self.hass)
-            p.entity_id: p.unique_id for p in self.hass.data[DOMAIN][KNOWN_PLAYERS]
+        for other_player_entity_id in group_members:
-        }
+            other_player = ent_reg.async_get(other_player_entity_id)
-
+            if other_player is None:
-        for other_player in group_members:
+                raise ServiceValidationError(
-            if other_player_id := player_ids.get(other_player):
+                    f"Could not find player with entity_id {other_player_entity_id}"
+                )
+            if other_player_id := other_player.unique_id:
                 await self._player.async_sync(other_player_id)
             else:
                 raise ServiceValidationError(
-                    f"Could not join unknown player {other_player}"
+                    f"Could not join unknown player {other_player_entity_id}"
                 )
 
     async def async_unjoin_player(self) -> None:
         """Unsync this Squeezebox player."""
         await self._player.async_unsync()
+        await self.coordinator.async_refresh()
 
     async def async_browse_media(
         self,

tests/components/squeezebox/conftest.py

@@ -207,7 +207,7 @@ def player_factory() -> MagicMock:
 def mock_pysqueezebox_player(uuid: str) -> MagicMock:
     """Mock a Lyrion Media Server player."""
     with patch(
-        "homeassistant.components.squeezebox.media_player.Player", autospec=True
+        "homeassistant.components.squeezebox.Player", autospec=True
     ) as mock_player:
         mock_player.async_browse = AsyncMock(side_effect=mock_async_browse)
         mock_player.generate_image_url_from_track_id = MagicMock(

tests/components/squeezebox/test_media_player.py

@@ -30,10 +30,14 @@ from homeassistant.components.media_player import (
     MediaType,
     RepeatMode,
 )
-from homeassistant.components.squeezebox.const import DOMAIN, SENSOR_UPDATE_INTERVAL
+from homeassistant.components.squeezebox.const import (
+    DISCOVERY_INTERVAL,
+    DOMAIN,
+    PLAYER_UPDATE_INTERVAL,
+    SENSOR_UPDATE_INTERVAL,
+)
 from homeassistant.components.squeezebox.media_player import (
     ATTR_PARAMETERS,
-    DISCOVERY_INTERVAL,
     SERVICE_CALL_METHOD,
     SERVICE_CALL_QUERY,
 )
@@ -101,12 +105,9 @@ async def test_squeezebox_player_rediscovery(
 
     # Make the player appear unavailable
     configured_player.connected = False
-    await hass.services.async_call(
+    freezer.tick(timedelta(seconds=PLAYER_UPDATE_INTERVAL))
-        MEDIA_PLAYER_DOMAIN,
+    async_fire_time_changed(hass)
-        SERVICE_TURN_ON,
+    await hass.async_block_till_done()
-        {ATTR_ENTITY_ID: "media_player.test_player"},
-        blocking=True,
-    )
     assert hass.states.get("media_player.test_player").state == STATE_UNAVAILABLE
 
     # Make the player available again
@@ -115,7 +116,7 @@ async def test_squeezebox_player_rediscovery(
     async_fire_time_changed(hass)
     await hass.async_block_till_done()
 
-    freezer.tick(timedelta(seconds=SENSOR_UPDATE_INTERVAL))
+    freezer.tick(timedelta(seconds=PLAYER_UPDATE_INTERVAL))
     async_fire_time_changed(hass)
     await hass.async_block_till_done()
     assert hass.states.get("media_player.test_player").state == MediaPlayerState.IDLE