Wire delegates on characteristics and services

docs/source/api/delegates.rst

@@ -26,3 +26,40 @@ call and enforces the context's deadline - there is no sync/async
 detection. An overridden method that is not a coroutine function fails
 registration with a :class:`TypeError`, so a wrong method is caught at
 its source.
+
+Wiring a delegate
+=================
+
+Assign to the ``delegate`` of a characteristic or a service, or pass
+it through :meth:`Service.configure_char
+<pyhap.service.Service.configure_char>` - an instance, or a class that
+is then instantiated with no arguments:
+
+.. code-block:: python
+
+   class MyLock(CharacteristicDelegate):
+       async def on_set(self, value, context):
+           ...
+
+   char = service.configure_char("LockTargetState", delegate=MyLock)
+   # is the same as
+   service.get_characteristic("LockTargetState").delegate = MyLock()
+
+The assignment is when everything about the delegate is decided: which
+methods it overrides is recorded, an overridden method that is not a
+coroutine function fails with a :class:`TypeError`, and the read and
+write paths never introspect the delegate again. ``None`` or ``del``
+removes it.
+
+One instance may serve many characteristics: the context of a call
+carries :attr:`~pyhap.context.CharReadContext.characteristic` and says
+which one a call is about, e.g. one delegate per accessory rather than
+one per characteristic.
+
+**Rollout state.** The driver stores and validates delegates from this
+release on; the write and read pipelines dispatch them in the following
+releases of the callback interface refactor. Until then, a delegate is
+wired but not yet called, and the legacy
+:attr:`~pyhap.characteristic.Characteristic.setter_callback` /
+:attr:`~pyhap.characteristic.Characteristic.getter_callback` attributes
+keep working unchanged - assigning a delegate does not remove them.

pyhap/characteristic.py

@@ -21,6 +21,7 @@
     HAP_REPR_VALID_VALUES,
     HAP_REPR_VALUE,
 )
+from .delegates import CharacteristicDelegate, _characteristic_delegate
 from .util import hap_type_to_uuid, uuid_to_hap_type
 
 if TYPE_CHECKING:
@@ -184,8 +185,10 @@ class Characteristic:
         "_properties",
         "type_id",
         "_value",
-        "getter_callback",
-        "setter_callback",
+        "_getter_callback",
+        "_setter_callback",
+        "_delegate",
+        "_delegate_hooks",
         "service",
         "_uuid_str",
         "_loader_display_name",
@@ -236,6 +239,8 @@ def __init__(
         self._properties: dict[str, Any] = properties
         self.type_id = type_id
         self._value = self._get_default_value()
+        self._delegate: CharacteristicDelegate | None = None
+        self._delegate_hooks: frozenset[str] = frozenset()
         self.getter_callback: Callable[[], Any] | None = None
         # Setters may take just the value or (value, sender_client_addr); see
         # ``_setter_wants_addr``. The return value is sent back as the write
@@ -269,6 +274,78 @@ def display_name(self, value: str) -> None:
         self._display_name = value
         self._clear_cache()
 
+    @property
+    def delegate(self) -> CharacteristicDelegate | None:
+        """The characteristic's delegate, or ``None`` without one.
+
+        Assign a :class:`~pyhap.delegates.CharacteristicDelegate` instance,
+        or its class to instantiate one with no arguments. Everything about
+        a delegate is decided at this assignment: the driver records which
+        methods it overrides, and an overridden method that is not a
+        coroutine function fails here with a ``TypeError`` - there is no
+        sync/async detection later. One instance may serve many
+        characteristics; the context of a call says which one it is about.
+        """
+        return self._delegate
+
+    @delegate.setter
+    def delegate(
+        self, delegate: CharacteristicDelegate | type[CharacteristicDelegate] | None
+    ) -> None:
+        """Wire the delegate; ``None`` removes it."""
+        self._delegate, self._delegate_hooks = (
+            (None, frozenset())
+            if delegate is None
+            else _characteristic_delegate(delegate)
+        )
+
+    @delegate.deleter
+    def delegate(self) -> None:
+        """Remove the delegate."""
+        self._delegate, self._delegate_hooks = None, frozenset()
+
+    @property
+    def getter_callback(self) -> Callable[[], Any] | None:
+        """The legacy getter, called with nothing on every read of the value.
+
+        A sync callable; a slow one stalls the server. Replaced over time by
+        a delegate's ``on_get``, assigning one does not remove this.
+        """
+        return self._getter_callback
+
+    @getter_callback.setter
+    def getter_callback(self, callback: Callable[[], Any] | None) -> None:
+        """Set the legacy getter; ``None`` removes it."""
+        self._getter_callback = callback
+
+    @getter_callback.deleter
+    def getter_callback(self) -> None:
+        """Remove the legacy getter."""
+        self._getter_callback = None
+
+    @property
+    def setter_callback(self) -> Callable[..., Any] | None:
+        """The legacy setter, called with the value on a controller's write.
+
+        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. May also be a coroutine function; a
+        controller's write is answered once it is done. Replaced over time
+        by a delegate's ``on_set``, assigning one does not remove this.
+        """
+        return self._setter_callback
+
+    @setter_callback.setter
+    def setter_callback(self, callback: Callable[..., Any] | None) -> None:
+        """Set the legacy setter; ``None`` removes it."""
+        self._setter_callback = callback
+
+    @setter_callback.deleter
+    def setter_callback(self) -> None:
+        """Remove the legacy setter."""
+        self._setter_callback = None
+
     @property
     def value(self) -> Any:
         """Return the value of the characteristic."""

pyhap/delegates.py

@@ -17,13 +17,82 @@
 from __future__ import annotations
 
 from collections.abc import Mapping
-from typing import TYPE_CHECKING, Any
+import inspect
+from typing import TYPE_CHECKING, Any, TypeVar
 
 from .context import CharReadContext, CharWriteContext
 
 if TYPE_CHECKING:
     from .characteristic import Characteristic
 
+_CHARACTERISTIC_HOOKS = ("on_get", "on_set", "on_subscribe", "on_unsubscribe")
+_SERVICE_HOOKS = ("on_set",)
+_Wired = TypeVar("_Wired")
+
+
+def _wire_delegate(
+    delegate: Any, base: type[_Wired], hooks: tuple[str, ...], what: str
+) -> tuple[_Wired, frozenset[str]]:
+    """Validate a delegate at its assignment; return it with its hooks.
+
+    This is the whole of the "handled in init only" promise: the driver
+    records what a delegate overrides and that every overridden hook is a
+    coroutine function, and the read and write paths never introspect a
+    delegate again. Returns the delegate instance, a class is instantiated
+    with no arguments, and the frozenset of its overridden hook names.
+
+    :param delegate: A delegate instance, or its class.
+    :param base: The base class the delegate must derive from.
+    :param hooks: The hook methods ``base`` defines.
+    :param what: What is being wired, e.g. "a characteristic's delegate",
+        for error messages.
+    :raises TypeError: If the delegate is not of the right class, or an
+        overridden hook is not a coroutine function.
+    """
+    if isinstance(delegate, type):
+        if not issubclass(delegate, base):
+            raise TypeError(f"{delegate!r} is not a {base.__name__} subclass")
+        try:
+            delegate = delegate()
+        except TypeError as e:
+            raise TypeError(
+                f"{delegate.__name__} is instantiated with no arguments when"
+                f" it wires {what}: {e}"
+            ) from e
+    elif not isinstance(delegate, base):
+        raise TypeError(f"{delegate!r} is not a {base.__name__} instance")
+
+    overridden = frozenset(
+        hook
+        for hook in hooks
+        if getattr(type(delegate), hook) is not getattr(base, hook)
+    )
+    for hook in overridden:
+        # Async by design: the driver awaits every call, so a hook that is
+        # not a coroutine function never runs. Fail at the assignment
+        # instead, where the mistake is made.
+        if not inspect.iscoroutinefunction(getattr(delegate, hook)):
+            raise TypeError(
+                f"{type(delegate).__name__}.{hook} of {what} is not a coroutine"
+                " function; delegate methods are async by design, declare"
+                f" 'async def {hook}'"
+            )
+    return delegate, overridden
+
+
+def _characteristic_delegate(
+    delegate: Any,
+) -> tuple[CharacteristicDelegate, frozenset[str]]:
+    """Wire a characteristic's delegate; see :func:`_wire_delegate`."""
+    return _wire_delegate(
+        delegate, CharacteristicDelegate, _CHARACTERISTIC_HOOKS, "a characteristic"
+    )
+
+
+def _service_delegate(delegate: Any) -> tuple[ServiceDelegate, frozenset[str]]:
+    """Wire a service's delegate; see :func:`_wire_delegate`."""
+    return _wire_delegate(delegate, ServiceDelegate, _SERVICE_HOOKS, "a service")
+
 
 class CharacteristicDelegate:
     """Delegate of a characteristic's reads, writes and subscriptions.

pyhap/service.py

@@ -13,6 +13,7 @@
 )
 
 from .characteristic import Characteristic
+from .delegates import CharacteristicDelegate, ServiceDelegate, _service_delegate
 from .util import hap_type_to_uuid, uuid_to_hap_type
 
 if TYPE_CHECKING:
@@ -34,7 +35,9 @@ class Service:
         "type_id",
         "linked_services",
         "is_primary_service",
-        "setter_callback",
+        "_setter_callback",
+        "_delegate",
+        "_delegate_hooks",
         "unique_id",
         "_uuid_str",
     )
@@ -52,10 +55,58 @@ def __init__(
         self.display_name = display_name
         self.type_id = type_id
         self.is_primary_service = None
+        self._delegate: ServiceDelegate | None = None
+        self._delegate_hooks: frozenset[str] = frozenset()
         self.setter_callback: Callable[[Any], None] | None = None
         self.unique_id = unique_id
         self._uuid_str = uuid_to_hap_type(type_id)
 
+    @property
+    def delegate(self) -> ServiceDelegate | None:
+        """The service's delegate, or ``None`` without one.
+
+        Assign a :class:`~pyhap.delegates.ServiceDelegate` instance, or its
+        class to instantiate one with no arguments. The assignment is when
+        everything about the delegate is decided: the driver records that its
+        ``on_set`` is overridden and a coroutine function, and a method that
+        is not fails with a ``TypeError``.
+        """
+        return self._delegate
+
+    @delegate.setter
+    def delegate(
+        self, delegate: ServiceDelegate | type[ServiceDelegate] | None
+    ) -> None:
+        """Wire the delegate; ``None`` removes it."""
+        self._delegate, self._delegate_hooks = (
+            (None, frozenset()) if delegate is None else _service_delegate(delegate)
+        )
+
+    @delegate.deleter
+    def delegate(self) -> None:
+        """Remove the delegate."""
+        self._delegate, self._delegate_hooks = None, frozenset()
+
+    @property
+    def setter_callback(self) -> Callable[[Any], None] | None:
+        """The legacy setter, called with the request's values by name.
+
+        Called with a ``{display_name: value}`` dict after the
+        per-characteristic setters of a write. Replaced over time by the
+        delegate's grouped ``on_set``, assigning one does not remove this.
+        """
+        return self._setter_callback
+
+    @setter_callback.setter
+    def setter_callback(self, callback: Callable[[Any], None] | None) -> None:
+        """Set the legacy setter; ``None`` removes it."""
+        self._setter_callback = callback
+
+    @setter_callback.deleter
+    def setter_callback(self) -> None:
+        """Remove the legacy setter."""
+        self._setter_callback = None
+
     def __repr__(self) -> str:
         """Return the representation of the service."""
         chars_dict = {c.display_name: c.value for c in self.characteristics}
@@ -108,8 +159,20 @@ def configure_char(
         value: Any = None,
         setter_callback: Any = None,
         getter_callback: Any = None,
+        delegate: CharacteristicDelegate | type[CharacteristicDelegate] | None = None,
     ) -> Characteristic:
-        """Helper method to return fully configured characteristic."""
+        """Helper method to return fully configured characteristic.
+
+        :param char_name: The name of the characteristic to configure.
+        :param properties: Properties to override on the characteristic.
+        :param valid_values: Valid values to set on the characteristic.
+        :param value: The value to set on the characteristic.
+        :param setter_callback: The legacy setter of the characteristic.
+        :param getter_callback: The legacy getter of the characteristic.
+        :param delegate: The characteristic's
+            :class:`~pyhap.delegates.CharacteristicDelegate`, an instance or
+            its class - instantiated with no arguments.
+        """
         char = self.get_characteristic(char_name)
         if properties or valid_values:
             char.override_properties(properties, valid_values)
@@ -119,6 +182,8 @@ def configure_char(
             char.setter_callback = setter_callback
         if getter_callback:
             char.getter_callback = getter_callback
+        if delegate is not None:
+            char.delegate = delegate
         return char
 
     def to_HAP(self, include_value: bool = True) -> dict[str, Any]:

tests/test_accessory_driver.py

@@ -707,7 +707,8 @@ def test_write_response_returned_when_requested(
     service_setter = MagicMock()
     assert char.service is not None
     char.service.setter_callback = service_setter
-    char.setter_callback = MagicMock(return_value=returned)
+    setter = MagicMock(return_value=returned)
+    char.setter_callback = setter
 
     response = _write(driver, acc, char, **{HAP_REPR_WRITE_RESPONSE: True})
 
@@ -721,7 +722,7 @@ def test_write_response_returned_when_requested(
             }
         ]
     }
-    char.setter_callback.assert_called_once_with(0)
+    setter.assert_called_once_with(0)
     service_setter.assert_called_once_with({"Value": 0})
     assert char.value == stored
 
@@ -744,10 +745,11 @@ def test_write_response_not_returned(
 ) -> None:
     """Without a request or the wr permission the write has no response body."""
     acc, char, _ = _add_bridged_or_standalone(driver, permissions, False)
-    char.setter_callback = MagicMock(return_value=1)
+    setter = MagicMock(return_value=1)
+    char.setter_callback = setter
 
     assert _write(driver, acc, char, **query) is None
-    char.setter_callback.assert_called_once_with(0)
+    setter.assert_called_once_with(0)
     assert char.value == 0
 
 

tests/test_admin_only.py

@@ -295,7 +295,8 @@ def test_admin_only_null_write_reaches_no_setter(
     assert driver.accessory is not None
     service = driver.accessory.get_service("MotionSensor")
     assert service is not None
-    service.setter_callback = MagicMock()
+    service_setter = MagicMock()
+    service.setter_callback = service_setter
     driver.accessory.setter_callback = MagicMock()
     query = {
         HAP_REPR_CHARS: [
@@ -307,7 +308,7 @@ def test_admin_only_null_write_reaches_no_setter(
 
     assert response == _response(char, REFUSED)
     char.setter.assert_not_called()
-    service.setter_callback.assert_not_called()
+    service_setter.assert_not_called()
     driver.accessory.setter_callback.assert_not_called()