From 638a3025df1ef85a4cb53244a223d9d629381911 Mon Sep 17 00:00:00 2001 From: "J. Nick Koston" Date: Tue, 25 Feb 2020 21:43:41 -1000 Subject: [PATCH] Reduce August doorbell detail updates (#32193) * Reduce August doorbell detail updates * Doorbell images now get updates from the activity feed * Tests for activity updates * py-august now provides bridge_is_online for available state * py-august now provides is_standby for available state * py-august now provides get_doorbell_image (eliminate requests) * remove debug * black after merge conflict --- homeassistant/components/august/__init__.py | 7 +-- .../components/august/binary_sensor.py | 7 ++- homeassistant/components/august/camera.py | 17 ++++-- homeassistant/components/august/const.py | 8 +-- homeassistant/components/august/lock.py | 4 +- tests/components/august/mocks.py | 49 +++++++++++++++- tests/components/august/test_binary_sensor.py | 28 +++++++++ tests/components/august/test_lock.py | 7 +-- .../august/get_activity.doorbell_motion.json | 58 +++++++++++++++++++ 9 files changed, 158 insertions(+), 27 deletions(-) create mode 100644 tests/fixtures/august/get_activity.doorbell_motion.json diff --git a/homeassistant/components/august/__init__.py b/homeassistant/components/august/__init__.py index c80101d5658..6a497920ce3 100644 --- a/homeassistant/components/august/__init__.py +++ b/homeassistant/components/august/__init__.py @@ -30,8 +30,7 @@ from .const import ( DOMAIN, LOGIN_METHODS, MIN_TIME_BETWEEN_ACTIVITY_UPDATES, - MIN_TIME_BETWEEN_DOORBELL_DETAIL_UPDATES, - MIN_TIME_BETWEEN_LOCK_DETAIL_UPDATES, + MIN_TIME_BETWEEN_DETAIL_UPDATES, VERIFICATION_CODE_KEY, ) from .exceptions import InvalidAuth, RequireValidation @@ -296,7 +295,7 @@ class AugustData: await self._async_update_doorbells_detail() return self._doorbell_detail_by_id.get(device_id) - @Throttle(MIN_TIME_BETWEEN_DOORBELL_DETAIL_UPDATES) + @Throttle(MIN_TIME_BETWEEN_DETAIL_UPDATES) async def _async_update_doorbells_detail(self): await self._hass.async_add_executor_job(self._update_doorbells_detail) @@ -324,7 +323,7 @@ class AugustData: if lock.device_id == device_id: return lock.device_name - @Throttle(MIN_TIME_BETWEEN_LOCK_DETAIL_UPDATES) + @Throttle(MIN_TIME_BETWEEN_DETAIL_UPDATES) async def _async_update_locks_detail(self): await self._hass.async_add_executor_job(self._update_locks_detail) diff --git a/homeassistant/components/august/binary_sensor.py b/homeassistant/components/august/binary_sensor.py index 4cec054cf7d..c2b5603759d 100644 --- a/homeassistant/components/august/binary_sensor.py +++ b/homeassistant/components/august/binary_sensor.py @@ -22,7 +22,7 @@ SCAN_INTERVAL = timedelta(seconds=5) async def _async_retrieve_online_state(data, detail): """Get the latest state of the sensor.""" - return detail.is_online or detail.status == "standby" + return detail.is_online or detail.is_standby async def _async_retrieve_motion_state(data, detail): @@ -137,12 +137,13 @@ class AugustDoorBinarySensor(BinarySensorDevice): update_lock_detail_from_activity(detail, door_activity) lock_door_state = None + self._available = False if detail is not None: lock_door_state = detail.door_state + self._available = detail.bridge_is_online self._firmware_version = detail.firmware_version self._model = detail.model - self._available = lock_door_state != LockDoorStatus.UNKNOWN self._state = lock_door_state == LockDoorStatus.OPEN @property @@ -208,7 +209,7 @@ class AugustDoorbellBinarySensor(BinarySensorDevice): self._available = True else: self._available = detail is not None and ( - detail.is_online or detail.status == "standby" + detail.is_online or detail.is_standby ) self._state = None diff --git a/homeassistant/components/august/camera.py b/homeassistant/components/august/camera.py index 4e0ef65c82d..02c3a6b1231 100644 --- a/homeassistant/components/august/camera.py +++ b/homeassistant/components/august/camera.py @@ -1,7 +1,8 @@ """Support for August camera.""" from datetime import timedelta -import requests +from august.activity import ActivityType +from august.util import update_doorbell_image_from_activity from homeassistant.components.camera import Camera @@ -66,6 +67,15 @@ class AugustCamera(Camera): self._doorbell_detail = await self._data.async_get_doorbell_detail( self._doorbell.device_id ) + doorbell_activity = await self._data.async_get_latest_device_activity( + self._doorbell.device_id, ActivityType.DOORBELL_MOTION + ) + + if doorbell_activity is not None: + update_doorbell_image_from_activity( + self._doorbell_detail, doorbell_activity + ) + if self._doorbell_detail is None: return None @@ -89,9 +99,8 @@ class AugustCamera(Camera): self._model = self._doorbell_detail.model def _camera_image(self): - """Return bytes of camera image via http get.""" - # Move this to py-august: see issue#32048 - return requests.get(self._image_url, timeout=self._timeout).content + """Return bytes of camera image.""" + return self._doorbell_detail.get_doorbell_image(timeout=self._timeout) @property def unique_id(self) -> str: diff --git a/homeassistant/components/august/const.py b/homeassistant/components/august/const.py index 6e367e96ac5..a7ba61efe1f 100644 --- a/homeassistant/components/august/const.py +++ b/homeassistant/components/august/const.py @@ -23,13 +23,7 @@ DOMAIN = "august" # Limit battery, online, and hardware updates to 1800 seconds # in order to reduce the number of api requests and # avoid hitting rate limits -MIN_TIME_BETWEEN_LOCK_DETAIL_UPDATES = timedelta(seconds=1800) - -# Doorbells need to update more frequently than locks -# since we get an image from the doorbell api. Once -# py-august 0.18.0 is released doorbell status updates -# can be reduced in the same was as locks have been -MIN_TIME_BETWEEN_DOORBELL_DETAIL_UPDATES = timedelta(seconds=20) +MIN_TIME_BETWEEN_DETAIL_UPDATES = timedelta(seconds=1800) # Activity needs to be checked more frequently as the # doorbell motion and rings are included here diff --git a/homeassistant/components/august/lock.py b/homeassistant/components/august/lock.py index d339dae8063..c335292ca54 100644 --- a/homeassistant/components/august/lock.py +++ b/homeassistant/components/august/lock.py @@ -67,9 +67,7 @@ class AugustLock(LockDevice): if detail is not None: lock_status = detail.lock_status - self._available = ( - lock_status is not None and lock_status != LockStatus.UNKNOWN - ) + self._available = detail.bridge_is_online if self._lock_status != lock_status: self._lock_status = lock_status diff --git a/tests/components/august/mocks.py b/tests/components/august/mocks.py index 7b7fcd9f28c..cb78049d149 100644 --- a/tests/components/august/mocks.py +++ b/tests/components/august/mocks.py @@ -5,7 +5,18 @@ import time from unittest.mock import MagicMock, PropertyMock from asynctest import mock -from august.activity import DoorOperationActivity, LockOperationActivity +from august.activity import ( + ACTIVITY_ACTIONS_DOOR_OPERATION, + ACTIVITY_ACTIONS_DOORBELL_DING, + ACTIVITY_ACTIONS_DOORBELL_MOTION, + ACTIVITY_ACTIONS_DOORBELL_VIEW, + ACTIVITY_ACTIONS_LOCK_OPERATION, + DoorbellDingActivity, + DoorbellMotionActivity, + DoorbellViewActivity, + DoorOperationActivity, + LockOperationActivity, +) from august.authenticator import AuthenticationState from august.doorbell import Doorbell, DoorbellDetail from august.lock import Lock, LockDetail @@ -45,9 +56,12 @@ async def _mock_setup_august(hass, api_instance, authenticate_mock, api_mock): return True -async def _create_august_with_devices(hass, devices, api_call_side_effects=None): +async def _create_august_with_devices( + hass, devices, api_call_side_effects=None, activities=None +): if api_call_side_effects is None: api_call_side_effects = {} + device_data = { "doorbells": [], "locks": [], @@ -89,6 +103,8 @@ async def _create_august_with_devices(hass, devices, api_call_side_effects=None) return _get_base_devices("doorbells") def get_house_activities_side_effect(access_token, house_id, limit=10): + if activities is not None: + return activities return [] def lock_return_activities_side_effect(access_token, device_id): @@ -234,6 +250,17 @@ async def _mock_inoperative_august_lock_detail(hass): return await _mock_lock_from_fixture(hass, "get_lock.offline.json") +async def _mock_activities_from_fixture(hass, path): + json_dict = await _load_json_fixture(hass, path) + activities = [] + for activity_json in json_dict: + activity = _activity_from_dict(activity_json) + if activity: + activities.append(activity) + + return activities + + async def _mock_lock_from_fixture(hass, path): json_dict = await _load_json_fixture(hass, path) return LockDetail(json_dict) @@ -279,3 +306,21 @@ def _mock_door_operation_activity(lock, action): "action": action, } ) + + +def _activity_from_dict(activity_dict): + action = activity_dict.get("action") + + activity_dict["dateTime"] = time.time() * 1000 + + if action in ACTIVITY_ACTIONS_DOORBELL_DING: + return DoorbellDingActivity(activity_dict) + if action in ACTIVITY_ACTIONS_DOORBELL_MOTION: + return DoorbellMotionActivity(activity_dict) + if action in ACTIVITY_ACTIONS_DOORBELL_VIEW: + return DoorbellViewActivity(activity_dict) + if action in ACTIVITY_ACTIONS_LOCK_OPERATION: + return LockOperationActivity(activity_dict) + if action in ACTIVITY_ACTIONS_DOOR_OPERATION: + return DoorOperationActivity(activity_dict) + return None diff --git a/tests/components/august/test_binary_sensor.py b/tests/components/august/test_binary_sensor.py index f64b2de7918..1ecca29985d 100644 --- a/tests/components/august/test_binary_sensor.py +++ b/tests/components/august/test_binary_sensor.py @@ -14,6 +14,7 @@ from homeassistant.const import ( from tests.components.august.mocks import ( _create_august_with_devices, + _mock_activities_from_fixture, _mock_doorbell_from_fixture, _mock_lock_from_fixture, ) @@ -70,6 +71,10 @@ async def test_create_doorbell(hass): "binary_sensor.k98gidt45gul_name_ding" ) assert binary_sensor_k98gidt45gul_name_ding.state == STATE_OFF + binary_sensor_k98gidt45gul_name_motion = hass.states.get( + "binary_sensor.k98gidt45gul_name_motion" + ) + assert binary_sensor_k98gidt45gul_name_motion.state == STATE_OFF async def test_create_doorbell_offline(hass): @@ -90,6 +95,29 @@ async def test_create_doorbell_offline(hass): assert binary_sensor_tmt100_name_ding.state == STATE_UNAVAILABLE +async def test_create_doorbell_with_motion(hass): + """Test creation of a doorbell.""" + doorbell_one = await _mock_doorbell_from_fixture(hass, "get_doorbell.json") + doorbell_details = [doorbell_one] + activities = await _mock_activities_from_fixture( + hass, "get_activity.doorbell_motion.json" + ) + await _create_august_with_devices(hass, doorbell_details, activities=activities) + + binary_sensor_k98gidt45gul_name_motion = hass.states.get( + "binary_sensor.k98gidt45gul_name_motion" + ) + assert binary_sensor_k98gidt45gul_name_motion.state == STATE_ON + binary_sensor_k98gidt45gul_name_online = hass.states.get( + "binary_sensor.k98gidt45gul_name_online" + ) + assert binary_sensor_k98gidt45gul_name_online.state == STATE_ON + binary_sensor_k98gidt45gul_name_ding = hass.states.get( + "binary_sensor.k98gidt45gul_name_ding" + ) + assert binary_sensor_k98gidt45gul_name_ding.state == STATE_OFF + + async def test_doorbell_device_registry(hass): """Test creation of a lock with doorsense and bridge ands up in the registry.""" doorbell_one = await _mock_doorbell_from_fixture(hass, "get_doorbell.offline.json") diff --git a/tests/components/august/test_lock.py b/tests/components/august/test_lock.py index a7027842b57..24e0cdafd46 100644 --- a/tests/components/august/test_lock.py +++ b/tests/components/august/test_lock.py @@ -6,7 +6,7 @@ from homeassistant.const import ( SERVICE_LOCK, SERVICE_UNLOCK, STATE_LOCKED, - STATE_UNAVAILABLE, + STATE_UNKNOWN, STATE_UNLOCKED, ) @@ -80,6 +80,5 @@ async def test_one_lock_unknown_state(hass): await _create_august_with_devices(hass, lock_details) lock_brokenid_name = hass.states.get("lock.brokenid_name") - # Once we have bridge_is_online support in py-august - # this can change to STATE_UNKNOWN - assert lock_brokenid_name.state == STATE_UNAVAILABLE + + assert lock_brokenid_name.state == STATE_UNKNOWN diff --git a/tests/fixtures/august/get_activity.doorbell_motion.json b/tests/fixtures/august/get_activity.doorbell_motion.json new file mode 100644 index 00000000000..bd9c07afa26 --- /dev/null +++ b/tests/fixtures/august/get_activity.doorbell_motion.json @@ -0,0 +1,58 @@ +[ + { + "otherUser" : { + "FirstName" : "Unknown", + "UserName" : "deleteduser", + "LastName" : "User", + "UserID" : "deleted", + "PhoneNo" : "deleted" + }, + "dateTime" : 1582663119959, + "deviceID" : "K98GiDT45GUL", + "info" : { + "videoUploadProgress" : "in_progress", + "image" : { + "resource_type" : "image", + "etag" : "fdsf", + "created_at" : "2020-02-25T20:38:39Z", + "type" : "upload", + "format" : "jpg", + "version" : 1582663119, + "secure_url" : "https://res.cloudinary.com/updated_image.jpg", + "signature" : "fdfdfd", + "url" : "http://res.cloudinary.com/updated_image.jpg", + "bytes" : 48545, + "placeholder" : false, + "original_filename" : "file", + "width" : 720, + "tags" : [], + "public_id" : "xnsj5gphpzij9brifpf4", + "height" : 576 + }, + "dvrID" : "dvr", + "videoAvailable" : false, + "hasSubscription" : false + }, + "callingUser" : { + "LastName" : "User", + "UserName" : "deleteduser", + "FirstName" : "Unknown", + "UserID" : "deleted", + "PhoneNo" : "deleted" + }, + "house" : { + "houseName" : "K98GiDT45GUL", + "houseID" : "na" + }, + "action" : "doorbell_motion_detected", + "deviceType" : "doorbell", + "entities" : { + "otherUser" : "deleted", + "house" : "na", + "device" : "K98GiDT45GUL", + "activity" : "de5585cfd4eae900bb5ba3dc", + "callingUser" : "deleted" + }, + "deviceName" : "Front Door" + } +]