Skip to content

Commit 154d678

Browse files
gijzelaerrclaude
andauthored
Backport robustness improvements from python-s7comm (#580)
Backports concrete robustness improvements from nikteliy/python-s7comm to the native Python S7 implementation: - 210+ S7 protocol error codes from Wireshark's S7 dissector covering USERDATA parameter errors (0xD0xx), protocol errors (0x8xxx), and resource errors. parse_response() now raises descriptive S7ProtocolError on header errors, and USERDATA response parsing logs warnings. - Stale packet detection with retry: new validate_pdu_reference() validates response sequence numbers. New _send_receive() helper wraps the send/receive/parse pattern with automatic retry on stale packets (up to 3 retries). Refactored ~15 call sites in client.py to use it. - Automatic PDU splitting for read_area()/write_area(): requests exceeding the negotiated PDU length are automatically chunked, preventing PLC rejections on large reads (e.g. db_get() on big DBs). - TPDUSize enum (ISO 8073) with configurable COTP negotiation: replaces hardcoded 0x0A with a proper IntEnum (128-8192 bytes). - MAX_VARS=20 limit on read_multi_vars()/write_multi_vars(): raises ValueError when exceeding the S7 protocol limit. Co-authored-by: Claude Opus 4.6 <noreply@anthropic.com>
1 parent c246947 commit 154d678

8 files changed

Lines changed: 815 additions & 254 deletions

File tree

File renamed without changes.

snap7/client.py

Lines changed: 142 additions & 216 deletions
Large diffs are not rendered by default.

snap7/connection.py

Lines changed: 29 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -8,11 +8,28 @@
88
import socket
99
import struct
1010
import logging
11+
from enum import IntEnum
1112
from typing import Optional, Type
1213
from types import TracebackType
1314

1415
from .error import S7ConnectionError, S7TimeoutError
1516

17+
18+
class TPDUSize(IntEnum):
19+
"""TPDU sizes per ISO 8073 / RFC 905.
20+
21+
The value is the exponent: actual size = 2^value bytes.
22+
"""
23+
24+
S_128 = 0x07
25+
S_256 = 0x08
26+
S_512 = 0x09
27+
S_1024 = 0x0A
28+
S_2048 = 0x0B
29+
S_4096 = 0x0C
30+
S_8192 = 0x0D
31+
32+
1633
logger = logging.getLogger(__name__)
1734

1835

@@ -44,7 +61,14 @@ class ISOTCPConnection:
4461
COTP_PARAM_CALLING_TSAP = 0xC1
4562
COTP_PARAM_CALLED_TSAP = 0xC2
4663

47-
def __init__(self, host: str, port: int = 102, local_tsap: int = 0x0100, remote_tsap: int = 0x0102):
64+
def __init__(
65+
self,
66+
host: str,
67+
port: int = 102,
68+
local_tsap: int = 0x0100,
69+
remote_tsap: int = 0x0102,
70+
tpdu_size: TPDUSize = TPDUSize.S_1024,
71+
):
4872
"""
4973
Initialize ISO TCP connection.
5074
@@ -53,11 +77,13 @@ def __init__(self, host: str, port: int = 102, local_tsap: int = 0x0100, remote_
5377
port: TCP port (default 102 for S7)
5478
local_tsap: Local Transport Service Access Point
5579
remote_tsap: Remote Transport Service Access Point
80+
tpdu_size: TPDU size to request during COTP negotiation
5681
"""
5782
self.host = host
5883
self.port = port
5984
self.local_tsap = local_tsap
6085
self.remote_tsap = remote_tsap
86+
self.tpdu_size = tpdu_size
6187
self.socket: Optional[socket.socket] = None
6288
self.connected = False
6389
self.pdu_size = 240 # Default PDU size, negotiated during connection
@@ -241,8 +267,8 @@ def _build_cotp_cr(self) -> bytes:
241267
calling_tsap = struct.pack(">BBH", self.COTP_PARAM_CALLING_TSAP, tsap_length, self.local_tsap)
242268
# Called TSAP (remote)
243269
called_tsap = struct.pack(">BBH", self.COTP_PARAM_CALLED_TSAP, tsap_length, self.remote_tsap)
244-
# PDU Size parameter (ISO 8073 code: 0x0A = 1024 bytes)
245-
pdu_size_param = struct.pack(">BBB", self.COTP_PARAM_PDU_SIZE, 1, 0x0A)
270+
# PDU Size parameter (ISO 8073 code, e.g. 0x0A = 1024 bytes)
271+
pdu_size_param = struct.pack(">BBB", self.COTP_PARAM_PDU_SIZE, 1, self.tpdu_size)
246272

247273
parameters = calling_tsap + called_tsap + pdu_size_param
248274

snap7/datatypes.py

Lines changed: 22 additions & 21 deletions
Original file line numberDiff line numberDiff line change
@@ -14,6 +14,12 @@ def _assert_never(value: NoReturn) -> NoReturn:
1414
raise AssertionError(f"Unhandled value: {value!r}")
1515

1616

17+
def _validate_bit_addr(bit_addr: int) -> None:
18+
"""Validate that a bit address is in the valid range 0-7."""
19+
if not 0 <= bit_addr <= 7:
20+
raise ValueError(f"Bit address must be 0-7, got {bit_addr}")
21+
22+
1723
class S7Area(IntEnum):
1824
"""S7 memory area identifiers."""
1925

@@ -123,12 +129,12 @@ def decode_s7_data(data: bytes, word_len: S7WordLen, count: int) -> List[Union[b
123129
values.append(bool(byte_val))
124130
offset += 1
125131

126-
elif word_len in [S7WordLen.BYTE, S7WordLen.CHAR]:
132+
elif word_len == S7WordLen.BYTE or word_len == S7WordLen.CHAR:
127133
# 8-bit values
128134
values.append(data[offset])
129135
offset += 1
130136

131-
elif word_len in [S7WordLen.WORD, S7WordLen.COUNTER, S7WordLen.TIMER]:
137+
elif word_len == S7WordLen.WORD or word_len == S7WordLen.COUNTER or word_len == S7WordLen.TIMER:
132138
# 16-bit unsigned values (big-endian)
133139
value = struct.unpack(">H", data[offset : offset + 2])[0]
134140
values.append(value)
@@ -158,6 +164,9 @@ def decode_s7_data(data: bytes, word_len: S7WordLen, count: int) -> List[Union[b
158164
values.append(value)
159165
offset += 4
160166

167+
else:
168+
_assert_never(word_len)
169+
161170
return values
162171

163172
@staticmethod
@@ -174,11 +183,11 @@ def encode_s7_data(values: Sequence[Union[bool, int, float]], word_len: S7WordLe
174183
# Single bit to byte
175184
data.append(0x01 if value else 0x00)
176185

177-
elif word_len in [S7WordLen.BYTE, S7WordLen.CHAR]:
186+
elif word_len == S7WordLen.BYTE or word_len == S7WordLen.CHAR:
178187
# 8-bit values
179188
data.append(int(value) & 0xFF)
180189

181-
elif word_len in [S7WordLen.WORD, S7WordLen.COUNTER, S7WordLen.TIMER]:
190+
elif word_len == S7WordLen.WORD or word_len == S7WordLen.COUNTER or word_len == S7WordLen.TIMER:
182191
# 16-bit unsigned values (big-endian)
183192
data.extend(struct.pack(">H", int(value) & 0xFFFF))
184193

@@ -199,7 +208,7 @@ def encode_s7_data(values: Sequence[Union[bool, int, float]], word_len: S7WordLe
199208
data.extend(struct.pack(">f", float(value)))
200209

201210
else:
202-
_assert_never(word_len) # type: ignore[arg-type]
211+
_assert_never(word_len)
203212

204213
return bytes(data)
205214

@@ -224,10 +233,8 @@ def parse_address(address_str: str) -> Tuple[S7Area, int, int]:
224233
# Bit address: DBX10.5
225234
if "." in addr_part:
226235
byte_addr, bit_addr = addr_part[3:].split(".")
227-
bit_val = int(bit_addr)
228-
if not 0 <= bit_val <= 7:
229-
raise ValueError(f"Bit address must be 0-7, got {bit_val}")
230-
offset = int(byte_addr) * 8 + bit_val
236+
_validate_bit_addr(int(bit_addr))
237+
offset = int(byte_addr) * 8 + int(bit_addr)
231238
else:
232239
offset = int(addr_part[3:]) * 8
233240
elif addr_part.startswith("DBB"):
@@ -249,10 +256,8 @@ def parse_address(address_str: str) -> Tuple[S7Area, int, int]:
249256
if "." in address_str:
250257
# Bit address: M10.5
251258
byte_addr, bit_addr = address_str[1:].split(".")
252-
bit_val = int(bit_addr)
253-
if not 0 <= bit_val <= 7:
254-
raise ValueError(f"Bit address must be 0-7, got {bit_val}")
255-
offset = int(byte_addr) * 8 + bit_val
259+
_validate_bit_addr(int(bit_addr))
260+
offset = int(byte_addr) * 8 + int(bit_addr)
256261
elif address_str.startswith("MW"):
257262
# Word address: MW20
258263
offset = int(address_str[2:])
@@ -270,10 +275,8 @@ def parse_address(address_str: str) -> Tuple[S7Area, int, int]:
270275
if "." in address_str:
271276
# Bit address: I0.0
272277
byte_addr, bit_addr = address_str[1:].split(".")
273-
bit_val = int(bit_addr)
274-
if not 0 <= bit_val <= 7:
275-
raise ValueError(f"Bit address must be 0-7, got {bit_val}")
276-
offset = int(byte_addr) * 8 + bit_val
278+
_validate_bit_addr(int(bit_addr))
279+
offset = int(byte_addr) * 8 + int(bit_addr)
277280
elif address_str.startswith("IW"):
278281
# Word address: IW10
279282
offset = int(address_str[2:])
@@ -291,10 +294,8 @@ def parse_address(address_str: str) -> Tuple[S7Area, int, int]:
291294
if "." in address_str:
292295
# Bit address: Q0.0
293296
byte_addr, bit_addr = address_str[1:].split(".")
294-
bit_val = int(bit_addr)
295-
if not 0 <= bit_val <= 7:
296-
raise ValueError(f"Bit address must be 0-7, got {bit_val}")
297-
offset = int(byte_addr) * 8 + bit_val
297+
_validate_bit_addr(int(bit_addr))
298+
offset = int(byte_addr) * 8 + int(bit_addr)
298299
elif address_str.startswith("QW"):
299300
# Word address: QW10
300301
offset = int(address_str[2:])

0 commit comments

Comments
 (0)