diff --git a/pyhap/accessory.py b/pyhap/accessory.py index 6dc97309..f8a95f57 100644 --- a/pyhap/accessory.py +++ b/pyhap/accessory.py @@ -222,7 +222,7 @@ def get_characteristic(self, aid: int, iid: int) -> Optional["Characteristic"]: return self.iid_manager.get_obj(iid) - def to_HAP(self) -> Dict[str, Any]: + def to_HAP(self, include_value: bool = True) -> Dict[str, Any]: """A HAP representation of this Accessory. :return: A HAP representation of this accessory. For example: @@ -241,7 +241,7 @@ def to_HAP(self) -> Dict[str, Any]: """ return { HAP_REPR_AID: self.aid, - HAP_REPR_SERVICES: [s.to_HAP() for s in self.services], + HAP_REPR_SERVICES: [s.to_HAP(include_value=include_value) for s in self.services], } def setup_message(self): @@ -386,12 +386,12 @@ def add_accessory(self, acc: "Accessory") -> None: self.accessories[acc.aid] = acc - def to_HAP(self) -> List[Dict[str, Any]]: + def to_HAP(self, include_value: bool = True) -> List[Dict[str, Any]]: """Returns a HAP representation of itself and all contained accessories. .. seealso:: Accessory.to_HAP """ - return [acc.to_HAP() for acc in (super(), *self.accessories.values())] + return [acc.to_HAP(include_value=include_value) for acc in (super(), *self.accessories.values())] def get_characteristic(self, aid: int, iid: int) -> Optional["Characteristic"]: """.. seealso:: Accessory.to_HAP""" diff --git a/pyhap/accessory_driver.py b/pyhap/accessory_driver.py index c767f590..3cc160fc 100644 --- a/pyhap/accessory_driver.py +++ b/pyhap/accessory_driver.py @@ -745,11 +745,18 @@ def setup_srp_verifier(self): @property def accessories_hash(self): """Hash the get_accessories response to track configuration changes.""" + # We pass include_value=False to avoid including the value + # of the characteristics in the hash. This is because the + # value of the characteristics is not used by iOS to determine + # if the accessory configuration has changed. It only uses the + # characteristics metadata. If we included the value in the hash + # then iOS would think the accessory configuration has changed + # every time a characteristic value changed. return hashlib.sha512( - util.to_sorted_hap_json(self.get_accessories()) + util.to_sorted_hap_json(self.get_accessories(include_value=False)) ).hexdigest() - def get_accessories(self): + def get_accessories(self, include_value: bool = True): """Returns the accessory in HAP format. :return: An example HAP representation is: @@ -774,7 +781,7 @@ def get_accessories(self): :rtype: dict """ - hap_rep = self.accessory.to_HAP() + hap_rep = self.accessory.to_HAP(include_value=include_value) if not isinstance(hap_rep, list): hap_rep = [ hap_rep, diff --git a/pyhap/characteristic.py b/pyhap/characteristic.py index a35612dd..3505a92a 100644 --- a/pyhap/characteristic.py +++ b/pyhap/characteristic.py @@ -125,10 +125,10 @@ class Characteristic: __slots__ = ( "broker", - "display_name", - "properties", + "_display_name", + "_properties", "type_id", - "value", + "_value", "getter_callback", "setter_callback", "service", @@ -136,6 +136,9 @@ class Characteristic: "_loader_display_name", "allow_invalid_client_values", "unique_id", + "_to_hap_cache_with_value", + "_to_hap_cache", + "_always_null", ) def __init__( @@ -169,34 +172,68 @@ def __init__( # to True and handle converting the Auto state to Cool or Heat # depending on the device. # + self._always_null = type_id in ALWAYS_NULL self.allow_invalid_client_values = allow_invalid_client_values - self.display_name = display_name - self.properties: Dict[str, Any] = properties + self._display_name = display_name + self._properties: Dict[str, Any] = properties self.type_id = type_id - self.value = self._get_default_value() + self._value = self._get_default_value() self.getter_callback: Optional[Callable[[], Any]] = None self.setter_callback: Optional[Callable[[Any], None]] = None self.service: Optional["Service"] = None self.unique_id = unique_id self._uuid_str = uuid_to_hap_type(type_id) self._loader_display_name: Optional[str] = None + self._to_hap_cache_with_value: Optional[Dict[str, Any]] = None + self._to_hap_cache: Optional[Dict[str, Any]] = None + + @property + def display_name(self) -> Optional[str]: + """Return the display name of the characteristic.""" + return self._display_name + + @display_name.setter + def display_name(self, value: str) -> None: + """Set the display name of the characteristic.""" + self._display_name = value + self._clear_cache() + + @property + def value(self) -> Any: + """Return the value of the characteristic.""" + return self._value + + @value.setter + def value(self, value: Any) -> None: + """Set the value of the characteristic.""" + self._value = value + self._clear_cache() + + @property + def properties(self) -> Dict[str, Any]: + """Return the properties of the characteristic. + + Properties should not be modified directly. Use override_properties instead. + """ + return self._properties def __repr__(self) -> str: """Return the representation of the characteristic.""" return ( - f"" + f"" ) def _get_default_value(self) -> Any: """Return default value for format.""" - if self.type_id in ALWAYS_NULL: + if self._always_null: return None - if self.properties.get(PROP_VALID_VALUES): - return min(self.properties[PROP_VALID_VALUES].values()) + valid_values = self._properties.get(PROP_VALID_VALUES) + if valid_values: + return min(valid_values.values()) - value = HAP_FORMAT_DEFAULTS[self.properties[PROP_FORMAT]] + value = HAP_FORMAT_DEFAULTS[self._properties[PROP_FORMAT]] return self.to_valid_value(value) def get_value(self) -> Any: @@ -207,43 +244,47 @@ def get_value(self) -> Any: if self.getter_callback: # pylint: disable=not-callable self.value = self.to_valid_value(value=self.getter_callback()) - return self.value + return self._value def valid_value_or_raise(self, value: Any) -> None: """Raise ValueError if PROP_VALID_VALUES is set and the value is not present.""" - if self.type_id in ALWAYS_NULL: + if self._always_null: return - valid_values = self.properties.get(PROP_VALID_VALUES) + valid_values = self._properties.get(PROP_VALID_VALUES) if not valid_values: return if value in valid_values.values(): return - error_msg = f"{self.display_name}: value={value} is an invalid value." + error_msg = f"{self._display_name}: value={value} is an invalid value." logger.error(error_msg) raise ValueError(error_msg) def to_valid_value(self, value: Any) -> Any: """Perform validation and conversion to valid value.""" - if self.properties[PROP_FORMAT] == HAP_FORMAT_STRING: - value = str(value)[ - : self.properties.get(HAP_REPR_MAX_LEN, DEFAULT_MAX_LENGTH) - ] - elif self.properties[PROP_FORMAT] == HAP_FORMAT_BOOL: - value = bool(value) - elif self.properties[PROP_FORMAT] in HAP_FORMAT_NUMERICS: + properties = self._properties + prop_format = properties[PROP_FORMAT] + + if prop_format == HAP_FORMAT_STRING: + return str(value)[: properties.get(HAP_REPR_MAX_LEN, DEFAULT_MAX_LENGTH)] + + if prop_format == HAP_FORMAT_BOOL: + return bool(value) + + if prop_format in HAP_FORMAT_NUMERICS: if not isinstance(value, (int, float)): error_msg = ( - f"{self.display_name}: value={value} is not a numeric value." + f"{self._display_name}: value={value} is not a numeric value." ) logger.error(error_msg) raise ValueError(error_msg) - min_step = self.properties.get(PROP_MIN_STEP) + min_step = properties.get(PROP_MIN_STEP) if value and min_step: value = round(min_step * round(value / min_step), 14) - value = min(self.properties.get(PROP_MAX_VALUE, value), value) - value = max(self.properties.get(PROP_MIN_VALUE, value), value) - if self.properties[PROP_FORMAT] != HAP_FORMAT_FLOAT: - value = int(value) + value = min(properties.get(PROP_MAX_VALUE, value), value) + value = max(properties.get(PROP_MIN_VALUE, value), value) + if prop_format != HAP_FORMAT_FLOAT: + return int(value) + return value def override_properties( @@ -264,23 +305,30 @@ def override_properties( if not properties and not valid_values: raise ValueError("No properties or valid_values specified to override.") + self._clear_cache() + if properties: _validate_properties(properties) - self.properties.update(properties) + self._properties.update(properties) if valid_values: - self.properties[PROP_VALID_VALUES] = valid_values + self._properties[PROP_VALID_VALUES] = valid_values - if self.type_id in ALWAYS_NULL: + if self._always_null: self.value = None return try: - self.value = self.to_valid_value(self.value) - self.valid_value_or_raise(self.value) + self.value = self.to_valid_value(self._value) + self.valid_value_or_raise(self._value) except ValueError: self.value = self._get_default_value() + def _clear_cache(self) -> None: + """Clear the cached HAP representation.""" + self._to_hap_cache = None + self._to_hap_cache_with_value = None + def set_value(self, value: Any, should_notify: bool = True) -> None: """Set the given raw value. It is checked if it is a valid value. @@ -300,14 +348,14 @@ def set_value(self, value: Any, should_notify: bool = True) -> None: subscribed clients. Notify will be performed if the broker is set. :type should_notify: bool """ - logger.debug("set_value: %s to %s", self.display_name, value) + logger.debug("set_value: %s to %s", self._display_name, value) value = self.to_valid_value(value) self.valid_value_or_raise(value) - changed = self.value != value + changed = self._value != value self.value = value if changed and should_notify and self.broker: self.notify() - if self.type_id in ALWAYS_NULL: + if self._always_null: self.value = None def client_update_value( @@ -318,27 +366,27 @@ def client_update_value( Change self.value to value and call callback. """ original_value = value - if self.type_id not in ALWAYS_NULL or original_value is not None: + if not self._always_null or original_value is not None: value = self.to_valid_value(value) if not self.allow_invalid_client_values: self.valid_value_or_raise(value) logger.debug( "client_update_value: %s to %s (original: %s) from client: %s", - self.display_name, + self._display_name, value, original_value, sender_client_addr, ) - previous_value = self.value + previous_value = self._value self.value = value response = None if self.setter_callback: # pylint: disable=not-callable response = self.setter_callback(value) - changed = self.value != previous_value + changed = self._value != previous_value if changed: self.notify(sender_client_addr) - if self.type_id in ALWAYS_NULL: + if self._always_null: self.value = None return response @@ -352,7 +400,7 @@ def notify(self, sender_client_addr: Optional[Tuple[str, int]] = None) -> None: self.broker.publish(self.value, self, sender_client_addr, immediate) # pylint: disable=invalid-name - def to_HAP(self) -> Dict[str, Any]: + def to_HAP(self, include_value: bool = True) -> Dict[str, Any]: """Create a HAP representation of this Characteristic. Used for json serialization. @@ -360,41 +408,51 @@ def to_HAP(self) -> Dict[str, Any]: :return: A HAP representation. :rtype: dict """ + if include_value: + if self._to_hap_cache_with_value is not None and not self.getter_callback: + return self._to_hap_cache_with_value + elif self._to_hap_cache is not None: + return self._to_hap_cache + + properties = self._properties + permissions = properties[PROP_PERMISSIONS] + prop_format = properties[PROP_FORMAT] hap_rep = { HAP_REPR_IID: self.broker.iid_manager.get_iid(self), HAP_REPR_TYPE: self._uuid_str, - HAP_REPR_PERM: self.properties[PROP_PERMISSIONS], - HAP_REPR_FORMAT: self.properties[PROP_FORMAT], + HAP_REPR_PERM: permissions, + HAP_REPR_FORMAT: prop_format, } # HAP_REPR_DESC (description) is optional and takes up # quite a bit of space in the payload. Only include it # if it has been changed from the default loader version - if ( - not self._loader_display_name - or self._loader_display_name != self.display_name - ): - hap_rep[HAP_REPR_DESC] = self.display_name - - value = self.get_value() - if self.properties[PROP_FORMAT] in HAP_FORMAT_NUMERICS: + loader_display_name = self._loader_display_name + display_name = self._display_name + if not loader_display_name or loader_display_name != display_name: + hap_rep[HAP_REPR_DESC] = display_name + + if prop_format in HAP_FORMAT_NUMERICS: hap_rep.update( - { - k: self.properties[k] - for k in PROP_NUMERIC.intersection(self.properties) - } + {k: properties[k] for k in PROP_NUMERIC.intersection(properties)} ) - if PROP_VALID_VALUES in self.properties: + if PROP_VALID_VALUES in properties: hap_rep[HAP_REPR_VALID_VALUES] = sorted( - self.properties[PROP_VALID_VALUES].values() + properties[PROP_VALID_VALUES].values() ) - elif self.properties[PROP_FORMAT] == HAP_FORMAT_STRING: - max_length = self.properties.get(HAP_REPR_MAX_LEN, DEFAULT_MAX_LENGTH) + elif prop_format == HAP_FORMAT_STRING: + max_length = properties.get(HAP_REPR_MAX_LEN, DEFAULT_MAX_LENGTH) if max_length != DEFAULT_MAX_LENGTH: hap_rep[HAP_REPR_MAX_LEN] = max_length - if HAP_PERMISSION_READ in self.properties[PROP_PERMISSIONS]: - hap_rep[HAP_REPR_VALUE] = value + if include_value and HAP_PERMISSION_READ in permissions: + hap_rep[HAP_REPR_VALUE] = self.get_value() + + if not include_value: + self._to_hap_cache = hap_rep + elif not self.getter_callback: + # Only cache if there is no getter_callback + self._to_hap_cache_with_value = hap_rep return hap_rep @classmethod diff --git a/pyhap/service.py b/pyhap/service.py index 1579809c..e31b3920 100644 --- a/pyhap/service.py +++ b/pyhap/service.py @@ -117,7 +117,7 @@ def configure_char( return char # pylint: disable=invalid-name - def to_HAP(self) -> Dict[str, Any]: + def to_HAP(self, include_value: bool = True) -> Dict[str, Any]: """Create a HAP representation of this Service. :return: A HAP representation. @@ -126,7 +126,7 @@ def to_HAP(self) -> Dict[str, Any]: hap = { HAP_REPR_IID: self.broker.iid_manager.get_iid(self), HAP_REPR_TYPE: self._uuid_str, - HAP_REPR_CHARS: [c.to_HAP() for c in self.characteristics], + HAP_REPR_CHARS: [c.to_HAP(include_value) for c in self.characteristics], } if self.is_primary_service is not None: diff --git a/tests/test_accessory_driver.py b/tests/test_accessory_driver.py index 98770a21..e7234fb1 100644 --- a/tests/test_accessory_driver.py +++ b/tests/test_accessory_driver.py @@ -147,7 +147,9 @@ def test_write_response_returned_when_not_requested(driver: AccessoryDriver): bridge = Bridge(driver, "mybridge") acc = Accessory(driver, "TestAcc", aid=2) service = Service(uuid1(), "NFCAccess") - char_nfc_access_control_point = Characteristic("NFCAccessControlPoint", uuid1(), CHAR_PROPS) + char_nfc_access_control_point = Characteristic( + "NFCAccessControlPoint", uuid1(), CHAR_PROPS + ) service.add_characteristic(char_nfc_access_control_point) mock_callback = MagicMock() @@ -159,7 +161,9 @@ def setter_with_write_response(value=0): char_nfc_access_control_point.setter_callback = setter_with_write_response acc.add_service(service) - char_nfc_access_control_point_iid = char_nfc_access_control_point.to_HAP()[HAP_REPR_IID] + char_nfc_access_control_point_iid = char_nfc_access_control_point.to_HAP()[ + HAP_REPR_IID + ] bridge.add_accessory(acc) driver.add_accessory(bridge) @@ -171,7 +175,7 @@ def setter_with_write_response(value=0): HAP_REPR_AID: acc.aid, HAP_REPR_IID: char_nfc_access_control_point_iid, HAP_REPR_VALUE: 0, - HAP_REPR_WRITE_RESPONSE: False + HAP_REPR_WRITE_RESPONSE: False, } ] }, @@ -198,7 +202,9 @@ def test_write_response_returned_when_requested(driver: AccessoryDriver): bridge = Bridge(driver, "mybridge") acc = Accessory(driver, "TestAcc", aid=2) service = Service(uuid1(), "NFCAccess") - char_nfc_access_control_point = Characteristic("NFCAccessControlPoint", uuid1(), CHAR_PROPS) + char_nfc_access_control_point = Characteristic( + "NFCAccessControlPoint", uuid1(), CHAR_PROPS + ) service.add_characteristic(char_nfc_access_control_point) mock_callback = MagicMock() @@ -210,7 +216,9 @@ def setter_with_write_response(value=0): char_nfc_access_control_point.setter_callback = setter_with_write_response acc.add_service(service) - char_nfc_access_control_point_iid = char_nfc_access_control_point.to_HAP()[HAP_REPR_IID] + char_nfc_access_control_point_iid = char_nfc_access_control_point.to_HAP()[ + HAP_REPR_IID + ] bridge.add_accessory(acc) driver.add_accessory(bridge) @@ -222,7 +230,7 @@ def setter_with_write_response(value=0): HAP_REPR_AID: acc.aid, HAP_REPR_IID: char_nfc_access_control_point_iid, HAP_REPR_VALUE: 0, - HAP_REPR_WRITE_RESPONSE: True + HAP_REPR_WRITE_RESPONSE: True, } ] }, @@ -234,7 +242,7 @@ def setter_with_write_response(value=0): HAP_REPR_AID: acc.aid, HAP_REPR_IID: char_nfc_access_control_point_iid, HAP_REPR_STATUS: 0, - HAP_REPR_VALUE: 1 + HAP_REPR_VALUE: 1, }, ] } @@ -1169,3 +1177,59 @@ async def test_bridge_with_multiple_sync_run_at_interval_accessories(async_zeroc assert acc.counter > 2 assert acc2.counter > 2 assert acc3.counter > 2 + + +def test_hash_ignores_values(driver: AccessoryDriver): + """The hash should change when the config changes but not for a value change.""" + bridge = Bridge(driver, "mybridge") + acc = Accessory(driver, "TestAcc", aid=2) + acc2 = UnavailableAccessory(driver, "TestAcc2", aid=3) + + 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) + + switch_service = Service(uuid1(), "Switch") + char_switch_on = Characteristic("On", uuid1(), CHAR_PROPS) + switch_service.add_characteristic(char_switch_on) + + mock_callback = MagicMock() + acc.setter_callback = mock_callback + + acc.add_service(service) + acc.add_service(switch_service) + bridge.add_accessory(acc) + + service2 = Service(uuid1(), "Lightbulb") + char_on2 = Characteristic("On", uuid1(), CHAR_PROPS) + char_brightness2 = Characteristic("Brightness", uuid1(), CHAR_PROPS) + + service2.add_characteristic(char_on2) + service2.add_characteristic(char_brightness2) + + mock_callback2 = MagicMock(side_effect=OSError) + acc2.setter_callback = mock_callback2 + + bridge.add_accessory(acc2) + + driver.add_accessory(bridge) + + original_hash = driver.accessories_hash + + char_on.set_value(False) + char_on2.set_value(False) + char_brightness.set_value(88) + char_switch_on.set_value(True) + + assert driver.accessories_hash == original_hash + + acc2.add_service(service2) + new_hash = driver.accessories_hash + assert new_hash != original_hash + + char_brightness2.set_value(43) + + assert driver.accessories_hash == new_hash diff --git a/tests/test_characteristic.py b/tests/test_characteristic.py index d7e2eca5..12d32713 100644 --- a/tests/test_characteristic.py +++ b/tests/test_characteristic.py @@ -220,6 +220,11 @@ def test_set_value_invalid_min_float(): # Ensure value is not modified assert char.value == 0 + char.value = 99 + assert char.value == 99 + char.set_value(0) + assert char.value == 0 + @pytest.mark.parametrize("int_format", HAP_FORMAT_INTS) def test_set_value_int(int_format): @@ -468,13 +473,16 @@ def test_to_HAP_string_max_length_override(): def test_to_HAP_bool(): """Test created HAP representation for booleans.""" + # pylint: disable=protected-access char = get_char(PROPERTIES.copy()) char.properties["Format"] = "bool" + char._clear_cache() with patch.object(char, "broker"): hap_repr = char.to_HAP() assert hap_repr["format"] == "bool" char.properties["Permissions"] = [] + char._clear_cache() with patch.object(char, "broker"): hap_repr = char.to_HAP() assert "value" not in hap_repr @@ -493,3 +501,74 @@ def test_from_dict(): assert char.display_name == "Test Char" assert char.type_id == uuid assert char.properties == {"Format": "int", "Permissions": "read"} + + +def test_getter_callback(): + """Test getter callback.""" + char = Characteristic( + display_name="Test Char", type_id="A1", properties=PROPERTIES.copy() + ) + char.set_value(3) + char.override_properties({"minValue": 3, "maxValue": 10}) + char.broker = Mock() + assert char.to_HAP() == { + "description": "Test Char", + "format": "int", + "iid": ANY, + "maxValue": 10, + "minValue": 3, + "perms": ["pr"], + "type": "A1", + "value": 3, + } + + assert char.to_HAP(include_value=False) == { + "description": "Test Char", + "format": "int", + "iid": ANY, + "maxValue": 10, + "minValue": 3, + "perms": ["pr"], + "type": "A1", + } + char.override_properties({"minValue": 4, "maxValue": 11}) + assert char.to_HAP() == { + "description": "Test Char", + "format": "int", + "iid": ANY, + "maxValue": 11, + "minValue": 4, + "perms": ["pr"], + "type": "A1", + "value": 4, + } + + assert char.to_HAP(include_value=False) == { + "description": "Test Char", + "format": "int", + "iid": ANY, + "maxValue": 11, + "minValue": 4, + "perms": ["pr"], + "type": "A1", + } + char.getter_callback = lambda: 5 + assert char.to_HAP() == { + "description": "Test Char", + "format": "int", + "iid": ANY, + "maxValue": 11, + "minValue": 4, + "perms": ["pr"], + "type": "A1", + "value": 5, + } + assert char.to_HAP(include_value=False) == { + "description": "Test Char", + "format": "int", + "iid": ANY, + "maxValue": 11, + "minValue": 4, + "perms": ["pr"], + "type": "A1", + }