From c0d5ceb18c9eb817125de0a88f202d990f7bbf79 Mon Sep 17 00:00:00 2001 From: starkillerOG Date: Sun, 1 Jan 2023 23:32:17 +0100 Subject: [PATCH] Process late feedback for Reolink (#84884) Co-authored-by: Martin Hjelmare --- homeassistant/components/reolink/__init__.py | 43 ++++++++------ homeassistant/components/reolink/camera.py | 25 +++++--- .../components/reolink/config_flow.py | 59 ++++++++++--------- homeassistant/components/reolink/const.py | 4 -- homeassistant/components/reolink/entity.py | 44 +++++++------- homeassistant/components/reolink/host.py | 27 ++++----- .../components/reolink/manifest.json | 2 - tests/components/reolink/test_config_flow.py | 17 +++++- 8 files changed, 120 insertions(+), 101 deletions(-) diff --git a/homeassistant/components/reolink/__init__.py b/homeassistant/components/reolink/__init__.py index 2b565a6d4b8..db61e4aa627 100644 --- a/homeassistant/components/reolink/__init__.py +++ b/homeassistant/components/reolink/__init__.py @@ -12,16 +12,19 @@ import async_timeout from reolink_ip.exceptions import ApiError, InvalidContentTypeError from homeassistant.config_entries import ConfigEntry -from homeassistant.const import EVENT_HOMEASSISTANT_STOP +from homeassistant.const import EVENT_HOMEASSISTANT_STOP, Platform from homeassistant.core import HomeAssistant from homeassistant.exceptions import ConfigEntryNotReady from homeassistant.helpers.update_coordinator import DataUpdateCoordinator -from .const import DEVICE_UPDATE_INTERVAL, DOMAIN, PLATFORMS +from .const import DOMAIN from .host import ReolinkHost _LOGGER = logging.getLogger(__name__) +PLATFORMS = [Platform.CAMERA] +DEVICE_UPDATE_INTERVAL = 60 + @dataclass class ReolinkData: @@ -31,14 +34,15 @@ class ReolinkData: device_coordinator: DataUpdateCoordinator -async def async_setup_entry(hass: HomeAssistant, entry: ConfigEntry) -> bool: +async def async_setup_entry(hass: HomeAssistant, config_entry: ConfigEntry) -> bool: """Set up Reolink from a config entry.""" - host = ReolinkHost(hass, dict(entry.data), dict(entry.options)) + host = ReolinkHost(hass, config_entry.data, config_entry.options) try: if not await host.async_init(): raise ConfigEntryNotReady( - f"Error while trying to setup {host.api.host}:{host.api.port}: failed to obtain data from device." + f"Error while trying to setup {host.api.host}:{host.api.port}: " + "failed to obtain data from device." ) except ( ClientConnectorError, @@ -50,14 +54,15 @@ async def async_setup_entry(hass: HomeAssistant, entry: ConfigEntry) -> bool: f'Error while trying to setup {host.api.host}:{host.api.port}: "{str(err)}".' ) from err - entry.async_on_unload( + config_entry.async_on_unload( hass.bus.async_listen_once(EVENT_HOMEASSISTANT_STOP, host.stop) ) async def async_device_config_update(): - """Perform the update of the host config-state cache, and renew the ONVIF-subscription.""" + """Update the host state cache and renew the ONVIF-subscription.""" async with async_timeout.timeout(host.api.timeout): - await host.update_states() # Login session is implicitly updated here, so no need to explicitly do it in a timer + # Login session is implicitly updated here + await host.update_states() coordinator_device_config_update = DataUpdateCoordinator( hass, @@ -69,30 +74,34 @@ async def async_setup_entry(hass: HomeAssistant, entry: ConfigEntry) -> bool: # Fetch initial data so we have data when entities subscribe await coordinator_device_config_update.async_config_entry_first_refresh() - hass.data.setdefault(DOMAIN, {})[entry.entry_id] = ReolinkData( + hass.data.setdefault(DOMAIN, {})[config_entry.entry_id] = ReolinkData( host=host, device_coordinator=coordinator_device_config_update, ) - await hass.config_entries.async_forward_entry_setups(entry, PLATFORMS) + await hass.config_entries.async_forward_entry_setups(config_entry, PLATFORMS) - entry.async_on_unload(entry.add_update_listener(entry_update_listener)) + config_entry.async_on_unload( + config_entry.add_update_listener(entry_update_listener) + ) return True -async def entry_update_listener(hass: HomeAssistant, entry: ConfigEntry): +async def entry_update_listener(hass: HomeAssistant, config_entry: ConfigEntry): """Update the configuration of the host entity.""" - await hass.config_entries.async_reload(entry.entry_id) + await hass.config_entries.async_reload(config_entry.entry_id) -async def async_unload_entry(hass: HomeAssistant, entry: ConfigEntry) -> bool: +async def async_unload_entry(hass: HomeAssistant, config_entry: ConfigEntry) -> bool: """Unload a config entry.""" - host: ReolinkHost = hass.data[DOMAIN][entry.entry_id].host + host: ReolinkHost = hass.data[DOMAIN][config_entry.entry_id].host await host.stop() - if unload_ok := await hass.config_entries.async_unload_platforms(entry, PLATFORMS): - hass.data[DOMAIN].pop(entry.entry_id) + if unload_ok := await hass.config_entries.async_unload_platforms( + config_entry, PLATFORMS + ): + hass.data[DOMAIN].pop(config_entry.entry_id) return unload_ok diff --git a/homeassistant/components/reolink/camera.py b/homeassistant/components/reolink/camera.py index 97369edcd64..aafc9686eea 100644 --- a/homeassistant/components/reolink/camera.py +++ b/homeassistant/components/reolink/camera.py @@ -8,9 +8,9 @@ from homeassistant.config_entries import ConfigEntry from homeassistant.core import HomeAssistant from homeassistant.helpers.entity_platform import AddEntitiesCallback +from . import ReolinkData from .const import DOMAIN from .entity import ReolinkCoordinatorEntity -from .host import ReolinkHost _LOGGER = logging.getLogger(__name__) @@ -18,10 +18,11 @@ _LOGGER = logging.getLogger(__name__) async def async_setup_entry( hass: HomeAssistant, config_entry: ConfigEntry, - async_add_devices: AddEntitiesCallback, + async_add_entities: AddEntitiesCallback, ) -> None: """Set up a Reolink IP Camera.""" - host: ReolinkHost = hass.data[DOMAIN][config_entry.entry_id].host + reolink_data: ReolinkData = hass.data[DOMAIN][config_entry.entry_id] + host = reolink_data.host cameras = [] for channel in host.api.channels: @@ -30,25 +31,31 @@ async def async_setup_entry( streams.append("ext") for stream in streams: - cameras.append(ReolinkCamera(hass, config_entry, channel, stream)) + cameras.append(ReolinkCamera(reolink_data, config_entry, channel, stream)) - async_add_devices(cameras, update_before_add=True) + async_add_entities(cameras, update_before_add=True) class ReolinkCamera(ReolinkCoordinatorEntity, Camera): """An implementation of a Reolink IP camera.""" _attr_supported_features: CameraEntityFeature = CameraEntityFeature.STREAM + _attr_has_entity_name = True - def __init__(self, hass, config, channel, stream): + def __init__( + self, + reolink_data: ReolinkData, + config_entry: ConfigEntry, + channel: int, + stream: str, + ) -> None: """Initialize Reolink camera stream.""" - ReolinkCoordinatorEntity.__init__(self, hass, config) + ReolinkCoordinatorEntity.__init__(self, reolink_data, config_entry, channel) Camera.__init__(self) - self._channel = channel self._stream = stream - self._attr_name = f"{self._host.api.camera_name(self._channel)} {self._stream}" + self._attr_name = self._stream self._attr_unique_id = f"{self._host.unique_id}_{self._channel}_{self._stream}" self._attr_entity_registry_enabled_default = stream == "sub" diff --git a/homeassistant/components/reolink/config_flow.py b/homeassistant/components/reolink/config_flow.py index 169e2624d46..c351a125056 100644 --- a/homeassistant/components/reolink/config_flow.py +++ b/homeassistant/components/reolink/config_flow.py @@ -2,7 +2,7 @@ from __future__ import annotations import logging -from typing import cast +from typing import Any from reolink_ip.exceptions import ApiError, CredentialsInvalidError import voluptuous as vol @@ -18,6 +18,8 @@ from .host import ReolinkHost _LOGGER = logging.getLogger(__name__) +DEFAULT_OPTIONS = {CONF_PROTOCOL: DEFAULT_PROTOCOL} + class ReolinkOptionsFlowHandler(config_entries.OptionsFlow): """Handle Reolink options.""" @@ -26,10 +28,12 @@ class ReolinkOptionsFlowHandler(config_entries.OptionsFlow): """Initialize ReolinkOptionsFlowHandler.""" self.config_entry = config_entry - async def async_step_init(self, user_input=None) -> FlowResult: + async def async_step_init( + self, user_input: dict[str, Any] | None = None + ) -> FlowResult: """Manage the Reolink options.""" if user_input is not None: - return self.async_create_entry(title="", data=user_input) + return self.async_create_entry(data=user_input) return self.async_show_form( step_id="init", @@ -37,9 +41,7 @@ class ReolinkOptionsFlowHandler(config_entries.OptionsFlow): { vol.Required( CONF_PROTOCOL, - default=self.config_entry.options.get( - CONF_PROTOCOL, DEFAULT_PROTOCOL - ), + default=self.config_entry.options[CONF_PROTOCOL], ): vol.In(["rtsp", "rtmp"]), } ), @@ -51,8 +53,6 @@ class ReolinkFlowHandler(config_entries.ConfigFlow, domain=DOMAIN): VERSION = 1 - host: ReolinkHost | None = None - @staticmethod @callback def async_get_options_flow( @@ -61,14 +61,16 @@ class ReolinkFlowHandler(config_entries.ConfigFlow, domain=DOMAIN): """Options callback for Reolink.""" return ReolinkOptionsFlowHandler(config_entry) - async def async_step_user(self, user_input=None) -> FlowResult: + async def async_step_user( + self, user_input: dict[str, Any] | None = None + ) -> FlowResult: """Handle the initial step.""" errors = {} placeholders = {} if user_input is not None: try: - await self.async_obtain_host_settings(self.hass, user_input) + host = await async_obtain_host_settings(self.hass, user_input) except CannotConnect: errors[CONF_HOST] = "cannot_connect" except CredentialsInvalidError: @@ -81,19 +83,17 @@ class ReolinkFlowHandler(config_entries.ConfigFlow, domain=DOMAIN): placeholders["error"] = str(err) errors[CONF_HOST] = "unknown" - self.host = cast(ReolinkHost, self.host) - if not errors: - user_input[CONF_PORT] = self.host.api.port - user_input[CONF_USE_HTTPS] = self.host.api.use_https + user_input[CONF_PORT] = host.api.port + user_input[CONF_USE_HTTPS] = host.api.use_https - await self.async_set_unique_id( - self.host.unique_id, raise_on_progress=False - ) + await self.async_set_unique_id(host.unique_id, raise_on_progress=False) self._abort_if_unique_id_configured(updates=user_input) return self.async_create_entry( - title=str(self.host.api.nvr_name), data=user_input + title=str(host.api.nvr_name), + data=user_input, + options=DEFAULT_OPTIONS, ) data_schema = vol.Schema( @@ -118,19 +118,20 @@ class ReolinkFlowHandler(config_entries.ConfigFlow, domain=DOMAIN): description_placeholders=placeholders, ) - async def async_obtain_host_settings( - self, hass: core.HomeAssistant, user_input: dict - ): - """Initialize the Reolink host and get the host information.""" - host = ReolinkHost(hass, user_input, {}) - try: - if not await host.async_init(): - raise CannotConnect - finally: - await host.stop() +async def async_obtain_host_settings( + hass: core.HomeAssistant, user_input: dict +) -> ReolinkHost: + """Initialize the Reolink host and get the host information.""" + host = ReolinkHost(hass, user_input, DEFAULT_OPTIONS) - self.host = host + try: + if not await host.async_init(): + raise CannotConnect + finally: + await host.stop() + + return host class CannotConnect(exceptions.HomeAssistantError): diff --git a/homeassistant/components/reolink/const.py b/homeassistant/components/reolink/const.py index 95bd5da3c96..180c3ccae11 100644 --- a/homeassistant/components/reolink/const.py +++ b/homeassistant/components/reolink/const.py @@ -1,13 +1,9 @@ """Constants for the Reolink Camera integration.""" DOMAIN = "reolink" -PLATFORMS = ["camera"] CONF_USE_HTTPS = "use_https" CONF_PROTOCOL = "protocol" DEFAULT_PROTOCOL = "rtsp" DEFAULT_TIMEOUT = 60 - -HOST = "host" -DEVICE_UPDATE_INTERVAL = 60 diff --git a/homeassistant/components/reolink/entity.py b/homeassistant/components/reolink/entity.py index 7e210114556..403ea278889 100644 --- a/homeassistant/components/reolink/entity.py +++ b/homeassistant/components/reolink/entity.py @@ -1,5 +1,7 @@ """Reolink parent entity class.""" +from __future__ import annotations +from homeassistant.config_entries import ConfigEntry from homeassistant.helpers.device_registry import CONNECTION_NETWORK_MAC from homeassistant.helpers.entity import DeviceInfo from homeassistant.helpers.update_coordinator import CoordinatorEntity @@ -11,24 +13,20 @@ from .const import DOMAIN class ReolinkCoordinatorEntity(CoordinatorEntity): """Parent class for Reolink Entities.""" - def __init__(self, hass, config): + def __init__( + self, reolink_data: ReolinkData, config_entry: ConfigEntry, channel: int | None + ) -> None: """Initialize ReolinkCoordinatorEntity.""" - self._hass = hass - entry_data: ReolinkData = self._hass.data[DOMAIN][config.entry_id] - coordinator = entry_data.device_coordinator + coordinator = reolink_data.device_coordinator super().__init__(coordinator) - self._host = entry_data.host - self._channel = None + self._host = reolink_data.host + self._channel = channel - @property - def device_info(self): - """Information about this entity/device.""" http_s = "https" if self._host.api.use_https else "http" conf_url = f"{http_s}://{self._host.api.host}:{self._host.api.port}" - if self._host.api.is_nvr and self._channel is not None: - return DeviceInfo( + self._attr_device_info = DeviceInfo( identifiers={(DOMAIN, f"{self._host.unique_id}_ch{self._channel}")}, via_device=(DOMAIN, self._host.unique_id), name=self._host.api.camera_name(self._channel), @@ -36,19 +34,19 @@ class ReolinkCoordinatorEntity(CoordinatorEntity): manufacturer=self._host.api.manufacturer, configuration_url=conf_url, ) - - return DeviceInfo( - identifiers={(DOMAIN, self._host.unique_id)}, - connections={(CONNECTION_NETWORK_MAC, self._host.api.mac_address)}, - name=self._host.api.nvr_name, - model=self._host.api.model, - manufacturer=self._host.api.manufacturer, - hw_version=self._host.api.hardware_version, - sw_version=self._host.api.sw_version, - configuration_url=conf_url, - ) + else: + self._attr_device_info = DeviceInfo( + identifiers={(DOMAIN, self._host.unique_id)}, + connections={(CONNECTION_NETWORK_MAC, self._host.api.mac_address)}, + name=self._host.api.nvr_name, + model=self._host.api.model, + manufacturer=self._host.api.manufacturer, + hw_version=self._host.api.hardware_version, + sw_version=self._host.api.sw_version, + configuration_url=conf_url, + ) @property def available(self) -> bool: """Return True if entity is available.""" - return self._host.api.session_active + return self._host.api.session_active and super().available diff --git a/homeassistant/components/reolink/host.py b/homeassistant/components/reolink/host.py index fbd88c94ccc..0a5e378a78d 100644 --- a/homeassistant/components/reolink/host.py +++ b/homeassistant/components/reolink/host.py @@ -2,7 +2,9 @@ from __future__ import annotations import asyncio +from collections.abc import Mapping import logging +from typing import Any import aiohttp from reolink_ip.api import Host @@ -16,7 +18,7 @@ from homeassistant.const import CONF_HOST, CONF_PASSWORD, CONF_PORT, CONF_USERNA from homeassistant.core import HomeAssistant from homeassistant.helpers.device_registry import format_mac -from .const import CONF_PROTOCOL, CONF_USE_HTTPS, DEFAULT_PROTOCOL, DEFAULT_TIMEOUT +from .const import CONF_PROTOCOL, CONF_USE_HTTPS, DEFAULT_TIMEOUT _LOGGER = logging.getLogger(__name__) @@ -27,18 +29,14 @@ class ReolinkHost: def __init__( self, hass: HomeAssistant, - config: dict, - options: dict, + config: Mapping[str, Any], + options: Mapping[str, Any], ) -> None: """Initialize Reolink Host. Could be either NVR, or Camera.""" self._hass: HomeAssistant = hass self._clientsession: aiohttp.ClientSession | None = None - self._unique_id: str | None = None - - cur_protocol = ( - DEFAULT_PROTOCOL if CONF_PROTOCOL not in options else options[CONF_PROTOCOL] - ) + self._unique_id: str = "" self._api = Host( config[CONF_HOST], @@ -46,12 +44,12 @@ class ReolinkHost: config[CONF_PASSWORD], port=config.get(CONF_PORT), use_https=config.get(CONF_USE_HTTPS), - protocol=cur_protocol, + protocol=options[CONF_PROTOCOL], timeout=DEFAULT_TIMEOUT, ) @property - def unique_id(self): + def unique_id(self) -> str: """Create the unique ID, base for all entities.""" return self._unique_id @@ -99,23 +97,22 @@ class ReolinkHost: ): if enable_onvif: _LOGGER.error( - "Unable to switch on ONVIF on %s. You need it to be ON to receive notifications", + "Failed to enable ONVIF on %s. Set it to ON to receive notifications", self._api.nvr_name, ) if enable_rtmp: _LOGGER.error( - "Unable to switch on RTMP on %s. You need it to be ON", + "Failed to enable RTMP on %s. Set it to ON", self._api.nvr_name, ) elif enable_rtsp: _LOGGER.error( - "Unable to switch on RTSP on %s. You need it to be ON", + "Failed to enable RTSP on %s. Set it to ON", self._api.nvr_name, ) - if self._unique_id is None: - self._unique_id = format_mac(self._api.mac_address) + self._unique_id = format_mac(self._api.mac_address) return True diff --git a/homeassistant/components/reolink/manifest.json b/homeassistant/components/reolink/manifest.json index 4db59caa42f..b5483be23ab 100644 --- a/homeassistant/components/reolink/manifest.json +++ b/homeassistant/components/reolink/manifest.json @@ -4,8 +4,6 @@ "config_flow": true, "documentation": "https://www.home-assistant.io/integrations/reolink", "requirements": ["reolink-ip==0.0.40"], - "dependencies": ["webhook"], - "after_dependencies": ["http"], "codeowners": ["@starkillerOG", "@JimStar"], "iot_class": "local_polling", "loggers": ["reolink-ip"] diff --git a/tests/components/reolink/test_config_flow.py b/tests/components/reolink/test_config_flow.py index fcf9280eb9a..ad017186075 100644 --- a/tests/components/reolink/test_config_flow.py +++ b/tests/components/reolink/test_config_flow.py @@ -82,6 +82,9 @@ async def test_config_flow_manual_success(hass): CONF_PORT: TEST_PORT, const.CONF_USE_HTTPS: TEST_USE_HTTPS, } + assert result["options"] == { + const.CONF_PROTOCOL: const.DEFAULT_PROTOCOL, + } async def test_config_flow_errors(hass): @@ -174,6 +177,9 @@ async def test_config_flow_errors(hass): CONF_PORT: TEST_PORT, const.CONF_USE_HTTPS: TEST_USE_HTTPS, } + assert result["options"] == { + const.CONF_PROTOCOL: const.DEFAULT_PROTOCOL, + } async def test_options_flow(hass): @@ -188,6 +194,9 @@ async def test_options_flow(hass): CONF_PORT: TEST_PORT, const.CONF_USE_HTTPS: TEST_USE_HTTPS, }, + options={ + const.CONF_PROTOCOL: "rtsp", + }, title=TEST_NVR_NAME, ) config_entry.add_to_hass(hass) @@ -202,12 +211,12 @@ async def test_options_flow(hass): result = await hass.config_entries.options.async_configure( result["flow_id"], - user_input={const.CONF_PROTOCOL: "rtsp"}, + user_input={const.CONF_PROTOCOL: "rtmp"}, ) assert result["type"] == data_entry_flow.FlowResultType.CREATE_ENTRY assert config_entry.options == { - const.CONF_PROTOCOL: "rtsp", + const.CONF_PROTOCOL: "rtmp", } @@ -223,6 +232,9 @@ async def test_change_connection_settings(hass): CONF_PORT: TEST_PORT, const.CONF_USE_HTTPS: TEST_USE_HTTPS, }, + options={ + const.CONF_PROTOCOL: const.DEFAULT_PROTOCOL, + }, title=TEST_NVR_NAME, ) config_entry.add_to_hass(hass) @@ -245,6 +257,7 @@ async def test_change_connection_settings(hass): ) assert result["type"] is data_entry_flow.FlowResultType.ABORT + assert result["reason"] == "already_configured" assert config_entry.data[CONF_HOST] == TEST_HOST2 assert config_entry.data[CONF_USERNAME] == TEST_USERNAME2 assert config_entry.data[CONF_PASSWORD] == TEST_PASSWORD2