Fix polling and update of camera state for synology_dsm (#43683)

Co-authored-by: J. Nick Koston <nick@koston.org>

homeassistant/components/synology_dsm/__init__.py

@@ -4,6 +4,7 @@ from datetime import timedelta
 import logging
 from typing import Dict
 
+import async_timeout
 from synology_dsm import SynologyDSM
 from synology_dsm.api.core.security import SynoCoreSecurity
 from synology_dsm.api.core.system import SynoCoreSystem
@@ -14,6 +15,7 @@ from synology_dsm.api.dsm.network import SynoDSMNetwork
 from synology_dsm.api.storage.storage import SynoStorage
 from synology_dsm.api.surveillance_station import SynoSurveillanceStation
 from synology_dsm.exceptions import (
+    SynologyDSMAPIErrorException,
     SynologyDSMLoginFailedException,
     SynologyDSMRequestException,
 )
@@ -44,10 +46,16 @@ from homeassistant.helpers.dispatcher import (
 from homeassistant.helpers.entity import Entity
 from homeassistant.helpers.event import async_track_time_interval
 from homeassistant.helpers.typing import HomeAssistantType
+from homeassistant.helpers.update_coordinator import (
+    CoordinatorEntity,
+    DataUpdateCoordinator,
+    UpdateFailed,
+)
 
 from .const import (
     CONF_SERIAL,
     CONF_VOLUMES,
+    COORDINATOR_SURVEILLANCE,
     DEFAULT_SCAN_INTERVAL,
     DEFAULT_USE_SSL,
     DEFAULT_VERIFY_SSL,
@@ -185,7 +193,7 @@ async def async_setup_entry(hass: HomeAssistantType, entry: ConfigEntry):
     try:
         await api.async_setup()
     except (SynologyDSMLoginFailedException, SynologyDSMRequestException) as err:
-        _LOGGER.debug("async_setup_entry - Unable to connect to DSM: %s", err)
+        _LOGGER.debug("async_setup_entry() - Unable to connect to DSM: %s", err)
         raise ConfigEntryNotReady from err
 
     undo_listener = entry.add_update_listener(_async_update_listener)
@@ -206,6 +214,35 @@ async def async_setup_entry(hass: HomeAssistantType, entry: ConfigEntry):
             entry, data={**entry.data, CONF_MAC: network.macs}
         )
 
+    # setup DataUpdateCoordinator
+    async def async_coordinator_update_data_surveillance_station():
+        """Fetch all surveillance station data from api."""
+        surveillance_station = api.surveillance_station
+        try:
+            async with async_timeout.timeout(10):
+                await hass.async_add_executor_job(surveillance_station.update)
+        except SynologyDSMAPIErrorException as err:
+            raise UpdateFailed(f"Error communicating with API: {err}") from err
+
+        if SynoSurveillanceStation.CAMERA_API_KEY not in api.dsm.apis:
+            return
+
+        return {
+            "cameras": {
+                camera.id: camera for camera in surveillance_station.get_all_cameras()
+            }
+        }
+
+    hass.data[DOMAIN][entry.unique_id][
+        COORDINATOR_SURVEILLANCE
+    ] = DataUpdateCoordinator(
+        hass,
+        _LOGGER,
+        name=f"{entry.unique_id}_surveillance_station",
+        update_method=async_coordinator_update_data_surveillance_station,
+        update_interval=timedelta(seconds=30),
+    )
+
     for platform in PLATFORMS:
         hass.async_create_task(
             hass.config_entries.async_forward_entry_setup(entry, platform)
@@ -314,6 +351,7 @@ class SynoApi:
 
     async def async_setup(self):
         """Start interacting with the NAS."""
+        # init SynologyDSM object and login
         self.dsm = SynologyDSM(
             self._entry.data[CONF_HOST],
             self._entry.data[CONF_PORT],
@@ -326,9 +364,14 @@ class SynoApi:
         )
         await self._hass.async_add_executor_job(self.dsm.login)
 
+        # check if surveillance station is used
         self._with_surveillance_station = bool(
             self.dsm.apis.get(SynoSurveillanceStation.CAMERA_API_KEY)
         )
+        _LOGGER.debug(
+            "SynoAPI.async_setup() - self._with_surveillance_station:%s",
+            self._with_surveillance_station,
+        )
 
         self._async_setup_api_requests()
 
@@ -348,6 +391,9 @@ class SynoApi:
     @callback
     def subscribe(self, api_key, unique_id):
         """Subscribe an entity from API fetches."""
+        _LOGGER.debug(
+            "SynoAPI.subscribe() - api_key:%s, unique_id:%s", api_key, unique_id
+        )
         if api_key not in self._fetching_entities:
             self._fetching_entities[api_key] = set()
         self._fetching_entities[api_key].add(unique_id)
@@ -362,8 +408,16 @@ class SynoApi:
     @callback
     def _async_setup_api_requests(self):
         """Determine if we should fetch each API, if one entity needs it."""
+        _LOGGER.debug(
+            "SynoAPI._async_setup_api_requests() - self._fetching_entities:%s",
+            self._fetching_entities,
+        )
+
         # Entities not added yet, fetch all
         if not self._fetching_entities:
+            _LOGGER.debug(
+                "SynoAPI._async_setup_api_requests() - Entities not added yet, fetch all"
+            )
             return
 
         # Determine if we should fetch an API
@@ -380,33 +434,39 @@ class SynoApi:
             self._fetching_entities.get(SynoDSMInformation.API_KEY)
         )
         self._with_surveillance_station = bool(
-            self._fetching_entities.get(SynoSurveillanceStation.CAMERA_API_KEY)
+            self.dsm.apis.get(SynoSurveillanceStation.CAMERA_API_KEY)
-        ) or bool(
-            self._fetching_entities.get(SynoSurveillanceStation.HOME_MODE_API_KEY)
         )
 
         # Reset not used API, information is not reset since it's used in device_info
         if not self._with_security:
+            _LOGGER.debug("SynoAPI._async_setup_api_requests() - disable security")
             self.dsm.reset(self.security)
             self.security = None
 
         if not self._with_storage:
+            _LOGGER.debug("SynoAPI._async_setup_api_requests() - disable storage")
             self.dsm.reset(self.storage)
             self.storage = None
 
         if not self._with_system:
+            _LOGGER.debug("SynoAPI._async_setup_api_requests() - disable system")
             self.dsm.reset(self.system)
             self.system = None
 
         if not self._with_upgrade:
+            _LOGGER.debug("SynoAPI._async_setup_api_requests() - disable upgrade")
             self.dsm.reset(self.upgrade)
             self.upgrade = None
 
         if not self._with_utilisation:
+            _LOGGER.debug("SynoAPI._async_setup_api_requests() - disable utilisation")
             self.dsm.reset(self.utilisation)
             self.utilisation = None
 
         if not self._with_surveillance_station:
+            _LOGGER.debug(
+                "SynoAPI._async_setup_api_requests() - disable surveillance_station"
+            )
             self.dsm.reset(self.surveillance_station)
             self.surveillance_station = None
 
@@ -417,34 +477,42 @@ class SynoApi:
         self.network.update()
 
         if self._with_security:
+            _LOGGER.debug("SynoAPI._fetch_device_configuration() - fetch security")
             self.security = self.dsm.security
 
         if self._with_storage:
+            _LOGGER.debug("SynoAPI._fetch_device_configuration() - fetch storage")
             self.storage = self.dsm.storage
 
         if self._with_upgrade:
+            _LOGGER.debug("SynoAPI._fetch_device_configuration() - fetch upgrade")
             self.upgrade = self.dsm.upgrade
 
         if self._with_system:
+            _LOGGER.debug("SynoAPI._fetch_device_configuration() - fetch system")
             self.system = self.dsm.system
 
         if self._with_utilisation:
+            _LOGGER.debug("SynoAPI._fetch_device_configuration() - fetch utilisation")
             self.utilisation = self.dsm.utilisation
 
         if self._with_surveillance_station:
+            _LOGGER.debug(
+                "SynoAPI._fetch_device_configuration() - fetch surveillance_station"
+            )
             self.surveillance_station = self.dsm.surveillance_station
 
     async def async_reboot(self):
         """Reboot NAS."""
         if not self.system:
-            _LOGGER.debug("async_reboot - System API not ready: %s", self)
+            _LOGGER.debug("SynoAPI.async_reboot() - System API not ready: %s", self)
             return
         await self._hass.async_add_executor_job(self.system.reboot)
 
     async def async_shutdown(self):
         """Shutdown NAS."""
         if not self.system:
-            _LOGGER.debug("async_shutdown - System API not ready: %s", self)
+            _LOGGER.debug("SynoAPI.async_shutdown() - System API not ready: %s", self)
             return
         await self._hass.async_add_executor_job(self.system.shutdown)
 
@@ -454,6 +522,7 @@ class SynoApi:
 
     async def async_update(self, now=None):
         """Update function for updating API information."""
+        _LOGGER.debug("SynoAPI.async_update()")
         self._async_setup_api_requests()
         try:
             await self._hass.async_add_executor_job(
@@ -463,13 +532,13 @@ class SynoApi:
             _LOGGER.warning(
                 "async_update - connection error during update, fallback by reloading the entry"
             )
-            _LOGGER.debug("async_update - exception: %s", err)
+            _LOGGER.debug("SynoAPI.async_update() - exception: %s", err)
             await self._hass.config_entries.async_reload(self._entry.entry_id)
             return
         async_dispatcher_send(self._hass, self.signal_sensor_update)
 
 
-class SynologyDSMEntity(Entity):
+class SynologyDSMBaseEntity(Entity):
     """Representation of a Synology NAS entry."""
 
     def __init__(
@@ -479,8 +548,6 @@ class SynologyDSMEntity(Entity):
         entity_info: Dict[str, str],
     ):
         """Initialize the Synology DSM entity."""
-        super().__init__()
-
         self._api = api
         self._api_key = entity_type.split(":")[0]
         self.entity_type = entity_type.split(":")[-1]
@@ -539,6 +606,20 @@ class SynologyDSMEntity(Entity):
         """Return if the entity should be enabled when first added to the entity registry."""
         return self._enable_default
 
+
+class SynologyDSMDispatcherEntity(SynologyDSMBaseEntity, Entity):
+    """Representation of a Synology NAS entry."""
+
+    def __init__(
+        self,
+        api: SynoApi,
+        entity_type: str,
+        entity_info: Dict[str, str],
+    ):
+        """Initialize the Synology DSM entity."""
+        super().__init__(api, entity_type, entity_info)
+        Entity.__init__(self)
+
     @property
     def should_poll(self) -> bool:
         """No polling needed."""
@@ -562,7 +643,22 @@ class SynologyDSMEntity(Entity):
         self.async_on_remove(self._api.subscribe(self._api_key, self.unique_id))
 
 
-class SynologyDSMDeviceEntity(SynologyDSMEntity):
+class SynologyDSMCoordinatorEntity(SynologyDSMBaseEntity, CoordinatorEntity):
+    """Representation of a Synology NAS entry."""
+
+    def __init__(
+        self,
+        api: SynoApi,
+        entity_type: str,
+        entity_info: Dict[str, str],
+        coordinator: DataUpdateCoordinator,
+    ):
+        """Initialize the Synology DSM entity."""
+        super().__init__(api, entity_type, entity_info)
+        CoordinatorEntity.__init__(self, coordinator)
+
+
+class SynologyDSMDeviceEntity(SynologyDSMDispatcherEntity):
     """Representation of a Synology NAS disk or volume entry."""
 
     def __init__(

homeassistant/components/synology_dsm/binary_sensor.py

@@ -6,7 +6,7 @@ from homeassistant.config_entries import ConfigEntry
 from homeassistant.const import CONF_DISKS
 from homeassistant.helpers.typing import HomeAssistantType
 
-from . import SynologyDSMDeviceEntity, SynologyDSMEntity
+from . import SynologyDSMDeviceEntity, SynologyDSMDispatcherEntity
 from .const import (
     DOMAIN,
     SECURITY_BINARY_SENSORS,
@@ -50,7 +50,7 @@ async def async_setup_entry(
     async_add_entities(entities)
 
 
-class SynoDSMSecurityBinarySensor(SynologyDSMEntity, BinarySensorEntity):
+class SynoDSMSecurityBinarySensor(SynologyDSMDispatcherEntity, BinarySensorEntity):
     """Representation a Synology Security binary sensor."""
 
     @property
@@ -78,7 +78,7 @@ class SynoDSMStorageBinarySensor(SynologyDSMDeviceEntity, BinarySensorEntity):
         return getattr(self._api.storage, self.entity_type)(self._device_id)
 
 
-class SynoDSMUpgradeBinarySensor(SynologyDSMEntity, BinarySensorEntity):
+class SynoDSMUpgradeBinarySensor(SynologyDSMDispatcherEntity, BinarySensorEntity):
     """Representation a Synology Upgrade binary sensor."""
 
     @property

homeassistant/components/synology_dsm/camera.py

@@ -1,15 +1,18 @@
 """Support for Synology DSM cameras."""
+import logging
 from typing import Dict
 
 from synology_dsm.api.surveillance_station import SynoSurveillanceStation
-from synology_dsm.api.surveillance_station.camera import SynoCamera
+from synology_dsm.exceptions import SynologyDSMAPIErrorException
 
 from homeassistant.components.camera import SUPPORT_STREAM, Camera
 from homeassistant.config_entries import ConfigEntry
 from homeassistant.helpers.typing import HomeAssistantType
+from homeassistant.helpers.update_coordinator import DataUpdateCoordinator
 
-from . import SynoApi, SynologyDSMEntity
+from . import SynoApi, SynologyDSMCoordinatorEntity
 from .const import (
+    COORDINATOR_SURVEILLANCE,
     DOMAIN,
     ENTITY_CLASS,
     ENTITY_ENABLE,
@@ -19,50 +22,72 @@ from .const import (
     SYNO_API,
 )
 
+_LOGGER = logging.getLogger(__name__)
+
 
 async def async_setup_entry(
     hass: HomeAssistantType, entry: ConfigEntry, async_add_entities
 ) -> None:
-    """Set up the Synology NAS binary sensor."""
+    """Set up the Synology NAS cameras."""
 
-    api = hass.data[DOMAIN][entry.unique_id][SYNO_API]
+    data = hass.data[DOMAIN][entry.unique_id]
+    api = data[SYNO_API]
 
     if SynoSurveillanceStation.CAMERA_API_KEY not in api.dsm.apis:
         return
 
-    surveillance_station = api.surveillance_station
+    # initial data fetch
-    await hass.async_add_executor_job(surveillance_station.update)
+    coordinator = data[COORDINATOR_SURVEILLANCE]
-    cameras = surveillance_station.get_all_cameras()
+    await coordinator.async_refresh()
-    entities = [SynoDSMCamera(api, camera) for camera in cameras]
 
-    async_add_entities(entities)
+    async_add_entities(
+        SynoDSMCamera(api, coordinator, camera_id)
+        for camera_id in coordinator.data["cameras"]
+    )
 
 
-class SynoDSMCamera(SynologyDSMEntity, Camera):
+class SynoDSMCamera(SynologyDSMCoordinatorEntity, Camera):
     """Representation a Synology camera."""
 
-    def __init__(self, api: SynoApi, camera: SynoCamera):
+    def __init__(
+        self, api: SynoApi, coordinator: DataUpdateCoordinator, camera_id: int
+    ):
         """Initialize a Synology camera."""
         super().__init__(
             api,
-            f"{SynoSurveillanceStation.CAMERA_API_KEY}:{camera.id}",
+            f"{SynoSurveillanceStation.CAMERA_API_KEY}:{camera_id}",
             {
-                ENTITY_NAME: camera.name,
+                ENTITY_NAME: coordinator.data["cameras"][camera_id].name,
+                ENTITY_ENABLE: coordinator.data["cameras"][camera_id].is_enabled,
                 ENTITY_CLASS: None,
                 ENTITY_ICON: None,
-                ENTITY_ENABLE: True,
                 ENTITY_UNIT: None,
             },
+            coordinator,
         )
-        self._camera = camera
+        Camera.__init__(self)
+
+        self._camera_id = camera_id
+        self._api = api
+
+    @property
+    def camera_data(self):
+        """Camera data."""
+        return self.coordinator.data["cameras"][self._camera_id]
 
     @property
     def device_info(self) -> Dict[str, any]:
         """Return the device information."""
         return {
-            "identifiers": {(DOMAIN, self._api.information.serial, self._camera.id)},
+            "identifiers": {
-            "name": self._camera.name,
+                (
-            "model": self._camera.model,
+                    DOMAIN,
+                    self._api.information.serial,
+                    self.camera_data.id,
+                )
+            },
+            "name": self.camera_data.name,
+            "model": self.camera_data.model,
             "via_device": (
                 DOMAIN,
                 self._api.information.serial,
@@ -73,7 +98,7 @@ class SynoDSMCamera(SynologyDSMEntity, Camera):
     @property
     def available(self) -> bool:
         """Return the availability of the camera."""
-        return self._camera.is_enabled
+        return self.camera_data.is_enabled and self.coordinator.last_update_success
 
     @property
     def supported_features(self) -> int:
@@ -83,29 +108,53 @@ class SynoDSMCamera(SynologyDSMEntity, Camera):
     @property
     def is_recording(self):
         """Return true if the device is recording."""
-        return self._camera.is_recording
+        return self.camera_data.is_recording
 
     @property
     def motion_detection_enabled(self):
         """Return the camera motion detection status."""
-        return self._camera.is_motion_detection_enabled
+        return self.camera_data.is_motion_detection_enabled
 
     def camera_image(self) -> bytes:
         """Return bytes of camera image."""
+        _LOGGER.debug(
+            "SynoDSMCamera.camera_image(%s)",
+            self.camera_data.name,
+        )
         if not self.available:
             return None
-        return self._api.surveillance_station.get_camera_image(self._camera.id)
+        try:
+            return self._api.surveillance_station.get_camera_image(self._camera_id)
+        except (SynologyDSMAPIErrorException) as err:
+            _LOGGER.debug(
+                "SynoDSMCamera.camera_image(%s) - Exception:%s",
+                self.camera_data.name,
+                err,
+            )
+            return None
 
     async def stream_source(self) -> str:
         """Return the source of the stream."""
+        _LOGGER.debug(
+            "SynoDSMCamera.stream_source(%s)",
+            self.camera_data.name,
+        )
         if not self.available:
             return None
-        return self._camera.live_view.rtsp
+        return self.camera_data.live_view.rtsp
 
     def enable_motion_detection(self):
         """Enable motion detection in the camera."""
-        self._api.surveillance_station.enable_motion_detection(self._camera.id)
+        _LOGGER.debug(
+            "SynoDSMCamera.enable_motion_detection(%s)",
+            self.camera_data.name,
+        )
+        self._api.surveillance_station.enable_motion_detection(self._camera_id)
 
     def disable_motion_detection(self):
         """Disable motion detection in camera."""
-        self._api.surveillance_station.disable_motion_detection(self._camera.id)
+        _LOGGER.debug(
+            "SynoDSMCamera.disable_motion_detection(%s)",
+            self.camera_data.name,
+        )
+        self._api.surveillance_station.disable_motion_detection(self._camera_id)

homeassistant/components/synology_dsm/const.py

@@ -19,6 +19,7 @@ from homeassistant.const import (
 
 DOMAIN = "synology_dsm"
 PLATFORMS = ["binary_sensor", "camera", "sensor", "switch"]
+COORDINATOR_SURVEILLANCE = "coordinator_surveillance_station"
 
 # Entry keys
 SYNO_API = "syno_api"

homeassistant/components/synology_dsm/sensor.py

@@ -15,7 +15,7 @@ from homeassistant.helpers.temperature import display_temp
 from homeassistant.helpers.typing import HomeAssistantType
 from homeassistant.util.dt import utcnow
 
-from . import SynoApi, SynologyDSMDeviceEntity, SynologyDSMEntity
+from . import SynoApi, SynologyDSMDeviceEntity, SynologyDSMDispatcherEntity
 from .const import (
     CONF_VOLUMES,
     DOMAIN,
@@ -68,7 +68,7 @@ async def async_setup_entry(
     async_add_entities(entities)
 
 
-class SynoDSMUtilSensor(SynologyDSMEntity):
+class SynoDSMUtilSensor(SynologyDSMDispatcherEntity):
     """Representation a Synology Utilisation sensor."""
 
     @property
@@ -117,7 +117,7 @@ class SynoDSMStorageSensor(SynologyDSMDeviceEntity):
         return attr
 
 
-class SynoDSMInfoSensor(SynologyDSMEntity):
+class SynoDSMInfoSensor(SynologyDSMDispatcherEntity):
     """Representation a Synology information sensor."""
 
     def __init__(self, api: SynoApi, entity_type: str, entity_info: Dict[str, str]):

homeassistant/components/synology_dsm/switch.py

@@ -1,4 +1,5 @@
 """Support for Synology DSM switch."""
+import logging
 from typing import Dict
 
 from synology_dsm.api.surveillance_station import SynoSurveillanceStation
@@ -7,9 +8,11 @@ from homeassistant.components.switch import ToggleEntity
 from homeassistant.config_entries import ConfigEntry
 from homeassistant.helpers.typing import HomeAssistantType
 
-from . import SynoApi, SynologyDSMEntity
+from . import SynoApi, SynologyDSMDispatcherEntity
 from .const import DOMAIN, SURVEILLANCE_SWITCH, SYNO_API
 
+_LOGGER = logging.getLogger(__name__)
+
 
 async def async_setup_entry(
     hass: HomeAssistantType, entry: ConfigEntry, async_add_entities
@@ -33,7 +36,7 @@ async def async_setup_entry(
     async_add_entities(entities, True)
 
 
-class SynoDSMSurveillanceHomeModeToggle(SynologyDSMEntity, ToggleEntity):
+class SynoDSMSurveillanceHomeModeToggle(SynologyDSMDispatcherEntity, ToggleEntity):
     """Representation a Synology Surveillance Station Home Mode toggle."""
 
     def __init__(
@@ -62,16 +65,28 @@ class SynoDSMSurveillanceHomeModeToggle(SynologyDSMEntity, ToggleEntity):
 
     async def async_update(self):
         """Update the toggle state."""
+        _LOGGER.debug(
+            "SynoDSMSurveillanceHomeModeToggle.async_update(%s)",
+            self._api.information.serial,
+        )
         self._state = await self.hass.async_add_executor_job(
             self._api.surveillance_station.get_home_mode_status
         )
 
     def turn_on(self, **kwargs) -> None:
         """Turn on Home mode."""
+        _LOGGER.debug(
+            "SynoDSMSurveillanceHomeModeToggle.turn_on(%s)",
+            self._api.information.serial,
+        )
         self._api.surveillance_station.set_home_mode(True)
 
     def turn_off(self, **kwargs) -> None:
         """Turn off Home mode."""
+        _LOGGER.debug(
+            "SynoDSMSurveillanceHomeModeToggle.turn_off(%s)",
+            self._api.information.serial,
+        )
         self._api.surveillance_station.set_home_mode(False)
 
     @property