Service callback for set_characteristics

For some services we want to send all the char value
changes at once.  This resolves an issue where we send
ON and then BRIGHTNESS and the light would go to 100%
and then dim to the brightness because each callback
would only send one char at a time.

Author: bdraco

pyhap/accessory_driver.py

@@ -54,6 +54,8 @@
 
 CHAR_STAT_OK = 0
 SERVICE_COMMUNICATION_FAILURE = -70402
+SERVICE_CALLBACK = 0
+SERVICE_CALLBACK_DATA = 1
 
 
 def callback(func):
@@ -633,6 +635,7 @@ def set_characteristics(self, chars_query, client_addr):
         :type chars_query: dict
         """
         # TODO: Add support for chars that do no support notifications.
+        service_callbacks = {}
         for cq in chars_query[HAP_REPR_CHARS]:
             aid, iid = cq[HAP_REPR_AID], cq[HAP_REPR_IID]
             char = self.accessory.get_characteristic(aid, iid)
@@ -647,6 +650,26 @@ def set_characteristics(self, chars_query, client_addr):
             if HAP_REPR_VALUE in cq:
                 # TODO: status needs to be based on success of set_value
                 char.client_update_value(cq[HAP_REPR_VALUE], client_addr)
+                # For some services we want to send all the char value
+                # changes at once.  This resolves an issue where we send
+                # ON and then BRIGHTNESS and the light would go to 100%
+                # and then dim to the brightness because each callback
+                # would only send one char at a time.
+                service = char.service
+
+                if service and service.setter_callback:
+                    service_callbacks.setdefault(
+                        service.display_name,
+                        [service.setter_callback, {}]
+                    )
+                    service_callbacks[service.display_name][
+                        SERVICE_CALLBACK_DATA
+                    ][char.display_name] = cq[HAP_REPR_VALUE]
+
+        for service_name in service_callbacks:
+            service_callbacks[service_name][SERVICE_CALLBACK](
+                service_callbacks[service_name][SERVICE_CALLBACK_DATA]
+            )
 
     def signal_handler(self, _signal, _frame):
         """Stops the AccessoryDriver for a given signal.

pyhap/characteristic.py

@@ -80,7 +80,7 @@ class Characteristic:
     """
 
     __slots__ = ('broker', 'display_name', 'properties', 'type_id',
-                 'value', 'getter_callback', 'setter_callback')
+                 'value', 'getter_callback', 'setter_callback', 'service')
 
     def __init__(self, display_name, type_id, properties):
         """Initialise with the given properties.
@@ -103,6 +103,7 @@ def __init__(self, display_name, type_id, properties):
         self.value = self._get_default_value()
         self.getter_callback = None
         self.setter_callback = None
+        self.service = None
 
     def __repr__(self):
         """Return the representation of the characteristic."""

pyhap/service.py

@@ -14,7 +14,7 @@ class Service:
     """
 
     __slots__ = ('broker', 'characteristics', 'display_name', 'type_id',
-                 'linked_services', 'is_primary_service')
+                 'linked_services', 'is_primary_service', 'setter_callback')
 
     def __init__(self, type_id, display_name=None):
         """Initialize a new Service object."""
@@ -24,6 +24,7 @@ def __init__(self, type_id, display_name=None):
         self.display_name = display_name
         self.type_id = type_id
         self.is_primary_service = None
+        self.setter_callback = None
 
     def __repr__(self):
         """Return the representation of the service."""
@@ -43,6 +44,7 @@ def add_characteristic(self, *chars):
         for char in chars:
             if not any(char.type_id == original_char.type_id
                        for original_char in self.characteristics):
+                char.service = self
                 self.characteristics.append(char)
 
     def get_characteristic(self, name):

tests/test_accessory_driver.py

@@ -1,11 +1,22 @@
 """Tests for pyhap.accessory_driver."""
 import tempfile
-from unittest.mock import patch
+from unittest.mock import MagicMock, patch
+from uuid import uuid1
 
 import pytest
 
-from pyhap.accessory import Accessory, STANDALONE_AID
+from pyhap.accessory import STANDALONE_AID, Accessory
 from pyhap.accessory_driver import AccessoryDriver
+from pyhap.characteristic import (HAP_FORMAT_INT, HAP_PERMISSION_READ,
+                                  PROP_FORMAT, PROP_PERMISSIONS,
+                                  Characteristic)
+from pyhap.const import HAP_REPR_IID, HAP_REPR_CHARS, HAP_REPR_AID, HAP_REPR_VALUE
+from pyhap.service import Service
+
+CHAR_PROPS = {
+    PROP_FORMAT: HAP_FORMAT_INT,
+    PROP_PERMISSIONS: HAP_PERMISSION_READ,
+}
 
 
 @pytest.fixture
@@ -43,6 +54,40 @@ def test_persist_load():
     assert driver.state.public_key == pk
 
 
+def test_service_callbacks(driver):
+    acc = Accessory(driver, 'TestAcc')
+
+    service = Service(uuid1(), 'Lightbulb')
+    char_on = Characteristic('On', uuid1(), CHAR_PROPS)
+    char_brightness = Characteristic('Brightness', uuid1(), CHAR_PROPS)
+
+    service.add_characteristic(char_on)
+    service.add_characteristic(char_brightness)
+
+    mock_callback = MagicMock()
+    service.setter_callback = mock_callback
+
+    acc.add_service(service)
+    driver.add_accessory(acc)
+
+    char_on_iid = char_on.to_HAP()[HAP_REPR_IID]
+    char_brightness_iid = char_brightness.to_HAP()[HAP_REPR_IID]
+
+    driver.set_characteristics({
+        HAP_REPR_CHARS: [{
+            HAP_REPR_AID: acc.aid,
+            HAP_REPR_IID: char_on_iid,
+            HAP_REPR_VALUE: True
+        }, {
+            HAP_REPR_AID: acc.aid,
+            HAP_REPR_IID: char_brightness_iid,
+            HAP_REPR_VALUE: 88
+        }]
+    }, "mock_addr")
+
+    mock_callback.assert_called_with({'On': True, 'Brightness': 88})
+
+
 def test_start_stop_sync_acc(driver):
     class Acc(Accessory):
         running = True