Restore august lock changed_by state on restart (#32340)

* Various code review items from previous PRs

* Add a test for fetching the doorbell camera image

* Switch to using UNIT_PERCENTAGE for battery charge unit

* Add tests for changed_by

homeassistant/components/august/binary_sensor.py

@@ -8,6 +8,7 @@ from august.util import update_lock_detail_from_activity
 
 from homeassistant.components.binary_sensor import (
     DEVICE_CLASS_CONNECTIVITY,
+    DEVICE_CLASS_DOOR,
     DEVICE_CLASS_MOTION,
     DEVICE_CLASS_OCCUPANCY,
     BinarySensorDevice,
@@ -116,24 +117,22 @@ class AugustDoorBinarySensor(AugustEntityMixin, BinarySensorDevice):
         self._data = data
         self._sensor_type = sensor_type
         self._device = device
-        self._state = None
-        self._available = False
         self._update_from_data()
 
     @property
     def available(self):
         """Return the availability of this sensor."""
-        return self._available
+        return self._detail.bridge_is_online
 
     @property
     def is_on(self):
         """Return true if the binary sensor is on."""
-        return self._state
+        return self._detail.door_state == LockDoorStatus.OPEN
 
     @property
     def device_class(self):
         """Return the class of this device."""
-        return "door"
+        return DEVICE_CLASS_DOOR
 
     @property
     def name(self):
@@ -146,15 +145,9 @@ class AugustDoorBinarySensor(AugustEntityMixin, BinarySensorDevice):
         door_activity = self._data.activity_stream.get_latest_device_activity(
             self._device_id, [ActivityType.DOOR_OPERATION]
         )
-        detail = self._detail
 
         if door_activity is not None:
-            update_lock_detail_from_activity(detail, door_activity)
+            update_lock_detail_from_activity(self._detail, door_activity)
-
-        lock_door_state = detail.door_state
-        self._available = detail.bridge_is_online
-
-        self._state = lock_door_state == LockDoorStatus.OPEN
 
     @property
     def unique_id(self) -> str:

homeassistant/components/august/entity.py

@@ -3,13 +3,14 @@
 import logging
 
 from homeassistant.core import callback
+from homeassistant.helpers.entity import Entity
 
 from . import DEFAULT_NAME, DOMAIN
 
 _LOGGER = logging.getLogger(__name__)
 
 
-class AugustEntityMixin:
+class AugustEntityMixin(Entity):
     """Base implementation for August device."""
 
     def __init__(self, data, device):

homeassistant/components/august/lock.py

@@ -5,9 +5,10 @@ from august.activity import ActivityType
 from august.lock import LockStatus
 from august.util import update_lock_detail_from_activity
 
-from homeassistant.components.lock import LockDevice
+from homeassistant.components.lock import ATTR_CHANGED_BY, LockDevice
 from homeassistant.const import ATTR_BATTERY_LEVEL
 from homeassistant.core import callback
+from homeassistant.helpers.restore_state import RestoreEntity
 
 from .const import DATA_AUGUST, DOMAIN
 from .entity import AugustEntityMixin
@@ -27,7 +28,7 @@ async def async_setup_entry(hass, config_entry, async_add_entities):
     async_add_entities(devices, True)
 
 
-class AugustLock(AugustEntityMixin, LockDevice):
+class AugustLock(AugustEntityMixin, RestoreEntity, LockDevice):
     """Representation of an August lock."""
 
     def __init__(self, data, device):
@@ -52,9 +53,8 @@ class AugustLock(AugustEntityMixin, LockDevice):
         activities = await self.hass.async_add_executor_job(
             lock_operation, self._device_id
         )
-        detail = self._detail
         for lock_activity in activities:
-            update_lock_detail_from_activity(detail, lock_activity)
+            update_lock_detail_from_activity(self._detail, lock_activity)
 
         if self._update_lock_status_from_detail():
             _LOGGER.debug(
@@ -64,26 +64,23 @@ class AugustLock(AugustEntityMixin, LockDevice):
             self._data.async_signal_device_id_update(self._device_id)
 
     def _update_lock_status_from_detail(self):
-        detail = self._detail
+        self._available = self._detail.bridge_is_online
-        lock_status = detail.lock_status
-        self._available = detail.bridge_is_online
 
-        if self._lock_status != lock_status:
+        if self._lock_status != self._detail.lock_status:
-            self._lock_status = lock_status
+            self._lock_status = self._detail.lock_status
             return True
         return False
 
     @callback
     def _update_from_data(self):
         """Get the latest state of the sensor and update activity."""
-        lock_detail = self._detail
         lock_activity = self._data.activity_stream.get_latest_device_activity(
             self._device_id, [ActivityType.LOCK_OPERATION]
         )
 
         if lock_activity is not None:
             self._changed_by = lock_activity.operated_by
-            update_lock_detail_from_activity(lock_detail, lock_activity)
+            update_lock_detail_from_activity(self._detail, lock_activity)
 
         self._update_lock_status_from_detail()
 
@@ -119,6 +116,17 @@ class AugustLock(AugustEntityMixin, LockDevice):
 
         return attributes
 
+    async def async_added_to_hass(self):
+        """Restore ATTR_CHANGED_BY on startup since it is likely no longer in the activity log."""
+        await super().async_added_to_hass()
+
+        last_state = await self.async_get_last_state()
+        if not last_state:
+            return
+
+        if ATTR_CHANGED_BY in last_state.attributes:
+            self._changed_by = last_state.attributes[ATTR_CHANGED_BY]
+
     @property
     def unique_id(self) -> str:
         """Get the unique id of the lock."""

homeassistant/components/august/sensor.py

@@ -2,6 +2,7 @@
 import logging
 
 from homeassistant.components.sensor import DEVICE_CLASS_BATTERY
+from homeassistant.const import UNIT_PERCENTAGE
 from homeassistant.core import callback
 from homeassistant.helpers.entity import Entity
 
@@ -111,7 +112,7 @@ class AugustBatterySensor(AugustEntityMixin, Entity):
     @property
     def unit_of_measurement(self):
         """Return the unit of measurement."""
-        return "%"  # UNIT_PERCENTAGE will be available after PR#32094
+        return UNIT_PERCENTAGE
 
     @property
     def device_class(self):

tests/components/august/test_binary_sensor.py

@@ -23,16 +23,14 @@ async def test_doorsense(hass):
     lock_one = await _mock_lock_from_fixture(
         hass, "get_lock.online_with_doorsense.json"
     )
-    lock_details = [lock_one]
+    await _create_august_with_devices(hass, [lock_one])
-    await _create_august_with_devices(hass, lock_details)
 
     binary_sensor_online_with_doorsense_name = hass.states.get(
         "binary_sensor.online_with_doorsense_name_open"
     )
     assert binary_sensor_online_with_doorsense_name.state == STATE_ON
 
-    data = {}
+    data = {ATTR_ENTITY_ID: "lock.online_with_doorsense_name"}
-    data[ATTR_ENTITY_ID] = "lock.online_with_doorsense_name"
     assert await hass.services.async_call(
         LOCK_DOMAIN, SERVICE_UNLOCK, data, blocking=True
     )
@@ -57,8 +55,7 @@ async def test_doorsense(hass):
 async def test_create_doorbell(hass):
     """Test creation of a doorbell."""
     doorbell_one = await _mock_doorbell_from_fixture(hass, "get_doorbell.json")
-    doorbell_details = [doorbell_one]
+    await _create_august_with_devices(hass, [doorbell_one])
-    await _create_august_with_devices(hass, doorbell_details)
 
     binary_sensor_k98gidt45gul_name_motion = hass.states.get(
         "binary_sensor.k98gidt45gul_name_motion"
@@ -81,8 +78,7 @@ async def test_create_doorbell(hass):
 async def test_create_doorbell_offline(hass):
     """Test creation of a doorbell that is offline."""
     doorbell_one = await _mock_doorbell_from_fixture(hass, "get_doorbell.offline.json")
-    doorbell_details = [doorbell_one]
+    await _create_august_with_devices(hass, [doorbell_one])
-    await _create_august_with_devices(hass, doorbell_details)
 
     binary_sensor_tmt100_name_motion = hass.states.get(
         "binary_sensor.tmt100_name_motion"
@@ -99,11 +95,10 @@ async def test_create_doorbell_offline(hass):
 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)
+    await _create_august_with_devices(hass, [doorbell_one], activities=activities)
 
     binary_sensor_k98gidt45gul_name_motion = hass.states.get(
         "binary_sensor.k98gidt45gul_name_motion"
@@ -122,8 +117,7 @@ async def test_create_doorbell_with_motion(hass):
 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")
-    doorbell_details = [doorbell_one]
+    await _create_august_with_devices(hass, [doorbell_one])
-    await _create_august_with_devices(hass, doorbell_details)
 
     device_registry = await hass.helpers.device_registry.async_get_registry()
 

tests/components/august/test_camera.py

@@ -1,5 +1,7 @@
 """The camera tests for the august platform."""
 
+from asynctest import mock
+
 from homeassistant.const import STATE_IDLE
 
 from tests.components.august.mocks import (
@@ -8,11 +10,26 @@ from tests.components.august.mocks import (
 )
 
 
-async def test_create_doorbell(hass):
+async def test_create_doorbell(hass, aiohttp_client):
     """Test creation of a doorbell."""
     doorbell_one = await _mock_doorbell_from_fixture(hass, "get_doorbell.json")
-    doorbell_details = [doorbell_one]
-    await _create_august_with_devices(hass, doorbell_details)
 
-    camera_k98gidt45gul_name_camera = hass.states.get("camera.k98gidt45gul_name_camera")
+    with mock.patch.object(
+        doorbell_one, "get_doorbell_image", create=False, return_value="image"
+    ):
+        await _create_august_with_devices(hass, [doorbell_one])
+
+        camera_k98gidt45gul_name_camera = hass.states.get(
+            "camera.k98gidt45gul_name_camera"
+        )
         assert camera_k98gidt45gul_name_camera.state == STATE_IDLE
+
+        url = hass.states.get("camera.k98gidt45gul_name_camera").attributes[
+            "entity_picture"
+        ]
+
+        client = await aiohttp_client(hass.http.app)
+        resp = await client.get(url)
+        assert resp.status == 200
+        body = await resp.text()
+        assert body == "image"

tests/components/august/test_lock.py

@@ -12,6 +12,7 @@ from homeassistant.const import (
 
 from tests.components.august.mocks import (
     _create_august_with_devices,
+    _mock_activities_from_fixture,
     _mock_doorsense_enabled_august_lock_detail,
     _mock_lock_from_fixture,
 )
@@ -20,8 +21,7 @@ from tests.components.august.mocks import (
 async def test_lock_device_registry(hass):
     """Test creation of a lock with doorsense and bridge ands up in the registry."""
     lock_one = await _mock_doorsense_enabled_august_lock_detail(hass)
-    lock_details = [lock_one]
+    await _create_august_with_devices(hass, [lock_one])
-    await _create_august_with_devices(hass, lock_details)
 
     device_registry = await hass.helpers.device_registry.async_get_registry()
 
@@ -34,11 +34,27 @@ async def test_lock_device_registry(hass):
     assert reg_device.manufacturer == "August"
 
 
+async def test_lock_changed_by(hass):
+    """Test creation of a lock with doorsense and bridge."""
+    lock_one = await _mock_doorsense_enabled_august_lock_detail(hass)
+
+    activities = await _mock_activities_from_fixture(hass, "get_activity.lock.json")
+    await _create_august_with_devices(hass, [lock_one], activities=activities)
+
+    lock_online_with_doorsense_name = hass.states.get("lock.online_with_doorsense_name")
+
+    assert lock_online_with_doorsense_name.state == STATE_LOCKED
+
+    assert (
+        lock_online_with_doorsense_name.attributes.get("changed_by")
+        == "Your favorite elven princess"
+    )
+
+
 async def test_one_lock_operation(hass):
     """Test creation of a lock with doorsense and bridge."""
     lock_one = await _mock_doorsense_enabled_august_lock_detail(hass)
-    lock_details = [lock_one]
+    await _create_august_with_devices(hass, [lock_one])
-    await _create_august_with_devices(hass, lock_details)
 
     lock_online_with_doorsense_name = hass.states.get("lock.online_with_doorsense_name")
 
@@ -50,8 +66,7 @@ async def test_one_lock_operation(hass):
         == "online_with_doorsense Name"
     )
 
-    data = {}
+    data = {ATTR_ENTITY_ID: "lock.online_with_doorsense_name"}
-    data[ATTR_ENTITY_ID] = "lock.online_with_doorsense_name"
     assert await hass.services.async_call(
         LOCK_DOMAIN, SERVICE_UNLOCK, data, blocking=True
     )
@@ -78,8 +93,7 @@ async def test_one_lock_unknown_state(hass):
     lock_one = await _mock_lock_from_fixture(
         hass, "get_lock.online.unknown_state.json",
     )
-    lock_details = [lock_one]
+    await _create_august_with_devices(hass, [lock_one])
-    await _create_august_with_devices(hass, lock_details)
 
     lock_brokenid_name = hass.states.get("lock.brokenid_name")
 

tests/fixtures/august/get_activity.lock.json

@@ -0,0 +1,34 @@
+[{
+   "entities" : {
+      "activity" : "mockActivity2",
+      "house" : "123",
+      "device" : "online_with_doorsense",
+      "callingUser" : "mockUserId2",
+      "otherUser" : "deleted"
+   },
+   "callingUser" : {
+      "LastName" : "elven princess",
+      "UserID" : "mockUserId2",
+      "FirstName" : "Your favorite"
+   },
+   "otherUser" : {
+      "LastName" : "User",
+      "UserName" : "deleteduser",
+      "FirstName" : "Unknown",
+      "UserID" : "deleted",
+      "PhoneNo" : "deleted"
+   },
+   "deviceType" : "lock",
+   "deviceName" : "MockHouseTDoor",
+   "action" : "lock",
+   "dateTime" : 1582007218000,
+   "info" : {
+      "remote" : true,
+      "DateLogActionID" : "ABC+Time"
+   },
+   "deviceID" : "online_with_doorsense",
+   "house" : {
+      "houseName" : "MockHouse",
+      "houseID" : "123"
+   }
+}]