[Codex] Add websocket heartbeat and idle reconnect#10
Conversation
![chatgpt-codex-connector[bot]](https://avatars.githubusercontent.com/in/1144995?s=60&v=4){kind=small}
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 52fa2d0b1c
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
|
|
||
| // Start listening for messages and monitoring connection health. | ||
| go c.listen() | ||
| go c.heartbeat() |
There was a problem hiding this comment.
When a disconnect is followed by a reconnect before the old heartbeat ticker wakes up, the heartbeat started for the previous socket does not exit because it only checks the client-wide stateConnected flag; after Connect sets that flag for the new socket, the old goroutine continues and operates on the shared c.conn. Each fast reconnect can therefore leave another ticker sending pings and running inactivity checks against the current connection, which can accumulate duplicate heartbeats and goroutines during network flapping. Tie the heartbeat to the specific connection or cancel it when that connection's listener exits.
Useful? React with 👍 / 👎.
| const ( | ||
| defaultPingInterval = 30 * time.Second | ||
| defaultPongTimeout = 60 * time.Second | ||
| defaultInactivityTimeout = 10 * time.Minute |
There was a problem hiding this comment.
With this default, any installation that has no subscribed Home Assistant event or command result for 10 minutes will be disconnected even though the websocket heartbeat is healthy, because markDataReceived only runs for data messages while pongs only extend the read deadline. In normal low-traffic homes this creates periodic reconnect windows and can drop events that happen during resubscription unless users discover and set a negative idle timeout, so the application-idle detector should be opt-in or count healthy heartbeat traffic separately from a truly stalled socket.
Useful? React with 👍 / 👎.
dansimau
changed the title
1 participant
Motivation
Description
WebSocketPingInterval,WebSocketPongTimeout, andWebSocketIdleTimeouttoConfigand plumbed them into the hasswsClientConfigso behavior is configurable at runtime.hassws.Client: periodic ping frames, aPongHandlerand read-deadline management, plus alastDataReceivedtimestamp updated on every frame.writeMumutex and added safe shutdown handling, and added theTestReconnectsAfterWebSocketIdleTimeoutregression test to verify idle-induced reconnection and resubscription.Testing
go test ./...and all tests completed successfully.TestReconnectsAfterWebSocketIdleTimeouttest was added and included in the test run and passed.Codex Task