Skip to content

Commit

Permalink
EZSP startup reset corner cases (#587)
Browse files Browse the repository at this point in the history
* Handle connection reset failure during reconnect

* Close EZSP connection when startup config writing fails
  • Loading branch information
puddly authored Oct 16, 2023
1 parent bd019fb commit b22db97
Show file tree
Hide file tree
Showing 4 changed files with 8 additions and 8 deletions.
7 changes: 3 additions & 4 deletions bellows/uart.py
Original file line number Diff line number Diff line change
Expand Up @@ -213,22 +213,21 @@ def connection_lost(self, exc):
"""Port was closed unexpectedly."""

LOGGER.debug("Connection lost: %r", exc)
reason = exc or ConnectionResetError("Remote server closed connection")

# XXX: The startup reset future must be resolved with an error *before* the
# "connection done" future is completed: the secondary thread has an attached
# callback to stop itself, which will cause the a future to propagate a
# `CancelledError` into the active event loop, breaking everything!
if self._startup_reset_future:
self._startup_reset_future.set_exception(
exc or ConnectionResetError("Remote server closed connection")
)
self._startup_reset_future.set_exception(reason)

if self._connection_done_future:
self._connection_done_future.set_result(exc)
self._connection_done_future = None

if self._reset_future:
self._reset_future.cancel()
self._reset_future.set_exception(reason)
self._reset_future = None

if self._send_task:
Expand Down
5 changes: 3 additions & 2 deletions bellows/zigbee/application.py
Original file line number Diff line number Diff line change
Expand Up @@ -139,12 +139,13 @@ async def connect(self) -> None:

try:
await ezsp.startup_reset()

# Writing config is required here because network info can't be loaded
await ezsp.write_config(self.config[CONF_EZSP_CONFIG])
except Exception:
ezsp.close()
raise

# Writing config is required here because network info can't be loaded otherwise
await ezsp.write_config(self.config[CONF_EZSP_CONFIG])
self._ezsp = ezsp

async def _ensure_network_running(self) -> bool:
Expand Down
2 changes: 1 addition & 1 deletion tests/test_application.py
Original file line number Diff line number Diff line change
Expand Up @@ -1802,7 +1802,7 @@ async def test_connect_failure(
app: ControllerApplication, ezsp_mock: ezsp.EZSP
) -> None:
"""Test that a failure to connect propagates."""
ezsp_mock.startup_reset = AsyncMock(side_effect=OSError())
ezsp_mock.write_config = AsyncMock(side_effect=OSError())
ezsp_mock.connect = AsyncMock()
app._ezsp = None

Expand Down
2 changes: 1 addition & 1 deletion tests/test_uart.py
Original file line number Diff line number Diff line change
Expand Up @@ -388,7 +388,7 @@ def on_transport_close():

asyncio.get_running_loop().call_later(0.1, gw.connection_lost, ValueError())

with pytest.raises(asyncio.CancelledError):
with pytest.raises(ValueError):
await gw.reset()

# Need to close to release thread
Expand Down

0 comments on commit b22db97

Please sign in to comment.