From 30994710e6a2001ff16d8355d8e3e67b0126fabd Mon Sep 17 00:00:00 2001 From: Louis Christ Date: Tue, 13 Aug 2024 12:55:01 +0200 Subject: [PATCH] Fix status update loop in bluesound integration (#123790) * Fix retry loop for status update * Use 'available' instead of _is_online * Fix tests --- .../components/bluesound/media_player.py | 37 ++++++++++--------- tests/components/bluesound/conftest.py | 35 ++++++++++++++++-- .../components/bluesound/test_config_flow.py | 4 +- 3 files changed, 55 insertions(+), 21 deletions(-) diff --git a/homeassistant/components/bluesound/media_player.py b/homeassistant/components/bluesound/media_player.py index c1b662fcddc..92f47977ee5 100644 --- a/homeassistant/components/bluesound/media_player.py +++ b/homeassistant/components/bluesound/media_player.py @@ -244,7 +244,6 @@ class BluesoundPlayer(MediaPlayerEntity): self._status: Status | None = None self._inputs: list[Input] = [] self._presets: list[Preset] = [] - self._is_online = False self._muted = False self._master: BluesoundPlayer | None = None self._is_master = False @@ -312,20 +311,24 @@ class BluesoundPlayer(MediaPlayerEntity): async def _start_poll_command(self): """Loop which polls the status of the player.""" - try: - while True: + while True: + try: await self.async_update_status() - - except (TimeoutError, ClientError): - _LOGGER.error("Node %s:%s is offline, retrying later", self.host, self.port) - await asyncio.sleep(NODE_OFFLINE_CHECK_TIMEOUT) - self.start_polling() - - except CancelledError: - _LOGGER.debug("Stopping the polling of node %s:%s", self.host, self.port) - except Exception: - _LOGGER.exception("Unexpected error in %s:%s", self.host, self.port) - raise + except (TimeoutError, ClientError): + _LOGGER.error( + "Node %s:%s is offline, retrying later", self.host, self.port + ) + await asyncio.sleep(NODE_OFFLINE_CHECK_TIMEOUT) + except CancelledError: + _LOGGER.debug( + "Stopping the polling of node %s:%s", self.host, self.port + ) + return + except Exception: + _LOGGER.exception( + "Unexpected error in %s:%s, retrying later", self.host, self.port + ) + await asyncio.sleep(NODE_OFFLINE_CHECK_TIMEOUT) async def async_added_to_hass(self) -> None: """Start the polling task.""" @@ -348,7 +351,7 @@ class BluesoundPlayer(MediaPlayerEntity): async def async_update(self) -> None: """Update internal status of the entity.""" - if not self._is_online: + if not self.available: return with suppress(TimeoutError): @@ -365,7 +368,7 @@ class BluesoundPlayer(MediaPlayerEntity): try: status = await self._player.status(etag=etag, poll_timeout=120, timeout=125) - self._is_online = True + self._attr_available = True self._last_status_update = dt_util.utcnow() self._status = status @@ -394,7 +397,7 @@ class BluesoundPlayer(MediaPlayerEntity): self.async_write_ha_state() except (TimeoutError, ClientError): - self._is_online = False + self._attr_available = False self._last_status_update = None self._status = None self.async_write_ha_state() diff --git a/tests/components/bluesound/conftest.py b/tests/components/bluesound/conftest.py index 5d81b6863c6..155d6b66e4e 100644 --- a/tests/components/bluesound/conftest.py +++ b/tests/components/bluesound/conftest.py @@ -3,7 +3,7 @@ from collections.abc import Generator from unittest.mock import AsyncMock, patch -from pyblu import SyncStatus +from pyblu import Status, SyncStatus import pytest from homeassistant.components.bluesound.const import DOMAIN @@ -39,6 +39,35 @@ def sync_status() -> SyncStatus: ) +@pytest.fixture +def status() -> Status: + """Return a status object.""" + return Status( + etag="etag", + input_id=None, + service=None, + state="playing", + shuffle=False, + album=None, + artist=None, + name=None, + image=None, + volume=10, + volume_db=22.3, + mute=False, + mute_volume=None, + mute_volume_db=None, + seconds=2, + total_seconds=123.1, + can_seek=False, + sleep=0, + group_name=None, + group_volume=None, + indexing=False, + stream_url=None, + ) + + @pytest.fixture def mock_setup_entry() -> Generator[AsyncMock]: """Override async_setup_entry.""" @@ -65,7 +94,7 @@ def mock_config_entry(hass: HomeAssistant) -> MockConfigEntry: @pytest.fixture -def mock_player() -> Generator[AsyncMock]: +def mock_player(status: Status) -> Generator[AsyncMock]: """Mock the player.""" with ( patch( @@ -78,7 +107,7 @@ def mock_player() -> Generator[AsyncMock]: ): player = mock_player.return_value player.__aenter__.return_value = player - player.status.return_value = None + player.status.return_value = status player.sync_status.return_value = SyncStatus( etag="etag", id="1.1.1.1:11000", diff --git a/tests/components/bluesound/test_config_flow.py b/tests/components/bluesound/test_config_flow.py index 32f36fcea58..8fecba7017d 100644 --- a/tests/components/bluesound/test_config_flow.py +++ b/tests/components/bluesound/test_config_flow.py @@ -41,7 +41,7 @@ async def test_user_flow_success( async def test_user_flow_cannot_connect( - hass: HomeAssistant, mock_player: AsyncMock + hass: HomeAssistant, mock_player: AsyncMock, mock_setup_entry: AsyncMock ) -> None: """Test we handle cannot connect error.""" result = await hass.config_entries.flow.async_init( @@ -76,6 +76,8 @@ async def test_user_flow_cannot_connect( CONF_PORT: 11000, } + mock_setup_entry.assert_called_once() + async def test_user_flow_aleady_configured( hass: HomeAssistant,