Merge pull request #339 from ikalchev/v3.4.1

v3.4.1

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

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."""

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

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

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)
 

tests/test_accessory.py

@@ -1,11 +1,13 @@
 """Tests for pyhap.accessory."""
+import asyncio
 from io import StringIO
 from unittest.mock import patch
 
 import pytest
 
 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