Dispatch STATE_CONNECTED only on auto-reconnect#19
Merged
SteveEasley merged 2 commits intoSteveEasley:mainfrom Apr 5, 2026
Merged
Conversation
Move the STATE_CONNECTED dispatch from _connect() to _reconnect() so that the signal only fires after auto-reconnect, not during initial connect(). During initial connect, the signal is redundant since the caller already knows the connection succeeded from the await returning. Dispatching it forces consumers to distinguish initial connect from reconnect with guard flags. Update all tests that previously waited for STATE_CONNECTED during initial connect to instead assert connection.state directly. Add test_connect_does_not_dispatch_connected to explicitly verify the signal is not dispatched on initial connect.
SteveEasley
pushed a commit
that referenced
this pull request
May 6, 2026
PR #18 (Refresh device state after auto-reconnect) added two tests that wait on `connect_signal` (STATE_CONNECTED) after the initial connect. PR #19 (Dispatch STATE_CONNECTED only on auto-reconnect) then moved the STATE_CONNECTED dispatch out of `_connect()` and into `_reconnect()`. PR #19 updated other tests with the same pattern, but #19 was opened before #18 merged, so the two tests added by #18 weren't updated. Result: `await connect_signal.wait()` after initial connect waits forever, hanging CI for the full 6h job timeout. Every CI run on main since #18 has been cancelled this way (#19, #20, #21, v1.1.6, #22). Match the pattern used by other tests updated in #19: assert `state == STATE_CONNECTED` directly after the initial connect, and keep `connect_signal.wait()` only for the post-reconnect signal where STATE_CONNECTED is actually dispatched. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
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
2 participants
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.
Summary
Connection._connect()dispatchesSTATE_CONNECTEDunconditionally.Since
_connect()is called from bothConnection.connect()(initial) andConnection._reconnect()(auto-reconnect), consumers receive the signal in both cases.During initial connect, this signal is redundant — the caller already knows the connection succeeded because
await device.connect()returned without raising. The signal's value is in notifying consumers about asynchronous events they didn't initiate, which only applies to auto-reconnect.In practice, this forces consumers to distinguish initial connect from reconnect.
For example, the Unfolded Circle kaleidescape integration needs a
_connectingguard flag as a workaround because theSTATE_CONNECTEDhandler runs duringDevice.connect()while device info is still being fetched viaasyncio.gather()— causing queries against incomplete state.Suggested change
There were tests that asserted the prior behavior, so this may have been an intentional design choice. But here is a suggested change.
Move
self._dispatcher.send(const.STATE_CONNECTED)from_connect()to theelseblock of_reconnect()._connect()still setsself._state = const.STATE_CONNECTEDso all state checks (includingis_connected) work correctly.The signal now only fires for asynchronous reconnection events.
Impact on consumers
The Home Assistant kaleidescape integration registers a generic dispatcher callback that calls
async_write_ha_state()for every dispatched event — it does not check the event type or specifically handleSTATE_CONNECTED. Suppressing the signal during initial connect removes one redundant state write during setup, which is harmless.Any consumer that currently waits for
STATE_CONNECTEDduring initial connect can simply checkdevice.is_connectedorconnection.stateafterawait connect()returns, which is both simpler and more reliable.Tests
STATE_CONNECTEDduring initial connect to assertconnection.statedirectly insteadtest_connect_does_not_dispatch_connectedto explicitly verify the signal is not dispatched on initial connect