From 1b884489144769079a95647dcb659f0052251cd0 Mon Sep 17 00:00:00 2001 From: starkillerOG Date: Wed, 26 Jun 2024 09:34:45 +0200 Subject: [PATCH] Do not wait for Reolink firmware check (#120377) --- homeassistant/components/reolink/__init__.py | 25 +++++++++---------- homeassistant/components/reolink/host.py | 1 + tests/components/reolink/conftest.py | 1 + tests/components/reolink/test_config_flow.py | 12 ++++----- tests/components/reolink/test_init.py | 26 +++++++++++++++++--- 5 files changed, 41 insertions(+), 24 deletions(-) diff --git a/homeassistant/components/reolink/__init__.py b/homeassistant/components/reolink/__init__.py index a3e49f1f526..150a23dc64e 100644 --- a/homeassistant/components/reolink/__init__.py +++ b/homeassistant/components/reolink/__init__.py @@ -67,7 +67,7 @@ async def async_setup_entry(hass: HomeAssistant, config_entry: ConfigEntry) -> b raise ConfigEntryNotReady( f"Error while trying to setup {host.api.host}:{host.api.port}: {err!s}" ) from err - except Exception: + except BaseException: await host.stop() raise @@ -75,8 +75,6 @@ async def async_setup_entry(hass: HomeAssistant, config_entry: ConfigEntry) -> b hass.bus.async_listen_once(EVENT_HOMEASSISTANT_STOP, host.stop) ) - starting = True - async def async_device_config_update() -> None: """Update the host state cache and renew the ONVIF-subscription.""" async with asyncio.timeout(host.api.timeout * (RETRY_ATTEMPTS + 2)): @@ -103,7 +101,7 @@ async def async_setup_entry(hass: HomeAssistant, config_entry: ConfigEntry) -> b try: await host.api.check_new_firmware(host.firmware_ch_list) except ReolinkError as err: - if starting: + if host.starting: _LOGGER.debug( "Error checking Reolink firmware update at startup " "from %s, possibly internet access is blocked", @@ -116,6 +114,8 @@ async def async_setup_entry(hass: HomeAssistant, config_entry: ConfigEntry) -> b "if the camera is blocked from accessing the internet, " "disable the update entity" ) from err + finally: + host.starting = False device_coordinator = DataUpdateCoordinator( hass, @@ -131,17 +131,15 @@ async def async_setup_entry(hass: HomeAssistant, config_entry: ConfigEntry) -> b update_method=async_check_firmware_update, update_interval=FIRMWARE_UPDATE_INTERVAL, ) + + # If camera WAN blocked, firmware check fails and takes long, do not prevent setup + config_entry.async_create_task(hass, firmware_coordinator.async_refresh()) # Fetch initial data so we have data when entities subscribe - results = await asyncio.gather( - device_coordinator.async_config_entry_first_refresh(), - firmware_coordinator.async_config_entry_first_refresh(), - return_exceptions=True, - ) - # If camera WAN blocked, firmware check fails, do not prevent setup - # so don't check firmware_coordinator exceptions - if isinstance(results[0], BaseException): + try: + await device_coordinator.async_config_entry_first_refresh() + except BaseException: await host.stop() - raise results[0] + raise hass.data.setdefault(DOMAIN, {})[config_entry.entry_id] = ReolinkData( host=host, @@ -159,7 +157,6 @@ async def async_setup_entry(hass: HomeAssistant, config_entry: ConfigEntry) -> b config_entry.add_update_listener(entry_update_listener) ) - starting = False return True diff --git a/homeassistant/components/reolink/host.py b/homeassistant/components/reolink/host.py index bccb5c5b684..c9989f2c02b 100644 --- a/homeassistant/components/reolink/host.py +++ b/homeassistant/components/reolink/host.py @@ -79,6 +79,7 @@ class ReolinkHost: ) self.firmware_ch_list: list[int | None] = [] + self.starting: bool = True self.credential_errors: int = 0 self.webhook_id: str | None = None diff --git a/tests/components/reolink/conftest.py b/tests/components/reolink/conftest.py index 3541aa1f856..105815bae1d 100644 --- a/tests/components/reolink/conftest.py +++ b/tests/components/reolink/conftest.py @@ -87,6 +87,7 @@ def reolink_connect_class() -> Generator[MagicMock]: host_mock.camera_name.return_value = TEST_NVR_NAME host_mock.camera_hardware_version.return_value = "IPC_00001" host_mock.camera_sw_version.return_value = "v1.1.0.0.0.0000" + host_mock.camera_sw_version_update_required.return_value = False host_mock.camera_uid.return_value = TEST_UID_CAM host_mock.channel_for_uid.return_value = 0 host_mock.get_encoding.return_value = "h264" diff --git a/tests/components/reolink/test_config_flow.py b/tests/components/reolink/test_config_flow.py index de1e7a0bc83..ba845dc1697 100644 --- a/tests/components/reolink/test_config_flow.py +++ b/tests/components/reolink/test_config_flow.py @@ -397,7 +397,7 @@ async def test_dhcp_flow(hass: HomeAssistant, mock_setup_entry: MagicMock) -> No None, None, TEST_HOST2, - [TEST_HOST, TEST_HOST2, TEST_HOST2], + [TEST_HOST, TEST_HOST2], ), ( True, @@ -475,8 +475,8 @@ async def test_dhcp_ip_update( const.DOMAIN, context={"source": config_entries.SOURCE_DHCP}, data=dhcp_data ) - expected_calls = [ - call( + for host in host_call_list: + expected_call = call( host, TEST_USERNAME, TEST_PASSWORD, @@ -485,10 +485,10 @@ async def test_dhcp_ip_update( protocol=DEFAULT_PROTOCOL, timeout=DEFAULT_TIMEOUT, ) - for host in host_call_list - ] + assert expected_call in reolink_connect_class.call_args_list - assert reolink_connect_class.call_args_list == expected_calls + for exc_call in reolink_connect_class.call_args_list: + assert exc_call[0][0] in host_call_list assert result["type"] is FlowResultType.ABORT assert result["reason"] == "already_configured" diff --git a/tests/components/reolink/test_init.py b/tests/components/reolink/test_init.py index 922fe0829f6..a6c798f9415 100644 --- a/tests/components/reolink/test_init.py +++ b/tests/components/reolink/test_init.py @@ -1,5 +1,6 @@ """Test the Reolink init.""" +import asyncio from datetime import timedelta from typing import Any from unittest.mock import AsyncMock, MagicMock, Mock, patch @@ -39,6 +40,11 @@ from tests.common import MockConfigEntry, async_fire_time_changed pytestmark = pytest.mark.usefixtures("reolink_connect", "reolink_platforms") +async def test_wait(*args, **key_args): + """Ensure a mocked function takes a bit of time to be able to timeout in test.""" + await asyncio.sleep(0) + + @pytest.mark.parametrize( ("attr", "value", "expected"), [ @@ -377,9 +383,13 @@ async def test_no_repair_issue( async def test_https_repair_issue( - hass: HomeAssistant, config_entry: MockConfigEntry, issue_registry: ir.IssueRegistry + hass: HomeAssistant, + config_entry: MockConfigEntry, + reolink_connect: MagicMock, + issue_registry: ir.IssueRegistry, ) -> None: """Test repairs issue is raised when https local url is used.""" + reolink_connect.get_states = test_wait await async_process_ha_core_config( hass, {"country": "GB", "internal_url": "https://test_homeassistant_address"} ) @@ -400,9 +410,13 @@ async def test_https_repair_issue( async def test_ssl_repair_issue( - hass: HomeAssistant, config_entry: MockConfigEntry, issue_registry: ir.IssueRegistry + hass: HomeAssistant, + config_entry: MockConfigEntry, + reolink_connect: MagicMock, + issue_registry: ir.IssueRegistry, ) -> None: """Test repairs issue is raised when global ssl certificate is used.""" + reolink_connect.get_states = test_wait assert await async_setup_component(hass, "webhook", {}) hass.config.api.use_ssl = True @@ -446,9 +460,13 @@ async def test_port_repair_issue( async def test_webhook_repair_issue( - hass: HomeAssistant, config_entry: MockConfigEntry, issue_registry: ir.IssueRegistry + hass: HomeAssistant, + config_entry: MockConfigEntry, + reolink_connect: MagicMock, + issue_registry: ir.IssueRegistry, ) -> None: """Test repairs issue is raised when the webhook url is unreachable.""" + reolink_connect.get_states = test_wait with ( patch("homeassistant.components.reolink.host.FIRST_ONVIF_TIMEOUT", new=0), patch( @@ -471,7 +489,7 @@ async def test_firmware_repair_issue( issue_registry: ir.IssueRegistry, ) -> None: """Test firmware issue is raised when too old firmware is used.""" - reolink_connect.sw_version_update_required = True + reolink_connect.camera_sw_version_update_required.return_value = True assert await hass.config_entries.async_setup(config_entry.entry_id) await hass.async_block_till_done()