Make process_changes return a set of what actually changed (#339)

aiohomekit/model/__init__.py

@@ -27,6 +27,7 @@
 from . import entity_map
 from .categories import Categories
 from .characteristics import (
+    EVENT_CHARACTERISTICS,
     Characteristic,
     CharacteristicFormats,
     CharacteristicPermissions,
@@ -407,16 +408,29 @@ def has_aid(self, aid: int) -> bool:
         """Return True if the given aid exists."""
         return aid in self._aid_to_accessory
 
-    def process_changes(self, changes: dict[tuple[int, int], dict[str, Any]]) -> None:
-        """Process changes from a HomeKit controller."""
-        for (aid, iid), value in changes.items():
+    def process_changes(
+        self, changes: dict[tuple[int, int], dict[str, Any]]
+    ) -> set[tuple[int, int]]:
+        """Process changes from a HomeKit controller.
+
+        Returns a set of the changes that were applied.
+        """
+        changed: set[tuple[int, int]] = set()
+        for aid_iid, value in changes.items():
+            (aid, iid) = aid_iid
             if not (char := self.aid(aid).characteristics.iid(iid)):
                 continue
 
             if "value" in value:
-                char.set_value(value["value"])
+                if char.set_value(value["value"]) or char.type in EVENT_CHARACTERISTICS:
+                    changed.add(aid_iid)
 
+            previous_status = char.status
             char.status = to_status_code(value.get("status", 0))
+            if previous_status != char.status:
+                changed.add(aid_iid)
+
+        return changed
 
 
 @dataclass

aiohomekit/model/characteristics/characteristic.py

@@ -175,16 +175,20 @@ def available(self) -> bool:
     def set_events(self, new_val: Any) -> None:
         self.ev = new_val
 
-    def set_value(self, new_val: Any) -> None:
+    def set_value(self, new_val: Any) -> bool:
         """
         This function sets the value of this characteristic.
+
+        Returns True if the value has changed, False otherwise.
         """
 
         if self.format == CharacteristicFormats.bool:
             # Device might return 1 or 0, so lets case to True/False
             new_val = bool(new_val)
 
+        changed = self._value != new_val
         self._value = new_val
+        return changed
 
     @property
     def value(self) -> Any:

aiohomekit/protocol/statuscodes.py

@@ -41,8 +41,7 @@ class HapStatusCode(EnumWithDescription):
 
 def to_status_code(status_code: int) -> HapStatusCode:
     # Some HAP implementations return positive values for error code (myq)
-    status_code = abs(status_code) * -1
-    return HapStatusCode(status_code)
+    return HapStatusCode(abs(status_code) * -1)
 
 
 class _HapBleStatusCodes:

aiohomekit/testing.py

@@ -34,7 +34,10 @@
 from aiohomekit.exceptions import AccessoryNotFoundError
 from aiohomekit.model import Accessories, AccessoriesState, Transport
 from aiohomekit.model.categories import Categories
-from aiohomekit.model.characteristics import CharacteristicsTypes
+from aiohomekit.model.characteristics import Characteristic, CharacteristicsTypes
+from aiohomekit.model.characteristics.characteristic_formats import (
+    CharacteristicFormats,
+)
 from aiohomekit.model.status_flags import StatusFlags
 from aiohomekit.protocol.statuscodes import HapStatusCode
 from aiohomekit.uuid import normalize_uuid
@@ -154,29 +157,39 @@ def update_named_service(self, name: str, new_values):
                     f"Unexpected characteristic {uuid!r} applied to service {name!r}"
                 )
 
-            char = service[uuid]
-            char.set_value(value)
-            changed.append((char.service.accessory.aid, char.iid))
+            char: Characteristic = service[uuid]
+            changed.append((char.service.accessory.aid, char.iid, value))
 
         self._send_events(changed)
 
     def update_aid_iid(self, characteristics):
-        changed = []
-        for aid, iid, value in characteristics:
-            self.characteristics[(aid, iid)].set_value(value)
-            changed.append((aid, iid))
+        self._send_events(characteristics)
 
-        self._send_events(changed)
+    def set_aid_iid_status(self, aid_iid_statuses: list[tuple[int, int, int]]):
+        """Set status for an aid iid pair."""
+        event = {
+            (aid, iid): {"status": status} for aid, iid, status in aid_iid_statuses
+        }
 
-    def _send_events(self, changed):
+        if not event:
+            return
+
+        for listener in self.pairing.listeners:
+            try:
+                listener(event)
+            except Exception:
+                _LOGGER.exception("Unhandled error when processing event")
+
+    def _send_events(self, characteristics):
         if not self.events_enabled:
             return
 
         event = {}
-        for aid, iid in changed:
-            if (aid, iid) not in self.pairing.subscriptions:
-                continue
-            event[(aid, iid)] = {"value": self.characteristics[(aid, iid)].get_value()}
+        for aid, iid, value in characteristics:
+            char: Characteristic = self.characteristics[(aid, iid)]
+            if char.format == CharacteristicFormats.bool:
+                value = bool(value)
+            event[(aid, iid)] = {"value": value}
 
         if not event:
             return

tests/test_model.py

@@ -215,7 +215,11 @@ def test_process_changes():
     on_char = accessories.aid(1).characteristics.iid(8)
     assert on_char.value is False
 
-    accessories.process_changes({(1, 8): {"value": True}})
+    changed = accessories.process_changes({(1, 8): {"value": True}})
+    assert changed == {(1, 8)}
+
+    changed = accessories.process_changes({(1, 8): {"value": True}})
+    assert changed == set()
 
     assert on_char.value is True
 
@@ -227,14 +231,16 @@ def test_process_changes_error():
     assert on_char.value is False
     assert on_char.status == HapStatusCode.SUCCESS
 
-    accessories.process_changes(
+    changed = accessories.process_changes(
         {(1, 8): {"status": HapStatusCode.UNABLE_TO_COMMUNICATE.value}}
     )
 
     assert on_char.value is False
     assert on_char.status == HapStatusCode.UNABLE_TO_COMMUNICATE
+    assert changed == {(1, 8)}
 
-    accessories.process_changes({(1, 8): {"value": True}})
+    changed = accessories.process_changes({(1, 8): {"value": True}})
+    assert changed == {(1, 8)}
     assert on_char.value is True
     assert on_char.status == HapStatusCode.SUCCESS
 
@@ -248,15 +254,17 @@ def test_process_changes_availability():
     assert on_char.service.available is True
     assert on_char.service.accessory.available is True
 
-    accessories.process_changes(
+    changed = accessories.process_changes(
         {(1, 8): {"status": HapStatusCode.UNABLE_TO_COMMUNICATE.value}}
     )
+    assert changed == {(1, 8)}
 
     assert on_char.available is False
     assert on_char.service.available is False
     assert on_char.service.accessory.available is False
 
-    accessories.process_changes({(1, 8): {"value": True}})
+    changed = accessories.process_changes({(1, 8): {"value": True}})
+    assert changed == {(1, 8)}
     assert on_char.available is True
     assert on_char.service.available is True
     assert on_char.service.accessory.available is True

tests/test_testing.py

@@ -29,6 +29,8 @@ async def test_get_and_set():
     finish_pairing = await discovery.async_start_pairing("alias")
     pairing = await finish_pairing("111-22-333")
 
+    pairing.dispatcher_connect(accessories.process_changes)
+
     chars = await pairing.get_characteristics([(1, 10)])
     assert chars == {(1, 10): {"value": 0}}
 
@@ -78,14 +80,13 @@ async def test_update_named_service_events():
     controller = FakeController()
     pairing = await controller.add_paired_device(accessories, "alias")
 
-    callback = mock.Mock()
     await pairing.subscribe([(1, 8)])
-    pairing.dispatcher_connect(callback)
+    pairing.dispatcher_connect(accessories.process_changes)
 
     # Simulate that the state was changed on the device itself.
     pairing.testing.update_named_service("Light Strip", {CharacteristicsTypes.ON: True})
 
-    assert callback.call_args_list == [mock.call({(1, 8): {"value": 1}})]
+    assert accessories.aid(1).characteristics.iid(8).value == 1
 
 
 async def test_update_named_service_events_manual_accessory():
@@ -166,14 +167,13 @@ async def test_events_are_filtered():
     controller = FakeController()
     pairing = await controller.add_paired_device(accessories, "alias")
 
-    callback = mock.Mock()
     await pairing.subscribe([(1, 10)])
-    pairing.dispatcher_connect(callback)
+    pairing.dispatcher_connect(accessories.process_changes)
 
     # Simulate that the state was changed on the device itself.
     pairing.testing.update_named_service("Light Strip", {CharacteristicsTypes.ON: True})
 
-    assert callback.call_args_list == []
+    assert accessories.aid(1).characteristics.iid(8).value == 1
 
 
 async def test_camera():