diff --git a/pyhap/accessory_driver.py b/pyhap/accessory_driver.py index 08e3e817..7f7326f2 100644 --- a/pyhap/accessory_driver.py +++ b/pyhap/accessory_driver.py @@ -20,8 +20,10 @@ import asyncio import base64 from collections import defaultdict +from collections.abc import Awaitable, Coroutine from concurrent.futures import ThreadPoolExecutor import hashlib +import inspect import logging import os import re @@ -29,7 +31,7 @@ import tempfile import threading import time -from typing import Any +from typing import Any, NamedTuple, TypeGuard from zeroconf import ServiceInfo from zeroconf.asyncio import AsyncZeroconf @@ -45,6 +47,7 @@ from pyhap.const import ( HAP_PERMISSION_NOTIFY, HAP_PERMISSION_READ, + HAP_PERMISSION_TIMED_WRITE, HAP_PERMISSION_WRITE, HAP_PERMISSION_WRITE_RESPONSE, HAP_PROTOCOL_SHORT_VERSION, @@ -93,6 +96,11 @@ MAX_PREPARED_WRITE_TTL_SECONDS = 30.0 +def _is_hap_int(value: Any) -> TypeGuard[int]: + """Whether ``value`` is a JSON integer, not a bool.""" + return isinstance(value, int) and not isinstance(value, bool) + + def _stored_value(char: Characteristic, value: HAPValue) -> HAPValue: """Return the value ``client_update_value`` stores for ``value``.""" try: @@ -107,7 +115,10 @@ def _wrap_char_setter( ) -> tuple[int, Any]: """Process an characteristic setter callback trapping and logging all exceptions.""" try: - response = char.client_update_value(value, client_addr, notify=notify) + # The driver finishes the update of a coroutine setter once it is done. + response = char.client_update_value( + value, client_addr, notify=notify, defer_notify=True + ) except Exception: logger.exception( "%s: Error while setting characteristic %s to %s", @@ -119,10 +130,24 @@ def _wrap_char_setter( return HAP_SERVER_STATUS.SUCCESS, response +def _close_unsupported_coroutine(result: Any, name: str) -> None: + """Close a coroutine an accessory or service setter returned. + + Only characteristic setters may be coroutine functions. + """ + if inspect.iscoroutine(result): + result.close() + logger.warning( + "%s: only characteristic setters may be coroutine functions", name + ) + + def _wrap_acc_setter(acc: Any, updates_by_service: Any, client_addr: Any) -> Any: """Process an accessory setter callback trapping and logging all exceptions.""" try: - acc.setter_callback(updates_by_service) + _close_unsupported_coroutine( + acc.setter_callback(updates_by_service), acc.display_name + ) except Exception: logger.exception( "%s: Error while setting characteristics to %s for the %s accessory", @@ -141,7 +166,9 @@ def _wrap_service_setter(service: Any, chars: Any, client_addr: Any) -> Any: # consumers of the data. service_chars = {char.display_name: value for char, value in chars.items()} try: - service.setter_callback(service_chars) + _close_unsupported_coroutine( + service.setter_callback(service_chars), service.display_name + ) except Exception: logger.exception( "%s: Error while setting characteristics to %s for the %s service", @@ -153,6 +180,43 @@ def _wrap_service_setter(service: Any, chars: Any, client_addr: Any) -> Any: return HAP_SERVER_STATUS.SUCCESS +class _PendingWrite(NamedTuple): + """A characteristic write whose setter returned a coroutine.""" + + result: dict[str, Any] # The result entry to complete. + coro: Coroutine[Any, Any, Any] + write_response: bool # Whether the controller asked for a write response. + char: Characteristic + previous_value: Any # The value before the write, to tell if it changed. + notify: bool # Whether subscribers learn the value once the setter is done. + + +def _retrieve_late_result(task: asyncio.Future[Any]) -> None: + """Retrieve the outcome of a setter that ran on after it was cancelled.""" + if not task.cancelled() and (err := task.exception()) is not None: + logger.debug("Setter failed after it was cancelled", exc_info=err) + + +def _set_characteristics_response( + results: dict[int, dict[int, dict[str, Any]]], +) -> dict[str, Any] | None: + """Build the write response, or None if every write succeeded without a value.""" + characteristics = [] + nonempty_results_exist = False + for aid, iid_results in results.items(): + for iid, result in iid_results.items(): + result[HAP_REPR_AID] = aid + result[HAP_REPR_IID] = iid + characteristics.append(result) + if ( + result[HAP_REPR_STATUS] != HAP_SERVER_STATUS.SUCCESS + or HAP_REPR_VALUE in result + ): + nonempty_results_exist = True + + return {HAP_REPR_CHARS: characteristics} if nonempty_results_exist else None + + class AccessoryMDNSServiceInfo(ServiceInfo): """A mDNS service info representation of an accessory.""" @@ -247,6 +311,7 @@ def __init__( async_zeroconf_instance: Any = None, zeroconf_server: Any = None, advertise: bool = True, + enforce_timed_writes: bool = False, ) -> None: """Initialize a new AccessoryDriver object. @@ -301,6 +366,13 @@ def __init__( the driver starts. Pass False to run the HAP server without mDNS advertising, e.g. for tests or benchmarks. :type advertise: bool + + :param enforce_timed_writes: Whether to reject a plain write to a + characteristic with the timed write ("tw") permission with -70410, + as the HAP specification requires. Off by default, because Home + hubs write some tw characteristics, e.g. HomeKit Camera Active, + without a timed write. + :type enforce_timed_writes: bool """ if loop is None: if sys.platform == "win32": @@ -333,6 +405,7 @@ def __init__( self.stop_event = threading.Event() self.safe_mode = False + self.enforce_timed_writes = enforce_timed_writes self.mdns_service_info: ServiceInfo | None = None self._srp_verifier: SrpServer | None = None @@ -1042,6 +1115,18 @@ def set_characteristics( ) -> dict[str, Any] | None: """Called from ``HAPServerHandler`` when iOS configures the characteristics. + Returns ``None`` when every write succeeded without a write response, + otherwise the per-characteristic results. + + A characteristic setter may be a coroutine function. Writes from a + controller wait for it before they are answered. Called directly, this + method only schedules the coroutine and does not wait for it. Either + way subscribers learn the value once the coroutine succeeded, and it + should use its argument, not the characteristic's value, which a write + response request has already put back. Accessory and service setters + must not be coroutine functions. A coroutine they return is closed + with a warning. + :param chars_query: A configuration query. For example: .. code-block:: python @@ -1058,8 +1143,84 @@ def set_characteristics( :type chars_query: dict """ + results, pending = self._apply_characteristic_writes(chars_query, client_addr) + for write in pending: + self.add_job(self._finish_pending_write(write, client_addr)) + return _set_characteristics_response(results) + + async def _finish_pending_write( + self, write: _PendingWrite, client_addr: tuple[str, int] + ) -> None: + """Run a coroutine setter of a direct call, then tell subscribers.""" + try: + await write.coro + except Exception: + logger.exception( + "%s: Error while setting characteristic %s", + client_addr, + write.char.display_name, + ) + return + write.char.client_update_finished( + write.previous_value, client_addr, notify=write.notify + ) + + def _set_characteristics_for_controller( + self, + chars_query: dict[str, Any], + client_addr: tuple[str, int], + timeout: float, + ) -> dict[str, Any] | None | Awaitable[dict[str, Any] | None]: + """Apply a controller's write, waiting for coroutine setters. + + Returns an awaitable of the response if a characteristic setter is a + coroutine function. Accessory and service setters run before those + coroutines start. A setter still running after ``timeout`` seconds is + cancelled and its characteristic answers -70408. Cancelling the + awaitable, e.g. because the connection closed, cancels the setters. + """ + results, pending = self._apply_characteristic_writes(chars_query, client_addr) + if pending: + return self._async_finish_set_characteristics( + results, pending, client_addr, timeout + ) + return _set_characteristics_response(results) + + def _apply_characteristic_writes( + self, chars_query: dict[str, Any], client_addr: tuple[str, int] + ) -> tuple[dict[int, dict[int, dict[str, Any]]], list[_PendingWrite]]: + """Run the setters of a write and collect the per-characteristic results.""" + pending: list[_PendingWrite] = [] + try: + return self._run_characteristic_writes(chars_query, client_addr, pending) + except BaseException: + # A malformed later entry must not leave a setter coroutine + # un-awaited. + for write in pending: + write.coro.close() + raise + + def _run_characteristic_writes( + self, + chars_query: dict[str, Any], + client_addr: tuple[str, int], + pending: list[_PendingWrite], + ) -> tuple[dict[int, dict[int, dict[str, Any]]], list[_PendingWrite]]: queries: list[dict[str, Any]] = chars_query[HAP_REPR_CHARS] + timed_write = HAP_REPR_PID in chars_query + if timed_write and not self._take_prepared_write( + chars_query[HAP_REPR_PID], client_addr + ): + # The spec requires a timed write that was never prepared, was + # already used or has expired to be ignored as a whole. + ignored: dict[int, dict[int, dict[str, Any]]] = defaultdict(dict) + for query in queries: + ignored[query[HAP_REPR_AID]][query[HAP_REPR_IID]] = { + HAP_REPR_STATUS: HAP_SERVER_STATUS.INVALID_VALUE_IN_REQUEST + } + return ignored, [] + not_notifiable = self._notify(queries, client_addr) updates_by_accessories_services: dict[ @@ -1070,12 +1231,6 @@ def set_characteristics( ) char_to_iid: dict[Characteristic, int] = {} - expired = False - if HAP_REPR_PID in chars_query: - pid = chars_query[HAP_REPR_PID] - expire_time = self.prepared_writes.get(client_addr, {}).pop(pid, None) - expired = expire_time is None or time.time() > expire_time - primary_accessory = self.accessory assert primary_accessory is not None # noqa: S101 primary_aid = primary_accessory.aid @@ -1090,7 +1245,7 @@ def set_characteristics( iid = query[HAP_REPR_IID] if (aid, iid) in not_notifiable: continue - if HAP_REPR_VALUE not in query and not expired: + if HAP_REPR_VALUE not in query: # Listed with status 0 if the response has other results. results[aid][iid] = {HAP_REPR_STATUS: HAP_SERVER_STATUS.SUCCESS} continue @@ -1120,10 +1275,26 @@ def set_characteristics( } continue + if ( + value is not None + and not timed_write + and HAP_PERMISSION_TIMED_WRITE in char.properties[PROP_PERMISSIONS] + ): + if self.enforce_timed_writes: + results[aid][iid] = { + HAP_REPR_STATUS: HAP_SERVER_STATUS.INVALID_VALUE_IN_REQUEST + } + continue + logger.debug( + "%s: %s requires a timed write but was written without one", + client_addr, + char.display_name, + ) + # Only characteristics with the write response permission answer # a write with a value, and only when the controller asks for it. write_response = ( - query.get(HAP_REPR_WRITE_RESPONSE, False) + bool(query.get(HAP_REPR_WRITE_RESPONSE)) and HAP_PERMISSION_WRITE_RESPONSE in char.properties[PROP_PERMISSIONS] ) # A new write replaces a former response of this connection. The @@ -1153,15 +1324,22 @@ def set_characteristics( if private_write and char.value == _stored_value(char, value): char.value = previous_value - if set_result_value is not None and write_response: - result = { - HAP_REPR_STATUS: set_result, - HAP_REPR_VALUE: self._store_write_response( - char, set_result_value, client_addr - ), - } - else: - result = {HAP_REPR_STATUS: set_result} + result: dict[str, Any] = {HAP_REPR_STATUS: set_result} + if inspect.iscoroutine(set_result_value): + pending.append( + _PendingWrite( + result, + set_result_value, + write_response, + char, + previous_value, + not private_write, + ) + ) + elif set_result_value is not None and write_response: + result[HAP_REPR_VALUE] = self._store_write_response( + char, set_result_value, client_addr + ) results[aid][iid] = result char_to_iid[char] = iid @@ -1193,20 +1371,88 @@ def set_characteristics( for char in chars: aid_results[char_to_iid[char]][HAP_REPR_STATUS] = set_result - characteristics = [] - nonempty_results_exist = False - for aid, iid_results in results.items(): - for iid, result in iid_results.items(): - result[HAP_REPR_AID] = aid - result[HAP_REPR_IID] = iid - characteristics.append(result) - if ( - result[HAP_REPR_STATUS] != HAP_SERVER_STATUS.SUCCESS - or HAP_REPR_VALUE in result - ): - nonempty_results_exist = True - - return {HAP_REPR_CHARS: characteristics} if nonempty_results_exist else None + return results, pending + + async def _async_finish_set_characteristics( + self, + results: dict[int, dict[int, dict[str, Any]]], + pending: list[_PendingWrite], + client_addr: tuple[str, int], + timeout: float, + ) -> dict[str, Any] | None: + """Wait for the coroutine setters, then build the response.""" + tasks = {asyncio.ensure_future(write.coro): write for write in pending} + try: + done, not_done = await asyncio.wait(tasks, timeout=timeout) + except asyncio.CancelledError: + for task, write in tasks.items(): + if task.done(): + self._finish_done_write(task, write, client_addr) + continue + task.cancel() + task.add_done_callback(_retrieve_late_result) + raise + for task in not_done: + task.cancel() + task.add_done_callback(_retrieve_late_result) + write = tasks[task] + logger.warning( + "%s: Timed out setting characteristic %s", + client_addr, + write.char.display_name, + ) + write.result[HAP_REPR_STATUS] = HAP_SERVER_STATUS.OPERATION_TIMED_OUT + for task in done: + write = tasks[task] + try: + outcome = task.result() + except (Exception, asyncio.CancelledError): + logger.exception( + "%s: Error while setting characteristic %s", + client_addr, + write.char.display_name, + ) + write.result[HAP_REPR_STATUS] = ( + HAP_SERVER_STATUS.SERVICE_COMMUNICATION_FAILURE + ) + continue + write.char.client_update_finished( + write.previous_value, client_addr, notify=write.notify + ) + if outcome is not None and write.write_response: + # A response queued when the connection closes is dropped by + # the deferred cleanup in connection_lost. + write.result[HAP_REPR_VALUE] = self._store_write_response( + write.char, outcome, client_addr + ) + return _set_characteristics_response(results) + + @staticmethod + def _finish_done_write( + task: asyncio.Future[Any], write: _PendingWrite, client_addr: tuple[str, int] + ) -> None: + """Tell subscribers about a setter done before the write was cancelled.""" + if task.cancelled(): + return + if (err := task.exception()) is not None: + logger.error( + "%s: Error while setting characteristic %s", + client_addr, + write.char.display_name, + exc_info=err, + ) + return + write.char.client_update_finished( + write.previous_value, client_addr, notify=write.notify + ) + + def _take_prepared_write(self, pid: Any, client_addr: tuple[str, int]) -> bool: + """Consume the prepared write ``pid`` and return whether it is still valid.""" + if not _is_hap_int(pid): + # Controller input. A list or dict is not even hashable. + return False + expires_at = self.prepared_writes.get(client_addr, {}).pop(pid, None) + return expires_at is not None and time.monotonic() <= expires_at def _store_write_response( self, @@ -1258,10 +1504,14 @@ def prepare( :type prepare_query: dict """ + if not isinstance(prepare_query, dict): + return {HAP_REPR_STATUS: HAP_SERVER_STATUS.INVALID_VALUE_IN_REQUEST} + ttl = prepare_query.get(HAP_REPR_TTL) + pid = prepare_query.get(HAP_REPR_PID) + if not (_is_hap_int(ttl) and _is_hap_int(pid)): + return {HAP_REPR_STATUS: HAP_SERVER_STATUS.INVALID_VALUE_IN_REQUEST} try: - ttl = prepare_query[HAP_REPR_TTL] - pid = prepare_query[HAP_REPR_PID] - now = time.time() + now = time.monotonic() pending = self.prepared_writes.setdefault(client_addr, {}) # Prune expired entries and cap the number of pending writes so a # paired client cannot grow prepared_writes without bound by @@ -1269,13 +1519,13 @@ def prepare( for stale_pid in [ prepared_pid for prepared_pid, expires_at in pending.items() - if expires_at <= now + if expires_at < now ]: del pending[stale_pid] if len(pending) >= MAX_PREPARED_WRITES_PER_CLIENT: del pending[min(pending, key=pending.get)] pending[pid] = now + min(ttl / 1000, MAX_PREPARED_WRITE_TTL_SECONDS) - except (KeyError, TypeError, ValueError): + except OverflowError: return {HAP_REPR_STATUS: HAP_SERVER_STATUS.INVALID_VALUE_IN_REQUEST} return {HAP_REPR_STATUS: HAP_SERVER_STATUS.SUCCESS} diff --git a/pyhap/characteristic.py b/pyhap/characteristic.py index 773aa57f..3dd44c9f 100644 --- a/pyhap/characteristic.py +++ b/pyhap/characteristic.py @@ -206,7 +206,8 @@ def __init__( # Setters may take just the value or (value, sender_client_addr); see # ``_setter_wants_addr``. The return value is sent back as the write # response when the characteristic has the "wr" permission and the - # controller asked for one. + # controller asked for one. A setter may also be a coroutine function; + # a controller's write is answered once it is done. self.setter_callback: Callable[..., Any] | None = None # Cache of whether ``setter_callback`` opts into the client address, # recomputed only when the setter object itself changes. @@ -402,12 +403,17 @@ def client_update_value( sender_client_addr: tuple[str, int] | None = None, *, notify: bool = True, + defer_notify: bool = False, ) -> Any: """Called from broker for value change in Home app. - Change self.value to value and call callback. With ``notify`` False, - subscribers are not told about the written value, e.g. for a request - that only the writing connection may read back. + Change self.value to value and call callback. Returns what the callback + returned. With ``notify`` False, subscribers are not told about the + written value, e.g. for a request that only the writing connection may + read back. With ``defer_notify``, a coroutine the callback returns is + for the caller to await, and the caller then calls + :meth:`client_update_finished`, so subscribers only learn the value + once the setter succeeded. """ original_value = value if not self._always_null or original_value is not None: @@ -429,12 +435,28 @@ def client_update_value( response = self.setter_callback(value, sender_client_addr) else: response = self.setter_callback(value) - changed = self._value != previous_value - if changed and notify: + if not (defer_notify and inspect.iscoroutine(response)): + self.client_update_finished( + previous_value, sender_client_addr, notify=notify + ) + return response + + def client_update_finished( + self, + previous_value: Any, + sender_client_addr: tuple[str, int] | None, + *, + notify: bool = True, + ) -> None: + """Finish a client's write, e.g. after awaiting its coroutine setter. + + Tells subscribers about the new value if it differs from + ``previous_value``, see :meth:`client_update_value`. + """ + if notify and self._value != previous_value: self.notify(sender_client_addr) if self._always_null: self.value = None - return response def _setter_wants_addr(self) -> bool: """Whether the current setter opts into the sender's client address. diff --git a/pyhap/const.py b/pyhap/const.py index 2f70f950..aa9468ab 100644 --- a/pyhap/const.py +++ b/pyhap/const.py @@ -56,6 +56,7 @@ HAP_PERMISSION_HIDDEN = "hd" HAP_PERMISSION_NOTIFY = "ev" HAP_PERMISSION_READ = "pr" +HAP_PERMISSION_TIMED_WRITE = "tw" HAP_PERMISSION_WRITE = "pw" HAP_PERMISSION_WRITE_RESPONSE = "wr" diff --git a/pyhap/hap_handler.py b/pyhap/hap_handler.py index e027811a..8c7844cd 100644 --- a/pyhap/hap_handler.py +++ b/pyhap/hap_handler.py @@ -6,6 +6,7 @@ import asyncio from collections.abc import Awaitable from http import HTTPStatus +import inspect import logging import sys from typing import TYPE_CHECKING, Any, cast @@ -711,16 +712,40 @@ def handle_set_characteristics(self) -> None: "%s: Set characteristics content: %s", self.client_address, requested_chars ) - response = self.accessory_handler.set_characteristics( - requested_chars, cast("tuple[str, int]", self.client_address) + result = self.accessory_handler._set_characteristics_for_controller( + requested_chars, + cast("tuple[str, int]", self.client_address), + RESPONSE_TIMEOUT, ) - if response is None: - self.send_response(HTTPStatus.NO_CONTENT) + assert self.response is not None # noqa: S101 + if inspect.isawaitable(result): + # A setter is still running, answer once it is done. The driver + # times out slow setters itself. + self.response.task = asyncio.ensure_future( + self._async_set_characteristics_body(self.response, result) + ) return + self._fill_set_characteristics_response(self.response, result) - self.send_response(HTTPStatus.MULTI_STATUS) - self.send_header("Content-Type", self.JSON_RESPONSE_TYPE) - self.end_response(to_hap_json(response)) + def _fill_set_characteristics_response( + self, response: HAPResponse, result: dict[str, Any] | None + ) -> None: + """Fill in the response to a characteristics write.""" + if result is None: + response.status_code = HTTPStatus.NO_CONTENT.value + response.reason = HTTPStatus.NO_CONTENT.phrase + return + response.status_code = HTTPStatus.MULTI_STATUS.value + response.reason = HTTPStatus.MULTI_STATUS.phrase + response.headers.append(("Content-Type", self.JSON_RESPONSE_TYPE)) + response.body = to_hap_json(result) + + async def _async_set_characteristics_body( + self, response: HAPResponse, result: Awaitable[dict[str, Any] | None] + ) -> bytes: + """Wait for the pending setters, then fill in the write response.""" + self._fill_set_characteristics_response(response, await result) + return response.body def handle_prepare(self) -> None: """Handles a client request to prepare to write.""" diff --git a/tests/test_accessory_driver.py b/tests/test_accessory_driver.py index 13a5d471..77d40b1c 100644 --- a/tests/test_accessory_driver.py +++ b/tests/test_accessory_driver.py @@ -1,20 +1,20 @@ """Tests for pyhap.accessory_driver.""" import asyncio -from concurrent.futures import ThreadPoolExecutor from functools import partial +import inspect import os import tempfile import time from typing import Any, NoReturn -from unittest.mock import AsyncMock, MagicMock, patch +from unittest.mock import ANY, AsyncMock, MagicMock, call, patch from uuid import uuid1 from cryptography.hazmat.primitives import serialization import pytest from zeroconf import InterfaceChoice -from pyhap import util +from pyhap import accessory_driver, util from pyhap.accessory import STANDALONE_AID, Accessory, Bridge from pyhap.accessory_driver import ( MAX_FAILED_PAIRING_ATTEMPTS, @@ -616,7 +616,7 @@ def test_listen_address(kwargs: dict[str, Any], expected: Any) -> None: def _write( driver: AccessoryDriver, acc: Accessory, char: Characteristic, **query: Any -) -> dict[str, Any] | None: +) -> Any: """Write 0 to ``char`` from a mock client with extra query fields.""" return driver.set_characteristics( { @@ -746,6 +746,825 @@ def test_write_response_value_of_write_only_char_not_readable( assert HAP_REPR_VALUE not in acc_rep[HAP_REPR_SERVICES][-1][HAP_REPR_CHARS][0] +ASYNC_TIMEOUT = 1.0 + + +async def _controller_write( + driver: AccessoryDriver, acc: Accessory, char: Characteristic, **query: Any +) -> dict[str, Any] | None: + """Write 0 to ``char`` like a controller and wait for the async setters.""" + result = driver._set_characteristics_for_controller( + { + HAP_REPR_CHARS: [ + { + HAP_REPR_AID: acc.aid, + HAP_REPR_IID: acc.iid_manager.get_iid(char), + HAP_REPR_VALUE: 0, + **query, + } + ] + }, + WRITER, + ASYNC_TIMEOUT, + ) + assert inspect.isawaitable(result) + return await result + + +async def _async_return_one(value: int) -> int: + await asyncio.sleep(0) + return 1 + + +async def _async_return_one_with_addr(value: int, sender_client_addr: Any) -> int: + await asyncio.sleep(0) + return 1 + + +async def _async_return_none(value: int) -> None: + await asyncio.sleep(0) + + +async def _async_raise(value: int) -> NoReturn: + await asyncio.sleep(0) + raise ValueError("setter failed") + + +@pytest.mark.parametrize("bridged", [False, True], ids=["standalone", "bridged"]) +@pytest.mark.parametrize( + ("setter", "permissions", "expected", "stored"), + [ + pytest.param( + _async_return_one, + ["pr", "pw", "wr"], + {HAP_REPR_STATUS: HAP_SERVER_STATUS.SUCCESS, HAP_REPR_VALUE: 1}, + 0, + id="write-response", + ), + pytest.param( + _async_return_one_with_addr, + ["pw", "wr"], + {HAP_REPR_STATUS: HAP_SERVER_STATUS.SUCCESS, HAP_REPR_VALUE: 1}, + 1, + id="write-response-with-addr", + ), + pytest.param( + _async_raise, + ["pr", "pw", "wr"], + {HAP_REPR_STATUS: HAP_SERVER_STATUS.SERVICE_COMMUNICATION_FAILURE}, + 0, + id="error", + ), + ], +) +async def test_async_setter_result( + driver: AccessoryDriver, + bridged: bool, + setter: Any, + permissions: list[str], + expected: dict[str, Any], + stored: int, +) -> None: + """The result of an async setter is awaited before the write is answered.""" + acc, char, _ = _add_bridged_or_standalone(driver, permissions, bridged) + char.setter_callback = setter + + result = await _controller_write( + driver, acc, char, **{HAP_REPR_WRITE_RESPONSE: True} + ) + + assert result == { + HAP_REPR_CHARS: [ + { + HAP_REPR_AID: acc.aid, + HAP_REPR_IID: acc.iid_manager.get_iid(char), + **expected, + } + ] + } + assert char.value == stored + + +@pytest.mark.parametrize( + ("reader", "expected"), + [ + pytest.param(WRITER, 1, id="writer"), + pytest.param(OTHER_CONTROLLER, 0, id="other-controller"), + ], +) +async def test_async_write_response_readable_only_by_writer( + driver: AccessoryDriver, reader: tuple[str, int], expected: int +) -> None: + """Only the controller that wrote reads an async setter's response back.""" + acc, char, _ = _add_bridged_or_standalone(driver, ["pr", "pw", "wr"], False) + char.getter_callback = None + char.setter_callback = _async_return_one + + await _controller_write( + driver, acc, char, **{HAP_REPR_VALUE: 5, HAP_REPR_WRITE_RESPONSE: True} + ) + + char_id = f"{acc.aid}.{acc.iid_manager.get_iid(char)}" + assert _read(driver, char_id, reader) == expected + + +async def test_async_write_response_dropped_when_closed_while_queued( + driver: AccessoryDriver, +) -> None: + """A response that completes as the connection closes is not kept.""" + # This covers a completion that is already queued when the connection + # closes. The cancel in ``HAPServerProtocol.close()`` covers the rest. + driver.loop = asyncio.get_running_loop() + acc, char, _ = _add_write_response_char(driver, False) + release = asyncio.Event() + + async def _setter(value: int) -> int: + await release.wait() + return 7 + + char.setter_callback = _setter + write = asyncio.ensure_future( + _controller_write( + driver, acc, char, **{HAP_REPR_VALUE: 5, HAP_REPR_WRITE_RESPONSE: True} + ) + ) + await asyncio.sleep(0) + release.set() + # Two loop steps: the setter is done and the write's completion is queued. + await asyncio.sleep(0) + await asyncio.sleep(0) + + driver.connection_lost(WRITER) + await write + await asyncio.sleep(0) + + assert driver._write_responses == {} + + +async def test_async_write_response_request_not_shared( + driver: AccessoryDriver, +) -> None: + """Other controllers don't read the written request while the setter runs.""" + acc, char, _ = _add_bridged_or_standalone(driver, ["pr", "pw", "wr"], False) + char.getter_callback = None + release = asyncio.Event() + + async def _setter(value: int) -> int: + await release.wait() + return 1 + + char.setter_callback = _setter + write = asyncio.ensure_future( + _controller_write( + driver, acc, char, **{HAP_REPR_VALUE: 5, HAP_REPR_WRITE_RESPONSE: True} + ) + ) + await asyncio.sleep(0) + + char_id = f"{acc.aid}.{acc.iid_manager.get_iid(char)}" + assert _read(driver, char_id, OTHER_CONTROLLER) == 0 + release.set() + await write + assert _read(driver, char_id, OTHER_CONTROLLER) == 0 + + +@pytest.mark.parametrize( + ("setter", "permissions", "query"), + [ + pytest.param(_async_return_one, ["pr", "pw", "wr"], {}, id="not-requested"), + pytest.param( + _async_return_one, + ["pr", "pw"], + {HAP_REPR_WRITE_RESPONSE: True}, + id="no-wr-permission", + ), + pytest.param( + _async_return_none, + ["pr", "pw", "wr"], + {HAP_REPR_WRITE_RESPONSE: True}, + id="no-value", + ), + ], +) +async def test_async_setter_without_write_response( + driver: AccessoryDriver, + setter: Any, + permissions: list[str], + query: dict[str, Any], +) -> None: + """A successful async setter without a write response leaves no body.""" + acc, char, _ = _add_bridged_or_standalone(driver, permissions, False) + char.setter_callback = setter + + assert await _controller_write(driver, acc, char, **query) is None + assert char.value == 0 + + +async def _async_level_setter(values: Any) -> None: + raise AssertionError("must not run") + + +@pytest.mark.parametrize( + "attach", + [ + pytest.param( + lambda acc, service: setattr(acc, "setter_callback", _async_level_setter), + id="accessory", + ), + pytest.param( + lambda acc, service: setattr( + service, "setter_callback", _async_level_setter + ), + id="service", + ), + ], +) +async def test_async_accessory_or_service_setter_is_closed( + driver: AccessoryDriver, attach: Any, caplog: pytest.LogCaptureFixture +) -> None: + """A coroutine from an accessory or service setter is closed with a warning.""" + acc = Accessory(driver, "Test Accessory") + service = Service(uuid1(), "Test Service") + char = Characteristic( + "Value", uuid1(), {PROP_FORMAT: HAP_FORMAT_INT, PROP_PERMISSIONS: ["pr", "pw"]} + ) + service.add_characteristic(char) + acc.add_service(service) + driver.add_accessory(acc) + attach(acc, service) + + result = driver._set_characteristics_for_controller( + { + HAP_REPR_CHARS: [ + { + HAP_REPR_AID: acc.aid, + HAP_REPR_IID: acc.iid_manager.get_iid(char), + HAP_REPR_VALUE: 2, + } + ] + }, + ("1.2.3.4", 5), + ASYNC_TIMEOUT, + ) + + assert result is None + assert char.value == 2 + assert "only characteristic setters may be coroutine functions" in caplog.text + + +async def test_async_and_sync_setters_in_one_write(driver: AccessoryDriver) -> None: + """Sync and async setters of one request share a single response.""" + acc = Accessory(driver, "Test Accessory") + service = Service(uuid1(), "Test Service") + async_char = Characteristic( + "Async", + uuid1(), + {PROP_FORMAT: HAP_FORMAT_INT, PROP_PERMISSIONS: ["pr", "pw", "wr"]}, + ) + async_char.setter_callback = _async_return_one + sync_char = Characteristic( + "Sync", uuid1(), {PROP_FORMAT: HAP_FORMAT_INT, PROP_PERMISSIONS: ["pr", "pw"]} + ) + sync_char.setter_callback = MagicMock(side_effect=ValueError) + service.add_characteristic(async_char, sync_char) + service_setter = MagicMock() + service.setter_callback = service_setter + acc.add_service(service) + driver.add_accessory(acc) + async_iid = acc.iid_manager.get_iid(async_char) + sync_iid = acc.iid_manager.get_iid(sync_char) + + result = driver._set_characteristics_for_controller( + { + HAP_REPR_CHARS: [ + { + HAP_REPR_AID: acc.aid, + HAP_REPR_IID: async_iid, + HAP_REPR_VALUE: 3, + HAP_REPR_WRITE_RESPONSE: True, + }, + {HAP_REPR_AID: acc.aid, HAP_REPR_IID: sync_iid, HAP_REPR_VALUE: 4}, + ] + }, + ("1.2.3.4", 5), + ASYNC_TIMEOUT, + ) + + service_setter.assert_called_once_with({"Async": 3, "Sync": 4}) + assert inspect.isawaitable(result) + assert await result == { + HAP_REPR_CHARS: [ + { + HAP_REPR_AID: acc.aid, + HAP_REPR_IID: async_iid, + HAP_REPR_STATUS: HAP_SERVER_STATUS.SUCCESS, + HAP_REPR_VALUE: 1, + }, + { + HAP_REPR_AID: acc.aid, + HAP_REPR_IID: sync_iid, + HAP_REPR_STATUS: HAP_SERVER_STATUS.SERVICE_COMMUNICATION_FAILURE, + }, + ] + } + + +class _SlowSetter: + """An async setter that runs until it is cancelled.""" + + def __init__(self) -> None: + self.started = asyncio.Event() + self.cancelled = asyncio.Event() + + async def __call__(self, value: int) -> None: + self.started.set() + try: + await asyncio.sleep(10) + except asyncio.CancelledError: + self.cancelled.set() + raise + + +class _RecordingSetter: + """An async setter that records that it ran and answers 1.""" + + def __init__(self) -> None: + self.ran = asyncio.Event() + + async def __call__(self, value: int) -> int: + self.ran.set() + return 1 + + +async def test_async_setter_timeout_keeps_other_results( + driver: AccessoryDriver, +) -> None: + """Only the setter that runs too long answers -70408, and it is cancelled.""" + acc = Accessory(driver, "Test Accessory") + service = Service(uuid1(), "Test Service") + slow = Characteristic( + "Slow", uuid1(), {PROP_FORMAT: HAP_FORMAT_INT, PROP_PERMISSIONS: ["pw"]} + ) + slow_setter = _SlowSetter() + slow.setter_callback = slow_setter + fast = Characteristic( + "Fast", uuid1(), {PROP_FORMAT: HAP_FORMAT_INT, PROP_PERMISSIONS: ["pw", "wr"]} + ) + fast.setter_callback = _async_return_one + service.add_characteristic(slow, fast) + acc.add_service(service) + driver.add_accessory(acc) + slow_iid = acc.iid_manager.get_iid(slow) + fast_iid = acc.iid_manager.get_iid(fast) + + result = driver._set_characteristics_for_controller( + { + HAP_REPR_CHARS: [ + {HAP_REPR_AID: acc.aid, HAP_REPR_IID: slow_iid, HAP_REPR_VALUE: 1}, + { + HAP_REPR_AID: acc.aid, + HAP_REPR_IID: fast_iid, + HAP_REPR_VALUE: 2, + HAP_REPR_WRITE_RESPONSE: True, + }, + ] + }, + ("1.2.3.4", 5), + 0.05, + ) + + assert inspect.isawaitable(result) + assert await result == { + HAP_REPR_CHARS: [ + { + HAP_REPR_AID: acc.aid, + HAP_REPR_IID: slow_iid, + HAP_REPR_STATUS: HAP_SERVER_STATUS.OPERATION_TIMED_OUT, + }, + { + HAP_REPR_AID: acc.aid, + HAP_REPR_IID: fast_iid, + HAP_REPR_STATUS: HAP_SERVER_STATUS.SUCCESS, + HAP_REPR_VALUE: 1, + }, + ] + } + await asyncio.wait_for(slow_setter.cancelled.wait(), ASYNC_TIMEOUT) + + +async def test_cancelled_write_cancels_async_setters( + driver: AccessoryDriver, +) -> None: + """Cancelling the pending write, e.g. on a closed connection, stops setters.""" + acc, char, _ = _add_bridged_or_standalone(driver, ["pw"], False) + setter = _SlowSetter() + char.setter_callback = setter + task = asyncio.ensure_future(_controller_write(driver, acc, char)) + await setter.started.wait() + + task.cancel() + + with pytest.raises(asyncio.CancelledError): + await task + await asyncio.wait_for(setter.cancelled.wait(), ASYNC_TIMEOUT) + + +class _CoroutineRecorder: + """A setter that hands out a coroutine and keeps a reference to it.""" + + def __init__(self) -> None: + self.coro: Any = None + + def __call__(self, value: int) -> Any: + self.coro = _async_return_none(value) + return self.coro + + +async def test_malformed_entry_closes_pending_setters(driver: AccessoryDriver) -> None: + """A write that fails on a later entry closes the coroutines it created.""" + acc, char, _ = _add_bridged_or_standalone(driver, ["pw"], False) + setter = _CoroutineRecorder() + char.setter_callback = setter + + with pytest.raises(KeyError): + driver._set_characteristics_for_controller( + { + HAP_REPR_CHARS: [ + { + HAP_REPR_AID: acc.aid, + HAP_REPR_IID: acc.iid_manager.get_iid(char), + HAP_REPR_VALUE: 0, + }, + {HAP_REPR_AID: acc.aid}, + ] + }, + ("1.2.3.4", 5), + ASYNC_TIMEOUT, + ) + + assert inspect.getcoroutinestate(setter.coro) == inspect.CORO_CLOSED + + +async def test_public_set_characteristics_schedules_async_setters( + driver: AccessoryDriver, +) -> None: + """Called directly, the write is answered at once and the setter runs later.""" + driver.loop = asyncio.get_running_loop() + acc, char, _ = _add_bridged_or_standalone(driver, ["pw", "wr"], False) + setter = _RecordingSetter() + char.setter_callback = setter + + assert _write(driver, acc, char, **{HAP_REPR_WRITE_RESPONSE: True}) is None + await asyncio.wait_for(setter.ran.wait(), ASYNC_TIMEOUT) + + +class _NotifyCheckingSetter: + """An async setter that records how often subscribers were told when it ran.""" + + def __init__(self, notify: MagicMock, fail: bool) -> None: + self.notify = notify + self.fail = fail + self.notified_before: list[int] = [] + + async def __call__(self, value: int) -> None: + await asyncio.sleep(0) + self.notified_before.append(self.notify.call_count) + if self.fail: + raise ValueError("setter failed") + + +@pytest.mark.parametrize( + ("fail", "calls"), + [ + pytest.param(False, [call(WRITER)], id="success"), + pytest.param(True, [], id="error"), + ], +) +async def test_async_setter_notifies_once_it_succeeded( + driver: AccessoryDriver, fail: bool, calls: list[Any] +) -> None: + """Subscribers learn a written value only after the async setter succeeded.""" + acc, char, _ = _add_bridged_or_standalone(driver, ["pr", "pw", "ev"], False) + char.value = 5 + + with patch.object(Characteristic, "notify") as notify: + setter = _NotifyCheckingSetter(notify, fail) + char.setter_callback = setter + await _controller_write(driver, acc, char) + + assert setter.notified_before == [0] + assert notify.call_args_list == calls + + +class _ValueSettingSetter: + """An async setter that stores a value of its own and answers 1.""" + + def __init__(self, char: Characteristic) -> None: + self.char = char + self.done = asyncio.Event() + + async def __call__(self, value: int) -> int: + await asyncio.sleep(0) + self.char.value = 7 + self.done.set() + return 1 + + +@pytest.mark.parametrize("own_value", [False, True], ids=["request", "own-value"]) +async def test_private_async_write_does_not_notify( + driver: AccessoryDriver, own_value: bool +) -> None: + """A write response request through an async setter tells no subscriber.""" + acc, char, _ = _add_bridged_or_standalone(driver, ["pr", "pw", "ev", "wr"], False) + char.value = 5 + setters = {False: _async_return_one, True: _ValueSettingSetter(char)} + char.setter_callback = setters[own_value] + + with patch.object(Characteristic, "notify") as notify: + await _controller_write(driver, acc, char, **{HAP_REPR_WRITE_RESPONSE: True}) + + notify.assert_not_called() + + +def test_direct_client_update_notifies_at_once(driver: AccessoryDriver) -> None: + """Without defer_notify, client_update_value notifies as before.""" + _, char, _ = _add_bridged_or_standalone(driver, ["pr", "pw", "ev"], False) + char.value = 5 + char.setter_callback = _async_return_none + + with patch.object(Characteristic, "notify") as notify: + coro = char.client_update_value(0, WRITER) + + coro.close() + notify.assert_called_once_with(WRITER) + + +async def test_public_write_notifies_once_the_async_setter_ran( + driver: AccessoryDriver, +) -> None: + """Called directly, subscribers also learn the value after the setter ran.""" + driver.loop = asyncio.get_running_loop() + acc, char, _ = _add_bridged_or_standalone(driver, ["pr", "pw", "ev"], False) + char.value = 5 + setter = _RecordingSetter() + char.setter_callback = setter + notified = asyncio.Event() + ran_when_notified: list[bool] = [] + + def _record(*_: Any) -> None: + ran_when_notified.append(setter.ran.is_set()) + notified.set() + + with patch.object(Characteristic, "notify", side_effect=_record) as notify: + assert _write(driver, acc, char) is None + await asyncio.wait_for(notified.wait(), ASYNC_TIMEOUT) + + assert ran_when_notified == [True] + notify.assert_called_once_with(WRITER) + + +async def test_public_write_logs_a_failing_async_setter( + driver: AccessoryDriver, +) -> None: + """Called directly, a failing async setter is logged with its name.""" + driver.loop = asyncio.get_running_loop() + acc, char, _ = _add_bridged_or_standalone(driver, ["pr", "pw", "ev"], False) + char.value = 5 + logged = asyncio.Event() + char.setter_callback = _async_raise + + with ( + patch.object(Characteristic, "notify") as notify, + patch.object( + accessory_driver.logger, + "exception", + side_effect=lambda *_: logged.set(), + ) as log_exception, + ): + assert _write(driver, acc, char) is None + await asyncio.wait_for(logged.wait(), ASYNC_TIMEOUT) + + log_exception.assert_called_once_with( + "%s: Error while setting characteristic %s", WRITER, char.display_name + ) + notify.assert_not_called() + + +@pytest.mark.parametrize( + ("fast_setter", "calls"), + [ + pytest.param(_async_return_none, [call(WRITER)], id="success"), + pytest.param(_async_raise, [], id="error"), + ], +) +async def test_cancelled_write_notifies_setters_that_finished( + driver: AccessoryDriver, fast_setter: Any, calls: list[Any] +) -> None: + """A setter done before the write is cancelled tells subscribers if it worked.""" + acc = Accessory(driver, "Test Accessory") + service = Service(uuid1(), "Test Service") + props = {PROP_FORMAT: HAP_FORMAT_INT, PROP_PERMISSIONS: ["pr", "pw", "ev"]} + fast = Characteristic("Fast", uuid1(), props) + fast.setter_callback = fast_setter + slow = Characteristic("Slow", uuid1(), props) + slow_setter = _SlowSetter() + slow.setter_callback = slow_setter + service.add_characteristic(fast, slow) + acc.add_service(service) + driver.add_accessory(acc) + fast.value = slow.value = 5 + fast_iid = acc.iid_manager.get_iid(fast) + slow_iid = acc.iid_manager.get_iid(slow) + query = { + HAP_REPR_CHARS: [ + {HAP_REPR_AID: acc.aid, HAP_REPR_IID: fast_iid, HAP_REPR_VALUE: 0}, + {HAP_REPR_AID: acc.aid, HAP_REPR_IID: slow_iid, HAP_REPR_VALUE: 0}, + ] + } + + with patch.object(Characteristic, "notify") as notify: + result = driver._set_characteristics_for_controller( + query, WRITER, ASYNC_TIMEOUT + ) + assert inspect.isawaitable(result) + task = asyncio.ensure_future(result) + await asyncio.wait_for(slow_setter.started.wait(), ASYNC_TIMEOUT) + await asyncio.sleep(0) + task.cancel() + await asyncio.gather(task, return_exceptions=True) + + assert notify.call_args_list == calls + assert task.cancelled() + + +async def test_cancelled_write_logs_a_setter_that_failed_before( + driver: AccessoryDriver, +) -> None: + """A setter that failed before the write is cancelled is logged once.""" + acc = Accessory(driver, "Test Accessory") + service = Service(uuid1(), "Test Service") + props = {PROP_FORMAT: HAP_FORMAT_INT, PROP_PERMISSIONS: ["pw"]} + failing = Characteristic("Failing", uuid1(), props) + failing.setter_callback = _async_raise + slow = Characteristic("Slow", uuid1(), props) + slow_setter = _SlowSetter() + slow.setter_callback = slow_setter + service.add_characteristic(failing, slow) + acc.add_service(service) + driver.add_accessory(acc) + failing_iid = acc.iid_manager.get_iid(failing) + slow_iid = acc.iid_manager.get_iid(slow) + query = { + HAP_REPR_CHARS: [ + {HAP_REPR_AID: acc.aid, HAP_REPR_IID: failing_iid, HAP_REPR_VALUE: 0}, + {HAP_REPR_AID: acc.aid, HAP_REPR_IID: slow_iid, HAP_REPR_VALUE: 0}, + ] + } + result = driver._set_characteristics_for_controller(query, WRITER, ASYNC_TIMEOUT) + assert inspect.isawaitable(result) + task = asyncio.ensure_future(result) + await asyncio.wait_for(slow_setter.started.wait(), ASYNC_TIMEOUT) + await asyncio.sleep(0) + + with ( + patch.object(accessory_driver.logger, "error") as log_error, + patch.object(accessory_driver.logger, "debug") as log_debug, + ): + task.cancel() + await asyncio.gather(task, return_exceptions=True) + await asyncio.wait_for(slow_setter.cancelled.wait(), ASYNC_TIMEOUT) + + log_error.assert_called_once_with( + "%s: Error while setting characteristic %s", + WRITER, + "Failing", + exc_info=ANY, + ) + log_debug.assert_not_called() + + +async def test_public_private_write_with_own_value_does_not_notify( + driver: AccessoryDriver, +) -> None: + """Called directly, a write response request tells no subscriber either.""" + driver.loop = asyncio.get_running_loop() + acc, char, _ = _add_bridged_or_standalone(driver, ["pr", "pw", "ev", "wr"], False) + char.value = 5 + setter = _ValueSettingSetter(char) + char.setter_callback = setter + + with patch.object(Characteristic, "notify") as notify: + _write(driver, acc, char, **{HAP_REPR_WRITE_RESPONSE: True}) + await asyncio.wait_for(setter.done.wait(), ASYNC_TIMEOUT) + await asyncio.sleep(0) + + notify.assert_not_called() + + +class _StubbornSetter: + """An async setter that ignores its cancellation and fails afterwards.""" + + def __init__(self) -> None: + self.started = asyncio.Event() + + async def __call__(self, value: int) -> None: + self.started.set() + try: + await asyncio.sleep(10) + except asyncio.CancelledError: + await asyncio.sleep(0) + raise ValueError("failed after the cancellation") from None + + +async def _write_and_cancel( + driver: AccessoryDriver, query: dict[str, Any], setter: _StubbornSetter +) -> None: + """Start a controller write and cancel it once the setter runs.""" + result = driver._set_characteristics_for_controller(query, WRITER, ASYNC_TIMEOUT) + assert inspect.isawaitable(result) + task = asyncio.ensure_future(result) + await asyncio.wait_for(setter.started.wait(), ASYNC_TIMEOUT) + task.cancel() + await asyncio.gather(task, return_exceptions=True) + + +async def _write_until_timeout( + driver: AccessoryDriver, query: dict[str, Any], setter: _StubbornSetter +) -> None: + """Make a controller write whose setter outlives the timeout.""" + result = driver._set_characteristics_for_controller(query, WRITER, 0.05) + assert inspect.isawaitable(result) + await result + + +@pytest.mark.parametrize( + "run", [_write_until_timeout, _write_and_cancel], ids=["timeout", "cancelled"] +) +async def test_setter_failing_after_its_cancellation_is_retrieved( + driver: AccessoryDriver, run: Any +) -> None: + """A setter that runs on after its cancellation has its error logged.""" + acc, char, _ = _add_bridged_or_standalone(driver, ["pw"], False) + setter = _StubbornSetter() + char.setter_callback = setter + logged = asyncio.Event() + query = { + HAP_REPR_CHARS: [ + { + HAP_REPR_AID: acc.aid, + HAP_REPR_IID: acc.iid_manager.get_iid(char), + HAP_REPR_VALUE: 0, + } + ] + } + + with patch.object( + accessory_driver.logger, "debug", side_effect=lambda *_, **__: logged.set() + ) as log_debug: + await run(driver, query, setter) + await asyncio.wait_for(logged.wait(), ASYNC_TIMEOUT) + + assert log_debug.call_args.args == ("Setter failed after it was cancelled",) + + +@pytest.mark.parametrize("flag", [True, 1], ids=["true", "one"]) +def test_write_response_flag_is_truthy(driver: AccessoryDriver, flag: Any) -> None: + """Any truthy "r" asks for a write response, as before.""" + acc, char, _ = _add_bridged_or_standalone(driver, ["pw", "wr"], False) + char.setter_callback = MagicMock(return_value=1) + + response = _write(driver, acc, char, **{HAP_REPR_WRITE_RESPONSE: flag}) + + assert response[HAP_REPR_CHARS][0][HAP_REPR_VALUE] == 1 + + +async def test_setter_returning_a_task_is_not_awaited( + driver: AccessoryDriver, +) -> None: + """A sync setter that starts a task is answered without waiting for it.""" + acc, char, _ = _add_bridged_or_standalone(driver, ["pw"], False) + task = asyncio.ensure_future(asyncio.sleep(10)) + char.setter_callback = MagicMock(return_value=task) + + result = driver._set_characteristics_for_controller( + { + HAP_REPR_CHARS: [ + { + HAP_REPR_AID: acc.aid, + HAP_REPR_IID: acc.iid_manager.get_iid(char), + HAP_REPR_VALUE: 0, + } + ] + }, + ("1.2.3.4", 5), + ASYNC_TIMEOUT, + ) + + assert result is None + assert not task.done() + task.cancel() + + def _add_write_response_char( driver: AccessoryDriver, bridged: bool ) -> tuple[Accessory, Characteristic, str]: @@ -1280,24 +2099,33 @@ def fail_callback(*_: Any) -> NoReturn: } -def test_start_from_sync(driver: AccessoryDriver) -> None: - """Start from sync.""" +def test_start_from_sync(async_zeroconf: Any) -> None: + """Start from sync, on the loop and executor the driver makes itself. + + Not the shared test loop: stopping shuts the executor down, which would + leave that loop unable to run later tests' executor jobs. + """ + with ( + patch("pyhap.accessory_driver.HAPServer.async_stop", new_callable=MagicMock), + patch("pyhap.accessory_driver.HAPServer.async_start", new_callable=AsyncMock), + patch("pyhap.accessory_driver.AccessoryDriver.persist"), + ): + driver = AccessoryDriver() - class Acc(Accessory): - @Accessory.run_at_interval(0) - async def run(self) -> None: - driver.executor = ThreadPoolExecutor() - driver.loop.set_default_executor(driver.executor) - await driver.async_stop() + class Acc(Accessory): + @Accessory.run_at_interval(0) + async def run(self) -> None: + await driver.async_stop() - def setup_message(self) -> None: - pass + def setup_message(self) -> None: + pass - acc = Acc(driver, "TestAcc") - driver.add_accessory(acc) - with patch.object(driver.loop, "close"): + acc = Acc(driver, "TestAcc") + driver.add_accessory(acc) driver.start() + assert driver.loop.is_closed() + def test_accessory_level_callbacks(driver: AccessoryDriver) -> None: bridge = Bridge(driver, "mybridge") @@ -2084,7 +2912,7 @@ def test_prepare_bounds_prepared_writes(driver: AccessoryDriver) -> None: # An absurd TTL is clamped to the maximum. driver.prepare({HAP_REPR_TTL: 10_000_000_000, HAP_REPR_PID: 1}, client_addr) assert ( - driver.prepared_writes[client_addr][1] - time.time() + driver.prepared_writes[client_addr][1] - time.monotonic() <= MAX_PREPARED_WRITE_TTL_SECONDS ) diff --git a/tests/test_hap_handler.py b/tests/test_hap_handler.py index fc390c95..02e8161f 100644 --- a/tests/test_hap_handler.py +++ b/tests/test_hap_handler.py @@ -1,5 +1,6 @@ """Tests for the HAPServerHandler.""" +import asyncio import json from typing import Any, NoReturn from unittest.mock import patch @@ -1246,3 +1247,85 @@ def test_invalid_pairing_three(driver: AccessoryDriver) -> None: hap_handler.HAP_TLV_TAGS.SEQUENCE_NUM: hap_handler.HAP_TLV_STATES.M6, hap_handler.HAP_TLV_TAGS.ERROR_CODE: hap_handler.HAP_TLV_ERRORS.AUTHENTICATION, } + + +def _dispatch_write_to_async_setter( + driver: AccessoryDriver, setter: Any, body: bytes +) -> hap_handler.HAPResponse: + """Dispatch a characteristics write to a wr characteristic with ``setter``.""" + acc = Accessory(driver, "TestAcc") + service = Service(uuid1(), "Test Service") + char = Characteristic( + "Value", uuid1(), {"Format": "int", "Permissions": ["pr", "pw", "wr"]} + ) + char.setter_callback = setter + service.add_characteristic(char) + acc.add_service(service) + driver.add_accessory(acc) + assert acc.iid_manager.get_iid(char) == 9 + + handler = hap_handler.HAPServerHandler(driver, "peername") + handler.is_encrypted = True + request = h11.Request( + method="PUT", target="/characteristics", headers=[(b"host", b"t")] + ) + return handler.dispatch(request, body) + + +async def _async_return_one(_value: int) -> int: + await asyncio.sleep(0) + return 1 + + +async def _async_sleep(_value: int) -> None: + await asyncio.sleep(10) + + +@pytest.mark.parametrize( + ("body", "status_code", "headers", "response_body"), + [ + pytest.param( + b'{"characteristics":[{"aid":1,"iid":9,"value":0,"r":true}]}', + 207, + [("Content-Type", "application/hap+json")], + b'{"characteristics":[{"status":0,"value":1,"aid":1,"iid":9}]}', + id="write-response", + ), + pytest.param( + b'{"characteristics":[{"aid":1,"iid":9,"value":0}]}', + 204, + [], + b"", + id="no-write-response", + ), + ], +) +async def test_handle_set_characteristics_async_setter( + driver: AccessoryDriver, + body: bytes, + status_code: int, + headers: list[tuple[str, str]], + response_body: bytes, +) -> None: + """A write to an async setter is answered once the setter is done.""" + response = _dispatch_write_to_async_setter(driver, _async_return_one, body) + + assert response.task is not None + assert await response.task == response_body + assert response.status_code == status_code + assert response.headers == headers + + +async def test_handle_set_characteristics_async_setter_timeout( + driver: AccessoryDriver, +) -> None: + """An async setter that does not finish in time answers -70408 for its char.""" + with patch.object(hap_handler, "RESPONSE_TIMEOUT", 0.01): + response = _dispatch_write_to_async_setter( + driver, _async_sleep, b'{"characteristics":[{"aid":1,"iid":9,"value":0}]}' + ) + assert response.task is not None + assert await response.task == ( + b'{"characteristics":[{"status":-70408,"aid":1,"iid":9}]}' + ) + assert response.status_code == 207 diff --git a/tests/test_hap_protocol.py b/tests/test_hap_protocol.py index 190dee6b..95de4e2e 100644 --- a/tests/test_hap_protocol.py +++ b/tests/test_hap_protocol.py @@ -5,15 +5,18 @@ import time from typing import Any, NoReturn, cast from unittest.mock import MagicMock, Mock, patch +from uuid import uuid1 from cryptography.exceptions import InvalidTag from pyhap import hap_handler, hap_protocol from pyhap.accessory import Accessory, Bridge, get_topic from pyhap.accessory_driver import AccessoryDriver +from pyhap.characteristic import Characteristic from pyhap.const import HAP_REPR_AID, HAP_REPR_IID, HAP_REPR_VALUE from pyhap.hap_crypto import HAPCrypto, hap_hkdf from pyhap.hap_handler import HAPResponse +from pyhap.service import Service class MockTransport(asyncio.Transport): @@ -925,6 +928,98 @@ async def _async_get_snapshot(*_: Any) -> bytes: hap_proto.close() +ASYNC_PUT_REQUEST = ( + b"PUT /characteristics HTTP/1.1\r\n" + b"Host: HASS12\\032AD1C22._hap._tcp.local\r\n" + b"Content-Length: 58\r\n" + b"Content-Type: application/hap+json\r\n" + b"\r\n" + b'{"characteristics":[{"aid":1,"iid":9,"value":0,"r":true}]}' +) +ASYNC_PUT_RESPONSE = ( + b"HTTP/1.1 207 Multi-Status\r\n" + b"Content-Type: application/hap+json\r\n" + b"Content-Length: 60\r\n\r\n" + b'{"characteristics":[{"status":0,"value":1,"aid":1,"iid":9}]}' +) + + +def _async_setter_protocol( + driver: AccessoryDriver, setter_done: asyncio.Event +) -> hap_protocol.HAPServerProtocol: + """A connection to an accessory whose char 1.9 has an async setter. + + The setter waits for ``setter_done`` and answers 1 in the write response. + """ + transport = MagicMock() + transport.is_closing = Mock(return_value=False) + + async def _setter(_value: int) -> int: + await setter_done.wait() + return 1 + + acc = Accessory(driver, "TestAcc") + service = Service(uuid1(), "Test Service") + char = Characteristic( + "Value", uuid1(), {"Format": "int", "Permissions": ["pr", "pw", "wr"]} + ) + char.setter_callback = _setter + service.add_characteristic(char) + acc.add_service(service) + driver.add_accessory(acc) + assert acc.iid_manager.get_iid(char) == 9 + + hap_proto = hap_protocol.HAPServerProtocol(MagicMock(), {}, driver) + hap_proto.connection_made(transport) + _enable_encryption(hap_proto) + return hap_proto + + +async def test_set_characteristics_async_setter_write_response( + driver: AccessoryDriver, +) -> None: + """The write response of an async setter is sent once the setter is done.""" + setter_done = asyncio.Event() + hap_proto = _async_setter_protocol(driver, setter_done) + + with patch.object(hap_proto.transport, "writelines") as writelines: + hap_proto.data_received(ASYNC_PUT_REQUEST) + writelines.assert_not_called() + setter_done.set() + await _await_response(hap_proto) + + hap_proto.close() + assert b"".join(writelines.call_args_list[0][0]) == ASYNC_PUT_RESPONSE + + +async def test_get_pipelined_behind_async_setter(driver: AccessoryDriver) -> None: + """A GET behind an async write is answered after the 207, with the new value.""" + setter_done = asyncio.Event() + hap_proto = _async_setter_protocol(driver, setter_done) + + with patch.object(hap_proto.transport, "writelines") as writelines: + hap_proto.data_received( + ASYNC_PUT_REQUEST + + b"GET /characteristics?id=1.9 HTTP/1.1\r\n" + + b"Host: HASS12\\032AD1C22._hap._tcp.local\r\n\r\n" + ) + await asyncio.sleep(0) + writelines.assert_not_called() + setter_done.set() + await _await_response(hap_proto) + await asyncio.sleep(0) + + hap_proto.close() + assert writelines.call_count == 2 + assert b"".join(writelines.call_args_list[0][0]) == ASYNC_PUT_RESPONSE + assert b"".join(writelines.call_args_list[1][0]) == ( + b"HTTP/1.1 200 OK\r\n" + b"Content-Type: application/hap+json\r\n" + b"Content-Length: 49\r\n\r\n" + b'{"characteristics":[{"aid":1,"iid":9,"value":1}]}' + ) + + def test_upgrade_to_encrypted(driver: AccessoryDriver) -> None: """Test we switch to encrypted wen we get a shared_key.""" loop = MagicMock() diff --git a/tests/test_timed_write.py b/tests/test_timed_write.py new file mode 100644 index 00000000..929c919a --- /dev/null +++ b/tests/test_timed_write.py @@ -0,0 +1,309 @@ +"""Tests for timed writes (prepare, then write with the pid).""" + +from collections.abc import Iterator +import logging +from typing import Any, NamedTuple +from unittest.mock import MagicMock, patch + +import pytest + +from pyhap.accessory import Accessory, get_topic +from pyhap.accessory_driver import AccessoryDriver +from pyhap.const import ( + HAP_PERMISSION_NOTIFY, + HAP_REPR_AID, + HAP_REPR_CHARS, + HAP_REPR_IID, + HAP_REPR_PID, + HAP_REPR_STATUS, + HAP_REPR_TTL, + HAP_REPR_VALUE, + HAP_SERVER_STATUS, +) +from pyhap.service import Service + +CLIENT = ("1.2.3.4", 5000) +NOW = 1000.0 +REJECTED = HAP_SERVER_STATUS.INVALID_VALUE_IN_REQUEST + + +class WritableChar(NamedTuple): + """A characteristic of the test accessory and its setter.""" + + setter: MagicMock + aid: int + iid: int + + +def _writable(acc: Accessory, service: Service, name: str) -> WritableChar: + """Give the char ``name`` of ``service`` a mock setter.""" + setter = MagicMock() + char = service.configure_char(name, setter_callback=setter) + iid = acc.iid_manager.get_iid(char) + assert acc.aid is not None + assert iid is not None + return WritableChar(setter, acc.aid, iid) + + +@pytest.fixture +def chars(driver: AccessoryDriver) -> Iterator[dict[str, WritableChar]]: + """Add a garage door opener with a camera operating mode service. + + Target Door State is a plain write characteristic that iOS still writes + with timed writes, HomeKit Camera Active has the timed write permission. + """ + acc = Accessory(driver, "Garage") + door = driver.loader.get_service("GarageDoorOpener") + camera = driver.loader.get_service("CameraOperatingMode") + acc.add_service(door, camera) + driver.add_accessory(acc) + with patch.object(driver, "publish"): + yield { + "TargetDoorState": _writable(acc, door, "TargetDoorState"), + "HomeKitCameraActive": _writable(acc, camera, "HomeKitCameraActive"), + } + + +@pytest.fixture +def door(chars: dict[str, WritableChar]) -> WritableChar: + """Return the Target Door State of the test accessory.""" + return chars["TargetDoorState"] + + +def _write(aid: int, iid: int, pid: int) -> dict[str, Any]: + """Return a timed write of value 1 that also subscribes to the char.""" + return { + HAP_REPR_PID: pid, + HAP_REPR_CHARS: [ + { + HAP_REPR_AID: aid, + HAP_REPR_IID: iid, + HAP_REPR_VALUE: 1, + HAP_PERMISSION_NOTIFY: True, + } + ], + } + + +def test_prepared_write_is_applied( + driver: AccessoryDriver, + door: WritableChar, +) -> None: + """Test a write with a prepared pid inside its TTL is applied.""" + with patch("pyhap.accessory_driver.time.monotonic", return_value=NOW) as now: + driver.prepare({HAP_REPR_TTL: 2500, HAP_REPR_PID: 7}, CLIENT) + now.return_value = NOW + 2.5 + assert driver.set_characteristics(_write(door.aid, door.iid, 7), CLIENT) is None + + door.setter.assert_called_once_with(1) + assert CLIENT in driver.topics[get_topic(door.aid, door.iid)] + + +@pytest.mark.parametrize( + ("write_pid", "delay"), + [(8, 0.0), (1, 2.6), ([1], 0.0), (True, 0.0)], + ids=["unknown_pid", "expired_pid", "unhashable_pid", "bool_pid"], +) +def test_unprepared_write_is_ignored( + driver: AccessoryDriver, + door: WritableChar, + write_pid: Any, + delay: float, +) -> None: + """Test a write with an unknown or expired pid is ignored as a whole.""" + with patch("pyhap.accessory_driver.time.monotonic", return_value=NOW) as now: + driver.prepare({HAP_REPR_TTL: 2500, HAP_REPR_PID: 1}, CLIENT) + now.return_value = NOW + delay + response = driver.set_characteristics( + _write(door.aid, door.iid, write_pid), CLIENT + ) + + assert response == { + HAP_REPR_CHARS: [ + { + HAP_REPR_AID: door.aid, + HAP_REPR_IID: door.iid, + HAP_REPR_STATUS: HAP_SERVER_STATUS.INVALID_VALUE_IN_REQUEST, + } + ] + } + door.setter.assert_not_called() + assert get_topic(door.aid, door.iid) not in driver.topics + + +def test_prepared_write_is_kept_until_its_ttl_ends( + driver: AccessoryDriver, + door: WritableChar, +) -> None: + """Test a later prepare at the last moment of a TTL keeps that write.""" + with patch("pyhap.accessory_driver.time.monotonic", return_value=NOW) as now: + driver.prepare({HAP_REPR_TTL: 2500, HAP_REPR_PID: 7}, CLIENT) + now.return_value = NOW + 2.5 + driver.prepare({HAP_REPR_TTL: 2500, HAP_REPR_PID: 8}, CLIENT) + assert driver.set_characteristics(_write(door.aid, door.iid, 7), CLIENT) is None + + door.setter.assert_called_once_with(1) + + +def test_unprepared_write_ignores_every_characteristic( + driver: AccessoryDriver, + chars: dict[str, WritableChar], +) -> None: + """Test a write with an unknown pid writes none of its characteristics.""" + door, camera = chars["TargetDoorState"], chars["HomeKitCameraActive"] + query = { + HAP_REPR_PID: 8, + HAP_REPR_CHARS: [ + {HAP_REPR_AID: door.aid, HAP_REPR_IID: door.iid, HAP_REPR_VALUE: 1}, + {HAP_REPR_AID: camera.aid, HAP_REPR_IID: camera.iid, HAP_REPR_VALUE: 0}, + ], + } + + response = driver.set_characteristics(query, CLIENT) + + assert _statuses(response) == [REJECTED, REJECTED] + door.setter.assert_not_called() + camera.setter.assert_not_called() + + +@pytest.mark.parametrize( + "prepare_query", + [ + {HAP_REPR_TTL: 2500, HAP_REPR_PID: "7"}, + {HAP_REPR_TTL: 2500, HAP_REPR_PID: 1.5}, + {HAP_REPR_TTL: 2500, HAP_REPR_PID: True}, + {HAP_REPR_TTL: "2500", HAP_REPR_PID: 7}, + {HAP_REPR_TTL: 2500.0, HAP_REPR_PID: 7}, + {HAP_REPR_TTL: 10**400, HAP_REPR_PID: 7}, + {HAP_REPR_PID: 7}, + {HAP_REPR_TTL: 2500}, + [2500, 7], + None, + ], + ids=[ + "str_pid", + "float_pid", + "bool_pid", + "str_ttl", + "float_ttl", + "overflowing_ttl", + "missing_ttl", + "missing_pid", + "list", + "null", + ], +) +def test_prepare_refuses_non_integers( + driver: AccessoryDriver, prepare_query: Any +) -> None: + """Test a prepare that is no object or lacks an integer ttl or pid is refused.""" + assert driver.prepare(prepare_query, CLIENT) == {HAP_REPR_STATUS: REJECTED} + assert driver.prepared_writes.get(CLIENT, {}) == {} + + +def test_prepared_pid_is_used_once( + driver: AccessoryDriver, + door: WritableChar, +) -> None: + """Test a prepared pid cannot be replayed.""" + driver.prepare({HAP_REPR_TTL: 2500, HAP_REPR_PID: 7}, CLIENT) + assert driver.set_characteristics(_write(door.aid, door.iid, 7), CLIENT) is None + + response = driver.set_characteristics(_write(door.aid, door.iid, 7), CLIENT) + + assert response is not None + assert response[HAP_REPR_CHARS][0][HAP_REPR_STATUS] == ( + HAP_SERVER_STATUS.INVALID_VALUE_IN_REQUEST + ) + door.setter.assert_called_once_with(1) + + +def test_prepared_pid_belongs_to_its_connection( + driver: AccessoryDriver, + door: WritableChar, +) -> None: + """Test a pid prepared on one connection is unknown on another.""" + driver.prepare({HAP_REPR_TTL: 2500, HAP_REPR_PID: 7}, CLIENT) + + response = driver.set_characteristics( + _write(door.aid, door.iid, 7), ("1.2.3.4", 5001) + ) + + assert response is not None + assert response[HAP_REPR_CHARS][0][HAP_REPR_STATUS] == ( + HAP_SERVER_STATUS.INVALID_VALUE_IN_REQUEST + ) + door.setter.assert_not_called() + + +def _statuses(response: dict[str, Any] | None) -> list[int]: + """Return the statuses of a write response, none for a plain success.""" + return [char[HAP_REPR_STATUS] for char in (response or {}).get(HAP_REPR_CHARS, [])] + + +def _highest_level(records: list[logging.LogRecord]) -> int: + """Return the highest log level in ``records``, 0 without any.""" + return max((record.levelno for record in records), default=0) + + +@pytest.mark.parametrize( + ("enforce", "name", "request_extra", "value", "statuses", "calls", "logged"), + [ + (True, "HomeKitCameraActive", {}, 0, [REJECTED], 0, 0), + (True, "HomeKitCameraActive", {HAP_REPR_PID: 7}, 0, [], 1, 0), + (True, "HomeKitCameraActive", {}, None, [REJECTED], 0, 0), + (True, "TargetDoorState", {}, 0, [], 1, 0), + (False, "HomeKitCameraActive", {}, 0, [], 1, 1), + (False, "HomeKitCameraActive", {}, None, [REJECTED], 0, 0), + (False, "HomeKitCameraActive", {HAP_REPR_PID: 7}, 0, [], 1, 0), + (False, "TargetDoorState", {}, 0, [], 1, 0), + ], + ids=[ + "enforced_plain_write", + "enforced_timed_write", + "enforced_null_value", + "enforced_plain_write_without_tw", + "lenient_plain_write", + "lenient_null_value", + "lenient_timed_write", + "lenient_plain_write_without_tw", + ], +) +def test_timed_write_permission( + driver: AccessoryDriver, + chars: dict[str, WritableChar], + caplog: pytest.LogCaptureFixture, + enforce: bool, + name: str, + request_extra: dict[str, Any], + value: int | None, + statuses: list[int], + calls: int, + logged: int, +) -> None: + """Test a plain write to a timed write characteristic is refused or logged.""" + caplog.set_level(logging.DEBUG, logger="pyhap.accessory_driver") + char = chars[name] + driver.enforce_timed_writes = enforce + driver.prepare({HAP_REPR_TTL: 2500, HAP_REPR_PID: 7}, CLIENT) + query = { + HAP_REPR_CHARS: [ + {HAP_REPR_AID: char.aid, HAP_REPR_IID: char.iid, HAP_REPR_VALUE: value} + ], + **request_extra, + } + + response = driver.set_characteristics(query, CLIENT) + + assert _statuses(response) == statuses + assert char.setter.call_count == calls + assert caplog.text.count("requires a timed write") == logged + # Lenient mode must not warn, hubs write these values all the time. + assert _highest_level(caplog.records) < logging.WARNING + + +def test_enforce_timed_writes_defaults_to_off(driver: AccessoryDriver) -> None: + """Test timed writes are only enforced on request.""" + assert driver.enforce_timed_writes is False + enforced = AccessoryDriver(loop=driver.loop, enforce_timed_writes=True) + assert enforced.enforce_timed_writes is True