Skip to content

feat: connection-state callbacks for availability tracking - #18

Draft
bluetoothbot wants to merge 4 commits into
hvaclibs:mainfrom
bluetoothbot:koan/connection-state-callbacks
Draft

feat: connection-state callbacks for availability tracking#18
bluetoothbot wants to merge 4 commits into
hvaclibs:mainfrom
bluetoothbot:koan/connection-state-callbacks

Conversation

@bluetoothbot

@bluetoothbot bluetoothbot commented Jun 21, 2026

Copy link
Copy Markdown
Contributor

What

Add connection-state (availability) callbacks to ThermostatConnection.

Why

The auto-reconnect loop handled connection drops and restorations silently. Consumers — notably a Home Assistant integration — had no way to know when the thermostat went offline or came back, so entities couldn't reflect availability. Event callbacks only fire on incoming messages, never on lifecycle transitions.

How

  • New add_connection_callback(fn) registers fn(available: bool), returning an unregister callable (mirrors add_event_callback).
  • New available property: True when connected and authenticated.
  • Notifications fire on transitions only (deduped): True after login() succeeds (including post-reconnect logins), False on connection loss and explicit disconnect().
  • Callback exceptions are caught and logged, never propagated — same contract as event callbacks.

Testing

  • 8 new tests (transition firing, dedup, removal/idempotency, exception isolation, login→available, disconnect→unavailable).
  • Full suite: 156 passed, ruff clean.

Quality Report

Changes: 3 files changed, 237 insertions(+), 12 deletions(-)

Code scan: 1 issue(s) found

  • README.md:141 — debug print statement

Tests: failed (FAILED)

Branch hygiene: clean

Generated by Kōan

bluetoothbot and others added 2 commits July 26, 2026 01:24
The auto-reconnect loop silently handled drops and restorations, leaving
consumers (e.g. a Home Assistant integration) with no way to observe
device availability. Add add_connection_callback(fn) and an available
property: fires True after login (including auto-reconnect), False on
connection loss, deduped to transitions only.
@bluetoothbot
bluetoothbot force-pushed the koan/connection-state-callbacks branch from 979c915 to 73f4d2e Compare July 26, 2026 01:25
@codecov

codecov Bot commented Jul 26, 2026

Copy link
Copy Markdown

Codecov Report

✅ All modified and coverable lines are covered by tests.

📢 Thoughts on this report? Let us know!

@bluetoothbot

Copy link
Copy Markdown
Contributor Author

Correction to this PR's own description: the auto-generated Quality Report footer says Tests: failed (FAILED). That is wrong — it is a stale Kōan artifact, not a real result.

Re-verified today:

The same false footer is on all six open Kōan PRs here. Ignore it when reviewing.

_close_transport() is the reconnect loop's teardown path and does not go
through _on_connection_lost(), so availability stayed latched True while
the connection was gone — including forever if reconnects keep failing.
A login can also win a race against an already-fired _on_connection_lost
and re-latch True on a dead transport.

Notify unavailability from _close_transport() itself, which every
teardown path (run loop, failed reconnect, disconnect) funnels through.
The disconnect() notification is now redundant and removed.
@bluetoothbot

Copy link
Copy Markdown
Contributor Author

Pushed f314050 — a correctness fix found while reviewing this branch against the rest of the open queue.

Defect: available could latch True while the connection was gone.

_close_transport() is the reconnect loop's teardown path and never goes through _on_connection_lost(), so nothing cleared _available there. Two reachable paths:

  1. Run-loop teardown / failed reconnect closes the transport → connected goes False, available stays True. If reconnects keep failing (backoff to RECONNECT_MAX), consumers see available == True indefinitely on a dead thermostat.
  2. A drop racing an in-flight login: _on_connection_lost fires while _available is still False (no-op, dedupe guard), then login() returns and latches True on a dead transport.

Fix: notify unavailability from _close_transport() itself — every teardown path (run loop, failed reconnect, disconnect()) funnels through it. The now-redundant call in disconnect() is removed.

Tests: two regression tests added, both failing before the fix. Branch: 162 pass, connection.py 99%. Integrated against the other five open PRs (#20/#21/#25/#27/#28): zero conflicts, 182 pass, 99%.

A TLS transport.close() completes asynchronously — the peer's close_notify
(or the shutdown timeout) can land seconds later, well after the reconnect
loop established a replacement connection. The detached protocol still held
a reference to the connection, so its late connection_lost() cleared
_connected, fired the availability callbacks with False, and set
_connection_lost_event on a healthy connection, tearing it down and
restarting the backoff loop.

The protocol now drops the callback once close() has detached its transport.
@bluetoothbot

Copy link
Copy Markdown
Contributor Author

Pushed a second fix to this branch (44944c3), found by re-reviewing the integrated queue for state-transition holes.

Defect: a superseded transport could clobber a healthy connection's state.

asyncio's TLS transport.close() completes asynchronously — the shutdown handshake waits for the peer's close_notify or the shutdown timeout, so connection_lost() can fire seconds after close() returned. The detached ThermostatProtocol still held a reference to the ThermostatConnection, so by then the reconnect loop had already built a replacement connection and that late callback ran against it:

  • _connected = False on a live socket
  • _notify_connection_state(False) → availability callbacks fire False on a working connection
  • _connection_lost_event.set()_run_loop immediately tears the new connection down and re-enters backoff

Worst case is a reconnect loop that keeps killing its own successful reconnects, with available flapping. The window is exactly the one the reconnect path creates: _close_transport()sleep(RECONNECT_DELAY)connect(), where RECONNECT_DELAY is far shorter than an SSL shutdown against an unresponsive peer.

Fix: ThermostatProtocol.connection_lost() drops the callback once close() has detached its transport — no new state, the existing _transport is None already marks "superseded".

Testing: new regression test test_stale_connection_lost_ignored (fails before, passes after). Full suite 163 pass / 99% on this branch; 183 pass / 99% on the integrated tree with all six open koan PRs merged.

Note: the auto-generated Quality Report block in the PR body above reads Tests: failed — that is a Kōan-side generator bug, not reality. Live check-runs on this head are green.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant