Merge pull request #229 from bdraco/service_callback_brightness_fix

Add support for service callbacks

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