From 46b59dcf026fd1e0cac06bf4424aafb541bbd44a Mon Sep 17 00:00:00 2001 From: Gijs Molenaar Date: Wed, 29 Jul 2026 13:05:40 +0200 Subject: [PATCH 1/2] fix: auto-reconnect not triggered on multi-block read path (#785) When read_tags() batched multiple variables into multi-block packets, the optimized execution path (_execute_packets_sequential and _execute_packets_parallel) called _send_receive() directly, bypassing _send_receive_with_reconnect(). This meant auto_reconnect only worked for the single-block path (few tags) but silently failed for multi-block reads (many tags in production). Fix _execute_packets_sequential to rebuild requests via _send_receive_with_reconnect so sequence counters are refreshed after reconnection. Wrap _execute_packets_parallel with reconnect logic that falls back to sequential execution after re-establishing the connection. Closes #785 Co-Authored-By: Claude Opus 4.6 (1M context) --- snap7/client.py | 24 ++++++++++++++++++++---- 1 file changed, 20 insertions(+), 4 deletions(-) diff --git a/snap7/client.py b/snap7/client.py index 7cc5ee32..c6b25956 100644 --- a/snap7/client.py +++ b/snap7/client.py @@ -1292,8 +1292,13 @@ def _read_multi_vars_optimized(self, dict_items: List[dict[str, Any]]) -> Tuple[ def _execute_packets_sequential(self, packet_requests: list[Tuple[int, bytes, ReadPacket]]) -> None: """Execute multi-block packets one at a time.""" - for _, request, packet in packet_requests: - response = self._send_receive(request) + for _, _request, packet in packet_requests: + block_specs = [(blk.area, blk.db_number, blk.start_offset, blk.byte_length) for blk in packet.blocks] + + def build_request(specs: list[tuple[int, int, int, int]] = block_specs) -> bytes: + return self.protocol.build_multi_read_request(specs) + + response = self._send_receive_with_reconnect(build_request) block_data_list = self.protocol.extract_multi_read_data(response, len(packet.blocks)) for blk, buf in zip(packet.blocks, block_data_list): blk.buffer = buf @@ -1302,9 +1307,20 @@ def _execute_packets_parallel(self, packet_requests: list[Tuple[int, bytes, Read """Execute multi-block packets using parallel dispatch. Sends up to *max_parallel* PDUs back-to-back before reading - responses, reducing round-trip overhead. + responses, reducing round-trip overhead. Falls back to + sequential reconnect-aware execution on connection loss. """ - # Process in chunks of max_parallel + try: + self._execute_packets_parallel_inner(packet_requests) + except (S7ConnectionError, OSError) as e: + if not self._auto_reconnect: + raise + logger.warning(f"Connection lost during parallel read: {e}") + self._do_reconnect() + self._execute_packets_sequential(packet_requests) + + def _execute_packets_parallel_inner(self, packet_requests: list[Tuple[int, bytes, ReadPacket]]) -> None: + """Inner parallel dispatch without reconnect handling.""" for chunk_start in range(0, len(packet_requests), self.max_parallel): chunk = packet_requests[chunk_start : chunk_start + self.max_parallel] requests = [(pkt_idx, pdu) for pkt_idx, pdu, _ in chunk] From 45d62393574f99b8408f3309793dc6f1aeb5e9eb Mon Sep 17 00:00:00 2001 From: Gijs Molenaar Date: Wed, 29 Jul 2026 15:36:24 +0200 Subject: [PATCH 2/2] test: add multi-block reconnect test to cover sequential reconnect path Adds test_reconnect_on_multi_block_read that exercises the _execute_packets_sequential reconnect wrapper with 20 INT tags, fixing coverage below the 75% threshold. Co-Authored-By: Claude Opus 4.6 (1M context) --- tests/test_reconnect.py | 23 ++++++++++++++++++++++- 1 file changed, 22 insertions(+), 1 deletion(-) diff --git a/tests/test_reconnect.py b/tests/test_reconnect.py index f36293e0..c61f3f08 100644 --- a/tests/test_reconnect.py +++ b/tests/test_reconnect.py @@ -10,7 +10,8 @@ from snap7.client import Client from snap7.error import S7ConnectionError from snap7.server import Server -from snap7.type import SrvArea +from snap7.tags import Tag +from snap7.type import Area, SrvArea logging.basicConfig(level=logging.WARNING) @@ -152,6 +153,26 @@ def test_reconnect_on_write_failure(self) -> None: finally: client.disconnect() + def test_reconnect_on_multi_block_read(self) -> None: + """Client reconnects when connection drops during multi-block sequential read.""" + client = Client(auto_reconnect=True, max_retries=3, retry_delay=0.1) + client.connect(ip, rack, slot, tcpport) + + try: + tags = [Tag(Area.DB, db_number, i * 2, "INT") for i in range(20)] + data = client.read_tags(tags) + assert len(data) == 20 + + if client.connection and client.connection.socket: + client.connection.socket.close() + client.connected = False + + data = client.read_tags(tags) + assert len(data) == 20 + assert client.connected is True + finally: + client.disconnect() + def test_no_reconnect_when_disabled(self) -> None: """Without auto_reconnect, connection errors propagate immediately.""" client = Client(auto_reconnect=False)