From cd1b355e4383db52c4c1a48206ea551d9e44ad17 Mon Sep 17 00:00:00 2001 From: Steven B <51370195+sdb9696@users.noreply.github.com> Date: Wed, 21 Aug 2024 15:39:57 +0100 Subject: [PATCH] Enable ring event listener to fix missing notifications --- homeassistant/components/ring/__init__.py | 38 +++-- homeassistant/components/ring/const.py | 5 +- homeassistant/components/ring/coordinator.py | 137 ++++++++++++++++--- homeassistant/components/ring/entity.py | 13 +- homeassistant/components/ring/event.py | 121 ++++++++++++++++ homeassistant/components/ring/strings.json | 8 +- tests/components/ring/conftest.py | 56 +++++++- tests/components/ring/device_mocks.py | 11 -- tests/components/ring/test_binary_sensor.py | 24 ---- tests/components/ring/test_init.py | 23 ++++ 10 files changed, 360 insertions(+), 76 deletions(-) create mode 100644 homeassistant/components/ring/event.py delete mode 100644 tests/components/ring/test_binary_sensor.py diff --git a/homeassistant/components/ring/__init__.py b/homeassistant/components/ring/__init__.py index 3714802b63a939..0dd77ff5db66c6 100644 --- a/homeassistant/components/ring/__init__.py +++ b/homeassistant/components/ring/__init__.py @@ -9,14 +9,18 @@ from ring_doorbell import Auth, Ring, RingDevices from homeassistant.config_entries import ConfigEntry -from homeassistant.const import APPLICATION_NAME, CONF_TOKEN, __version__ +from homeassistant.const import APPLICATION_NAME, CONF_TOKEN from homeassistant.core import HomeAssistant, ServiceCall, callback -from homeassistant.helpers import device_registry as dr, entity_registry as er +from homeassistant.helpers import ( + device_registry as dr, + entity_registry as er, + instance_id, +) from homeassistant.helpers.aiohttp_client import async_get_clientsession from homeassistant.helpers.issue_registry import IssueSeverity, async_create_issue -from .const import DOMAIN, PLATFORMS -from .coordinator import RingDataCoordinator, RingNotificationsCoordinator +from .const import CONF_LISTEN_CREDENTIALS, DOMAIN, PLATFORMS +from .coordinator import RingDataCoordinator, RingListenCoordinator _LOGGER = logging.getLogger(__name__) @@ -28,7 +32,7 @@ class RingData: api: Ring devices: RingDevices devices_coordinator: RingDataCoordinator - notifications_coordinator: RingNotificationsCoordinator + listen_coordinator: RingListenCoordinator type RingConfigEntry = ConfigEntry[RingData] @@ -44,26 +48,39 @@ def token_updater(token: dict[str, Any]) -> None: data={**entry.data, CONF_TOKEN: token}, ) + def listen_credentials_updater(token: dict[str, Any]) -> None: + """Handle from async context when token is updated.""" + hass.config_entries.async_update_entry( + entry, + data={**entry.data, CONF_LISTEN_CREDENTIALS: token}, + ) + + hardware_id = await instance_id.async_get(hass) + client_session = async_get_clientsession(hass) auth = Auth( - f"{APPLICATION_NAME}/{__version__}", + f"{APPLICATION_NAME}/{DOMAIN}-integration", entry.data[CONF_TOKEN], token_updater, - http_client_session=async_get_clientsession(hass), + hardware_id=hardware_id, + http_client_session=client_session, ) ring = Ring(auth) await _migrate_old_unique_ids(hass, entry.entry_id) devices_coordinator = RingDataCoordinator(hass, ring) - notifications_coordinator = RingNotificationsCoordinator(hass, ring) + listen_credentials = entry.data.get(CONF_LISTEN_CREDENTIALS) + listen_coordinator = RingListenCoordinator( + hass, ring, listen_credentials, listen_credentials_updater + ) + await devices_coordinator.async_config_entry_first_refresh() - await notifications_coordinator.async_config_entry_first_refresh() entry.runtime_data = RingData( api=ring, devices=ring.devices(), devices_coordinator=devices_coordinator, - notifications_coordinator=notifications_coordinator, + listen_coordinator=listen_coordinator, ) await hass.config_entries.async_forward_entry_setups(entry, PLATFORMS) @@ -91,7 +108,6 @@ async def async_refresh_all(_: ServiceCall) -> None: ) for loaded_entry in hass.config_entries.async_loaded_entries(DOMAIN): await loaded_entry.runtime_data.devices_coordinator.async_refresh() - await loaded_entry.runtime_data.notifications_coordinator.async_refresh() # register service hass.services.async_register(DOMAIN, "update", async_refresh_all) diff --git a/homeassistant/components/ring/const.py b/homeassistant/components/ring/const.py index 70813a78c76b9e..d5d891d140dadd 100644 --- a/homeassistant/components/ring/const.py +++ b/homeassistant/components/ring/const.py @@ -15,9 +15,9 @@ DEFAULT_ENTITY_NAMESPACE = "ring" PLATFORMS = [ - Platform.BINARY_SENSOR, Platform.BUTTON, Platform.CAMERA, + Platform.EVENT, Platform.LIGHT, Platform.SENSOR, Platform.SIREN, @@ -26,6 +26,7 @@ SCAN_INTERVAL = timedelta(minutes=1) -NOTIFICATIONS_SCAN_INTERVAL = timedelta(seconds=5) +SESSION_REFRESH_INTERVAL = timedelta(minutes=10) CONF_2FA = "2fa" +CONF_LISTEN_CREDENTIALS = "listen_token" diff --git a/homeassistant/components/ring/coordinator.py b/homeassistant/components/ring/coordinator.py index 600743005ebcb9..746820d262397a 100644 --- a/homeassistant/components/ring/coordinator.py +++ b/homeassistant/components/ring/coordinator.py @@ -3,15 +3,28 @@ from asyncio import TaskGroup from collections.abc import Callable, Coroutine import logging -from typing import Any - -from ring_doorbell import AuthenticationError, Ring, RingDevices, RingError, RingTimeout - -from homeassistant.core import HomeAssistant +from typing import TYPE_CHECKING, Any + +from ring_doorbell import ( + AuthenticationError, + Ring, + RingDevices, + RingError, + RingEvent, + RingTimeout, +) +from ring_doorbell.listen import RingEventListener + +from homeassistant import config_entries +from homeassistant.core import CALLBACK_TYPE, HomeAssistant, callback from homeassistant.exceptions import ConfigEntryAuthFailed -from homeassistant.helpers.update_coordinator import DataUpdateCoordinator, UpdateFailed +from homeassistant.helpers.update_coordinator import ( + BaseDataUpdateCoordinatorProtocol, + DataUpdateCoordinator, + UpdateFailed, +) -from .const import NOTIFICATIONS_SCAN_INTERVAL, SCAN_INTERVAL +from .const import SCAN_INTERVAL _LOGGER = logging.getLogger(__name__) @@ -91,19 +104,105 @@ async def _async_update_data(self) -> RingDevices: return devices -class RingNotificationsCoordinator(DataUpdateCoordinator[None]): +class RingListenCoordinator(BaseDataUpdateCoordinatorProtocol): """Global notifications coordinator.""" - def __init__(self, hass: HomeAssistant, ring_api: Ring) -> None: + config_entry: config_entries.ConfigEntry + + def __init__( + self, + hass: HomeAssistant, + ring_api: Ring, + listen_credentials: dict[str, Any] | None, + listen_credentials_updater: Callable[[dict[str, Any]], None], + ) -> None: """Initialize my coordinator.""" - super().__init__( - hass, - logger=_LOGGER, - name="active dings", - update_interval=NOTIFICATIONS_SCAN_INTERVAL, - ) + self.hass = hass + self.logger = _LOGGER self.ring_api: Ring = ring_api - - async def _async_update_data(self) -> None: - """Fetch data from API endpoint.""" - await _call_api(self.hass, self.ring_api.async_update_dings) + self.event_listener = RingEventListener( + ring_api, listen_credentials, listen_credentials_updater + ) + self._listeners: dict[CALLBACK_TYPE, tuple[CALLBACK_TYPE, object | None]] = {} + self._listen_callback_id: int | None = None + + config_entry = config_entries.current_entry.get() + if TYPE_CHECKING: + assert config_entry + self.config_entry = config_entry + self.start_timeout = 10 + self.config_entry.async_on_unload(self.async_shutdown) + self.alerts = ring_api.active_alerts() + + async def async_shutdown(self) -> None: + """Cancel any scheduled call, and ignore new runs.""" + if self.event_listener.started: + await self._async_stop_listen() + + async def _async_stop_listen(self) -> None: + self.logger.debug("Stopped ring listener") + await self.event_listener.stop() + self.logger.debug("Stopped ring listener") + + async def _async_start_listen(self) -> None: + """Start listening for realtime events.""" + self.logger.debug("Starting ring listener.") + await self.event_listener.start( + timeout=self.start_timeout, + ) + if self.event_listener.started is True: + self.logger.debug("Started ring listener") + else: + self.logger.warning( + "Ring event listener failed to start after %s seconds", + self.start_timeout, + ) + self._listen_callback_id = self.event_listener.add_notification_callback( + self._on_event + ) + self.alerts = self.ring_api.active_alerts() + # Update the listeners so they switch from Unavailable to Unknown + self._async_update_listeners() + + def _on_event(self, event: RingEvent) -> None: + self.logger.debug("Ring event received: %s", event) + self.alerts = self.ring_api.active_alerts() + self._async_update_listeners(event.doorbot_id) + + @callback + def _async_update_listeners(self, doorbot_id: int | None = None) -> None: + """Update all registered listeners.""" + for update_callback, device_api_id in list(self._listeners.values()): + if not doorbot_id or device_api_id == doorbot_id: + update_callback() + + @callback + def async_add_listener( + self, update_callback: CALLBACK_TYPE, context: Any = None + ) -> Callable[[], None]: + """Listen for data updates.""" + start_listen = not self._listeners + + @callback + def remove_listener() -> None: + """Remove update listener.""" + self._listeners.pop(remove_listener) + if not self._listeners: + self.config_entry.async_create_task( + self.hass, + self._async_stop_listen(), + "Ring event listener stop", + eager_start=True, + ) + + self._listeners[remove_listener] = (update_callback, context) + + # This is the first listener, start the event listener. + if start_listen: + self.config_entry.async_create_task( + self.hass, + self._async_start_listen(), + "Ring event listener start", + eager_start=True, + ) + return remove_listener diff --git a/homeassistant/components/ring/entity.py b/homeassistant/components/ring/entity.py index 72deb09b76fc17..f17ac1555fec89 100644 --- a/homeassistant/components/ring/entity.py +++ b/homeassistant/components/ring/entity.py @@ -15,16 +15,19 @@ from homeassistant.core import callback from homeassistant.exceptions import HomeAssistantError from homeassistant.helpers.device_registry import DeviceInfo -from homeassistant.helpers.update_coordinator import CoordinatorEntity +from homeassistant.helpers.update_coordinator import ( + BaseCoordinatorEntity, + CoordinatorEntity, +) from .const import ATTRIBUTION, DOMAIN -from .coordinator import RingDataCoordinator, RingNotificationsCoordinator +from .coordinator import RingDataCoordinator, RingListenCoordinator RingDeviceT = TypeVar("RingDeviceT", bound=RingGeneric, default=RingGeneric) _RingCoordinatorT = TypeVar( "_RingCoordinatorT", - bound=(RingDataCoordinator | RingNotificationsCoordinator), + bound=(RingDataCoordinator | RingListenCoordinator), ) @@ -52,7 +55,7 @@ async def _wrap(self: _RingBaseEntityT, *args: _P.args, **kwargs: _P.kwargs) -> class RingBaseEntity( - CoordinatorEntity[_RingCoordinatorT], Generic[_RingCoordinatorT, RingDeviceT] + BaseCoordinatorEntity[_RingCoordinatorT], Generic[_RingCoordinatorT, RingDeviceT] ): """Base implementation for Ring device.""" @@ -77,7 +80,7 @@ def __init__( ) -class RingEntity(RingBaseEntity[RingDataCoordinator, RingDeviceT]): +class RingEntity(RingBaseEntity[RingDataCoordinator, RingDeviceT], CoordinatorEntity): """Implementation for Ring devices.""" def _get_coordinator_data(self) -> RingDevices: diff --git a/homeassistant/components/ring/event.py b/homeassistant/components/ring/event.py new file mode 100644 index 00000000000000..5138b757177b30 --- /dev/null +++ b/homeassistant/components/ring/event.py @@ -0,0 +1,121 @@ +"""Component providing support for ring events.""" + +from dataclasses import dataclass +from typing import Generic + +from ring_doorbell import RingCapability, RingEvent as RingAlert +from ring_doorbell.const import KIND_DING, KIND_INTERCOM_UNLOCK, KIND_MOTION + +from homeassistant.components.event import ( + EventDeviceClass, + EventEntity, + EventEntityDescription, +) +from homeassistant.core import HomeAssistant, callback +from homeassistant.helpers.entity_platform import AddEntitiesCallback + +from . import RingConfigEntry +from .coordinator import RingListenCoordinator +from .entity import RingBaseEntity, RingDeviceT + + +@dataclass(frozen=True, kw_only=True) +class RingEventEntityDescription(EventEntityDescription, Generic[RingDeviceT]): + """Base class for event entity description.""" + + capability: RingCapability + + +EVENT_DESCRIPTIONS: tuple[RingEventEntityDescription, ...] = ( + RingEventEntityDescription( + key=KIND_DING, + translation_key=KIND_DING, + device_class=EventDeviceClass.DOORBELL, + event_types=[KIND_DING], + entity_registry_enabled_default=True, + capability=RingCapability.DING, + ), + RingEventEntityDescription( + key=KIND_MOTION, + translation_key=KIND_MOTION, + device_class=EventDeviceClass.MOTION, + event_types=[KIND_MOTION], + entity_registry_enabled_default=True, + capability=RingCapability.MOTION_DETECTION, + ), + RingEventEntityDescription( + key=KIND_INTERCOM_UNLOCK, + translation_key=KIND_INTERCOM_UNLOCK, + device_class=EventDeviceClass.BUTTON, + event_types=[KIND_INTERCOM_UNLOCK], + entity_registry_enabled_default=True, + capability=RingCapability.OPEN, + ), +) + + +async def async_setup_entry( + hass: HomeAssistant, + entry: RingConfigEntry, + async_add_entities: AddEntitiesCallback, +) -> None: + """Set up a sensor for a Ring device.""" + ring_data = entry.runtime_data + listen_coordinator = ring_data.listen_coordinator + + entities = [ + RingEvent(device, listen_coordinator, description) + for description in EVENT_DESCRIPTIONS + for device in ring_data.devices.all_devices + if device.has_capability(description.capability) + ] + + async_add_entities(entities) + + +class RingEvent(RingBaseEntity[RingListenCoordinator, RingDeviceT], EventEntity): + """A sensor implementation for Ring device.""" + + entity_description: RingEventEntityDescription[RingDeviceT] + + def __init__( + self, + device: RingDeviceT, + coordinator: RingListenCoordinator, + description: RingEventEntityDescription[RingDeviceT], + ) -> None: + """Initialize a sensor for Ring device.""" + super().__init__(device, coordinator) + self.entity_description = description + self._attr_unique_id = f"{device.id}-{description.key}" + self._attr_entity_registry_enabled_default = ( + description.entity_registry_enabled_default + ) + + @callback + def _async_handle_event(self, event: str) -> None: + """Handle the event.""" + self._trigger_event(event) + + def _get_coordinator_alert(self) -> RingAlert | None: + alerts = ( + alert + for alert in self.coordinator.ring_api.active_alerts() + if alert.kind == self.entity_description.key + and alert.doorbot_id == self._device.device_api_id + ) + return next(alerts, None) + + @callback + def _handle_coordinator_update(self) -> None: + if alert := self._get_coordinator_alert(): + self._async_handle_event(alert.kind) + super()._handle_coordinator_update() + + @property + def available(self) -> bool: + """Return if entity is available.""" + return self.coordinator.event_listener.started + + async def async_update(self) -> None: + """All updates are passive.""" diff --git a/homeassistant/components/ring/strings.json b/homeassistant/components/ring/strings.json index ed0319b7a4b8b9..6812aeb19a3095 100644 --- a/homeassistant/components/ring/strings.json +++ b/homeassistant/components/ring/strings.json @@ -32,9 +32,15 @@ } }, "entity": { - "binary_sensor": { + "event": { "ding": { "name": "Ding" + }, + "motion": { + "name": "Motion" + }, + "intercom_unlock": { + "name": "Unlock" } }, "button": { diff --git a/tests/components/ring/conftest.py b/tests/components/ring/conftest.py index 4456a9daa26278..37f6a0994c912a 100644 --- a/tests/components/ring/conftest.py +++ b/tests/components/ring/conftest.py @@ -4,6 +4,7 @@ from itertools import chain from unittest.mock import AsyncMock, Mock, create_autospec, patch +from google.protobuf.json_format import Parse as JsonParse import pytest import ring_doorbell @@ -11,9 +12,9 @@ from homeassistant.const import CONF_USERNAME from homeassistant.core import HomeAssistant -from .device_mocks import get_active_alerts, get_devices_data, get_mock_devices +from .device_mocks import get_devices_data, get_mock_devices -from tests.common import MockConfigEntry +from tests.common import MockConfigEntry, load_fixture from tests.components.light.conftest import mock_light_profiles # noqa: F401 @@ -103,7 +104,7 @@ def mock_ring_client(mock_ring_auth, mock_ring_devices): mock_client = create_autospec(ring_doorbell.Ring) mock_client.return_value.devices_data = get_devices_data() mock_client.return_value.devices.return_value = mock_ring_devices - mock_client.return_value.active_alerts.side_effect = get_active_alerts + mock_client.return_value.active_alerts.return_value = [] with patch("homeassistant.components.ring.Ring", new=mock_client): yield mock_client.return_value @@ -135,3 +136,52 @@ async def mock_added_config_entry( assert await hass.config_entries.async_setup(mock_config_entry.entry_id) await hass.async_block_till_done() return mock_config_entry + + +def load_fixture_as_protobuf_msg(filename, msg_class): + """Load a fixture.""" + msg = msg_class() + JsonParse(load_fixture(filename, "ring"), msg) + return msg + + +@pytest.fixture(autouse=True) +def mock_listener(request): + """Fixture to mock the push client connect and disconnect.""" + + f = _FakeRingListener() + with patch( + "homeassistant.components.ring.coordinator.RingEventListener", return_value=f + ): + yield f + + +class _FakeRingListener: + """Test class to replace the ring_doorbell event listener for testing.""" + + def __init__(self, *_, **__): + self._callbacks = {} + self._subscription_counter = 1 + self.started = False + self.do_not_start = False + + async def start(self, *_, **__): + if self.do_not_start: + return False + self.started = True + return True + + async def stop(self, *_, **__): + self.started = False + + def add_notification_callback(self, callback): + self._callbacks[self._subscription_counter] = callback + self._subscription_counter += 1 + return self._subscription_counter + + def remove_notification_callback(self, subscription_id): + del self._callbacks[subscription_id] + + def notify(self, ring_event: ring_doorbell.RingEvent): + for callback in self._callbacks.values(): + callback(ring_event) diff --git a/tests/components/ring/device_mocks.py b/tests/components/ring/device_mocks.py index d2671c3896db09..0a00a9b64deb85 100644 --- a/tests/components/ring/device_mocks.py +++ b/tests/components/ring/device_mocks.py @@ -7,9 +7,7 @@ Mocks the api calls on the devices such as history() and health(). """ -from copy import deepcopy from datetime import datetime -from time import time from unittest.mock import AsyncMock, MagicMock from ring_doorbell import ( @@ -30,7 +28,6 @@ INTERCOM_HISTORY = load_json_value_fixture("intercom_history.json", DOMAIN) DOORBOT_HEALTH = load_json_value_fixture("doorbot_health_attrs.json", DOMAIN) CHIME_HEALTH = load_json_value_fixture("chime_health_attrs.json", DOMAIN) -DEVICE_ALERTS = load_json_value_fixture("ding_active.json", DOMAIN) def get_mock_devices(): @@ -54,14 +51,6 @@ def get_devices_data(): } -def get_active_alerts(): - """Return active alerts set to now.""" - dings_fixture = deepcopy(DEVICE_ALERTS) - for ding in dings_fixture: - ding["now"] = time() - return dings_fixture - - DEVICE_TYPES = { "doorbots": RingDoorBell, "authorized_doorbots": RingDoorBell, diff --git a/tests/components/ring/test_binary_sensor.py b/tests/components/ring/test_binary_sensor.py deleted file mode 100644 index 16bc6e872c1d52..00000000000000 --- a/tests/components/ring/test_binary_sensor.py +++ /dev/null @@ -1,24 +0,0 @@ -"""The tests for the Ring binary sensor platform.""" - -from homeassistant.const import Platform -from homeassistant.core import HomeAssistant - -from .common import setup_platform - - -async def test_binary_sensor(hass: HomeAssistant, mock_ring_client) -> None: - """Test the Ring binary sensors.""" - await setup_platform(hass, Platform.BINARY_SENSOR) - - motion_state = hass.states.get("binary_sensor.front_door_motion") - assert motion_state is not None - assert motion_state.state == "on" - assert motion_state.attributes["device_class"] == "motion" - - front_ding_state = hass.states.get("binary_sensor.front_door_ding") - assert front_ding_state is not None - assert front_ding_state.state == "off" - - ingress_ding_state = hass.states.get("binary_sensor.ingress_ding") - assert ingress_ding_state is not None - assert ingress_ding_state.state == "off" diff --git a/tests/components/ring/test_init.py b/tests/components/ring/test_init.py index 97392e0c93bdb0..c3e671c177651e 100644 --- a/tests/components/ring/test_init.py +++ b/tests/components/ring/test_init.py @@ -413,3 +413,26 @@ async def test_token_updated( async_fire_time_changed(hass) await hass.async_block_till_done() assert mock_config_entry.data[CONF_TOKEN] == {"access_token": "new-mock-token"} + + +async def test_no_listen_start( + hass: HomeAssistant, + caplog: pytest.LogCaptureFixture, + mock_listener, + mock_ring_client, +) -> None: + """Test behaviour if listener doesn't start.""" + mock_entry = MockConfigEntry( + domain=DOMAIN, + version=1, + data={"username": "foo", "token": {}}, + ) + mock_listener.do_not_start = True + + mock_entry.add_to_hass(hass) + await hass.config_entries.async_setup(mock_entry.entry_id) + await hass.async_block_till_done() + + assert "Ring event listener failed to start after 10 seconds" in [ + record.message for record in caplog.records if record.levelname == "WARNING" + ]