Propagate unexpected API disconnects to the ESPHome async transport - #113
Closed
balloobbot wants to merge 3 commits into
Closed
Propagate unexpected API disconnects to the ESPHome async transport#113balloobbot wants to merge 3 commits into
balloobbot wants to merge 3 commits into
Conversation
The ESPHome transport connected without an `on_stop` callback, so when the device restarted (or the API connection dropped for any other reason) the transport was never told: `connection_lost()` was never dispatched, readers blocked forever, and writes were silently discarded inside the client loop. Consumers saw a healthy port that simply never produced data again. Wire `APIClient.connect(on_stop=...)` for connections the transport owns and translate a stop into `connection_lost()`: a clean device-initiated disconnect surfaces as EOF and an unexpected drop as a `SerialException`, matching how the descriptor transport reports fatal errors. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
The callback is now a constructor argument passed straight through to `APIClient.connect` (which accepts None), and the transport handler runs directly as the on_stop coroutine: it is only wired for owned API connections, which are created on the transport's loop, so no cross-loop dispatch is needed. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
Owner
|
I believe #112 handles the same problem but more directly. |
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.
Problem
ESPHomeSerialTransportconnects withAPIClient.connect(login=True)— noon_stopcallback — and_call_protocol_connection_lost()is only ever dispatched from a localclose(). When the ESPHome device restarts (or the API connection drops for any other reason), aioesphomeapi cleans up its connection internally and nobody tells the transport:protocol.connection_lost()never fires, so aStreamReaderblocks inread()forever.serial_proxy_writeis scheduled withcall_soon_threadsafe, and theAPIConnectionErrorit raises inside the client loop is swallowed by the loop exception handler.Consumers see a healthy port that never produces data again. For request/response protocols on top (e.g. the
samsung-exlink/lg-rs232-tvHome Assistant integrations), every command just times out — indistinguishable from a device in standby — and their reconnect logic never triggers, even after the proxy comes back.Fix
Pass
on_stoptoAPIClient.connect()for connections the transport owns, and translate a stop intoconnection_lost()on the transport's loop (marshalled cross-loop the same way as_on_data):connection_lost(SerialException(...)), and the serial is marked broken — matching how the descriptor transport reports fatal errors;DisconnectRequestbefore a reboot, e.g. OTA) →connection_lost(None), i.e. EOF.The handler is a no-op when
close()already ran (_closingguard), andclose()after a remote stop still works as before. Externally-passedAPIClientinstances are untouched: their owner calledconnect()and owns the lifecycle.The sync
ESPHomeSerialAPI is intentionally unchanged (the hook defaults to doing nothing there).Testing
New test
test_daemon_death_propagates_connection_lostkills the host daemon mid-connection and asserts the reader unblocks (EOF orSerialException) and the port reports closed. It times out after 10s of a blocked reader without the fix, and passes with it. Existing ESPHome transport tests (18) pass;prek(ruff, mypy, pylint, etc.) is clean.🤖 Generated with Claude Code