Skip to content

fix(s7commplus): reconnect around symbolic reads so browse() works on RST-happy firmware - #801

Merged
gijzelaerr merged 1 commit into
gijzelaerr:fix-tls-layering-v2from
tommasofaedo:fix/browse-reconnect-tls
Aug 4, 2026
Merged

fix(s7commplus): reconnect around symbolic reads so browse() works on RST-happy firmware#801
gijzelaerr merged 1 commit into
gijzelaerr:fix-tls-layering-v2from
tommasofaedo:fix/browse-reconnect-tls

Conversation

@tommasofaedo

Copy link
Copy Markdown
Contributor

Follow-up to the TLS validation in #793, on top of #775 (fix-tls-layering-v2).

Problem

browse() runs an EXPLORE (list_datablocks) → a symbolic GetMultiVariables read (_read_typeinfo_rid) → another EXPLORE (the type-info container). On firmware that sends a TCP RST after the first symbolic read per connection — e.g. S7-1200 FW V4.1, the behaviour already documented in read_symbolic()'s own docstring ("TCP RST after first I/Q/M read… reconnect before each I/Q/M read") — the final EXPLORE runs on a dead socket and browse() blows up:

S7ConnectionError('Not connected')
  at _explore_type_info_container -> send_request -> _tls_flush_outgoing -> _iso_conn.send_data

Fix

A lazy reconnect-and-retry:

  • connect() stores its arguments; a new _open_connection() / _reconnect() rebuild the session with them.
  • _with_reconnect(op) runs op and, only if the PLC dropped the socket (S7ConnectionError), reconnects once and retries. Well-behaved firmware never triggers the retry (the first call succeeds), so there is no behaviour change there.
  • browse() wraps the per-DB reads and the container EXPLORE with it; _read_typeinfo_rid now propagates S7ConnectionError (other errors still mean "skip DB").

Also: downgrade the misleading "CreateObject returned error … PLC may require TLS" warning to debug when TLS is already active — this PLC returns a non-zero CreateObject value on a fully functional TLS session, so the warning is noise.

Validation

Live on an S7-1200 FW V4.1 over TLS (secure PG/PC comms). Before: browse() raised Not connected. After:

IArea.input_1    BOOL    (50.A)
QArea.output_1   BOOL    (51.9)
MArea.merker_1   BOOL    (52.9)
MArea.mtag_byte  BYTE    %MB100  (52.B)
MArea.mtag_word  WORD    %MW102  (52.D)
MArea.mtag_dword DWORD   %MD104  (52.E)
MArea.mtag_bool  BOOL    (52.10)

Types and addresses cross-checked against TIA Portal.

  • pytest tests/test_client.py tests/test_tag_browser.py tests/test_client_e2e.py → 107 passed, 48 skipped
  • ruff check + ruff format clean
  • no new mypy errors from the change
  • Only validated on FW V4.1 (I don't have other RST-happy firmware to hand); the retry is a no-op on firmware that keeps the socket open.

Refs #793, #775

… RST-happy firmware

browse() does an EXPLORE (list_datablocks) then a symbolic GetMultiVariables
read (_read_typeinfo_rid) then another EXPLORE (type-info container). On
firmware that sends a TCP RST after the first symbolic read per connection
(e.g. S7-1200 FW V4.1 — the behaviour already documented in read_symbolic's
docstring), the final EXPLORE runs on a dead socket and browse() raises
S7ConnectionError("Not connected").

Add a lazy reconnect-and-retry: connect() now stores its arguments, and a new
_with_reconnect() helper retries an operation once on a fresh session if the
PLC dropped the socket. browse() wraps the per-DB reads and the container
EXPLORE with it; _read_typeinfo_rid propagates S7ConnectionError so the retry
can act. Well-behaved firmware never triggers the retry (the first call
succeeds), so there is no behaviour change there.

Also downgrade the misleading "CreateObject returned error ... PLC may require
TLS" warning to debug when TLS is already active: this PLC returns a non-zero
CreateObject value on a fully functional TLS session.

Validated live on an S7-1200 FW V4.1 over TLS: browse() now returns the full
I/Q/M symbol tree with correct types (input_1 BOOL, mtag_byte BYTE %MB100,
mtag_word WORD %MW102, ...). Tests: 107 passed, ruff + format clean.

Refs gijzelaerr#793, gijzelaerr#775
gijzelaerr added a commit that referenced this pull request Aug 4, 2026
…connect

Add `rst_after_symbolic_read` flag to S7CommPlusServer that closes the
TCP connection after responding to GetMultiVariables — matching the
behaviour of S7-1200 FW V4.1 that RSTs after a symbolic read.

This lets the reconnect logic from PR #801 be exercised without real
hardware.
@gijzelaerr
gijzelaerr merged commit 06fb8be into gijzelaerr:fix-tls-layering-v2 Aug 4, 2026
2 checks passed
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.

2 participants