From 009989d7ae1b9efda191858d980261e043e836af Mon Sep 17 00:00:00 2001 From: Philip Vanloo <26272906+dukeofphilberg@users.noreply.github.com> Date: Tue, 3 Sep 2024 13:34:47 +0200 Subject: [PATCH] Add Linkplay mTLS/HTTPS and improve logging (#124307) * Work * Implement 0.0.8 changes, fixup tests * Cleanup * Implement new playmodes, close clientsession upon ha close * Implement new playmodes, close clientsession upon ha close * Add test for zeroconf bridge failure * Bump 0.0.9 Address old comments in 113940 * Exact _async_register_default_clientsession_shutdown --- homeassistant/components/linkplay/__init__.py | 24 ++++++---- .../components/linkplay/config_flow.py | 40 ++++++++++++---- homeassistant/components/linkplay/const.py | 1 + .../components/linkplay/manifest.json | 2 +- .../components/linkplay/media_player.py | 11 +++++ homeassistant/components/linkplay/utils.py | 27 +++++++++++ requirements_all.txt | 2 +- requirements_test_all.txt | 2 +- tests/components/linkplay/conftest.py | 9 +++- tests/components/linkplay/test_config_flow.py | 48 +++++++++++++------ 10 files changed, 128 insertions(+), 38 deletions(-) diff --git a/homeassistant/components/linkplay/__init__.py b/homeassistant/components/linkplay/__init__.py index c0fe711a61b2f..808f2f93ce2a7 100644 --- a/homeassistant/components/linkplay/__init__.py +++ b/homeassistant/components/linkplay/__init__.py @@ -1,17 +1,22 @@ """Support for LinkPlay devices.""" +from dataclasses import dataclass + +from aiohttp import ClientSession from linkplay.bridge import LinkPlayBridge -from linkplay.discovery import linkplay_factory_bridge +from linkplay.discovery import linkplay_factory_httpapi_bridge +from linkplay.exceptions import LinkPlayRequestException from homeassistant.config_entries import ConfigEntry from homeassistant.const import CONF_HOST from homeassistant.core import HomeAssistant from homeassistant.exceptions import ConfigEntryNotReady -from homeassistant.helpers.aiohttp_client import async_get_clientsession from .const import PLATFORMS +from .utils import async_get_client_session +@dataclass class LinkPlayData: """Data for LinkPlay.""" @@ -24,16 +29,17 @@ class LinkPlayData: async def async_setup_entry(hass: HomeAssistant, entry: LinkPlayConfigEntry) -> bool: """Async setup hass config entry. Called when an entry has been setup.""" - session = async_get_clientsession(hass) - if ( - bridge := await linkplay_factory_bridge(entry.data[CONF_HOST], session) - ) is None: + session: ClientSession = await async_get_client_session(hass) + bridge: LinkPlayBridge | None = None + + try: + bridge = await linkplay_factory_httpapi_bridge(entry.data[CONF_HOST], session) + except LinkPlayRequestException as exception: raise ConfigEntryNotReady( f"Failed to connect to LinkPlay device at {entry.data[CONF_HOST]}" - ) + ) from exception - entry.runtime_data = LinkPlayData() - entry.runtime_data.bridge = bridge + entry.runtime_data = LinkPlayData(bridge=bridge) await hass.config_entries.async_forward_entry_setups(entry, PLATFORMS) return True diff --git a/homeassistant/components/linkplay/config_flow.py b/homeassistant/components/linkplay/config_flow.py index 0f9c40d0fd4be..7dfdce238ff20 100644 --- a/homeassistant/components/linkplay/config_flow.py +++ b/homeassistant/components/linkplay/config_flow.py @@ -1,16 +1,22 @@ """Config flow to configure LinkPlay component.""" +import logging from typing import Any -from linkplay.discovery import linkplay_factory_bridge +from aiohttp import ClientSession +from linkplay.bridge import LinkPlayBridge +from linkplay.discovery import linkplay_factory_httpapi_bridge +from linkplay.exceptions import LinkPlayRequestException import voluptuous as vol from homeassistant.components import zeroconf from homeassistant.config_entries import ConfigFlow, ConfigFlowResult from homeassistant.const import CONF_HOST, CONF_MODEL -from homeassistant.helpers.aiohttp_client import async_get_clientsession from .const import DOMAIN +from .utils import async_get_client_session + +_LOGGER = logging.getLogger(__name__) class LinkPlayConfigFlow(ConfigFlow, domain=DOMAIN): @@ -25,10 +31,15 @@ async def async_step_zeroconf( ) -> ConfigFlowResult: """Handle Zeroconf discovery.""" - session = async_get_clientsession(self.hass) - bridge = await linkplay_factory_bridge(discovery_info.host, session) + session: ClientSession = await async_get_client_session(self.hass) + bridge: LinkPlayBridge | None = None - if bridge is None: + try: + bridge = await linkplay_factory_httpapi_bridge(discovery_info.host, session) + except LinkPlayRequestException: + _LOGGER.exception( + "Failed to connect to LinkPlay device at %s", discovery_info.host + ) return self.async_abort(reason="cannot_connect") self.data[CONF_HOST] = discovery_info.host @@ -66,14 +77,26 @@ async def async_step_user( """Handle a flow initialized by the user.""" errors: dict[str, str] = {} if user_input: - session = async_get_clientsession(self.hass) - bridge = await linkplay_factory_bridge(user_input[CONF_HOST], session) + session: ClientSession = await async_get_client_session(self.hass) + bridge: LinkPlayBridge | None = None + + try: + bridge = await linkplay_factory_httpapi_bridge( + user_input[CONF_HOST], session + ) + except LinkPlayRequestException: + _LOGGER.exception( + "Failed to connect to LinkPlay device at %s", user_input[CONF_HOST] + ) + errors["base"] = "cannot_connect" if bridge is not None: self.data[CONF_HOST] = user_input[CONF_HOST] self.data[CONF_MODEL] = bridge.device.name - await self.async_set_unique_id(bridge.device.uuid) + await self.async_set_unique_id( + bridge.device.uuid, raise_on_progress=False + ) self._abort_if_unique_id_configured( updates={CONF_HOST: self.data[CONF_HOST]} ) @@ -83,7 +106,6 @@ async def async_step_user( data={CONF_HOST: self.data[CONF_HOST]}, ) - errors["base"] = "cannot_connect" return self.async_show_form( step_id="user", data_schema=vol.Schema({vol.Required(CONF_HOST): str}), diff --git a/homeassistant/components/linkplay/const.py b/homeassistant/components/linkplay/const.py index 48ae225dd98e2..91a427d5eb815 100644 --- a/homeassistant/components/linkplay/const.py +++ b/homeassistant/components/linkplay/const.py @@ -4,3 +4,4 @@ DOMAIN = "linkplay" PLATFORMS = [Platform.MEDIA_PLAYER] +CONF_SESSION = "session" diff --git a/homeassistant/components/linkplay/manifest.json b/homeassistant/components/linkplay/manifest.json index 5212f3f99b8e5..66a719c640ea3 100644 --- a/homeassistant/components/linkplay/manifest.json +++ b/homeassistant/components/linkplay/manifest.json @@ -6,6 +6,6 @@ "documentation": "https://www.home-assistant.io/integrations/linkplay", "integration_type": "hub", "iot_class": "local_polling", - "requirements": ["python-linkplay==0.0.8"], + "requirements": ["python-linkplay==0.0.9"], "zeroconf": ["_linkplay._tcp.local."] } diff --git a/homeassistant/components/linkplay/media_player.py b/homeassistant/components/linkplay/media_player.py index 398add235bd78..0b62b4dbceedb 100644 --- a/homeassistant/components/linkplay/media_player.py +++ b/homeassistant/components/linkplay/media_player.py @@ -48,6 +48,17 @@ PlayingMode.XLR: "XLR", PlayingMode.HDMI: "HDMI", PlayingMode.OPTICAL_2: "Optical 2", + PlayingMode.EXTERN_BLUETOOTH: "External Bluetooth", + PlayingMode.PHONO: "Phono", + PlayingMode.ARC: "ARC", + PlayingMode.COAXIAL_2: "Coaxial 2", + PlayingMode.TF_CARD_1: "SD Card 1", + PlayingMode.TF_CARD_2: "SD Card 2", + PlayingMode.CD: "CD", + PlayingMode.DAB: "DAB Radio", + PlayingMode.FM: "FM Radio", + PlayingMode.RCA: "RCA", + PlayingMode.UDISK: "USB", } SOURCE_MAP_INV: dict[str, PlayingMode] = {v: k for k, v in SOURCE_MAP.items()} diff --git a/homeassistant/components/linkplay/utils.py b/homeassistant/components/linkplay/utils.py index 7532c9b354a77..7f15e2971456c 100644 --- a/homeassistant/components/linkplay/utils.py +++ b/homeassistant/components/linkplay/utils.py @@ -2,6 +2,14 @@ from typing import Final +from aiohttp import ClientSession +from linkplay.utils import async_create_unverified_client_session + +from homeassistant.const import EVENT_HOMEASSISTANT_CLOSE +from homeassistant.core import Event, HomeAssistant, callback + +from .const import CONF_SESSION, DOMAIN + MANUFACTURER_ARTSOUND: Final[str] = "ArtSound" MANUFACTURER_ARYLIC: Final[str] = "Arylic" MANUFACTURER_IEAST: Final[str] = "iEAST" @@ -44,3 +52,22 @@ def get_info_from_project(project: str) -> tuple[str, str]: return MANUFACTURER_IEAST, MODELS_IEAST_AUDIOCAST_M5 case _: return MANUFACTURER_GENERIC, MODELS_GENERIC + + +async def async_get_client_session(hass: HomeAssistant) -> ClientSession: + """Get a ClientSession that can be used with LinkPlay devices.""" + hass.data.setdefault(DOMAIN, {}) + if CONF_SESSION not in hass.data[DOMAIN]: + clientsession: ClientSession = await async_create_unverified_client_session() + + @callback + def _async_close_websession(event: Event) -> None: + """Close websession.""" + clientsession.detach() + + hass.bus.async_listen_once(EVENT_HOMEASSISTANT_CLOSE, _async_close_websession) + hass.data[DOMAIN][CONF_SESSION] = clientsession + return clientsession + + session: ClientSession = hass.data[DOMAIN][CONF_SESSION] + return session diff --git a/requirements_all.txt b/requirements_all.txt index b42d34a761e0d..179e0512d0161 100644 --- a/requirements_all.txt +++ b/requirements_all.txt @@ -2316,7 +2316,7 @@ python-juicenet==1.1.0 python-kasa[speedups]==0.7.2 # homeassistant.components.linkplay -python-linkplay==0.0.8 +python-linkplay==0.0.9 # homeassistant.components.lirc # python-lirc==1.2.3 diff --git a/requirements_test_all.txt b/requirements_test_all.txt index de793acb135fb..e5bc24881dc23 100644 --- a/requirements_test_all.txt +++ b/requirements_test_all.txt @@ -1834,7 +1834,7 @@ python-juicenet==1.1.0 python-kasa[speedups]==0.7.2 # homeassistant.components.linkplay -python-linkplay==0.0.8 +python-linkplay==0.0.9 # homeassistant.components.matter python-matter-server==6.3.0 diff --git a/tests/components/linkplay/conftest.py b/tests/components/linkplay/conftest.py index b3d65422e08d3..be83dd2412d32 100644 --- a/tests/components/linkplay/conftest.py +++ b/tests/components/linkplay/conftest.py @@ -3,6 +3,7 @@ from collections.abc import Generator from unittest.mock import AsyncMock, patch +from aiohttp import ClientSession from linkplay.bridge import LinkPlayBridge, LinkPlayDevice import pytest @@ -14,11 +15,15 @@ @pytest.fixture def mock_linkplay_factory_bridge() -> Generator[AsyncMock]: - """Mock for linkplay_factory_bridge.""" + """Mock for linkplay_factory_httpapi_bridge.""" with ( patch( - "homeassistant.components.linkplay.config_flow.linkplay_factory_bridge" + "homeassistant.components.linkplay.config_flow.async_get_client_session", + return_value=AsyncMock(spec=ClientSession), + ), + patch( + "homeassistant.components.linkplay.config_flow.linkplay_factory_httpapi_bridge", ) as factory, ): bridge = AsyncMock(spec=LinkPlayBridge) diff --git a/tests/components/linkplay/test_config_flow.py b/tests/components/linkplay/test_config_flow.py index 641f09893c206..3fd1fbea95ebb 100644 --- a/tests/components/linkplay/test_config_flow.py +++ b/tests/components/linkplay/test_config_flow.py @@ -3,6 +3,9 @@ from ipaddress import ip_address from unittest.mock import AsyncMock +from linkplay.exceptions import LinkPlayRequestException +import pytest + from homeassistant.components.linkplay.const import DOMAIN from homeassistant.components.zeroconf import ZeroconfServiceInfo from homeassistant.config_entries import SOURCE_USER, SOURCE_ZEROCONF @@ -47,10 +50,9 @@ ) +@pytest.mark.usefixtures("mock_linkplay_factory_bridge", "mock_setup_entry") async def test_user_flow( hass: HomeAssistant, - mock_linkplay_factory_bridge: AsyncMock, - mock_setup_entry: AsyncMock, ) -> None: """Test user setup config flow.""" result = await hass.config_entries.flow.async_init( @@ -74,10 +76,9 @@ async def test_user_flow( assert result["result"].unique_id == UUID +@pytest.mark.usefixtures("mock_linkplay_factory_bridge") async def test_user_flow_re_entry( hass: HomeAssistant, - mock_linkplay_factory_bridge: AsyncMock, - mock_setup_entry: AsyncMock, ) -> None: """Test user setup config flow when an entry with the same unique id already exists.""" @@ -105,10 +106,9 @@ async def test_user_flow_re_entry( assert result["reason"] == "already_configured" +@pytest.mark.usefixtures("mock_linkplay_factory_bridge", "mock_setup_entry") async def test_zeroconf_flow( hass: HomeAssistant, - mock_linkplay_factory_bridge: AsyncMock, - mock_setup_entry: AsyncMock, ) -> None: """Test Zeroconf flow.""" result = await hass.config_entries.flow.async_init( @@ -133,10 +133,9 @@ async def test_zeroconf_flow( assert result["result"].unique_id == UUID +@pytest.mark.usefixtures("mock_linkplay_factory_bridge") async def test_zeroconf_flow_re_entry( hass: HomeAssistant, - mock_linkplay_factory_bridge: AsyncMock, - mock_setup_entry: AsyncMock, ) -> None: """Test Zeroconf flow when an entry with the same unique id already exists.""" @@ -160,16 +159,35 @@ async def test_zeroconf_flow_re_entry( assert result["reason"] == "already_configured" -async def test_flow_errors( +@pytest.mark.usefixtures("mock_setup_entry") +async def test_zeroconf_flow_errors( + hass: HomeAssistant, + mock_linkplay_factory_bridge: AsyncMock, +) -> None: + """Test flow when the device discovered through Zeroconf cannot be reached.""" + + # Temporarily make the mock_linkplay_factory_bridge throw an exception + mock_linkplay_factory_bridge.side_effect = (LinkPlayRequestException("Error"),) + + result = await hass.config_entries.flow.async_init( + DOMAIN, + context={"source": SOURCE_ZEROCONF}, + data=ZEROCONF_DISCOVERY, + ) + + assert result["type"] is FlowResultType.ABORT + assert result["reason"] == "cannot_connect" + + +@pytest.mark.usefixtures("mock_setup_entry") +async def test_user_flow_errors( hass: HomeAssistant, mock_linkplay_factory_bridge: AsyncMock, - mock_setup_entry: AsyncMock, ) -> None: """Test flow when the device cannot be reached.""" - # Temporarily store bridge in a separate variable and set factory to return None - bridge = mock_linkplay_factory_bridge.return_value - mock_linkplay_factory_bridge.return_value = None + # Temporarily make the mock_linkplay_factory_bridge throw an exception + mock_linkplay_factory_bridge.side_effect = (LinkPlayRequestException("Error"),) result = await hass.config_entries.flow.async_init( DOMAIN, @@ -188,8 +206,8 @@ async def test_flow_errors( assert result["step_id"] == "user" assert result["errors"] == {"base": "cannot_connect"} - # Make linkplay_factory_bridge return a mock bridge again - mock_linkplay_factory_bridge.return_value = bridge + # Make mock_linkplay_factory_bridge_exception no longer throw an exception + mock_linkplay_factory_bridge.side_effect = None result = await hass.config_entries.flow.async_configure( result["flow_id"],