Avoid delaying startup in dlna_dmr (#109836)

* Avoid delaying startup in dlna_dmr

fixes #109834

* make sure device info is linked up at startup

* fixes

* update tests

* startup only

* override device info if we have it

* fixes

* make sure its set right away when adding the device

* revert test changes

* coverage

* coverage

* coverage

* coverage

* adjust

* fixes

* more fixes

* coverage

* coverage

* coverage

* tweaks

* tweaks

* Revert "revert test changes"

This reverts commit 014d29297dac9dda45a50ec9eb2fa63a735538ac.

* coverage

* coverage

homeassistant/components/dlna_dmr/media_player.py

@@ -19,6 +19,7 @@ from homeassistant import config_entries
 from homeassistant.components import media_source, ssdp
 from homeassistant.components.media_player import (
     ATTR_MEDIA_EXTRA,
+    DOMAIN as MEDIA_PLAYER_DOMAIN,
     BrowseMedia,
     MediaPlayerEntity,
     MediaPlayerEntityFeature,
@@ -28,7 +29,7 @@ from homeassistant.components.media_player import (
     async_process_play_media_url,
 )
 from homeassistant.const import CONF_DEVICE_ID, CONF_MAC, CONF_TYPE, CONF_URL
-from homeassistant.core import HomeAssistant
+from homeassistant.core import CoreState, HomeAssistant
 from homeassistant.helpers import device_registry as dr, entity_registry as er
 from homeassistant.helpers.entity_platform import AddEntitiesCallback
 
@@ -37,6 +38,7 @@ from .const import (
     CONF_CALLBACK_URL_OVERRIDE,
     CONF_LISTEN_PORT,
     CONF_POLL_AVAILABILITY,
+    DOMAIN,
     LOGGER as _LOGGER,
     MEDIA_METADATA_DIDL,
     MEDIA_TYPE_MAP,
@@ -87,9 +89,32 @@ async def async_setup_entry(
     """Set up the DlnaDmrEntity from a config entry."""
     _LOGGER.debug("media_player.async_setup_entry %s (%s)", entry.entry_id, entry.title)
 
+    udn = entry.data[CONF_DEVICE_ID]
+    ent_reg = er.async_get(hass)
+    dev_reg = dr.async_get(hass)
+
+    if (
+        (
+            existing_entity_id := ent_reg.async_get_entity_id(
+                domain=MEDIA_PLAYER_DOMAIN, platform=DOMAIN, unique_id=udn
+            )
+        )
+        and (existing_entry := ent_reg.async_get(existing_entity_id))
+        and (device_id := existing_entry.device_id)
+        and (device_entry := dev_reg.async_get(device_id))
+        and (dr.CONNECTION_UPNP, udn) not in device_entry.connections
+    ):
+        # If the existing device is missing the udn connection, add it
+        # now to ensure that when the entity gets added it is linked to
+        # the correct device.
+        dev_reg.async_update_device(
+            device_id,
+            merge_connections={(dr.CONNECTION_UPNP, udn)},
+        )
+
     # Create our own device-wrapping entity
     entity = DlnaDmrEntity(
-        udn=entry.data[CONF_DEVICE_ID],
+        udn=udn,
         device_type=entry.data[CONF_TYPE],
         name=entry.title,
         event_port=entry.options.get(CONF_LISTEN_PORT) or 0,
@@ -98,6 +123,7 @@ async def async_setup_entry(
         location=entry.data[CONF_URL],
         mac_address=entry.data.get(CONF_MAC),
         browse_unfiltered=entry.options.get(CONF_BROWSE_UNFILTERED, False),
+        config_entry=entry,
     )
 
     async_add_entities([entity])
@@ -143,6 +169,7 @@ class DlnaDmrEntity(MediaPlayerEntity):
         location: str,
         mac_address: str | None,
         browse_unfiltered: bool,
+        config_entry: config_entries.ConfigEntry,
     ) -> None:
         """Initialize DLNA DMR entity."""
         self.udn = udn
@@ -154,25 +181,17 @@ class DlnaDmrEntity(MediaPlayerEntity):
         self.mac_address = mac_address
         self.browse_unfiltered = browse_unfiltered
         self._device_lock = asyncio.Lock()
+        self._background_setup_task: asyncio.Task[None] | None = None
+        self._updated_registry: bool = False
+        self._config_entry = config_entry
+        self._attr_device_info = dr.DeviceInfo(connections={(dr.CONNECTION_UPNP, udn)})
 
     async def async_added_to_hass(self) -> None:
         """Handle addition."""
         # Update this entity when the associated config entry is modified
-        if self.registry_entry and self.registry_entry.config_entry_id:
+        self.async_on_remove(
-            config_entry = self.hass.config_entries.async_get_entry(
+            self._config_entry.add_update_listener(self.async_config_update_listener)
-                self.registry_entry.config_entry_id
+        )
-            )
-            assert config_entry is not None
-            self.async_on_remove(
-                config_entry.add_update_listener(self.async_config_update_listener)
-            )
-
-        # Try to connect to the last known location, but don't worry if not available
-        if not self._device:
-            try:
-                await self._device_connect(self.location)
-            except UpnpError as err:
-                _LOGGER.debug("Couldn't connect immediately: %r", err)
 
         # Get SSDP notifications for only this device
         self.async_on_remove(
@@ -193,8 +212,29 @@ class DlnaDmrEntity(MediaPlayerEntity):
             )
         )
 
+        if not self._device:
+            if self.hass.state is CoreState.running:
+                await self._async_setup()
+            else:
+                self._background_setup_task = self.hass.async_create_background_task(
+                    self._async_setup(), f"dlna_dmr {self.name} setup"
+                )
+
+    async def _async_setup(self) -> None:
+        # Try to connect to the last known location, but don't worry if not available
+        try:
+            await self._device_connect(self.location)
+        except UpnpError as err:
+            _LOGGER.debug("Couldn't connect immediately: %r", err)
+
     async def async_will_remove_from_hass(self) -> None:
         """Handle removal."""
+        if self._background_setup_task:
+            self._background_setup_task.cancel()
+            with contextlib.suppress(asyncio.CancelledError):
+                await self._background_setup_task
+            self._background_setup_task = None
+
         await self._device_disconnect()
 
     async def async_ssdp_callback(
@@ -351,25 +391,28 @@ class DlnaDmrEntity(MediaPlayerEntity):
 
     def _update_device_registry(self, set_mac: bool = False) -> None:
         """Update the device registry with new information about the DMR."""
-        if not self._device:
+        if (
-            return  # Can't get all the required information without a connection
+            # Can't get all the required information without a connection
+            not self._device
+            or
+            # No new information
+            (not set_mac and self._updated_registry)
+        ):
+            return
 
-        if not self.registry_entry or not self.registry_entry.config_entry_id:
-            return  # No config registry entry to link to
-
-        if self.registry_entry.device_id and not set_mac:
-            return  # No new information
-
-        connections = set()
         # Connections based on the root device's UDN, and the DMR embedded
         # device's UDN. They may be the same, if the DMR is the root device.
-        connections.add(
+        connections = {
             (
                 dr.CONNECTION_UPNP,
                 self._device.profile_device.root_device.udn,
-            )
+            ),
-        )
+            (dr.CONNECTION_UPNP, self._device.udn),
-        connections.add((dr.CONNECTION_UPNP, self._device.udn))
+            (
+                dr.CONNECTION_UPNP,
+                self.udn,
+            ),
+        }
 
         if self.mac_address:
             # Connection based on MAC address, if known
@@ -378,23 +421,27 @@ class DlnaDmrEntity(MediaPlayerEntity):
                 (dr.CONNECTION_NETWORK_MAC, self.mac_address)
             )
 
-        # Create linked HA DeviceEntry now the information is known.
+        device_info = dr.DeviceInfo(
-        dev_reg = dr.async_get(self.hass)
-        device_entry = dev_reg.async_get_or_create(
-            config_entry_id=self.registry_entry.config_entry_id,
             connections=connections,
             default_manufacturer=self._device.manufacturer,
             default_model=self._device.model_name,
             default_name=self._device.name,
         )
+        self._attr_device_info = device_info
+
+        self._updated_registry = True
+        # Create linked HA DeviceEntry now the information is known.
+        device_entry = dr.async_get(self.hass).async_get_or_create(
+            config_entry_id=self._config_entry.entry_id, **device_info
+        )
 
         # Update entity registry to link to the device
-        ent_reg = er.async_get(self.hass)
+        er.async_get(self.hass).async_get_or_create(
-        ent_reg.async_get_or_create(
+            MEDIA_PLAYER_DOMAIN,
-            self.registry_entry.domain,
+            DOMAIN,
-            self.registry_entry.platform,
             self.unique_id,
             device_id=device_entry.id,
+            config_entry=self._config_entry,
         )
 
     async def _device_disconnect(self) -> None:
@@ -419,6 +466,10 @@ class DlnaDmrEntity(MediaPlayerEntity):
 
     async def async_update(self) -> None:
         """Retrieve the latest data."""
+        if self._background_setup_task:
+            await self._background_setup_task
+            self._background_setup_task = None
+
         if not self._device:
             if not self.poll_availability:
                 return

tests/components/dlna_dmr/test_init.py

@@ -6,6 +6,7 @@ from homeassistant.components import media_player
 from homeassistant.components.dlna_dmr.const import DOMAIN as DLNA_DOMAIN
 from homeassistant.core import HomeAssistant
 from homeassistant.helpers import entity_registry as er
+from homeassistant.helpers.entity_component import async_update_entity
 from homeassistant.setup import async_setup_component
 
 from tests.common import MockConfigEntry
@@ -31,6 +32,10 @@ async def test_resource_lifecycle(
     )
     assert len(entries) == 1
     entity_id = entries[0].entity_id
+
+    await async_update_entity(hass, entity_id)
+    await hass.async_block_till_done()
+
     mock_state = hass.states.get(entity_id)
     assert mock_state is not None
     assert mock_state.state == media_player.STATE_IDLE

tests/components/dlna_dmr/test_media_player.py

@@ -26,6 +26,7 @@ from homeassistant.components.dlna_dmr.const import (
     CONF_CALLBACK_URL_OVERRIDE,
     CONF_LISTEN_PORT,
     CONF_POLL_AVAILABILITY,
+    DOMAIN,
 )
 from homeassistant.components.dlna_dmr.data import EventListenAddr
 from homeassistant.components.dlna_dmr.media_player import DlnaDmrEntity
@@ -46,7 +47,7 @@ from homeassistant.const import (
     CONF_TYPE,
     CONF_URL,
 )
-from homeassistant.core import HomeAssistant
+from homeassistant.core import CoreState, HomeAssistant
 from homeassistant.helpers.device_registry import (
     CONNECTION_NETWORK_MAC,
     CONNECTION_UPNP,
@@ -216,6 +217,9 @@ async def test_setup_entry_no_options(
     """
     config_entry_mock.options = MappingProxyType({})
     mock_entity_id = await setup_mock_component(hass, config_entry_mock)
+    await async_update_entity(hass, mock_entity_id)
+    await hass.async_block_till_done()
+
     mock_state = hass.states.get(mock_entity_id)
     assert mock_state is not None
 
@@ -266,17 +270,23 @@ async def test_setup_entry_no_options(
     assert mock_state.state == ha_const.STATE_UNAVAILABLE
 
 
+@pytest.mark.parametrize(
+    "core_state",
+    (CoreState.not_running, CoreState.running),
+)
 async def test_setup_entry_with_options(
     hass: HomeAssistant,
     domain_data_mock: Mock,
     ssdp_scanner_mock: Mock,
     config_entry_mock: MockConfigEntry,
     dmr_device_mock: Mock,
+    core_state: CoreState,
 ) -> None:
     """Test setting options leads to a DlnaDmrEntity with custom event_handler.
 
     Check that the device is constructed properly as part of the test.
     """
+    hass.set_state(core_state)
     config_entry_mock.options = MappingProxyType(
         {
             CONF_LISTEN_PORT: 2222,
@@ -285,6 +295,8 @@ async def test_setup_entry_with_options(
         }
     )
     mock_entity_id = await setup_mock_component(hass, config_entry_mock)
+    await async_update_entity(hass, mock_entity_id)
+    await hass.async_block_till_done()
     mock_state = hass.states.get(mock_entity_id)
     assert mock_state is not None
 
@@ -343,8 +355,9 @@ async def test_setup_entry_mac_address(
     dmr_device_mock: Mock,
 ) -> None:
     """Entry with a MAC address will set up and set the device registry connection."""
-    await setup_mock_component(hass, config_entry_mock)
+    mock_entity_id = await setup_mock_component(hass, config_entry_mock)
-
+    await async_update_entity(hass, mock_entity_id)
+    await hass.async_block_till_done()
     # Check the device registry connections for MAC address
     dev_reg = async_get_dr(hass)
     device = dev_reg.async_get_device(
@@ -363,8 +376,9 @@ async def test_setup_entry_no_mac_address(
     dmr_device_mock: Mock,
 ) -> None:
     """Test setting up an entry without a MAC address will succeed."""
-    await setup_mock_component(hass, config_entry_mock_no_mac)
+    mock_entity_id = await setup_mock_component(hass, config_entry_mock_no_mac)
-
+    await async_update_entity(hass, mock_entity_id)
+    await hass.async_block_till_done()
     # Check the device registry connections does not include the MAC address
     dev_reg = async_get_dr(hass)
     device = dev_reg.async_get_device(
@@ -382,6 +396,8 @@ async def test_event_subscribe_failure(
     dmr_device_mock.async_subscribe_services.side_effect = UpnpError
 
     mock_entity_id = await setup_mock_component(hass, config_entry_mock)
+    await async_update_entity(hass, mock_entity_id)
+    await hass.async_block_till_done()
     mock_state = hass.states.get(mock_entity_id)
     assert mock_state is not None
 
@@ -412,6 +428,8 @@ async def test_event_subscribe_rejected(
     dmr_device_mock.async_subscribe_services.side_effect = UpnpResponseError(status=501)
 
     mock_entity_id = await setup_mock_component(hass, config_entry_mock)
+    await async_update_entity(hass, mock_entity_id)
+    await hass.async_block_till_done()
     mock_state = hass.states.get(mock_entity_id)
     assert mock_state is not None
 
@@ -432,6 +450,8 @@ async def test_available_device(
 ) -> None:
     """Test a DlnaDmrEntity with a connected DmrDevice."""
     # Check hass device information is filled in
+    await async_update_entity(hass, mock_entity_id)
+    await hass.async_block_till_done()
     dev_reg = async_get_dr(hass)
     device = dev_reg.async_get_device(
         connections={(CONNECTION_UPNP, MOCK_DEVICE_UDN)},
@@ -1235,14 +1255,20 @@ async def test_playback_update_state(
     dmr_device_mock.async_update.assert_not_awaited()
 
 
+@pytest.mark.parametrize(
+    "core_state",
+    (CoreState.not_running, CoreState.running),
+)
 async def test_unavailable_device(
     hass: HomeAssistant,
     domain_data_mock: Mock,
     ssdp_scanner_mock: Mock,
     config_entry_mock: MockConfigEntry,
+    core_state: CoreState,
 ) -> None:
     """Test a DlnaDmrEntity with out a connected DmrDevice."""
     # Cause connection attempts to fail
+    hass.set_state(core_state)
     domain_data_mock.upnp_factory.async_create_device.side_effect = UpnpConnectionError
 
     with patch(
@@ -1336,7 +1362,9 @@ async def test_unavailable_device(
         connections={(CONNECTION_UPNP, MOCK_DEVICE_UDN)},
         identifiers=set(),
     )
-    assert device is None
+    assert device is not None
+    assert device.name is None
+    assert device.manufacturer is None
 
     # Unload config entry to clean up
     assert await hass.config_entries.async_remove(config_entry_mock.entry_id) == {
@@ -1355,15 +1383,21 @@ async def test_unavailable_device(
     assert mock_state.state == ha_const.STATE_UNAVAILABLE
 
 
+@pytest.mark.parametrize(
+    "core_state",
+    (CoreState.not_running, CoreState.running),
+)
 async def test_become_available(
     hass: HomeAssistant,
     domain_data_mock: Mock,
     ssdp_scanner_mock: Mock,
     config_entry_mock: MockConfigEntry,
     dmr_device_mock: Mock,
+    core_state: CoreState,
 ) -> None:
     """Test a device becoming available after the entity is constructed."""
     # Cause connection attempts to fail before adding entity
+    hass.set_state(core_state)
     domain_data_mock.upnp_factory.async_create_device.side_effect = UpnpConnectionError
     mock_entity_id = await setup_mock_component(hass, config_entry_mock)
     mock_state = hass.states.get(mock_entity_id)
@@ -1376,7 +1410,7 @@ async def test_become_available(
         connections={(CONNECTION_UPNP, MOCK_DEVICE_UDN)},
         identifiers=set(),
     )
-    assert device is None
+    assert device is not None
 
     # Mock device is now available.
     domain_data_mock.upnp_factory.async_create_device.side_effect = None
@@ -1440,13 +1474,19 @@ async def test_become_available(
     assert mock_state.state == ha_const.STATE_UNAVAILABLE
 
 
+@pytest.mark.parametrize(
+    "core_state",
+    (CoreState.not_running, CoreState.running),
+)
 async def test_alive_but_gone(
     hass: HomeAssistant,
     domain_data_mock: Mock,
     ssdp_scanner_mock: Mock,
     mock_disconnected_entity_id: str,
+    core_state: CoreState,
 ) -> None:
     """Test a device sending an SSDP alive announcement, but not being connectable."""
+    hass.set_state(core_state)
     domain_data_mock.upnp_factory.async_create_device.side_effect = UpnpError
 
     # Send an SSDP notification from the still missing device
@@ -2275,3 +2315,162 @@ async def test_config_update_mac_address(
     )
     assert device is not None
     assert (CONNECTION_NETWORK_MAC, MOCK_MAC_ADDRESS) in device.connections
+
+
+@pytest.mark.parametrize(
+    "core_state",
+    (CoreState.not_running, CoreState.running),
+)
+async def test_connections_restored(
+    hass: HomeAssistant,
+    domain_data_mock: Mock,
+    ssdp_scanner_mock: Mock,
+    config_entry_mock: MockConfigEntry,
+    dmr_device_mock: Mock,
+    core_state: CoreState,
+) -> None:
+    """Test previous connections restored."""
+    # Cause connection attempts to fail before adding entity
+    hass.set_state(core_state)
+    domain_data_mock.upnp_factory.async_create_device.side_effect = UpnpConnectionError
+    mock_entity_id = await setup_mock_component(hass, config_entry_mock)
+    mock_state = hass.states.get(mock_entity_id)
+    assert mock_state is not None
+    assert mock_state.state == ha_const.STATE_UNAVAILABLE
+
+    # Check hass device information has not been filled in yet
+    dev_reg = async_get_dr(hass)
+    device = dev_reg.async_get_device(
+        connections={(CONNECTION_UPNP, MOCK_DEVICE_UDN)},
+        identifiers=set(),
+    )
+    assert device is not None
+
+    # Mock device is now available.
+    domain_data_mock.upnp_factory.async_create_device.side_effect = None
+    domain_data_mock.upnp_factory.async_create_device.reset_mock()
+
+    # Send an SSDP notification from the now alive device
+    ssdp_callback = ssdp_scanner_mock.async_register_callback.call_args.args[0]
+    await ssdp_callback(
+        ssdp.SsdpServiceInfo(
+            ssdp_usn=MOCK_DEVICE_USN,
+            ssdp_location=NEW_DEVICE_LOCATION,
+            ssdp_st=MOCK_DEVICE_TYPE,
+            upnp={},
+        ),
+        ssdp.SsdpChange.ALIVE,
+    )
+    await hass.async_block_till_done()
+
+    # Check device was created from the supplied URL
+    domain_data_mock.upnp_factory.async_create_device.assert_awaited_once_with(
+        NEW_DEVICE_LOCATION
+    )
+    # Check event notifiers are acquired
+    domain_data_mock.async_get_event_notifier.assert_awaited_once_with(
+        EventListenAddr(LOCAL_IP, 0, None), hass
+    )
+    # Check UPnP services are subscribed
+    dmr_device_mock.async_subscribe_services.assert_awaited_once_with(
+        auto_resubscribe=True
+    )
+    assert dmr_device_mock.on_event is not None
+    # Quick check of the state to verify the entity has a connected DmrDevice
+    mock_state = hass.states.get(mock_entity_id)
+    assert mock_state is not None
+    assert mock_state.state == MediaPlayerState.IDLE
+    # Check hass device information is now filled in
+    dev_reg = async_get_dr(hass)
+    device = dev_reg.async_get_device(
+        connections={(CONNECTION_UPNP, MOCK_DEVICE_UDN)},
+        identifiers=set(),
+    )
+    assert device is not None
+    previous_connections = device.connections
+    assert device.manufacturer == "device_manufacturer"
+    assert device.model == "device_model_name"
+    assert device.name == "device_name"
+
+    # Reload the config entry
+    assert await hass.config_entries.async_reload(config_entry_mock.entry_id)
+    await async_update_entity(hass, mock_entity_id)
+
+    # Confirm SSDP notifications unregistered
+    assert ssdp_scanner_mock.async_register_callback.return_value.call_count == 2
+
+    # Confirm the entity has disconnected from the device
+    domain_data_mock.async_release_event_notifier.assert_awaited_once()
+    dmr_device_mock.async_unsubscribe_services.assert_awaited_once()
+
+    # Check hass device information has not been filled in yet
+    dev_reg = async_get_dr(hass)
+    device = dev_reg.async_get_device(
+        connections={(CONNECTION_UPNP, MOCK_DEVICE_UDN)},
+        identifiers=set(),
+    )
+    assert device is not None
+    assert device.connections == previous_connections
+
+    # Verify the entity remains linked to the device
+    ent_reg = async_get_er(hass)
+    entry = ent_reg.async_get(mock_entity_id)
+    assert entry is not None
+    assert entry.device_id == device.id
+
+    # Verify the entity has an idle state
+    mock_state = hass.states.get(mock_entity_id)
+    assert mock_state is not None
+    assert mock_state.state == MediaPlayerState.IDLE
+
+    # Unload config entry to clean up
+    assert await hass.config_entries.async_unload(config_entry_mock.entry_id)
+
+
+async def test_udn_upnp_connection_added_if_missing(
+    hass: HomeAssistant,
+    domain_data_mock: Mock,
+    ssdp_scanner_mock: Mock,
+    config_entry_mock: MockConfigEntry,
+    dmr_device_mock: Mock,
+) -> None:
+    """Test missing upnp connection added.
+
+    We did not always add the upnp connection to the device registry, so we need to
+    check that it is added if missing as otherwise we might end up creating a new
+    device entry.
+    """
+    config_entry_mock.add_to_hass(hass)
+
+    # Cause connection attempts to fail before adding entity
+    ent_reg = async_get_er(hass)
+    entry = ent_reg.async_get_or_create(
+        MP_DOMAIN,
+        DOMAIN,
+        MOCK_DEVICE_UDN,
+        config_entry=config_entry_mock,
+    )
+    mock_entity_id = entry.entity_id
+
+    dev_reg = async_get_dr(hass)
+    device = dev_reg.async_get_or_create(
+        config_entry_id=config_entry_mock.entry_id,
+        connections={(CONNECTION_NETWORK_MAC, MOCK_MAC_ADDRESS)},
+        identifiers=set(),
+    )
+
+    ent_reg.async_update_entity(mock_entity_id, device_id=device.id)
+
+    domain_data_mock.upnp_factory.async_create_device.side_effect = UpnpConnectionError
+    assert await hass.config_entries.async_setup(config_entry_mock.entry_id) is True
+    await hass.async_block_till_done()
+
+    mock_state = hass.states.get(mock_entity_id)
+    assert mock_state is not None
+    assert mock_state.state == ha_const.STATE_UNAVAILABLE
+
+    # Check hass device information has not been filled in yet
+    dev_reg = async_get_dr(hass)
+    device = dev_reg.async_get(device.id)
+    assert device is not None
+    assert (CONNECTION_UPNP, MOCK_DEVICE_UDN) in device.connections