Enable strict typing checking for bluesound integration (#123821)

* Enable strict typing

* Fix types

* Update to pyblu 0.5.2 for typing support

* Update pyblu to 1.0.0

* Update pyblu to 1.0.1

* Update error handling

* Fix tests

* Remove return None from methods only returning None

.strict-typing

@@ -110,6 +110,7 @@ homeassistant.components.bitcoin.*
 homeassistant.components.blockchain.*
 homeassistant.components.blue_current.*
 homeassistant.components.blueprint.*
+homeassistant.components.bluesound.*
 homeassistant.components.bluetooth.*
 homeassistant.components.bluetooth_adapters.*
 homeassistant.components.bluetooth_tracker.*

homeassistant/components/bluesound/__init__.py

@@ -2,8 +2,8 @@
 
 from dataclasses import dataclass
 
-import aiohttp
 from pyblu import Player, SyncStatus
+from pyblu.errors import PlayerUnreachableError
 
 from homeassistant.config_entries import ConfigEntry
 from homeassistant.const import CONF_HOST, CONF_PORT, Platform
@@ -22,14 +22,14 @@ PLATFORMS = [Platform.MEDIA_PLAYER]
 
 
 @dataclass
-class BluesoundData:
+class BluesoundRuntimeData:
     """Bluesound data class."""
 
     player: Player
     sync_status: SyncStatus
 
 
-type BluesoundConfigEntry = ConfigEntry[BluesoundData]
+type BluesoundConfigEntry = ConfigEntry[BluesoundRuntimeData]
 
 
 async def async_setup(hass: HomeAssistant, config: ConfigType) -> bool:
@@ -51,14 +51,10 @@ async def async_setup_entry(
     async with Player(host, port, session=session, default_timeout=10) as player:
         try:
             sync_status = await player.sync_status(timeout=1)
-        except TimeoutError as ex:
-            raise ConfigEntryNotReady(
-                f"Timeout while connecting to {host}:{port}"
-            ) from ex
-        except aiohttp.ClientError as ex:
+        except PlayerUnreachableError as ex:
             raise ConfigEntryNotReady(f"Error connecting to {host}:{port}") from ex
 
-    config_entry.runtime_data = BluesoundData(player, sync_status)
+    config_entry.runtime_data = BluesoundRuntimeData(player, sync_status)
 
     await hass.config_entries.async_forward_entry_setups(config_entry, PLATFORMS)
 

homeassistant/components/bluesound/config_flow.py

@@ -3,8 +3,8 @@
 import logging
 from typing import Any
 
-import aiohttp
 from pyblu import Player, SyncStatus
+from pyblu.errors import PlayerUnreachableError
 import voluptuous as vol
 
 from homeassistant.components import zeroconf
@@ -43,7 +43,7 @@ class BluesoundConfigFlow(ConfigFlow, domain=DOMAIN):
             ) as player:
                 try:
                     sync_status = await player.sync_status(timeout=1)
-                except (TimeoutError, aiohttp.ClientError):
+                except PlayerUnreachableError:
                     errors["base"] = "cannot_connect"
                 else:
                     await self.async_set_unique_id(
@@ -79,7 +79,7 @@ class BluesoundConfigFlow(ConfigFlow, domain=DOMAIN):
         ) as player:
             try:
                 sync_status = await player.sync_status(timeout=1)
-            except (TimeoutError, aiohttp.ClientError):
+            except PlayerUnreachableError:
                 return self.async_abort(reason="cannot_connect")
 
         await self.async_set_unique_id(
@@ -105,7 +105,7 @@ class BluesoundConfigFlow(ConfigFlow, domain=DOMAIN):
                 discovery_info.host, self._port, session=session
             ) as player:
                 sync_status = await player.sync_status(timeout=1)
-        except (TimeoutError, aiohttp.ClientError):
+        except PlayerUnreachableError:
             return self.async_abort(reason="cannot_connect")
 
         await self.async_set_unique_id(format_unique_id(sync_status.mac, self._port))
@@ -127,7 +127,9 @@ class BluesoundConfigFlow(ConfigFlow, domain=DOMAIN):
         )
         return await self.async_step_confirm()
 
-    async def async_step_confirm(self, user_input=None) -> ConfigFlowResult:
+    async def async_step_confirm(
+        self, user_input: dict[str, Any] | None = None
+    ) -> ConfigFlowResult:
         """Confirm the zeroconf setup."""
         assert self._sync_status is not None
         assert self._host is not None

homeassistant/components/bluesound/manifest.json

@@ -6,7 +6,7 @@
   "config_flow": true,
   "documentation": "https://www.home-assistant.io/integrations/bluesound",
   "iot_class": "local_polling",
-  "requirements": ["pyblu==0.4.0"],
+  "requirements": ["pyblu==1.0.1"],
   "zeroconf": [
     {
       "type": "_musc._tcp.local."

homeassistant/components/bluesound/media_player.py

@@ -9,8 +9,8 @@ from datetime import datetime, timedelta
 import logging
 from typing import TYPE_CHECKING, Any, NamedTuple
 
-from aiohttp.client_exceptions import ClientError
 from pyblu import Input, Player, Preset, Status, SyncStatus
+from pyblu.errors import PlayerUnreachableError
 import voluptuous as vol
 
 from homeassistant.components import media_source
@@ -239,7 +239,7 @@ class BluesoundPlayer(MediaPlayerEntity):
         self.port = port
         self._polling_task: Task[None] | None = None  # The actual polling task.
         self._id = sync_status.id
-        self._last_status_update = None
+        self._last_status_update: datetime | None = None
         self._sync_status = sync_status
         self._status: Status | None = None
         self._inputs: list[Input] = []
@@ -247,7 +247,7 @@ class BluesoundPlayer(MediaPlayerEntity):
         self._muted = False
         self._master: BluesoundPlayer | None = None
         self._is_master = False
-        self._group_name = None
+        self._group_name: str | None = None
         self._group_list: list[str] = []
         self._bluesound_device_name = sync_status.name
         self._player = player
@@ -273,14 +273,6 @@ class BluesoundPlayer(MediaPlayerEntity):
                 via_device=(DOMAIN, format_mac(sync_status.mac)),
             )
 
-    @staticmethod
-    def _try_get_index(string, search_string):
-        """Get the index."""
-        try:
-            return string.index(search_string)
-        except ValueError:
-            return -1
-
     async def force_update_sync_status(self) -> bool:
         """Update the internal status."""
         sync_status = await self._player.sync_status()
@@ -309,12 +301,12 @@ class BluesoundPlayer(MediaPlayerEntity):
 
         return True
 
-    async def _poll_loop(self):
+    async def _poll_loop(self) -> None:
         """Loop which polls the status of the player."""
         while True:
             try:
                 await self.async_update_status()
-            except (TimeoutError, ClientError):
+            except PlayerUnreachableError:
                 _LOGGER.error(
                     "Node %s:%s is offline, retrying later", self.host, self.port
                 )
@@ -324,9 +316,9 @@ class BluesoundPlayer(MediaPlayerEntity):
                     "Stopping the polling of node %s:%s", self.host, self.port
                 )
                 return
-            except Exception:
+            except:  # noqa: E722 - this loop should never stop
                 _LOGGER.exception(
-                    "Unexpected error in %s:%s, retrying later", self.host, self.port
+                    "Unexpected error for %s:%s, retrying later", self.host, self.port
                 )
                 await asyncio.sleep(NODE_OFFLINE_CHECK_TIMEOUT)
 
@@ -356,12 +348,12 @@ class BluesoundPlayer(MediaPlayerEntity):
         if not self.available:
             return
 
-        with suppress(TimeoutError):
+        with suppress(PlayerUnreachableError):
             await self.async_update_sync_status()
             await self.async_update_presets()
             await self.async_update_captures()
 
-    async def async_update_status(self):
+    async def async_update_status(self) -> None:
         """Use the poll session to always get the status of the player."""
         etag = None
         if self._status is not None:
@@ -394,11 +386,11 @@ class BluesoundPlayer(MediaPlayerEntity):
                 # the device is playing. This would solve a lot of
                 # problems. This change will be done when the
                 # communication is moved to a separate library
-                with suppress(TimeoutError):
+                with suppress(PlayerUnreachableError):
                     await self.force_update_sync_status()
 
             self.async_write_ha_state()
-        except (TimeoutError, ClientError):
+        except PlayerUnreachableError:
             self._attr_available = False
             self._last_status_update = None
             self._status = None
@@ -409,7 +401,7 @@ class BluesoundPlayer(MediaPlayerEntity):
             )
             raise
 
-    async def async_trigger_sync_on_all(self):
+    async def async_trigger_sync_on_all(self) -> None:
         """Trigger sync status update on all devices."""
         _LOGGER.debug("Trigger sync status on all devices")
 
@@ -417,7 +409,7 @@ class BluesoundPlayer(MediaPlayerEntity):
             await player.force_update_sync_status()
 
     @Throttle(SYNC_STATUS_INTERVAL)
-    async def async_update_sync_status(self):
+    async def async_update_sync_status(self) -> None:
         """Update sync status."""
         await self.force_update_sync_status()
 
@@ -506,8 +498,6 @@ class BluesoundPlayer(MediaPlayerEntity):
             return None
 
         position = self._status.seconds
-        if position is None:
-            return None
 
         if mediastate == MediaPlayerState.PLAYING:
             position += (dt_util.utcnow() - self._last_status_update).total_seconds()
@@ -524,7 +514,7 @@ class BluesoundPlayer(MediaPlayerEntity):
         if duration is None:
             return None
 
-        return duration
+        return int(duration)
 
     @property
     def media_position_updated_at(self) -> datetime | None:
@@ -660,7 +650,7 @@ class BluesoundPlayer(MediaPlayerEntity):
 
         return shuffle
 
-    async def async_join(self, master):
+    async def async_join(self, master: str) -> None:
         """Join the player to a group."""
         master_device = [
             device
@@ -711,7 +701,7 @@ class BluesoundPlayer(MediaPlayerEntity):
             if entity.bluesound_device_name in device_group
         ]
 
-    async def async_unjoin(self):
+    async def async_unjoin(self) -> None:
         """Unjoin the player from a group."""
         if self._master is None:
             return
@@ -719,11 +709,11 @@ class BluesoundPlayer(MediaPlayerEntity):
         _LOGGER.debug("Trying to unjoin player: %s", self.id)
         await self._master.async_remove_slave(self)
 
-    async def async_add_slave(self, slave_device: BluesoundPlayer):
+    async def async_add_slave(self, slave_device: BluesoundPlayer) -> None:
         """Add slave to master."""
         await self._player.add_slave(slave_device.host, slave_device.port)
 
-    async def async_remove_slave(self, slave_device: BluesoundPlayer):
+    async def async_remove_slave(self, slave_device: BluesoundPlayer) -> None:
         """Remove slave to master."""
         await self._player.remove_slave(slave_device.host, slave_device.port)
 
@@ -731,7 +721,7 @@ class BluesoundPlayer(MediaPlayerEntity):
         """Increase sleep time on player."""
         return await self._player.sleep_timer()
 
-    async def async_clear_timer(self):
+    async def async_clear_timer(self) -> None:
         """Clear sleep timer on player."""
         sleep = 1
         while sleep > 0:
@@ -755,6 +745,9 @@ class BluesoundPlayer(MediaPlayerEntity):
             if preset.name == source:
                 url = preset.url
 
+        if url is None:
+            raise ServiceValidationError(f"Source {source} not found")
+
         await self._player.play_url(url)
 
     async def async_clear_playlist(self) -> None:
@@ -826,20 +819,20 @@ class BluesoundPlayer(MediaPlayerEntity):
     async def async_volume_up(self) -> None:
         """Volume up the media player."""
         if self.volume_level is None:
-            return None
+            return
 
         new_volume = self.volume_level + 0.01
         new_volume = min(1, new_volume)
-        return await self.async_set_volume_level(new_volume)
+        await self.async_set_volume_level(new_volume)
 
     async def async_volume_down(self) -> None:
         """Volume down the media player."""
         if self.volume_level is None:
-            return None
+            return
 
         new_volume = self.volume_level - 0.01
         new_volume = max(0, new_volume)
-        return await self.async_set_volume_level(new_volume)
+        await self.async_set_volume_level(new_volume)
 
     async def async_set_volume_level(self, volume: float) -> None:
         """Send volume_up command to media player."""

mypy.ini

@@ -855,6 +855,16 @@ disallow_untyped_defs = true
 warn_return_any = true
 warn_unreachable = true
 
+[mypy-homeassistant.components.bluesound.*]
+check_untyped_defs = true
+disallow_incomplete_defs = true
+disallow_subclassing_any = true
+disallow_untyped_calls = true
+disallow_untyped_decorators = true
+disallow_untyped_defs = true
+warn_return_any = true
+warn_unreachable = true
+
 [mypy-homeassistant.components.bluetooth.*]
 check_untyped_defs = true
 disallow_incomplete_defs = true

requirements_all.txt

@@ -1763,7 +1763,7 @@ pybbox==0.0.5-alpha
 pyblackbird==0.6
 
 # homeassistant.components.bluesound
-pyblu==0.4.0
+pyblu==1.0.1
 
 # homeassistant.components.neato
 pybotvac==0.0.25

requirements_test_all.txt

@@ -1428,7 +1428,7 @@ pybalboa==1.0.2
 pyblackbird==0.6
 
 # homeassistant.components.bluesound
-pyblu==0.4.0
+pyblu==1.0.1
 
 # homeassistant.components.neato
 pybotvac==0.0.25

tests/components/bluesound/test_config_flow.py

@@ -2,7 +2,7 @@
 
 from unittest.mock import AsyncMock
 
-from aiohttp import ClientConnectionError
+from pyblu.errors import PlayerUnreachableError
 
 from homeassistant.components.bluesound.const import DOMAIN
 from homeassistant.components.zeroconf import ZeroconfServiceInfo
@@ -49,7 +49,7 @@ async def test_user_flow_cannot_connect(
         context={"source": SOURCE_USER},
     )
 
-    mock_player.sync_status.side_effect = ClientConnectionError
+    mock_player.sync_status.side_effect = PlayerUnreachableError("Player not reachable")
     result = await hass.config_entries.flow.async_configure(
         result["flow_id"],
         {
@@ -129,7 +129,7 @@ async def test_import_flow_cannot_connect(
     hass: HomeAssistant, mock_player: AsyncMock
 ) -> None:
     """Test we handle cannot connect error."""
-    mock_player.sync_status.side_effect = ClientConnectionError
+    mock_player.sync_status.side_effect = PlayerUnreachableError("Player not reachable")
     result = await hass.config_entries.flow.async_init(
         DOMAIN,
         context={"source": SOURCE_IMPORT},
@@ -200,7 +200,7 @@ async def test_zeroconf_flow_cannot_connect(
     hass: HomeAssistant, mock_player: AsyncMock
 ) -> None:
     """Test we handle cannot connect error."""
-    mock_player.sync_status.side_effect = ClientConnectionError
+    mock_player.sync_status.side_effect = PlayerUnreachableError("Player not reachable")
     result = await hass.config_entries.flow.async_init(
         DOMAIN,
         context={"source": SOURCE_ZEROCONF},