From 8117856c2734b6ab06321997ca357b7cd1b0901d Mon Sep 17 00:00:00 2001 From: "J. Nick Koston" Date: Thu, 19 Mar 2020 05:12:09 +0000 Subject: [PATCH] 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. --- pyhap/accessory_driver.py | 23 ++++++++++++++++ pyhap/characteristic.py | 3 ++- pyhap/service.py | 4 ++- tests/test_accessory_driver.py | 49 ++++++++++++++++++++++++++++++++-- 4 files changed, 75 insertions(+), 4 deletions(-) diff --git a/pyhap/accessory_driver.py b/pyhap/accessory_driver.py index dfe6163e..6a4161e8 100644 --- a/pyhap/accessory_driver.py +++ b/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. diff --git a/pyhap/characteristic.py b/pyhap/characteristic.py index 881a87ba..bdd2888f 100644 --- a/pyhap/characteristic.py +++ b/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.""" diff --git a/pyhap/service.py b/pyhap/service.py index c68686db..f2c2d1f1 100644 --- a/pyhap/service.py +++ b/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): diff --git a/tests/test_accessory_driver.py b/tests/test_accessory_driver.py index 74c66dbd..00849662 100644 --- a/tests/test_accessory_driver.py +++ b/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