Repository navigation
Await async characteristic setters before answering the write - #537
Merged
Merged
Conversation
GET /characteristics returned the stored value of every characteristic, including write-only ones such as Identify or a camera key. The HAP spec only allows reads of characteristics with the paired read (pr) permission and answers everything else with -70405. HAP-NodeJS does the same. The getter is not called for such characteristics. The /accessories database already omitted their value; this adds tests for it.
Any characteristic could be subscribed to, whether or not it has the notify (ev) permission. A controller could then receive the values that another controller wrote to a write-only characteristic. Per the HAP spec, (un)subscribing a characteristic without ev now answers -70406 and leaves the subscriptions unchanged, the same as HAP-NodeJS. A value in the same request entry is not written. Some tests subscribed to Version, which has no ev permission. They now use CurrentDoorState.
A value written to a characteristic without the pw permission still reached its setter. Answer it with -70404 like HAP-NodeJS instead, without calling the characteristic, service or accessory setters. A subscription in the same entry and the other characteristics of the request are still handled.
A write carrying a pid that was never prepared on this connection, was already used or has outlived its TTL was still applied, and its event subscriptions were changed. Only characteristics without a value got -70410. The HAP specification says such a request must be ignored and answered with -70410. Do that for every characteristic in the request, as HAP-NodeJS does. The TTL is now measured with the monotonic clock, so a wall clock jump, e.g. from NTP right after boot, can't expire a timed write.
No behaviour change. The next commit fills in the response after awaiting async setters, which needs the result building in the driver and the HTTP response filling in the handler to work on their own.
A write with "r": true got the setter's return value back for any characteristic. The HAP spec ties write responses to the write response (wr) permission, and HAP-NodeJS ignores the request for other characteristics. Do the same. When a write response is sent, it becomes the characteristic's value instead of the written one, which HAP-NodeJS does as well. The sent value is the stored one, after the usual format checks. A response the characteristic can't hold is still sent and the written value is kept. The write response tests are now parametrized and also cover a wr characteristic without paired read: neither the written value nor the response can be read back.
The HAP specification requires a plain write to a characteristic with
the timed write ("tw") permission to be refused with -70410. HAP-NodeJS
does this.
A Home hub has been seen writing HomeKit Camera Active without a timed
write, and the HomeKit Secure Video guide gives Streaming Enabled and
Motion Enabled tw as well. Other controllers may not support timed
writes at all. So the check is opt-in: with enforce_timed_writes=True
the driver refuses such a write, otherwise it applies it and only logs
at debug level.
A setter that is a coroutine function was called but never awaited. With "r": true the coroutine was passed to the JSON encoder, which raised TypeError, so the whole write failed with a 500. Writes from a controller now wait for coroutine setters before they are answered, through the existing delayed response. Accessory and service setters still run first. A setter still running after the response timeout is cancelled and its characteristic answers -70408; the other characteristics keep their results. A setter that raises answers -70402, and a closed connection cancels the setters. The return value becomes the write response and the stored value under the same rules as for sync setters. The public set_characteristics() keeps its signature. Called directly, it schedules coroutine setters and answers at once. Only coroutines are awaited; a sync setter that returns a task or future works as before.
Codecov Report❌ Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## dev #537 +/- ##
==========================================
+ Coverage 91.53% 91.71% +0.18%
==========================================
Files 27 27
Lines 4219 4335 +116
Branches 532 553 +21
==========================================
+ Hits 3862 3976 +114
- Misses 250 251 +1
- Partials 107 108 +1
🚀 New features to boost your workflow:
|
A write response of a characteristic that also has pr was stored as its value, so any paired controller could read it with GET /characteristics or /accessories. Keep it per connection instead: the connection that wrote reads its last response until it closes. Other connections read the characteristic's own value, which the write leaves as it was, also when the setter raises or returns nothing. Any new write of a connection drops its former response. Characteristics without pr keep the response as the value.
The writing connection reads an async setter's response back, other connections don't, the request stays private while the setter runs, and a response that completes as the connection closes is not kept.
Contributor
Author
|
Before I take this out of draft: in #499 you mentioned a future setter/getter API similar to |
This was referenced Nov 10, 2026
RaHehl
marked this pull request as ready for review
November 10, 2026 13:13
client_update_value takes notify=False for a write response request on a readable characteristic, so ev subscribers don't get a request that only the writer may read, e.g. a WiFi configuration with a key. HAPValue moves to pyhap.characteristic next to the formats.
A pid of another type, e.g. a list, raised a TypeError instead of answering -70410.
# Conflicts: # pyhap/characteristic.py
A write asks for a response only with "r": true.
It set a new default executor on the shared test loop and shut it down when the driver stopped, so later tests on the same xdist worker failed with "cannot schedule new futures after shutdown". The driver now makes its own loop and executor, as a sync start does.
Owner
Leave it as is for now, I don't have an action plan yet. Can you resolve the conflicts? @RaHehl |
# Conflicts: # pyhap/accessory_driver.py # tests/test_accessory_driver.py
The write only accepts an integer pid, so a prepare with another pid could never be used. A ttl too large for a float is refused too, and an entry expires at the same moment for prepare and write. Tests cover a rejected timed write with several characteristics and an explicit null value on a tw characteristic.
A bool pid, a float ttl and the last moment of a TTL are covered now.
# Conflicts: # pyhap/accessory_driver.py
Contributor
Author
|
Sounds good, I'll leave it as is. The conflicts are resolved, here and in the other open PRs. |
A coroutine setter that fails or times out no longer sends an event for the value it did not set, the same as a sync setter that raises. A setter that runs on after its cancellation has its error retrieved and logged at debug level. Any truthy "r" asks for a write response again.
The driver asks for the deferred notify with defer_notify and finishes the update with the public client_update_finished. A failing coroutine setter of a direct set_characteristics call is logged with its name, and setters done before a write is cancelled still tell subscribers.
This was referenced Nov 10, 2026
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Builds on #536.
A setter defined with
async defwas called but never awaited. With"r": truethe JSON encoder raised on the coroutine and the write got a 500. The WebRTC and Camera Client CSR characteristics from the HomeKit Secure Video guide (#531) answer in the write response with data from async work, so they need this.Writes from a controller now wait for coroutine setters through the existing delayed response. Accessory and service setters still run first and must stay sync. A coroutine they return is closed with a warning. A setter that is still running after the timeout is cancelled and answers -70408, the rest of the request keeps its results. A setter that raises answers -70402, and a closed connection cancels the setters. The public
set_characteristics()keeps its signature: called directly it schedules coroutine setters and answers at once. A sync setter that returns a task or future works as before. Controller writes now go through an internal method, so a subclass that overridesset_characteristicsno longer sees them.Subscribers only get the event for an async write once its setter succeeded, like for a sync setter. For that the driver calls
client_update_value(..., defer_notify=True)and then the newclient_update_finished(). Withoutdefer_notify,client_update_valuenotifies at once as before.The first commit only moves the result building into helpers.
AI use: written with the help of an AI coding assistant (Opus 5.5). I reviewed and tested the changes myself (pytest, ruff, ty).