From 06fb8beb8a94136ba38e8f8edee5f2af053c3b9f Mon Sep 17 00:00:00 2001 From: tommasofaedo Date: Tue, 4 Aug 2026 06:51:19 +0200 Subject: [PATCH] fix(s7commplus): reconnect around symbolic reads so browse() works on RST-happy firmware MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit 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 #793, #775 --- s7commplus/client.py | 80 ++++++++++++++++++++++++++++++++++------ s7commplus/connection.py | 7 +++- 2 files changed, 74 insertions(+), 13 deletions(-) diff --git a/s7commplus/client.py b/s7commplus/client.py index 03ca1da4..9f52c39d 100644 --- a/s7commplus/client.py +++ b/s7commplus/client.py @@ -5,11 +5,12 @@ import logging import struct -from typing import Any, Optional +from typing import Any, Callable, Optional, TypeVar from . import typeinfo from .blob_decompressor import find_and_decompress from .connection import S7CommPlusConnection +from snap7.error import S7ConnectionError from .protocol import FunctionCode, Ids, ElementID, DataType, ObjectId from .vlq import encode_uint32_vlq, decode_uint32_vlq, decode_uint64_vlq from .codec import ( @@ -21,6 +22,8 @@ logger = logging.getLogger(__name__) +_T = TypeVar("_T") + class S7CommPlusClient: """S7CommPlus client for S7-1200/1500 PLCs. @@ -30,6 +33,9 @@ class S7CommPlusClient: def __init__(self) -> None: self._connection: Optional[S7CommPlusConnection] = None + # Last-used connect() arguments, kept so operations can transparently + # reconnect on firmware that RSTs the session after a symbolic read. + self._connect_params: Optional[dict[str, Any]] = None @property def connected(self) -> bool: @@ -88,24 +94,67 @@ def connect( tls_ca: Path to CA certificate for PLC verification (PEM) password: PLC password for legitimation (V2+ with TLS) """ - self._connection = S7CommPlusConnection(host=host, port=port) + self._connect_params = { + "host": host, + "port": port, + "use_tls": use_tls, + "tls_cert": tls_cert, + "tls_key": tls_key, + "tls_ca": tls_ca, + "password": password, + } + self._open_connection() + + def _open_connection(self) -> None: + """(Re)open the connection using the stored ``connect()`` arguments.""" + if self._connect_params is None: + raise RuntimeError("Not connected") + p = self._connect_params + self._connection = S7CommPlusConnection(host=p["host"], port=p["port"]) self._connection.connect( - use_tls=use_tls, - tls_cert=tls_cert, - tls_key=tls_key, - tls_ca=tls_ca, - password=password or "", + use_tls=p["use_tls"], + tls_cert=p["tls_cert"], + tls_key=p["tls_key"], + tls_ca=p["tls_ca"], + password=p["password"] or "", ) - - if password is not None and self._connection.tls_active and not self._connection.requires_substreamed: + if p["password"] is not None and self._connection.tls_active and not self._connection.requires_substreamed: logger.info("Performing PLC legitimation (password authentication)") - self._connection.authenticate(password) + self._connection.authenticate(p["password"]) + + def _reconnect(self) -> None: + """Tear down and re-establish the connection with the same parameters. + + Some firmware (e.g. S7-1200 FW V4.1) sends a TCP RST after the first + symbolic ``GetMultiVariables`` read per connection, so multi-step flows + such as :meth:`browse` need a fresh session to continue. + """ + if self._connection is not None: + try: + self._connection.disconnect() + except Exception: + pass + self._open_connection() + + def _with_reconnect(self, op: Callable[[], "_T"]) -> "_T": + """Run ``op``; if the socket was RST by the PLC, reconnect once and retry. + + Well-behaved firmware never triggers the retry (the first call succeeds); + RST-happy firmware reconnects only when a send actually fails. + """ + try: + return op() + except S7ConnectionError as exc: + logger.info("Connection dropped by PLC (%s); reconnecting and retrying", exc) + self._reconnect() + return op() def disconnect(self) -> None: """Disconnect from PLC.""" if self._connection: self._connection.disconnect() self._connection = None + self._connect_params = None def db_read(self, db_number: int, start: int, size: int) -> bytes: """Read raw bytes from a data block. @@ -485,7 +534,9 @@ def browse(self) -> list[dict[str, Any]]: for db_info in self.list_datablocks(): if db_info.get("number", 0) <= 0 or db_info.get("rid", 0) == 0: continue - ti_rid = self._read_typeinfo_rid(db_info["rid"]) + # A symbolic read may prompt a TCP RST on RST-happy firmware; retry once + # on a fresh session so the read still resolves. + ti_rid = self._with_reconnect(lambda: self._read_typeinfo_rid(db_info["rid"])) if ti_rid == 0: continue # load-memory-only DB, skip root_nodes.append( @@ -507,7 +558,9 @@ def browse(self) -> list[dict[str, Any]]: ) # Phase D: explore the OMS type-info container (a large, multi-fragment PDU). - type_objects = self._explore_type_info_container() + # The symbolic reads above may have left the socket RST on some firmware; + # reconnect and retry if so. + type_objects = self._with_reconnect(self._explore_type_info_container) # Phase E: recombine type-info with the DB/area nodes and flatten. typeinfo.build_tree(root_nodes, type_objects) @@ -534,6 +587,9 @@ def _read_typeinfo_rid(self, db_rid: int) -> int: """Read LID=1 of a DB to get its type-info RID (0 if the DB has no readable value).""" try: raw = self.read_symbolic(db_rid, [1], 0) + except S7ConnectionError: + # Socket was RST by the PLC — let the caller reconnect and retry. + raise except Exception: return 0 return struct.unpack(">I", raw[:4])[0] if len(raw) >= 4 else 0 diff --git a/s7commplus/connection.py b/s7commplus/connection.py index ecd00ee6..e12e04ce 100644 --- a/s7commplus/connection.py +++ b/s7commplus/connection.py @@ -1021,7 +1021,12 @@ def _wstring_attr(attr_id: int, s: str) -> bytes: logger.debug(f"Session created: id=0x{self._session_id:08X} ({self._session_id}), version=V{version}") if return_value != 0: - logger.warning(f"CreateObject returned error 0x{return_value:X} — PLC may require TLS (use_tls=True)") + if self._tls_active: + # Some firmware (e.g. S7-1200 FW V4.1) returns a non-zero CreateObject + # value on a perfectly usable TLS session, so this is informational only. + logger.debug(f"CreateObject returned non-zero 0x{return_value:X} on an active TLS session (session still usable)") + else: + logger.warning(f"CreateObject returned error 0x{return_value:X} — PLC may require TLS (use_tls=True)") # Parse remaining payload (the ResponseObject tree) for session attributes attrs = parse_create_object_attributes(response[offset:])