Skip to content

SocketDevice: notice dropped connections, and don't duplicate handlers on reopen - #87

Open
pleasantone wants to merge 1 commit into
nutechsoftware:masterfrom
pleasantone:socket-disconnect-detection
Open

pleasantone wants to merge 1 commit into
nutechsoftware:masterfrom
pleasantone:socket-disconnect-detection

Conversation

@pleasantone

Copy link
Copy Markdown
Contributor

I run my AD2 behind ser2sock on a Pi, with Home Assistant connecting over the network. Every so often HA's alarm
entities just froze on their last state. Nothing went unavailable and nothing logged, and they stayed frozen until
the integration was reloaded. I'd been papering over it with a watchdog automation that reloads it when the panel
goes quiet. This PR fixes the cause.

ref: My AD2 is connected to a Vista V20P. I was unable to test any DSC device. FWIW, I believe the messages come out
at the same rate, but that's only a belief. Suffice it to say the old code was insanely fragile.

What's going on

  • SocketDevice.read_line() ignores an empty recv(). On a socket that select() says is readable, an empty read
    means the other end closed the connection. So when ser2sock restarts, the reader just spins on a dead socket.
    CommError is never raised, on_close never fires, and anything that reconnects on on_close (HA does) never
    gets the chance.
  • A link that dies without a close is worse: Wi-Fi drops, or a firewall flushing its state table. That happened to me
    after a firewall change, and ser2sock gave up on the connection 15 min later with errno 110. Nothing on a
    read-only socket ever notices that. ReadThread swallows the read timeouts forever.
  • AlarmDecoder.open() wires the device events every time it's called, and the handler lists don't de-duplicate, so
    each reconnect doubles the message handling. The failed-open path was meant to undo it, but it's
    self._unwire_events without the (). So while the device is down, every failed reconnect adds another copy of
    every handler.

Changes

  • An empty read raises CommError. The reader thread already turns that into close() and on_close.
  • TCP keepalive on the socket: 30 s idle, then 3 probes 10 s apart (best effort; options the platform doesn't have are
    skipped, and on macOS the idle time is TCP_KEEPALIVE). A dead link is noticed in about 60 s.
  • _wire_events() is idempotent, _unwire_events() tolerates handlers that aren't wired, and a failed open()
    actually unwires.

No new API, and nothing changes for a connection that's working.
The 1-byte reads behind #66 are a separate PR, so this one stays easy to review.

Testing

  • New tests: 5, in test_devices.py and test_ad2.py. All 5 fail on current master and pass with this change.
  • Full suite, run the way the repo's PR workflow runs it (Python 3.10, pip install pytest mock build,
    python -m build, pip install -e ., pytest):
    build OK, 75 pass.
  • Real sockets: I also tested this branch against a fake ser2sock (clean close, bursts and split lines,
    reconnects, refused connects). With this keepalive, cutting the link to my real ser2sock silently with a firewall
    rule was noticed 61 s after the last message on Linux and 60 s on macOS, and HA reconnected on its own once the
    link was back.

… reopen

read_line() ignored an empty recv(), so when ser2sock or the network closed
the connection the reader spun on a dead socket and on_close never fired.
Anything reconnecting on on_close (Home Assistant does) never got the
chance. An empty read now raises CommError, which the reader thread already
turns into close() and on_close.

A link that dies without a close (Wi-Fi drop, a firewall flushing its state
table) can't be seen from a read-only socket, so TCP keepalive is now on:
30 s idle, then 3 probes 10 s apart (best effort; options the platform
doesn't have are skipped, and on macOS the idle time is TCP_KEEPALIVE).
A dead link is noticed in about a minute.

AlarmDecoder.open() wired the device events again on every call, and the
handler lists don't de-duplicate, so every reconnect doubled message
handling. The failed-open path meant to undo it but was missing the ().
Wiring is now idempotent and a failed open unwires.
@pleasantone
pleasantone marked this pull request as ready for review October 21, 2026 22:13
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