diff --git a/CHANGELOG.md b/CHANGELOG.md index 9988a977..1f32717c 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -16,6 +16,12 @@ Sections ### Developers --> +## [3.4.1] - 2021-03-28 + +# Fixed +- Fix `run_at_interval` with multiple accessories. [#335](https://github.com/ikalchev/HAP-python/pull/335) +- Ensure HTTP 200 status is sent when there are no failures for `get_characteristics`. Improves battery life. [#337](https://github.com/ikalchev/HAP-python/pull/337) + ## [3.4.0] - 2021-03-06 ### Added diff --git a/pyhap/accessory.py b/pyhap/accessory.py index 4896c325..b59eb731 100644 --- a/pyhap/accessory.py +++ b/pyhap/accessory.py @@ -368,7 +368,7 @@ def get_characteristic(self, aid, iid): async def run(self): """Schedule tasks for each of the accessories' run method.""" for acc in self.accessories.values(): - await self.driver.async_add_job(acc.run) + self.driver.async_add_job(acc.run) async def stop(self): """Calls stop() on all contained accessories.""" diff --git a/pyhap/accessory_driver.py b/pyhap/accessory_driver.py index 69daf79a..a3aed1d7 100644 --- a/pyhap/accessory_driver.py +++ b/pyhap/accessory_driver.py @@ -51,12 +51,11 @@ from pyhap.params import get_srp_context from pyhap.state import State +from .const import HAP_SERVER_STATUS from .util import callback logger = logging.getLogger(__name__) -CHAR_STAT_OK = 0 -SERVICE_COMMUNICATION_FAILURE = -70402 SERVICE_CALLBACK = "callback" SERVICE_CHARS = "chars" SERVICE_IIDS = "iids" @@ -682,7 +681,7 @@ def get_characteristics(self, char_ids): rep = { HAP_REPR_AID: aid, HAP_REPR_IID: iid, - HAP_REPR_STATUS: SERVICE_COMMUNICATION_FAILURE, + HAP_REPR_STATUS: HAP_SERVER_STATUS.SERVICE_COMMUNICATION_FAILURE, } try: @@ -698,7 +697,7 @@ def get_characteristics(self, char_ids): if available: rep[HAP_REPR_VALUE] = char.get_value() - rep[HAP_REPR_STATUS] = CHAR_STAT_OK + rep[HAP_REPR_STATUS] = HAP_SERVER_STATUS.SUCCESS except CharacteristicError: logger.error("Error getting value for characteristic %s.", id) except Exception: # pylint: disable=broad-except @@ -761,10 +760,12 @@ def set_characteristics(self, chars_query, client_addr): char.display_name, value, ) - setter_results[aid][iid] = SERVICE_COMMUNICATION_FAILURE + setter_results[aid][ + iid + ] = HAP_SERVER_STATUS.SERVICE_COMMUNICATION_FAILURE had_error = True else: - setter_results[aid][iid] = CHAR_STAT_OK + setter_results[aid][iid] = HAP_SERVER_STATUS.SUCCESS # For some services we want to send all the char value # changes at once. This resolves an issue where we send @@ -798,10 +799,10 @@ def set_characteristics(self, chars_query, client_addr): client_addr, service_name, ) - set_result = SERVICE_COMMUNICATION_FAILURE + set_result = HAP_SERVER_STATUS.SERVICE_COMMUNICATION_FAILURE had_error = True else: - set_result = CHAR_STAT_OK + set_result = HAP_SERVER_STATUS.SUCCESS for iid in service_data[SERVICE_IIDS]: setter_results[aid][iid] = set_result diff --git a/pyhap/const.py b/pyhap/const.py index 473165cc..2264e75c 100644 --- a/pyhap/const.py +++ b/pyhap/const.py @@ -1,9 +1,9 @@ """This module contains constants used by other modules.""" MAJOR_VERSION = 3 MINOR_VERSION = 4 -PATCH_VERSION = 0 -__short_version__ = '{}.{}'.format(MAJOR_VERSION, MINOR_VERSION) -__version__ = '{}.{}'.format(__short_version__, PATCH_VERSION) +PATCH_VERSION = 1 +__short_version__ = "{}.{}".format(MAJOR_VERSION, MINOR_VERSION) +__version__ = "{}.{}".format(__short_version__, PATCH_VERSION) REQUIRED_PYTHON_VER = (3, 5) # ### Misc ### @@ -51,26 +51,42 @@ # ### HAP Permissions ### -HAP_PERMISSION_HIDDEN = 'hd' -HAP_PERMISSION_NOTIFY = 'ev' -HAP_PERMISSION_READ = 'pr' -HAP_PERMISSION_WRITE = 'pw' -HAP_PERMISSION_WRITE_RESPONSE = 'wr' +HAP_PERMISSION_HIDDEN = "hd" +HAP_PERMISSION_NOTIFY = "ev" +HAP_PERMISSION_READ = "pr" +HAP_PERMISSION_WRITE = "pw" +HAP_PERMISSION_WRITE_RESPONSE = "wr" # ### HAP representation ### -HAP_REPR_ACCS = 'accessories' -HAP_REPR_AID = 'aid' -HAP_REPR_CHARS = 'characteristics' -HAP_REPR_DESC = 'description' -HAP_REPR_FORMAT = 'format' -HAP_REPR_IID = 'iid' -HAP_REPR_MAX_LEN = 'maxLen' -HAP_REPR_PERM = 'perms' -HAP_REPR_PRIMARY = 'primary' -HAP_REPR_SERVICES = 'services' -HAP_REPR_LINKED = 'linked' -HAP_REPR_STATUS = 'status' -HAP_REPR_TYPE = 'type' -HAP_REPR_VALUE = 'value' -HAP_REPR_VALID_VALUES = 'valid-values' +HAP_REPR_ACCS = "accessories" +HAP_REPR_AID = "aid" +HAP_REPR_CHARS = "characteristics" +HAP_REPR_DESC = "description" +HAP_REPR_FORMAT = "format" +HAP_REPR_IID = "iid" +HAP_REPR_MAX_LEN = "maxLen" +HAP_REPR_PERM = "perms" +HAP_REPR_PRIMARY = "primary" +HAP_REPR_SERVICES = "services" +HAP_REPR_LINKED = "linked" +HAP_REPR_STATUS = "status" +HAP_REPR_TYPE = "type" +HAP_REPR_VALUE = "value" +HAP_REPR_VALID_VALUES = "valid-values" + + +# Status codes for underlying HAP calls +class HAP_SERVER_STATUS: + SUCCESS = 0 + INSUFFICIENT_PRIVILEGES = -70401 + SERVICE_COMMUNICATION_FAILURE = -70402 + RESOURCE_BUSY = -70403 + READ_ONLY_CHARACTERISTIC = -70404 + WRITE_ONLY_CHARACTERISTIC = -70405 + NOTIFICATION_NOT_SUPPORTED = -70406 + OUT_OF_RESOURCE = -70407 + OPERATION_TIMED_OUT = -70408 + RESOURCE_DOES_NOT_EXIST = -70409 + INVALID_VALUE_IN_REQUEST = -70410 + INSUFFICIENT_AUTHORIZATION = -70411 diff --git a/pyhap/hap_handler.py b/pyhap/hap_handler.py index 02977d06..305378da 100644 --- a/pyhap/hap_handler.py +++ b/pyhap/hap_handler.py @@ -14,7 +14,12 @@ import curve25519 import ed25519 -from pyhap.const import CATEGORY_BRIDGE +from pyhap.const import ( + CATEGORY_BRIDGE, + HAP_REPR_CHARS, + HAP_REPR_STATUS, + HAP_SERVER_STATUS, +) import pyhap.tlv as tlv from pyhap.util import long_to_bytes @@ -74,22 +79,6 @@ class HAP_TLV_TAGS: PERMISSIONS = b"\x0B" -# Status codes for underlying HAP calls -class HAP_SERVER_STATUS: - SUCCESS = 0 - INSUFFICIENT_PRIVILEGES = -70401 - SERVICE_COMMUNICATION_FAILURE = -70402 - RESOURCE_BUSY = -70403 - READ_ONLY_CHARACTERISTIC = -70404 - WRITE_ONLY_CHARACTERISTIC = -70405 - NOTIFICATION_NOT_SUPPORTED = -70406 - OUT_OF_RESOURCE = -70407 - OPERATION_TIMED_OUT = -70408 - RESOURCE_DOES_NOT_EXIST = -70409 - INVALID_VALUE_IN_REQUEST = -70410 - INSUFFICIENT_AUTHORIZATION = -70411 - - class HAP_PERMISSIONS: USER = b"\x00" ADMIN = b"\x01" @@ -576,10 +565,22 @@ def handle_get_characteristics(self): # Check that char exists and ... params = parse_qs(urlparse(self.path).query) - chars = self.accessory_handler.get_characteristics(params["id"][0].split(",")) + response = self.accessory_handler.get_characteristics( + params["id"][0].split(",") + ) + chars = response[HAP_REPR_CHARS] - data = json.dumps(chars).encode("utf-8") - self.send_response(HTTPStatus.MULTI_STATUS) + had_failure = any( + result[HAP_REPR_STATUS] != HAP_SERVER_STATUS.SUCCESS for result in chars + ) + if had_failure: + self.send_response(HTTPStatus.MULTI_STATUS) + else: + self.send_response(HTTPStatus.OK) + for result in chars: + del result[HAP_REPR_STATUS] + + data = json.dumps(response).encode("utf-8") self.send_header("Content-Type", self.JSON_RESPONSE_TYPE) self.end_response(data) diff --git a/tests/test_accessory.py b/tests/test_accessory.py index c4eebe04..7989eeb9 100644 --- a/tests/test_accessory.py +++ b/tests/test_accessory.py @@ -1,4 +1,5 @@ """Tests for pyhap.accessory.""" +import asyncio from io import StringIO from unittest.mock import patch @@ -6,6 +7,7 @@ from pyhap import accessory from pyhap.accessory import Accessory, Bridge +from pyhap.accessory_driver import AccessoryDriver from pyhap.const import ( CATEGORY_CAMERA, CATEGORY_TARGET_CONTROLLER, @@ -22,6 +24,21 @@ # ##################### +class TestAccessory(Accessory): + """An accessory that keeps track of if its stopped.""" + + def __init__(self, *args, **kwargs): + super().__init__(*args, **kwargs) + self._stopped = False + + async def stop(self): + self._stopped = True + + @property + def stopped(self): + return self._stopped + + def test_acc_init(mock_driver): Accessory(mock_driver, "Test Accessory") @@ -383,15 +400,27 @@ def test_to_hap(mock_driver): @pytest.mark.asyncio -async def test_bridge_run_stop(mock_driver): - mock_driver.async_add_job = AsyncMock() - bridge = Bridge(mock_driver, "Test Bridge") - acc = Accessory(mock_driver, "Test Accessory", aid=2) - assert acc.available is True - bridge.add_accessory(acc) - acc2 = Accessory(mock_driver, "Test Accessory 2") - bridge.add_accessory(acc2) +async def test_bridge_run_stop(): + with patch( + "pyhap.accessory_driver.HAPServer.async_stop", new_callable=AsyncMock + ), patch( + "pyhap.accessory_driver.HAPServer.async_start", new_callable=AsyncMock + ), patch( + "pyhap.accessory_driver.Zeroconf" + ), patch( + "pyhap.accessory_driver.AccessoryDriver.persist" + ), patch( + "pyhap.accessory_driver.AccessoryDriver.load" + ): + driver = AccessoryDriver(loop=asyncio.get_event_loop()) + bridge = Bridge(driver, "Test Bridge") + acc = TestAccessory(driver, "Test Accessory", aid=2) + assert acc.available is True + bridge.add_accessory(acc) + acc2 = TestAccessory(driver, "Test Accessory 2") + bridge.add_accessory(acc2) - await bridge.run() - assert mock_driver.async_add_job.called - await bridge.stop() + await bridge.run() + await bridge.stop() + assert acc.stopped is True + assert acc2.stopped is True diff --git a/tests/test_accessory_driver.py b/tests/test_accessory_driver.py index 39f163ae..f8e6845a 100644 --- a/tests/test_accessory_driver.py +++ b/tests/test_accessory_driver.py @@ -9,11 +9,7 @@ from pyhap import util from pyhap.accessory import STANDALONE_AID, Accessory, Bridge -from pyhap.accessory_driver import ( - SERVICE_COMMUNICATION_FAILURE, - AccessoryDriver, - AccessoryMDNSServiceInfo, -) +from pyhap.accessory_driver import AccessoryDriver, AccessoryMDNSServiceInfo from pyhap.characteristic import ( HAP_FORMAT_INT, HAP_PERMISSION_READ, @@ -27,6 +23,7 @@ HAP_REPR_IID, HAP_REPR_STATUS, HAP_REPR_VALUE, + HAP_SERVER_STATUS, ) from pyhap.service import Service from pyhap.state import State @@ -39,6 +36,38 @@ } +class AsyncIntervalAccessory(Accessory): + """An accessory increments a counter at interval.""" + + def __init__(self, *args, **kwargs): + super().__init__(*args, **kwargs) + self._counter = 0 + + @Accessory.run_at_interval(0.001) # Run this method every 0.001 seconds + async def run(self): + self._counter += 1 + + @property + def counter(self): + return self._counter + + +class SyncIntervalAccessory(Accessory): + """An accessory increments a counter at interval.""" + + def __init__(self, *args, **kwargs): + super().__init__(*args, **kwargs) + self._counter = 0 + + @Accessory.run_at_interval(0.001) # Run this method every 0.001 seconds + def run(self): # pylint: disable=invalid-overridden-method + self._counter += 1 + + @property + def counter(self): + return self._counter + + class UnavailableAccessory(Accessory): """An accessory that is not available.""" @@ -271,12 +300,12 @@ def fail_callback(*_): { HAP_REPR_AID: acc.aid, HAP_REPR_IID: char_on_iid, - HAP_REPR_STATUS: SERVICE_COMMUNICATION_FAILURE, + HAP_REPR_STATUS: HAP_SERVER_STATUS.SERVICE_COMMUNICATION_FAILURE, }, { HAP_REPR_AID: acc.aid, HAP_REPR_IID: char_brightness_iid, - HAP_REPR_STATUS: SERVICE_COMMUNICATION_FAILURE, + HAP_REPR_STATUS: HAP_SERVER_STATUS.SERVICE_COMMUNICATION_FAILURE, }, { HAP_REPR_AID: acc2.aid, @@ -364,17 +393,17 @@ def fail_callback(*_): { HAP_REPR_AID: acc.aid, HAP_REPR_IID: char_on_iid, - HAP_REPR_STATUS: SERVICE_COMMUNICATION_FAILURE, + HAP_REPR_STATUS: HAP_SERVER_STATUS.SERVICE_COMMUNICATION_FAILURE, }, { HAP_REPR_AID: acc.aid, HAP_REPR_IID: char_brightness_iid, - HAP_REPR_STATUS: SERVICE_COMMUNICATION_FAILURE, + HAP_REPR_STATUS: HAP_SERVER_STATUS.SERVICE_COMMUNICATION_FAILURE, }, { HAP_REPR_AID: acc2.aid, HAP_REPR_IID: char_on2_iid, - HAP_REPR_STATUS: SERVICE_COMMUNICATION_FAILURE, + HAP_REPR_STATUS: HAP_SERVER_STATUS.SERVICE_COMMUNICATION_FAILURE, }, { HAP_REPR_AID: acc2.aid, @@ -670,3 +699,67 @@ def callback_test(): await asyncio.sleep(0) await asyncio.sleep(0) assert called is True + + +@pytest.mark.asyncio +async def test_bridge_with_multiple_async_run_at_interval_accessories(): + with patch( + "pyhap.accessory_driver.HAPServer.async_stop", new_callable=AsyncMock + ), patch( + "pyhap.accessory_driver.HAPServer.async_start", new_callable=AsyncMock + ), patch( + "pyhap.accessory_driver.Zeroconf" + ), patch( + "pyhap.accessory_driver.AccessoryDriver.persist" + ), patch( + "pyhap.accessory_driver.AccessoryDriver.load" + ): + driver = AccessoryDriver(loop=asyncio.get_event_loop()) + bridge = Bridge(driver, "mybridge") + acc = AsyncIntervalAccessory(driver, "TestAcc", aid=2) + acc2 = AsyncIntervalAccessory(driver, "TestAcc2", aid=3) + acc3 = AsyncIntervalAccessory(driver, "TestAcc3", aid=4) + bridge.add_accessory(acc) + bridge.add_accessory(acc2) + bridge.add_accessory(acc3) + driver.add_accessory(bridge) + driver.start_service() + await asyncio.sleep(0.5) + assert not driver.loop.is_closed() + await driver.async_stop() + + assert acc.counter > 2 + assert acc2.counter > 2 + assert acc3.counter > 2 + + +@pytest.mark.asyncio +async def test_bridge_with_multiple_sync_run_at_interval_accessories(): + with patch( + "pyhap.accessory_driver.HAPServer.async_stop", new_callable=AsyncMock + ), patch( + "pyhap.accessory_driver.HAPServer.async_start", new_callable=AsyncMock + ), patch( + "pyhap.accessory_driver.Zeroconf" + ), patch( + "pyhap.accessory_driver.AccessoryDriver.persist" + ), patch( + "pyhap.accessory_driver.AccessoryDriver.load" + ): + driver = AccessoryDriver(loop=asyncio.get_event_loop()) + bridge = Bridge(driver, "mybridge") + acc = SyncIntervalAccessory(driver, "TestAcc", aid=2) + acc2 = SyncIntervalAccessory(driver, "TestAcc2", aid=3) + acc3 = SyncIntervalAccessory(driver, "TestAcc3", aid=4) + bridge.add_accessory(acc) + bridge.add_accessory(acc2) + bridge.add_accessory(acc3) + driver.add_accessory(bridge) + driver.start_service() + await asyncio.sleep(0.5) + assert not driver.loop.is_closed() + await driver.async_stop() + + assert acc.counter > 2 + assert acc2.counter > 2 + assert acc3.counter > 2 diff --git a/tests/test_hap_handler.py b/tests/test_hap_handler.py index 7cd095cf..ce0e324e 100644 --- a/tests/test_hap_handler.py +++ b/tests/test_hap_handler.py @@ -4,6 +4,7 @@ from unittest.mock import patch from uuid import UUID +import json import pytest from pyhap import hap_handler @@ -408,7 +409,10 @@ def test_handle_get_characteristics_encrypted(driver): handler.path = "/characteristics?id=1.9" handler.handle_get_characteristics() - assert response.status_code == 207 + assert response.status_code == 200 + decoded_response = json.loads(response.body.decode()) + assert "characteristics" in decoded_response + assert "status" not in decoded_response["characteristics"][0] assert b'"value": 0' in response.body with patch.object(acc.iid_manager, "get_obj", side_effect=CharacteristicError): @@ -418,7 +422,10 @@ def test_handle_get_characteristics_encrypted(driver): handler.handle_get_characteristics() assert response.status_code == 207 - assert b"-70402" in response.body + decoded_response = json.loads(response.body.decode()) + assert "characteristics" in decoded_response + assert "status" in decoded_response["characteristics"][0] + assert decoded_response["characteristics"][0]["status"] == -70402 def test_invalid_pairing_two(driver): diff --git a/tests/test_hap_server.py b/tests/test_hap_server.py index 8941878a..062042c4 100644 --- a/tests/test_hap_server.py +++ b/tests/test_hap_server.py @@ -43,6 +43,9 @@ async def test_we_can_connect(): assert server.connections == {} _, port = sock.getsockname() _, writer = await asyncio.open_connection("127.0.0.1", port) + # flush out any call_soon + for _ in range(3): + await asyncio.sleep(0) assert server.connections != {} server.async_stop() writer.close() diff --git a/tests/test_tlv.py b/tests/test_tlv.py index 28260c60..8e9f300c 100644 --- a/tests/test_tlv.py +++ b/tests/test_tlv.py @@ -1,6 +1,7 @@ """Tests for pyhap.tlv.""" import pytest + from pyhap import tlv