Merge pull request #319 from ikalchev/v3.3.2

V3.3.2

CHANGELOG.md

@@ -16,6 +16,12 @@ Sections
 ### Developers
 -->
 
+## [3.3.2] - 2021-03-01
+
+### Fixed
+- Resolve unavailable condition on restart. [#318](https://github.com/ikalchev/HAP-python/pull/318)
+- Resolve config version overflow. [#318](https://github.com/ikalchev/HAP-python/pull/318)
+
 ## [3.3.1] - 2021-02-28
 
 ### Changed

pyhap/accessory_driver.py

@@ -35,6 +35,7 @@
 from pyhap.accessory import get_topic
 from pyhap.characteristic import CharacteristicError
 from pyhap.const import (
+    MAX_CONFIG_VERSION,
     HAP_PERMISSION_NOTIFY,
     HAP_REPR_ACCS,
     HAP_REPR_AID,
@@ -338,6 +339,7 @@ async def async_stop(self):
             self.state.address,
             self.state.port,
         )
+
         await self.async_add_job(self.accessory.stop)
 
         logger.debug(
@@ -498,6 +500,8 @@ def config_changed(self):
         to fetch new data.
         """
         self.state.config_version += 1
+        if self.state.config_version > MAX_CONFIG_VERSION:
+            self.state.config_version = 1
         self.persist()
         self.update_advertisement()
 

pyhap/const.py

@@ -1,19 +1,20 @@
 """This module contains constants used by other modules."""
 MAJOR_VERSION = 3
 MINOR_VERSION = 3
-PATCH_VERSION = 1
+PATCH_VERSION = 2
 __short_version__ = '{}.{}'.format(MAJOR_VERSION, MINOR_VERSION)
 __version__ = '{}.{}'.format(__short_version__, PATCH_VERSION)
 REQUIRED_PYTHON_VER = (3, 5)
 
 # ### Misc ###
 STANDALONE_AID = 1  # Standalone accessory ID (i.e. not bridged)
 
-
 # ### Default values ###
 DEFAULT_CONFIG_VERSION = 2
 DEFAULT_PORT = 51827
 
+# ### Configuration version ###
+MAX_CONFIG_VERSION = 65535
 
 # ### CATEGORY values ###
 # Category is a hint to iOS clients about what "type" of Accessory this

pyhap/hap_handler.py

@@ -187,13 +187,13 @@ def _set_encryption_ctx(
             "pre_session_key": pre_session_key,
         }
 
-    def send_response(self, code, message=None):
+    def send_response(self, http_status):
         """Add the response header to the headers buffer and log the
         response code.
         Does not add Server or Date
         """
-        self.response.status_code = int(code)
-        self.response.reason = message or "OK"
+        self.response.status_code = http_status.value
+        self.response.reason = http_status.phrase
 
     def send_header(self, header, value):
         """Add the response header to the headers buffer."""
@@ -226,7 +226,7 @@ def dispatch(self, request, body=None):
             getattr(self, self.HANDLERS[self.command][path])()
         except UnprivilegedRequestException:
             self.send_response_with_status(
-                401, HAP_SERVER_STATUS.INSUFFICIENT_PRIVILEGES
+                HTTPStatus.UNAUTHORIZED, HAP_SERVER_STATUS.INSUFFICIENT_PRIVILEGES
             )
         except TimeoutException:
             self.send_response_with_status(500, HAP_SERVER_STATUS.OPERATION_TIMED_OUT)
@@ -235,17 +235,24 @@ def dispatch(self, request, body=None):
                 "%s: Failed to process request for: %s", self.client_address, path
             )
             self.send_response_with_status(
-                500, HAP_SERVER_STATUS.SERVICE_COMMUNICATION_FAILURE
+                HTTPStatus.INTERNAL_SERVER_ERROR,
+                HAP_SERVER_STATUS.SERVICE_COMMUNICATION_FAILURE,
             )
 
+        body_len = len(self.response.body)
+        if body_len:
+            # Force Content-Length as iOS can sometimes
+            # stall if it gets chunked encoding
+            self.send_header("Content-Length", str(body_len))
         self.response = None
         return response
 
     def generic_failure_response(self):
         """Generate a generic failure response."""
         self.response = HAPResponse()
         self.send_response_with_status(
-            500, HAP_SERVER_STATUS.SERVICE_COMMUNICATION_FAILURE
+            HTTPStatus.INTERNAL_SERVER_ERROR,
+            HAP_SERVER_STATUS.SERVICE_COMMUNICATION_FAILURE,
         )
         response = self.response
         self.response = None
@@ -425,7 +432,8 @@ def _pairing_five(self, client_username, client_ltpk, encryption_key):
 
         if not should_confirm:
             self.send_response_with_status(
-                500, HAP_SERVER_STATUS.INVALID_VALUE_IN_REQUEST
+                HTTPStatus.INTERNAL_SERVER_ERROR,
+                HAP_SERVER_STATUS.INVALID_VALUE_IN_REQUEST,
             )
             return
 
@@ -567,7 +575,7 @@ def handle_accessories(self):
 
         hap_rep = self.accessory_handler.get_accessories()
         data = json.dumps(hap_rep).encode("utf-8")
-        self.send_response(200)
+        self.send_response(HTTPStatus.OK)
         self.send_header("Content-Type", self.JSON_RESPONSE_TYPE)
         self.end_response(data)
 
@@ -581,7 +589,7 @@ def handle_get_characteristics(self):
         chars = self.accessory_handler.get_characteristics(params["id"][0].split(","))
 
         data = json.dumps(chars).encode("utf-8")
-        self.send_response(207)
+        self.send_response(HTTPStatus.MULTI_STATUS)
         self.send_header("Content-Type", self.JSON_RESPONSE_TYPE)
         self.end_response(data)
 
@@ -606,7 +614,7 @@ def handle_set_characteristics(self):
             self.send_response(HTTPStatus.NO_CONTENT)
             return
 
-        self.send_response(207)
+        self.send_response(HTTPStatus.MULTI_STATUS)
         self.send_header("Content-Type", self.JSON_RESPONSE_TYPE)
         self.end_response(json.dumps(response).encode("utf-8"))
 
@@ -696,7 +704,7 @@ def _send_authentication_error_tlv_response(self, sequence):
 
     def _send_tlv_pairing_response(self, data):
         """Send a TLV encoded pairing response."""
-        self.send_response(200)
+        self.send_response(HTTPStatus.OK)
         self.send_header("Content-Type", self.PAIRING_RESPONSE_TYPE)
         self.end_response(data)
 
@@ -725,6 +733,6 @@ def handle_resource(self):
             )
 
         task = asyncio.ensure_future(asyncio.wait_for(coro, SNAPSHOT_TIMEOUT))
-        self.send_response(200)
+        self.send_response(HTTPStatus.OK)
         self.send_header("Content-Type", "image/jpeg")
         self.response.task = task

tests/conftest.py

@@ -16,10 +16,16 @@ def mock_driver():
 
 @pytest.fixture
 def driver():
+    try:
+        loop = asyncio.get_event_loop()
+    except RuntimeError:
+        loop = asyncio.new_event_loop()
+        asyncio.set_event_loop(loop)
     with patch("pyhap.accessory_driver.HAPServer"), patch(
         "pyhap.accessory_driver.Zeroconf"
     ), patch("pyhap.accessory_driver.AccessoryDriver.persist"):
-        yield AccessoryDriver()
+
+        yield AccessoryDriver(loop=loop)
 
 
 class MockDriver:
@@ -30,4 +36,4 @@ def publish(self, data, client_addr=None):
         pass
 
     def add_job(self, target, *args):  # pylint: disable=no-self-use
-        asyncio.get_event_loop().run_until_complete(target(*args))
+        asyncio.new_event_loop().run_until_complete(target(*args))

tests/test_accessory_driver.py

@@ -1,15 +1,17 @@
 """Tests for pyhap.accessory_driver."""
+import asyncio
 import tempfile
 from unittest.mock import MagicMock, patch
 from uuid import uuid1
+from concurrent.futures import ThreadPoolExecutor
 
 import pytest
 
 from pyhap.accessory import STANDALONE_AID, Accessory, Bridge
 from pyhap.accessory_driver import (
+    SERVICE_COMMUNICATION_FAILURE,
     AccessoryDriver,
     AccessoryMDNSServiceInfo,
-    SERVICE_COMMUNICATION_FAILURE,
 )
 from pyhap.characteristic import (
     HAP_FORMAT_INT,
@@ -22,8 +24,8 @@
     HAP_REPR_AID,
     HAP_REPR_CHARS,
     HAP_REPR_IID,
-    HAP_REPR_VALUE,
     HAP_REPR_STATUS,
+    HAP_REPR_VALUE,
 )
 from pyhap.service import Service
 from pyhap.state import State
@@ -380,37 +382,78 @@ def fail_callback(*_):
     }
 
 
-def test_start_stop_sync_acc(driver):
-    class Acc(Accessory):
-        running = True
+def test_start_from_sync(driver):
+    """Start from sync."""
 
+    class Acc(Accessory):
         @Accessory.run_at_interval(0)
-        def run(self):  # pylint: disable=invalid-overridden-method
-            self.running = False
-            driver.stop()
+        async def run(self):
+            driver.executor = ThreadPoolExecutor()
+            driver.loop.set_default_executor(driver.executor)
+            await driver.async_stop()
 
         def setup_message(self):
             pass
 
     acc = Acc(driver, "TestAcc")
     driver.add_accessory(acc)
     driver.start()
-    assert not acc.running
 
 
-def test_start_stop_async_acc(driver):
-    class Acc(Accessory):
-        @Accessory.run_at_interval(0)
-        async def run(self):
-            driver.stop()
+@pytest.mark.asyncio
+async def test_start_stop_sync_acc():
+    with patch("pyhap.accessory_driver.HAPServer"), patch(
+        "pyhap.accessory_driver.Zeroconf"
+    ), patch("pyhap.accessory_driver.AccessoryDriver.persist"), patch(
+        "pyhap.accessory_driver.AccessoryDriver.load"
+    ):
+        driver = AccessoryDriver(loop=asyncio.get_event_loop())
+        run_event = asyncio.Event()
 
-        def setup_message(self):
-            pass
+        class Acc(Accessory):
+            @Accessory.run_at_interval(0)
+            def run(self):  # pylint: disable=invalid-overridden-method
+                run_event.set()
 
-    acc = Acc(driver, "TestAcc")
-    driver.add_accessory(acc)
-    driver.start()
-    assert driver.loop.is_closed()
+            def setup_message(self):
+                pass
+
+        acc = Acc(driver, "TestAcc")
+        driver.add_accessory(acc)
+        driver.start_service()
+        await run_event.wait()
+        assert not driver.loop.is_closed()
+        await driver.async_stop()
+        assert not driver.loop.is_closed()
+
+
+@pytest.mark.asyncio
+async def test_start_stop_async_acc():
+    """Verify run_at_interval closes the driver."""
+    with patch("pyhap.accessory_driver.HAPServer"), patch(
+        "pyhap.accessory_driver.Zeroconf"
+    ), patch("pyhap.accessory_driver.AccessoryDriver.persist"), patch(
+        "pyhap.accessory_driver.AccessoryDriver.load"
+    ):
+        driver = AccessoryDriver(loop=asyncio.get_event_loop())
+        run_event = asyncio.Event()
+
+        class Acc(Accessory):
+            @Accessory.run_at_interval(0)
+            async def run(self):
+                run_event.set()
+
+            def setup_message(self):
+                pass
+
+        acc = Acc(driver, "TestAcc")
+        driver.add_accessory(acc)
+        driver.start_service()
+        await asyncio.sleep(0)
+        await run_event.wait()
+        assert not driver.loop.is_closed()
+        await driver.async_stop()
+        assert not driver.loop.is_closed()
 
 
 def test_start_without_accessory(driver):
@@ -492,3 +535,29 @@ def test_mdns_service_info(driver):
         "sf": "1",
         "sh": "+KjpzQ==",
     }
+
+
+@pytest.mark.asyncio
+async def test_start_service_and_update_config():
+    """Test starting service and updating the config."""
+    with patch("pyhap.accessory_driver.HAPServer"), patch(
+        "pyhap.accessory_driver.Zeroconf"
+    ), patch("pyhap.accessory_driver.AccessoryDriver.persist"), patch(
+        "pyhap.accessory_driver.AccessoryDriver.load"
+    ):
+        driver = AccessoryDriver(loop=asyncio.get_event_loop())
+        acc = Accessory(driver, "TestAcc")
+        driver.add_accessory(acc)
+        driver.start_service()
+
+        assert driver.state.config_version == 2
+        driver.config_changed()
+        assert driver.state.config_version == 3
+        driver.state.config_version = 65535
+        driver.config_changed()
+        assert driver.state.config_version == 1
+
+        await driver.async_stop()
+        await asyncio.sleep(0)
+        assert not driver.loop.is_closed()
+        assert driver.aio_stop_event.is_set()

tests/test_hap_protocol.py

@@ -187,7 +187,12 @@ def test_get_characteristics_with_crypto(driver):
         )
 
     hap_proto.close()
+    assert b"Content-Length:" in writer.call_args_list[0][0][0]
+    assert b"Transfer-Encoding: chunked\r\n\r\n" not in writer.call_args_list[0][0][0]
     assert b"-70402" in writer.call_args_list[0][0][0]
+
+    assert b"Content-Length:" in writer.call_args_list[1][0][0]
+    assert b"Transfer-Encoding: chunked\r\n\r\n" not in writer.call_args_list[1][0][0]
     assert b"TestAcc" in writer.call_args_list[1][0][0]
 
 
@@ -215,7 +220,7 @@ def test_set_characteristics_with_crypto(driver):
         )
 
     hap_proto.close()
-    assert writer.call_args_list[0][0][0] == b"HTTP/1.1 204 OK\r\n\r\n"
+    assert writer.call_args_list[0][0][0] == b"HTTP/1.1 204 No Content\r\n\r\n"
 
 
 def test_crypto_failure_closes_connection(driver):