diff --git a/homeassistant/components/dlna_dmr/media_player.py b/homeassistant/components/dlna_dmr/media_player.py index ada43f37f81..fd89c5be2d0 100644 --- a/homeassistant/components/dlna_dmr/media_player.py +++ b/homeassistant/components/dlna_dmr/media_player.py @@ -6,7 +6,7 @@ from collections.abc import Awaitable, Callable, Coroutine, Sequence import contextlib from datetime import datetime, timedelta import functools -from typing import TYPE_CHECKING, Any, TypeVar +from typing import Any, TypeVar from async_upnp_client import UpnpService, UpnpStateVariable from async_upnp_client.const import NotificationSubType @@ -70,6 +70,8 @@ _T = TypeVar("_T", bound="DlnaDmrEntity") _R = TypeVar("_R") _P = ParamSpec("_P") +Func = TypeVar("Func", bound=Callable[..., Any]) + def catch_request_errors( func: Callable[Concatenate[_T, _P], Awaitable[_R]] # type: ignore[misc] @@ -131,6 +133,7 @@ class DlnaDmrEntity(MediaPlayerEntity): _device_lock: asyncio.Lock # Held when connecting or disconnecting the device _device: DmrDevice | None = None check_available: bool = False + _ssdp_connect_failed: bool = False # Track BOOTID in SSDP advertisements for device changes _bootid: int | None = None @@ -228,22 +231,34 @@ class DlnaDmrEntity(MediaPlayerEntity): # Nothing left to do until ssdp:alive comes through return - if self._bootid is not None and self._bootid != bootid and self._device: - # Device has rebooted, drop existing connection and maybe reconnect - await self._device_disconnect() + if self._bootid is not None and self._bootid != bootid: + # Device has rebooted + # Maybe connection will succeed now + self._ssdp_connect_failed = False + if self._device: + # Drop existing connection and maybe reconnect + await self._device_disconnect() self._bootid = bootid - if change == ssdp.SsdpChange.BYEBYE and self._device: - # Device is going away, disconnect - await self._device_disconnect() + if change == ssdp.SsdpChange.BYEBYE: + # Device is going away + if self._device: + # Disconnect from gone device + await self._device_disconnect() + # Maybe the next alive message will result in a successful connection + self._ssdp_connect_failed = False - if change == ssdp.SsdpChange.ALIVE and not self._device: - if TYPE_CHECKING: - assert info.ssdp_location + if ( + change == ssdp.SsdpChange.ALIVE + and not self._device + and not self._ssdp_connect_failed + ): + assert info.ssdp_location location = info.ssdp_location try: await self._device_connect(location) except UpnpError as err: + self._ssdp_connect_failed = True _LOGGER.warning( "Failed connecting to recently alive device at %s: %r", location, diff --git a/tests/components/dlna_dmr/test_media_player.py b/tests/components/dlna_dmr/test_media_player.py index 2d59c31c666..3cb4b2a726a 100644 --- a/tests/components/dlna_dmr/test_media_player.py +++ b/tests/components/dlna_dmr/test_media_player.py @@ -43,6 +43,7 @@ from .conftest import ( LOCAL_IP, MOCK_DEVICE_LOCATION, MOCK_DEVICE_NAME, + MOCK_DEVICE_TYPE, MOCK_DEVICE_UDN, MOCK_DEVICE_USN, NEW_DEVICE_LOCATION, @@ -50,8 +51,6 @@ from .conftest import ( from tests.common import MockConfigEntry -MOCK_DEVICE_ST = "mock_st" - # Auto-use the domain_data_mock fixture for every test in this module pytestmark = pytest.mark.usefixtures("domain_data_mock") @@ -1024,7 +1023,7 @@ async def test_become_available( ssdp.SsdpServiceInfo( ssdp_usn=MOCK_DEVICE_USN, ssdp_location=NEW_DEVICE_LOCATION, - ssdp_st=MOCK_DEVICE_ST, + ssdp_st=MOCK_DEVICE_TYPE, upnp={}, ), ssdp.SsdpChange.ALIVE, @@ -1088,18 +1087,90 @@ async def test_alive_but_gone( ssdp.SsdpServiceInfo( ssdp_usn=MOCK_DEVICE_USN, ssdp_location=NEW_DEVICE_LOCATION, - ssdp_st=MOCK_DEVICE_ST, + ssdp_st=MOCK_DEVICE_TYPE, + ssdp_headers={ssdp.ATTR_SSDP_BOOTID: "1"}, upnp={}, ), ssdp.SsdpChange.ALIVE, ) await hass.async_block_till_done() + # There should be a connection attempt to the device + domain_data_mock.upnp_factory.async_create_device.assert_awaited() + # Device should still be unavailable mock_state = hass.states.get(mock_disconnected_entity_id) assert mock_state is not None assert mock_state.state == ha_const.STATE_UNAVAILABLE + # Send the same SSDP notification, expecting no extra connection attempts + domain_data_mock.upnp_factory.async_create_device.reset_mock() + await ssdp_callback( + ssdp.SsdpServiceInfo( + ssdp_usn=MOCK_DEVICE_USN, + ssdp_location=NEW_DEVICE_LOCATION, + ssdp_st=MOCK_DEVICE_TYPE, + ssdp_headers={ssdp.ATTR_SSDP_BOOTID: "1"}, + upnp={}, + ), + ssdp.SsdpChange.ALIVE, + ) + await hass.async_block_till_done() + domain_data_mock.upnp_factory.async_create_device.assert_not_called() + domain_data_mock.upnp_factory.async_create_device.assert_not_awaited() + mock_state = hass.states.get(mock_disconnected_entity_id) + assert mock_state is not None + assert mock_state.state == ha_const.STATE_UNAVAILABLE + + # Send an SSDP notification with a new BOOTID, indicating the device has rebooted + domain_data_mock.upnp_factory.async_create_device.reset_mock() + await ssdp_callback( + ssdp.SsdpServiceInfo( + ssdp_usn=MOCK_DEVICE_USN, + ssdp_location=NEW_DEVICE_LOCATION, + ssdp_st=MOCK_DEVICE_TYPE, + ssdp_headers={ssdp.ATTR_SSDP_BOOTID: "2"}, + upnp={}, + ), + ssdp.SsdpChange.ALIVE, + ) + await hass.async_block_till_done() + + # Rebooted device (seen via BOOTID) should mean a new connection attempt + domain_data_mock.upnp_factory.async_create_device.assert_awaited() + mock_state = hass.states.get(mock_disconnected_entity_id) + assert mock_state is not None + assert mock_state.state == ha_const.STATE_UNAVAILABLE + + # Send byebye message to indicate device is going away. Next alive message + # should result in a reconnect attempt even with same BOOTID. + domain_data_mock.upnp_factory.async_create_device.reset_mock() + await ssdp_callback( + ssdp.SsdpServiceInfo( + ssdp_usn=MOCK_DEVICE_USN, + ssdp_st=MOCK_DEVICE_TYPE, + upnp={}, + ), + ssdp.SsdpChange.BYEBYE, + ) + await ssdp_callback( + ssdp.SsdpServiceInfo( + ssdp_usn=MOCK_DEVICE_USN, + ssdp_location=NEW_DEVICE_LOCATION, + ssdp_st=MOCK_DEVICE_TYPE, + ssdp_headers={ssdp.ATTR_SSDP_BOOTID: "2"}, + upnp={}, + ), + ssdp.SsdpChange.ALIVE, + ) + await hass.async_block_till_done() + + # Rebooted device (seen via byebye/alive) should mean a new connection attempt + domain_data_mock.upnp_factory.async_create_device.assert_awaited() + mock_state = hass.states.get(mock_disconnected_entity_id) + assert mock_state is not None + assert mock_state.state == ha_const.STATE_UNAVAILABLE + async def test_multiple_ssdp_alive( hass: HomeAssistant, @@ -1129,7 +1200,7 @@ async def test_multiple_ssdp_alive( ssdp.SsdpServiceInfo( ssdp_usn=MOCK_DEVICE_USN, ssdp_location=NEW_DEVICE_LOCATION, - ssdp_st=MOCK_DEVICE_ST, + ssdp_st=MOCK_DEVICE_TYPE, upnp={}, ), ssdp.SsdpChange.ALIVE, @@ -1138,7 +1209,7 @@ async def test_multiple_ssdp_alive( ssdp.SsdpServiceInfo( ssdp_usn=MOCK_DEVICE_USN, ssdp_location=NEW_DEVICE_LOCATION, - ssdp_st=MOCK_DEVICE_ST, + ssdp_st=MOCK_DEVICE_TYPE, upnp={}, ), ssdp.SsdpChange.ALIVE, @@ -1170,7 +1241,7 @@ async def test_ssdp_byebye( ssdp_usn=MOCK_DEVICE_USN, ssdp_udn=MOCK_DEVICE_UDN, ssdp_headers={"NTS": "ssdp:byebye"}, - ssdp_st=MOCK_DEVICE_ST, + ssdp_st=MOCK_DEVICE_TYPE, upnp={}, ), ssdp.SsdpChange.BYEBYE, @@ -1189,7 +1260,7 @@ async def test_ssdp_byebye( ssdp_usn=MOCK_DEVICE_USN, ssdp_udn=MOCK_DEVICE_UDN, ssdp_headers={"NTS": "ssdp:byebye"}, - ssdp_st=MOCK_DEVICE_ST, + ssdp_st=MOCK_DEVICE_TYPE, upnp={}, ), ssdp.SsdpChange.BYEBYE, @@ -1222,7 +1293,7 @@ async def test_ssdp_update_seen_bootid( ssdp_usn=MOCK_DEVICE_USN, ssdp_location=MOCK_DEVICE_LOCATION, ssdp_headers={ssdp.ATTR_SSDP_BOOTID: "1"}, - ssdp_st=MOCK_DEVICE_ST, + ssdp_st=MOCK_DEVICE_TYPE, upnp={}, ), ssdp.SsdpChange.ALIVE, @@ -1239,7 +1310,7 @@ async def test_ssdp_update_seen_bootid( ssdp.ATTR_SSDP_BOOTID: "1", ssdp.ATTR_SSDP_NEXTBOOTID: "2", }, - ssdp_st=MOCK_DEVICE_ST, + ssdp_st=MOCK_DEVICE_TYPE, upnp={}, ), ssdp.SsdpChange.UPDATE, @@ -1264,7 +1335,7 @@ async def test_ssdp_update_seen_bootid( ssdp.ATTR_SSDP_BOOTID: "1", ssdp.ATTR_SSDP_NEXTBOOTID: "2", }, - ssdp_st=MOCK_DEVICE_ST, + ssdp_st=MOCK_DEVICE_TYPE, upnp={}, ), ssdp.SsdpChange.UPDATE, @@ -1289,7 +1360,7 @@ async def test_ssdp_update_seen_bootid( ssdp.ATTR_SSDP_BOOTID: "2", ssdp.ATTR_SSDP_NEXTBOOTID: "7c848375-a106-4bd1-ac3c-8e50427c8e4f", }, - ssdp_st=MOCK_DEVICE_ST, + ssdp_st=MOCK_DEVICE_TYPE, upnp={}, ), ssdp.SsdpChange.UPDATE, @@ -1310,7 +1381,7 @@ async def test_ssdp_update_seen_bootid( ssdp_usn=MOCK_DEVICE_USN, ssdp_location=MOCK_DEVICE_LOCATION, ssdp_headers={ssdp.ATTR_SSDP_BOOTID: "2"}, - ssdp_st=MOCK_DEVICE_ST, + ssdp_st=MOCK_DEVICE_TYPE, upnp={}, ), ssdp.SsdpChange.ALIVE, @@ -1349,7 +1420,7 @@ async def test_ssdp_update_missed_bootid( ssdp_usn=MOCK_DEVICE_USN, ssdp_location=MOCK_DEVICE_LOCATION, ssdp_headers={ssdp.ATTR_SSDP_BOOTID: "1"}, - ssdp_st=MOCK_DEVICE_ST, + ssdp_st=MOCK_DEVICE_TYPE, upnp={}, ), ssdp.SsdpChange.ALIVE, @@ -1366,7 +1437,7 @@ async def test_ssdp_update_missed_bootid( ssdp.ATTR_SSDP_BOOTID: "2", ssdp.ATTR_SSDP_NEXTBOOTID: "3", }, - ssdp_st=MOCK_DEVICE_ST, + ssdp_st=MOCK_DEVICE_TYPE, upnp={}, ), ssdp.SsdpChange.UPDATE, @@ -1387,7 +1458,7 @@ async def test_ssdp_update_missed_bootid( ssdp_usn=MOCK_DEVICE_USN, ssdp_location=MOCK_DEVICE_LOCATION, ssdp_headers={ssdp.ATTR_SSDP_BOOTID: "3"}, - ssdp_st=MOCK_DEVICE_ST, + ssdp_st=MOCK_DEVICE_TYPE, upnp={}, ), ssdp.SsdpChange.ALIVE, @@ -1426,7 +1497,7 @@ async def test_ssdp_bootid( ssdp_usn=MOCK_DEVICE_USN, ssdp_location=MOCK_DEVICE_LOCATION, ssdp_headers={ssdp.ATTR_SSDP_BOOTID: "1"}, - ssdp_st=MOCK_DEVICE_ST, + ssdp_st=MOCK_DEVICE_TYPE, upnp={}, ), ssdp.SsdpChange.ALIVE, @@ -1446,7 +1517,7 @@ async def test_ssdp_bootid( ssdp_usn=MOCK_DEVICE_USN, ssdp_location=MOCK_DEVICE_LOCATION, ssdp_headers={ssdp.ATTR_SSDP_BOOTID: "1"}, - ssdp_st=MOCK_DEVICE_ST, + ssdp_st=MOCK_DEVICE_TYPE, upnp={}, ), ssdp.SsdpChange.ALIVE, @@ -1466,7 +1537,7 @@ async def test_ssdp_bootid( ssdp_usn=MOCK_DEVICE_USN, ssdp_location=MOCK_DEVICE_LOCATION, ssdp_headers={ssdp.ATTR_SSDP_BOOTID: "2"}, - ssdp_st=MOCK_DEVICE_ST, + ssdp_st=MOCK_DEVICE_TYPE, upnp={}, ), ssdp.SsdpChange.ALIVE,