From 084bb73cff6cf6f5d0b099eade15d131c102b218 Mon Sep 17 00:00:00 2001 From: Ivan Kalchev Date: Fri, 6 Apr 2018 12:43:43 +0300 Subject: [PATCH] Merge char improvements, doc changes, version 1.1.9 (#79) * Remove breaking change description from README. * Characteristic improvements (#73) * Merged set_value and get_hap_value * Small improvements * Added helper method '_validate_value' * Added helper method '_get_default_value' * Raise ValueError if properties are invalid * Added __slots__ * Additional changes * New method: update_value (use for value updates from HAP-python) * Rename: to_HAP to to_dict * New method: from_dict (use during loading of chars) * Addressed comments * Fixed errors, small mistakes * Added debug statements * Changed '_LOGGER' to 'logger' * Changed raise error to return None * Raising ValueErrors with custom msg * Bugfix, added additional logger.error statements * Rearanged functions * Added error_msg parameter * Added tests * Added '/htmlcov' to .gitignore * Bump version number to 1.1.9 --- .gitignore | 1 + README.md | 14 -- pyhap/__init__.py | 2 +- pyhap/accessory.py | 4 +- pyhap/accessory_driver.py | 2 +- pyhap/characteristic.py | 198 ++++++++++++-------------- pyhap/service.py | 2 +- setup.py | 2 +- tests/test_characteristic.py | 260 +++++++++++++++++++++++------------ 9 files changed, 266 insertions(+), 219 deletions(-) diff --git a/.gitignore b/.gitignore index 1c941fd8..4d2d0278 100644 --- a/.gitignore +++ b/.gitignore @@ -19,6 +19,7 @@ docs/build/doctrees docs/build/html .coverage .coverage.* +/htmlcov # python venv bin diff --git a/README.md b/README.md index 68a22c51..74ed0f45 100644 --- a/README.md +++ b/README.md @@ -1,18 +1,4 @@ [![PyPI version](https://badge.fury.io/py/HAP-python.svg)](https://badge.fury.io/py/HAP-python) [![Build Status](https://travis-ci.org/ikalchev/HAP-python.svg?branch=master)](https://travis-ci.org/ikalchev/HAP-python) [![codecov](https://codecov.io/gh/ikalchev/HAP-python/branch/master/graph/badge.svg)](https://codecov.io/gh/ikalchev/HAP-python) [![Documentation Status](https://readthedocs.org/projects/hap-python/badge/?version=latest)](http://hap-python.readthedocs.io/en/latest/?badge=latest) -# Breaking change: 1.1.1 to 1.1.2+ -Since version 1.1.2 you no longer need to unpickle the accessory to retain its state! -Have a look at [main.py](main.py) to see the new, simpler workflow. (the state is also -1/10 the size and is python and HAP-python forward-compatible) - -If you want to convert the pickle state to the new format, run the `pickle_to_state.py` -script, passing your .pickle file and the name of the new state file, e.g.: -```sh -$ python3 pickle_to_state.py accessory.pickle accessory.state -``` -After that, you can just change your code to pass the `.state` file -as the `persist_file` to the driver. Alternatively, just delete the state and re-pair with the Home app. - -Aplogies for any inconvenience! # HAP-python HomeKit Accessory Protocol implementation in python 3 (tested with 3.4, 3.5 and 3.6). diff --git a/pyhap/__init__.py b/pyhap/__init__.py index 9d646f98..4c8fe5c0 100644 --- a/pyhap/__init__.py +++ b/pyhap/__init__.py @@ -6,7 +6,7 @@ CHARACTERISTICS_FILE = os.path.join(_RESOURCE_DIR, "characteristics.json") SERVICES_FILE = os.path.join(_RESOURCE_DIR, "services.json") -HAP_PYTHON_VERSION = (1, 1, 8) +HAP_PYTHON_VERSION = (1, 1, 9) """ HAP-python current version. """ diff --git a/pyhap/accessory.py b/pyhap/accessory.py index 54a71931..feee7b17 100755 --- a/pyhap/accessory.py +++ b/pyhap/accessory.py @@ -390,7 +390,7 @@ def stop(self): # Broker - def publish(self, data, sender): + def publish(self, value, sender): """Append AID and IID of the sender and forward it to the broker. Characteristics call this method to send updates. @@ -409,7 +409,7 @@ def publish(self, data, sender): acc_data = { "aid": self.aid, "iid": self.iid_manager.get_iid(sender), - "value": data["value"], + "value": value, } self.broker.publish(acc_data) diff --git a/pyhap/accessory_driver.py b/pyhap/accessory_driver.py index 18a4009e..01d9b9f8 100755 --- a/pyhap/accessory_driver.py +++ b/pyhap/accessory_driver.py @@ -440,7 +440,7 @@ def set_characteristics(self, chars_query, client_addr): } if "value" in cq: # TODO: status needs to be based on success of set_value - char.set_value(cq["value"], should_notify=True) + char.client_update_value(cq["value"]) if "r" in cq: response["value"] = char.value diff --git a/pyhap/characteristic.py b/pyhap/characteristic.py index 99bbdbcd..a844e2b7 100644 --- a/pyhap/characteristic.py +++ b/pyhap/characteristic.py @@ -4,6 +4,10 @@ A Characteristic is the smallest unit of the smart home, e.g. a temperature measuring or a device status. """ +import logging + +logger = logging.getLogger(__name__) + class HAP_FORMAT: BOOL = 'bool' @@ -56,21 +60,7 @@ class CharacteristicError(Exception): pass -class NotConfiguredError(Exception): - """Raised when an operation is attempted on a characteristic that has not been - fully configured. - """ - pass - - -_HAP_NUMERIC_FIELDS = {"maxValue", "minValue", "minStep", "unit"} -"""Fields that should be included in the HAP representation of the characteristic. - -That is, if they are present in the specification of a numeric-value characteristic. -""" - - -class Characteristic(object): +class Characteristic: """Represents a HAP characteristic, the smallest unit of the smart home. A HAP characteristic is some measurement or state, like battery status or @@ -79,7 +69,10 @@ class Characteristic(object): like format, min and max values, valid values and others. """ - def __init__(self, display_name, type_id, properties, value=None, broker=None): + __slots__ = ('display_name', 'type_id', 'properties', 'broker', + 'setter_callback', 'value') + + def __init__(self, display_name, type_id, properties): """Initialise with the given properties. :param display_name: Name that will be displayed for this characteristic, i.e. @@ -91,60 +84,48 @@ def __init__(self, display_name, type_id, properties, value=None, broker=None): :param properties: A dict of properties, such as Format, ValidValues, etc. :type properties: dict - - :param value: The initial value to set to this characteristic. If no value is given, - the assigned value happens as: - - if there is a ValidValue property, use some value from it. - - else use `HAP_FORMAT.DEFAULT` for the format of this characteristic. - :type value: Depends on `properties["Format"]` """ - assert "Format" in properties and "Permissions" in properties self.display_name = display_name self.type_id = type_id self.properties = properties - if value: - self.value = value - else: - if self.properties.get('ValidValues'): - self.value = min(self.properties['ValidValues'].values()) - else: - self.value = HAP_FORMAT.DEFAULT[properties["Format"]] - self.broker = broker + self.broker = None self.setter_callback = None + self.value = self._get_default_value() def __repr__(self): """Return the representation of the characteristic.""" - return "" \ + return '' \ .format(self.display_name, self.value, self.properties) - def set_value(self, value, should_notify=True, should_callback=True): - """Set the given raw value. It is checked if it is a valid value. - - :param value: The value to assign as this Characteristic's value. - :type value: Depends on properties["Format"] - - :param should_notify: Whether a the change should be sent to subscribed clients. - The notification is called _after_ the setter callback. Notify will be - performed if and only if the broker is set, i.e. not None. - :type should_notify: bool - - :param should_callback: Whether to invoke the callback, if such is set. This - is useful in cases where you and HAP clients can both update the value and - you don't want your callback called when you set the value, but want it - called when clients do. Defaults to True. - :type should_callback: bool - - :raise ValueError: When the value being assigned is not one of the valid values - for this Characteristic. - """ - if self.properties.get('ValidValues') and \ - value not in self.properties['ValidValues'].values(): - raise ValueError - self.value = value - if self.setter_callback is not None and should_callback: - self.setter_callback(value) - if should_notify and self.broker is not None: - self.notify() + def _get_default_value(self): + """Helper method. Return default value for format.""" + if self.properties.get('ValidValues'): + return min(self.properties['ValidValues'].values()) + else: + value = HAP_FORMAT.DEFAULT[self.properties['Format']] + return self.to_valid_value(value) + + def to_valid_value(self, value): + """Perform validation and conversion to valid value""" + if self.properties.get('ValidValues'): + if value not in self.properties['ValidValues'].values(): + error_msg = '{}: value={} is an invalid value.' \ + .format(self.display_name, value) + logger.error(error_msg) + raise ValueError(error_msg) + elif self.properties['Format'] == HAP_FORMAT.STRING: + value = str(value)[:256] + elif self.properties['Format'] == HAP_FORMAT.BOOL: + value = bool(value) + elif self.properties['Format'] in HAP_FORMAT.NUMERIC: + if not isinstance(value, (int, float)): + error_msg = '{}: value={} is not a numeric value.' \ + .format(self.display_name, value) + logger.error(error_msg) + raise ValueError(error_msg) + value = min(self.properties.get('maxValue', value), value) + value = max(self.properties.get('minValue', value), value) + return value def override_properties(self, properties=None, valid_values=None): """Override characteristic property values and valid values. @@ -163,70 +144,69 @@ def override_properties(self, properties=None, valid_values=None): if valid_values: self.properties['ValidValues'] = valid_values - def get_hap_value(self): - """Get the value of the characteristic, constrained with the HAP properties. + def set_value(self, value, should_notify=True): + """Set the given raw value. It is checked if it is a valid value. + + If not set_value will be aborted and an error message will be + displayed. + + :param value: The value to assign as this Characteristic's value. + :type value: Depends on properties["Format"] + + :param should_notify: Whether a the change should be sent to + subscribed clients. Notify will be performed if the broker is set. + :type should_notify: bool + """ + logger.debug('%s: Set value to %s', self.display_name, value) + value = self.to_valid_value(value) + self.value = value + if should_notify and self.broker: + self.notify() + + def client_update_value(self, value): + """Called from broker for value change in Home app. + + Change self.value to value and call callback. """ - val = self.value - if self.properties["Format"] == HAP_FORMAT.STRING: - val = val[:256] - elif self.properties["Format"] in HAP_FORMAT.NUMERIC: - if "maxValue" in self.properties: - val = min(self.properties["maxValue"], val) - if "minValue" in self.properties: - val = max(self.properties["minValue"], val) - return val + logger.debug('%s: Client update value to %s', + self.display_name, value) + self.value = value + self.notify() + if self.setter_callback: + self.setter_callback(value) def notify(self): - """Notify clients about a value change. + """Notify clients about a value change. Sends the value. - .. note:: Non-blocking, i.e. does not wait for the update to be sent. - .. note:: Uses the `get_hap_value`, i.e. sends the HAP value. + .. seealso:: accessory.publish .. seealso:: accessory_driver.publish - - :raise NotConfiguredError: When the broker is not set. """ - if self.broker is None: - raise NotConfiguredError("Attempted to notify when `broker` is None. " - "Consider adding the characteristic to a " - "Service and then to an Accessory.") - - data = { - "type_id": self.type_id, - "value": self.get_hap_value(), - } - self.broker.publish(data, self) + self.broker.publish(self.value, self) - def to_HAP(self, iid_manager): + def to_HAP(self): """Create a HAP representation of this Characteristic. - .. note:: Uses the `get_hap_value`, i.e. sends the HAP value. - - :param iid_manager: IID manager to query for this object's IID. - :type iid_manager: IIDManager + Used for json serialization. :return: A HAP representation. :rtype: dict """ hap_rep = { - "iid": iid_manager.get_iid(self), - "type": str(self.type_id).upper(), - "description": self.display_name, - "perms": self.properties["Permissions"], - "format": self.properties["Format"], + 'iid': self.broker.iid_manager.get_iid(self), + 'type': str(self.type_id).upper(), + 'description': self.display_name, + 'perms': self.properties['Permissions'], + 'format': self.properties['Format'], } - if self.properties["Format"] in HAP_FORMAT.NUMERIC: - value_info = {k: self.properties[k] for k in - self.properties.keys() & _HAP_NUMERIC_FIELDS} - else: - value_info = dict() - - val = self.get_hap_value() - if self.properties["Format"] == HAP_FORMAT.STRING: - if len(val) > 64: - value_info["maxLen"] = min(len(val), 256) - if HAP_PERMISSIONS.READ in self.properties["Permissions"]: - value_info["value"] = val + if self.properties['Format'] in HAP_FORMAT.NUMERIC: + hap_rep.update({k: self.properties[k] for k in + self.properties.keys() & + ('maxValue', 'minValue', 'minStep', 'unit')}) + elif self.properties['Format'] == HAP_FORMAT.STRING: + if len(self.value) > 64: + hap_rep['maxLen'] = min(len(self.value), 256) + if HAP_PERMISSIONS.READ in self.properties['Permissions']: + hap_rep['value'] = self.value - hap_rep.update(value_info) return hap_rep diff --git a/pyhap/service.py b/pyhap/service.py index d9f48d09..7b51727e 100644 --- a/pyhap/service.py +++ b/pyhap/service.py @@ -60,7 +60,7 @@ def to_HAP(self, iid_manager=None): :rtype: dict. """ assert iid_manager is not None - characteristics = [c.to_HAP(iid_manager) for c in self.characteristics] + characteristics = [c.to_HAP() for c in self.characteristics] hap_rep = { "iid": iid_manager.get_iid(self), diff --git a/setup.py b/setup.py index e4de1501..e7853cba 100644 --- a/setup.py +++ b/setup.py @@ -4,7 +4,7 @@ name="HAP-python", description="HomeKit Accessory Protocol implementation in python3", author="Ivan Kalchev", - version="1.1.8", + version="1.1.9", url="https://github.com/ikalchev/HAP-python.git", classifiers=[ "Development Status :: 5 - Production/Stable", diff --git a/tests/test_characteristic.py b/tests/test_characteristic.py index c8cd4b0d..fd009b86 100644 --- a/tests/test_characteristic.py +++ b/tests/test_characteristic.py @@ -1,98 +1,178 @@ -""" -Tests for pyhap.characteristic -""" +"""Tests for pyhap.characteristic.""" +import unittest +from unittest.mock import Mock, patch, ANY import uuid -from unittest import mock -import pytest +from pyhap.characteristic import Characteristic, HAP_FORMAT, HAP_PERMISSIONS -import pyhap.characteristic as characteristic -from pyhap.characteristic import Characteristic PROPERTIES = { - "Format": characteristic.HAP_FORMAT.INT, - "Permissions": [characteristic.HAP_PERMISSIONS.READ] + 'Format': HAP_FORMAT.INT, + 'Permissions': [HAP_PERMISSIONS.READ] } + def get_char(props, valid=None, min_value=None, max_value=None): - if valid is not None: - props["ValidValues"] = valid - if min_value is not None: - props["minValue"] = min_value - if max_value is not None: - props["maxValue"] = max_value - c = Characteristic(display_name="Test Char", - type_id=uuid.uuid1(), - properties=props) - return c - -def test_default_value(): - char = get_char(PROPERTIES.copy()) - assert (characteristic.HAP_FORMAT.DEFAULT[PROPERTIES["Format"]] - == char.value) - -def test_default_valid_value(): - valid_values = {"foo": 2, "bar": 3} - char = get_char(PROPERTIES.copy(), valid=valid_values) - assert char.value in valid_values.values() - -def test_set_value(): - char = get_char(PROPERTIES.copy()) - new_value = 3 - char.set_value(new_value) - assert char.value == new_value - -def test_set_value_valid_values(): - valid_values = {"foo": 2, "bar": 3, } - char = get_char(PROPERTIES.copy(), valid=valid_values) - with pytest.raises(ValueError): - char.set_value(4) - -def test_set_value_callback_toggle(): - char = get_char(PROPERTIES.copy()) - char.setter_callback = mock.Mock() - char.set_value(3, should_callback=False) - assert not char.setter_callback.called - char.set_value(3, should_callback=True) - assert char.setter_callback.called - -def test_override_properties_properties(): - new_properties = {'minValue': 10, 'maxValue': 20, 'step': 1} - char = get_char(PROPERTIES.copy(), min_value=0, max_value=1) - char.override_properties(properties=new_properties) - assert char.properties['minValue'] == new_properties['minValue'] - assert char.properties['maxValue'] == new_properties['maxValue'] - assert char.properties['step'] == new_properties['step'] - -def test_override_properties_valid_values(): - new_valid_values = {'foo2': 2, 'bar2': 3} - char = get_char(PROPERTIES.copy(), valid={'foo': 1, 'bar': 2}) - char.override_properties(valid_values=new_valid_values) - assert char.properties['ValidValues'] == new_valid_values - -def test_get_hap_value(): - max_value = 5 - raw_value = 6 - char = get_char(PROPERTIES.copy(), max_value=max_value) - char.set_value(raw_value, should_notify=False) - assert char.value == raw_value - assert char.get_hap_value() == max_value - -def test_notify(): - char = get_char(PROPERTIES.copy()) - broker_mock = mock.Mock() - char.broker = broker_mock - notify_value = 3 - expected = { - "type_id": char.type_id, - "value": notify_value, - } - char.value = notify_value - char.notify() - assert broker_mock.publish.called - broker_mock.publish.assert_called_with(expected, char) - -def test_notify_except_no_broker(): - char = get_char(PROPERTIES.copy()) - with pytest.raises(characteristic.NotConfiguredError): - char.notify() + if valid: + props['ValidValues'] = valid + if min_value: + props['minValue'] = min_value + if max_value: + props['maxValue'] = max_value + return Characteristic(display_name='Test Char', type_id=uuid.uuid1(), + properties=props) + + +class TestCharacteristic(unittest.TestCase): + + def test_repr(self): + char = get_char(PROPERTIES.copy()) + del char.properties['Permissions'] + self.assertEqual( + '', char.__repr__()) + + def test_default_value(self): + char = get_char(PROPERTIES.copy()) + self.assertEqual(char.value, HAP_FORMAT.DEFAULT[PROPERTIES['Format']]) + + def test_get_default_value(self): + valid_values = {'foo': 2, 'bar': 3} + char = get_char(PROPERTIES.copy(), valid=valid_values) + self.assertEqual(char.value, 2) + self.assertIn(char.value, valid_values.values()) + char = get_char(PROPERTIES.copy(), min_value=3, max_value=10) + self.assertEqual(char.value, 3) + + def test_to_valid_value(self): + char = get_char(PROPERTIES.copy(), valid={'foo': 2, 'bar': 3}, + min_value=2, max_value=7) + with self.assertRaises(ValueError): + char.to_valid_value(1) + self.assertEqual(char.to_valid_value(2), 2) + + del char.properties['ValidValues'] + for value in ('2', None): + with self.assertRaises(ValueError): + char.to_valid_value(value) + self.assertEqual(char.to_valid_value(1), 2) + self.assertEqual(char.to_valid_value(5), 5) + self.assertEqual(char.to_valid_value(8), 7) + + char.properties['Format'] = 'string' + self.assertEqual(char.to_valid_value(24), '24') + + char.properties['Format'] = 'bool' + self.assertTrue(char.to_valid_value(1)) + self.assertFalse(char.to_valid_value(0)) + + char.properties['Format'] = 'dictionary' + self.assertEqual(char.to_valid_value({'a': 1}), {'a': 1}) + + def test_override_properties_properties(self): + new_properties = {'minValue': 10, 'maxValue': 20, 'step': 1} + char = get_char(PROPERTIES.copy(), min_value=0, max_value=1) + char.override_properties(properties=new_properties) + self.assertEqual(char.properties['minValue'], + new_properties['minValue']) + self.assertEqual(char.properties['maxValue'], + new_properties['maxValue']) + self.assertEqual(char.properties['step'], new_properties['step']) + + def test_override_properties_valid_values(self): + new_valid_values = {'foo2': 2, 'bar2': 3} + char = get_char(PROPERTIES.copy(), valid={'foo': 1, 'bar': 2}) + char.override_properties(valid_values=new_valid_values) + self.assertEqual(char.properties['ValidValues'], new_valid_values) + + def test_set_value(self): + path = 'pyhap.characteristic.Characteristic.notify' + char = get_char(PROPERTIES.copy(), min_value=3, max_value=7) + + with patch(path) as mock_notify: + char.set_value(5) + self.assertEqual(char.value, 5) + self.assertFalse(mock_notify.called) + + char.broker = Mock() + char.set_value(8, should_notify=False) + self.assertEqual(char.value, 7) + self.assertFalse(mock_notify.called) + + char.set_value(1) + self.assertEqual(char.value, 3) + self.assertEqual(mock_notify.call_count, 1) + + def test_client_update_value(self): + path_notify = 'pyhap.characteristic.Characteristic.notify' + char = get_char(PROPERTIES.copy()) + + with patch(path_notify) as mock_notify: + char.client_update_value(4) + self.assertEqual(char.value, 4) + with patch.object(char, 'setter_callback') as mock_callback: + char.client_update_value(3) + + self.assertEqual(char.value, 3) + self.assertEqual(mock_notify.call_count, 2) + mock_callback.assert_called_with(3) + + def test_notify(self): + char = get_char(PROPERTIES.copy()) + + char.value = 2 + with self.assertRaises(AttributeError): + char.notify() + + with patch.object(char, 'broker') as mock_broker: + char.notify() + mock_broker.publish.assert_called_with(2, char) + + def test_to_HAP_numberic(self): + char = get_char(PROPERTIES.copy(), min_value=1, max_value=2) + with patch.object(char, 'broker') as mock_broker: + mock_iid = mock_broker.iid_manager.get_iid + mock_iid.return_value = 2 + hap_rep = char.to_HAP() + mock_iid.assert_called_with(char) + + self.assertEqual( + hap_rep, + { + 'iid': 2, + 'type': ANY, + 'description': 'Test Char', + 'perms': ['pr'], + 'format': 'int', + 'maxValue': 2, + 'minValue': 1, + 'value': 1, + }) + + def test_to_HAP_string(self): + char = get_char(PROPERTIES.copy()) + char.properties['Format'] = 'string' + char.value = 'aaa' + with patch.object(char, 'broker') as mock_broker: + hap_rep = char.to_HAP() + self.assertEqual(hap_rep['format'], 'string') + self.assertNotIn('maxLen', hap_rep) + + char.value = 'aaaaaaaaaabbbbbbbbbbccccccccccddddddddddeeeeeeeeee' \ + 'ffffffffffgggggggggg' + with patch.object(char, 'broker') as mock_broker: + hap_rep = char.to_HAP() + self.assertEqual(hap_rep['maxLen'], 70) + self.assertEqual(hap_rep['value'], char.value) + + def test_to_HAP_bool(self): + char = get_char(PROPERTIES.copy()) + char.properties['Format'] = 'bool' + with patch.object(char, 'broker') as mock_broker: + hap_rep = char.to_HAP() + self.assertEqual(hap_rep['format'], 'bool') + + char.properties['Permissions'] = [] + with patch.object(char, 'broker') as mock_broker: + hap_rep = char.to_HAP() + self.assertNotIn('value', hap_rep)