Skip to content

Commit

Permalink
Handle cancellation while waiting for an encrypted IP response (#354)
Browse files Browse the repository at this point in the history
  • Loading branch information
bdraco authored Jan 13, 2024
1 parent 6e8e374 commit 4cd3ccc
Show file tree
Hide file tree
Showing 2 changed files with 39 additions and 5 deletions.
15 changes: 10 additions & 5 deletions aiohomekit/controller/ip/connection.py
Original file line number Diff line number Diff line change
Expand Up @@ -103,8 +103,6 @@ async def send_bytes(self, payload: bytes) -> HttpResponse:
# queued writes can happy.
raise AccessoryDisconnectedError("Transport is closed")

self.transport.write(payload)

# We return a future so that our caller can block on a reply
# We can send many requests and dispatch the results in order
# Should mean we don't need locking around request/reply cycles
Expand All @@ -114,12 +112,19 @@ async def send_bytes(self, payload: bytes) -> HttpResponse:
timeout_handle = loop.call_at(loop.time() + 30, self._handle_timeout, result)
timeout_expired = False
try:
self.transport.write(payload)
return await result
except asyncio.TimeoutError:
timeout_expired = True
except (asyncio.TimeoutError, BaseException) as ex:
# If we get a timeout or any other exception then we need to
# close the connection as we are now out of sync with the device
# and any future requests will fail since the encryption counters
# will be out of sync.
self.transport.write_eof()
self.transport.close()
raise AccessoryDisconnectedError("Timeout while waiting for response")
if isinstance(ex, asyncio.TimeoutError):
timeout_expired = True
raise AccessoryDisconnectedError("Timeout while waiting for response")
raise
finally:
if not timeout_expired:
timeout_handle.cancel()
Expand Down
29 changes: 29 additions & 0 deletions tests/test_ip_pairing.py
Original file line number Diff line number Diff line change
Expand Up @@ -6,6 +6,7 @@
import pytest

from aiohomekit.controller.ip.pairing import IpPairing
from aiohomekit.exceptions import AccessoryDisconnectedError
from aiohomekit.model import Transport
from aiohomekit.protocol.statuscodes import HapStatusCode

Expand Down Expand Up @@ -127,6 +128,34 @@ async def test_put_characteristics(pairing: IpPairing):
assert characteristics[(1, 9)] == {"value": True}


async def test_put_characteristics_cancelled(pairing: IpPairing):
characteristics = await pairing.put_characteristics([(1, 9, True)])
characteristics = await pairing.get_characteristics([(1, 9)])

with mock.patch.object(pairing.connection.transport, "write"):
task = asyncio.create_task(pairing.put_characteristics([(1, 9, False)]))
await asyncio.sleep(0)
for future in pairing.connection.protocol.result_cbs:
future.cancel()
await asyncio.sleep(0)
with pytest.raises(asyncio.CancelledError):
await task

# We should wait a few seconds to see if the
# connection can be re-established and the write can be
# completed. But this is not currently possible because
# we do not wait for the connection to be re-established
# before we try to write the data. When we implement
# reconnection we should remove this pytest.raises
# and the sleep below.
with pytest.raises(AccessoryDisconnectedError):
await pairing.get_characteristics([(1, 9)])

await asyncio.sleep(0)
characteristics = await pairing.get_characteristics([(1, 9)])
assert characteristics[(1, 9)] == {"value": True}


async def test_put_characteristics_callbacks(pairing: IpPairing):
events = []

Expand Down

0 comments on commit 4cd3ccc

Please sign in to comment.