From a1be61ac0c7518d23e2ec6b8dda2fcbd5168909f Mon Sep 17 00:00:00 2001 From: Gijs Molenaar Date: Wed, 11 Nov 2026 14:57:10 +0200 Subject: [PATCH] Read the real CPU state from SZL 0x0424 get_cpu_state() sent a read request whose parameter section was one byte long and returned "S7CpuStatusRun" without looking at the reply. It only appeared to work because the server answered a read request that has no address with the first four bytes of DB1. Both clients now read SZL 0x0424 index 0 through read_szl and decode it with snap7.szl.parse_cpu_state_szl. The payload is the partial-list header (LENTHDR = 0x0014, N_DR = 1) followed by one 20-byte record: ereig(2) ae(1) bzu-id(1) res(4) anlinfo1..4(4) time(8) bzu-id holds the requested mode in bits 0-3 and the previous mode in bits 4-7: 1-4 are STOP variants, 5 and 7 startup, 8 RUN, 0xA HOLD, 0xD DEFECT. Sources for the layout: - Wireshark, epan/dissectors/packet-s7comm_szl_ids.c, s7comm_decode_szl_id_0424_idx_0000 and szl_0424_0000_bzu_id_names. - Snap7 (github.com/SCADACS/snap7), src/core/s7_micro_client.cpp: opGetPlcStatus reads opData[7], the same byte after the 4-byte partial-list header. In src/core/s7_server.cpp, SZL_ID424 writes the CPU status at offset 15 of the reply data (8 bytes of data and SZL header, 4 bytes of partial-list header, record offset 3). The Siemens reference manual "System Software for S7-300/400 System and Standard Functions" (05/2010, A5E02789976-01) has no section for SSL-ID W#16#xy24, so it could not be used. The two sources read the byte differently. Snap7 compares the whole byte with 0x00, 0x04 and 0x08 and reports anything else as STOP. This change reports a zero byte as Unknown, a requested-mode nibble of 8 as Run and everything else as Stop. That agrees with Snap7 for 0x00, 0x03, 0x04 and 0x08, and differs only when RUN comes with a non-zero previous mode, which Snap7 would report as Stop. Not tested on hardware. The record layout and the mapping come from the sources above and from the server in this repository only. Also: - The server answers SZL 0x0424 from its CPU state and lists the ID in SZL 0x0000. - The server refuses a read request without an address specification instead of falling back to DB1, offset 0, 4 bytes. - The heartbeat of the synchronous client treats an answered refusal as a live connection. A CPU that rejects SZL 0x0424 would otherwise be reconnected on every interval. Transport failures and lost or stale replies still mark the connection dead. - S7Protocol.build_cpu_state_request and extract_cpu_state are kept and documented as deprecated. --- CHANGES.md | 12 ++++++ snap7/async_client.py | 16 +++++--- snap7/client.py | 35 ++++++++++++----- snap7/s7protocol.py | 28 ++++++++------ snap7/server/__init__.py | 18 +++++++-- snap7/szl.py | 36 ++++++++++++++++++ tests/test_async_client.py | 10 +++-- tests/test_server.py | 77 +++++++++++++++++++++++++++++++++++--- 8 files changed, 195 insertions(+), 37 deletions(-) diff --git a/CHANGES.md b/CHANGES.md index bac5322e..11945ee4 100644 --- a/CHANGES.md +++ b/CHANGES.md @@ -1,6 +1,18 @@ CHANGES ======== +Unreleased +---------- + +### Bug fixes + +* `get_cpu_state()` now reads the CPU's operating state from SZL 0x0424, in both clients. Previously it sent a read + request with no address and returned `"S7CpuStatusRun"` whatever the PLC answered. Startup, HOLD and DEFECT are + reported as `"S7CpuStatusStop"`, as native Snap7 does. A PLC that refuses SZL 0x0424 now raises `S7ProtocolError`; + the heartbeat of the synchronous client counts such a refusal as a live connection. Not yet tested on hardware. +* The server answers SZL 0x0424 from its CPU state, and refuses a read request that has no address specification. + Previously it answered such a request with the first four bytes of DB1. + 3.4.0 ----- diff --git a/snap7/async_client.py b/snap7/async_client.py index 7e5dd290..df2fac73 100644 --- a/snap7/async_client.py +++ b/snap7/async_client.py @@ -23,7 +23,7 @@ from .client_base import ClientMixin, _instrumented from .tags import Tag, decode_tag, encode_tag from .client import Client -from .szl import parse_cp_info_szl, parse_cpu_info_szl, parse_order_code_szl, parse_protection_szl +from .szl import parse_cp_info_szl, parse_cpu_info_szl, parse_cpu_state_szl, parse_order_code_szl, parse_protection_szl from .client import _parse_force_szl from .rate_limiter import RateLimitAlgorithm, RateLimitBehavior, RequestRateLimiter from .type import ( @@ -1115,10 +1115,16 @@ async def get_cpu_info(self) -> S7CpuInfo: return parse_cpu_info_szl(await self.read_szl(0x001C, 0)) async def get_cpu_state(self) -> str: - """Get CPU state (running/stopped).""" - request = self.protocol.build_cpu_state_request() - response = await self._send_receive(request) - return self.protocol.extract_cpu_state(response) + """Get the CPU operating state by reading SZL 0x0424. + + Returns ``"S7CpuStatusRun"``, ``"S7CpuStatusStop"`` or ``"S7CpuStatusUnknown"``. + Startup, HOLD and DEFECT are reported as ``"S7CpuStatusStop"``, as native Snap7 does. + + Raises: + S7ConnectionError: If the client is not connected. + S7ProtocolError: If the PLC refuses SZL 0x0424 or its reply is too short. + """ + return parse_cpu_state_szl(await self.read_szl(0x0424, 0)) # --------------------------------------------------------------- # Upload / Download / Delete diff --git a/snap7/client.py b/snap7/client.py index 8a76362f..057aaa00 100644 --- a/snap7/client.py +++ b/snap7/client.py @@ -24,14 +24,14 @@ from .connection import ISOTCPConnection from .s7protocol import S7Protocol, S7UserDataGroup, S7UserDataSubfunction, get_return_code_description from .datatypes import S7DataTypes, S7WordLen -from .error import S7Error, S7ConnectionError, S7ProtocolError, S7StalePacketError, S7TimeoutError +from .error import S7Error, S7ConnectionError, S7PacketLostError, S7ProtocolError, S7StalePacketError, S7TimeoutError from .client_base import ClientMixin, _instrumented from .log import PLCLoggerAdapter, OperationLogger from .optimizer import ReadItem, ReadPacket, sort_items, merge_items, packetize, extract_results from .rate_limiter import RateLimitAlgorithm, RateLimitBehavior, RequestRateLimiter from .tags import Tag, decode_tag, encode_tag -from .szl import parse_cp_info_szl, parse_cpu_info_szl, parse_order_code_szl, parse_protection_szl +from .szl import parse_cp_info_szl, parse_cpu_info_szl, parse_cpu_state_szl, parse_order_code_szl, parse_protection_szl from .type import ( Area, Block, @@ -421,7 +421,7 @@ def _heartbeat_loop(self) -> None: try: with self._reconnect_lock: if self.connected and self.connection is not None: - self.get_cpu_state() + self._heartbeat_probe() self._is_alive = True except Exception as e: logger.warning(f"Heartbeat probe failed: {e}") @@ -434,6 +434,20 @@ def _heartbeat_loop(self) -> None: except S7ConnectionError: logger.warning("Heartbeat reconnection failed") + def _heartbeat_probe(self) -> None: + """Ask the PLC for its CPU state to prove the connection is alive. + + A PLC that answers with a refusal (for example a CPU without + SZL 0x0424) has still answered, so only a transport failure or a lost + or stale reply is raised. + """ + try: + self.get_cpu_state() + except (S7PacketLostError, S7StalePacketError): + raise + except (S7ProtocolError, RuntimeError) as e: + logger.debug(f"Heartbeat probe was refused, connection is alive: {e}") + def connect(self, address: str, rack: int, slot: int, tcp_port: int = 102) -> "Client": """ Connect to S7 PLC. @@ -1387,15 +1401,18 @@ def get_cpu_info(self) -> S7CpuInfo: def get_cpu_state(self) -> str: """ - Get CPU state (running/stopped). + Get the CPU operating state by reading SZL 0x0424. Returns: - CPU state string - """ - request = self.protocol.build_cpu_state_request() - response = self._send_receive(request) + ``"S7CpuStatusRun"``, ``"S7CpuStatusStop"`` or ``"S7CpuStatusUnknown"``. + Startup, HOLD and DEFECT are reported as ``"S7CpuStatusStop"``, as + native Snap7 does. - return self.protocol.extract_cpu_state(response) + Raises: + S7ConnectionError: If the client is not connected. + S7ProtocolError: If the PLC refuses SZL 0x0424 or its reply is too short. + """ + return parse_cpu_state_szl(self.read_szl(0x0424, 0)) def get_block_info(self, block_type: Block, db_number: int) -> TS7BlockInfo: """ diff --git a/snap7/s7protocol.py b/snap7/s7protocol.py index 095903fe..11b0c51a 100644 --- a/snap7/s7protocol.py +++ b/snap7/s7protocol.py @@ -1487,12 +1487,16 @@ def check_userdata_response( def build_cpu_state_request(self) -> bytes: """ - Build CPU state request. + Build the request ``get_cpu_state()`` used to send. + + .. deprecated:: 3.5 + This is a read request without an address specification, which is + not a valid S7 PDU. The clients read SZL 0x0424 instead; see + :func:`snap7.szl.parse_cpu_state_szl`. Kept for backwards compatibility. Returns: - Complete S7 PDU for CPU state query + S7 PDU for the old CPU state query """ - # Simple CPU state request - in real S7 this would be a userdata function header = struct.pack( ">BBHHHH", 0x32, # Protocol ID @@ -1503,24 +1507,26 @@ def build_cpu_state_request(self) -> bytes: 0x0000, # Data length ) - # Use a custom function code for CPU state - parameters = struct.pack(">B", 0x04) # Use READ_AREA function for simplicity + parameters = struct.pack(">B", 0x04) # READ_AREA function code, nothing else return header + parameters def extract_cpu_state(self, response: Dict[str, Any]) -> str: """ - Extract CPU state from response. + Return ``"S7CpuStatusRun"`` whatever the response holds. + + .. deprecated:: 3.5 + This never looked at the response. The clients read SZL 0x0424 + instead; see :func:`snap7.szl.parse_cpu_state_szl`. Kept for + backwards compatibility. Args: - response: Parsed S7 response + response: Parsed S7 response (ignored) Returns: - CPU state string in S7CpuStatus format (e.g., 'S7CpuStatusRun') + Always ``"S7CpuStatusRun"`` """ - # Map internal states to S7 status format for API compatibility with master branch - # The cpu_statuses dict in type.py uses: {0: "S7CpuStatusUnknown", 4: "S7CpuStatusStop", 8: "S7CpuStatusRun"} - return "S7CpuStatusRun" # Default state for pure Python server + return "S7CpuStatusRun" def parse_response(self, pdu: bytes) -> Dict[str, Any]: """ diff --git a/snap7/server/__init__.py b/snap7/server/__init__.py index 380a32e2..47e637bb 100644 --- a/snap7/server/__init__.py +++ b/snap7/server/__init__.py @@ -1021,9 +1021,9 @@ def _parse_read_address(self, request: Dict[str, Any]) -> Optional[Tuple[S7Area, ) return (area, db_number, start, byte_count) - # Fallback to defaults if parsing failed - logger.warning("Using default address values - address parsing may have failed") - return (S7Area.DB, 1, 0, 4) + # A read request without an address specification is malformed + logger.warning("Read request carries no address specification") + return None except Exception as e: logger.error(f"Error parsing read address: {e}") @@ -1837,6 +1837,7 @@ def _handle_szl(self, request: Dict[str, Any], userdata_params: Dict[str, Any], - 0x001C: Component identification (for get_cpu_info) - 0x0011: Module identification (for get_order_code) - 0x0131: Communication parameters (for get_cp_info) + - 0x0424: Current operating mode transition (for get_cpu_state) - 0x0232: Protection level (for get_protection) Args: @@ -1933,10 +1934,19 @@ def _get_szl_data(self, szl_id: int, szl_index: int) -> Optional[bytes]: # anl_sch: factory setting protection return struct.pack(">HHHHH", 1, 0, 0, 0, 0) # No protection + # SZL 0x0424: current operating mode transition (get_cpu_state) + # Record layout: ereig(2) + ae(1) + bzu-id(1) + res(4) + anlinfo1..4(4) + time(8) = 20 bytes. + # bzu-id carries the requested mode in bits 0-3: 4 is STOP, 8 is RUN. + elif szl_id == 0x0424: + # Event IDs: 0x4302 "STARTUP to RUN", 0x4304 "STOP caused by PG STOP operation" + event_ids = {CPUState.RUN: 0x4302, CPUState.STOP: 0x4304} + record = struct.pack(">HBB", event_ids.get(self.cpu_state, 0x0000), 0xFF, int(self.cpu_state)) + bytes(16) + return struct.pack(">HH", len(record), 1) + record + # SZL 0x0000: SZL list elif szl_id == 0x0000: # Return list of available SZL IDs - available_ids = [0x0000, 0x0011, 0x001C, 0x0131, 0x0232] + available_ids = [0x0000, 0x0011, 0x001C, 0x0131, 0x0232, 0x0424] szl_bytes = b"" for id_val in available_ids: szl_bytes += struct.pack(">H", id_val) diff --git a/snap7/szl.py b/snap7/szl.py index 1d539362..c3c97996 100644 --- a/snap7/szl.py +++ b/snap7/szl.py @@ -14,6 +14,7 @@ import struct +from .error import S7ProtocolError from .type import S7CpInfo, S7CpuInfo, S7OrderCode, S7Protection, S7SZL @@ -56,6 +57,41 @@ def parse_cpu_info_szl(szl: S7SZL) -> S7CpuInfo: return info +def parse_cpu_state_szl(szl: S7SZL) -> str: + """Decode SZL 0x0424 (current operating mode transition) into an ``S7CpuStatus*`` string. + + The payload is the 4-byte partial-list header (``LENTHDR``, ``N_DR``) + followed by one 20-byte record:: + + ereig(2) ae(1) bzu-id(1) res(4) anlinfo1..4(4) time(8) + + ``bzu-id`` holds the requested operating mode in bits 0-3 and the previous + one in bits 4-7 (1-4 are STOP variants, 5 and 7 are startup, 8 is RUN, + 0xA is HOLD, 0xD is DEFECT). The layout follows Wireshark's S7comm + dissector (``packet-s7comm_szl_ids.c``) and matches the byte native Snap7 + reads in ``opGetPlcStatus``. + + Like native Snap7, only RUN is reported as ``S7CpuStatusRun``, a zero byte + as ``S7CpuStatusUnknown``, and every other mode (startup, HOLD and DEFECT + included) as ``S7CpuStatusStop``. Snap7 compares the whole byte against + 0x08; this looks at the requested-mode nibble only, so a RUN reply that + also carries a previous mode is still reported as RUN. + + Raises: + S7ProtocolError: If the reply is too short to hold the mode byte. + """ + data = _szl_data(szl) + if len(data) < 8: + raise S7ProtocolError(f"SZL 0x0424 reply too short for an operating mode: {len(data)} bytes") + + mode = data[7] + if mode == 0x00: + return "S7CpuStatusUnknown" + if mode & 0x0F == 0x08: + return "S7CpuStatusRun" + return "S7CpuStatusStop" + + def parse_cp_info_szl(szl: S7SZL) -> S7CpInfo: """Decode SZL 0x0131 (communication processor info) into an :class:`S7CpInfo`. diff --git a/tests/test_async_client.py b/tests/test_async_client.py index 97e9d52b..55d959d7 100644 --- a/tests/test_async_client.py +++ b/tests/test_async_client.py @@ -406,9 +406,13 @@ async def test_download_collects_all_fragments(client: AsyncClient) -> None: @pytest.mark.asyncio -async def test_get_cpu_state(client: AsyncClient) -> None: - state = await client.get_cpu_state() - assert isinstance(state, str) +async def test_get_cpu_state(client: AsyncClient, server: Server) -> None: + try: + for status, expected in ((4, "S7CpuStatusStop"), (0, "S7CpuStatusUnknown"), (8, "S7CpuStatusRun")): + server.set_cpu_status(status) + assert await client.get_cpu_state() == expected + finally: + server.set_cpu_status(8) @pytest.mark.asyncio diff --git a/tests/test_server.py b/tests/test_server.py index eb2d1109..6d00a6a6 100644 --- a/tests/test_server.py +++ b/tests/test_server.py @@ -17,7 +17,8 @@ from snap7.error import S7ConnectionError, S7ProtocolError, error_text, server_errors from snap7.s7protocol import S7Function, S7PDUType from snap7.server import EVC_DATA_READ, EVC_DATA_WRITE, EVC_SERVER_STARTED, EVC_SERVER_STOPPED, Server, ServerISOConnection -from snap7.type import Block, Parameter, SrvArea, SrvEvent, mkEvent, mkLog +from snap7.szl import parse_cpu_state_szl +from snap7.type import S7SZL, Block, Parameter, SrvArea, SrvEvent, mkEvent, mkLog logging.basicConfig(level=logging.WARNING) @@ -1074,10 +1075,76 @@ def test_set_plc_system_datetime(self) -> None: # ------------------------------------------------------------------ # get_cpu_state (SZL-based CPU state request) # ------------------------------------------------------------------ - def test_get_cpu_state(self) -> None: - """get_cpu_state should return a string state.""" - state = self.client.get_cpu_state() - self.assertIsInstance(state, str) + def test_get_cpu_state_follows_server(self) -> None: + """get_cpu_state reports the state the server is in, read from SZL 0x0424.""" + try: + for status, expected in ((4, "S7CpuStatusStop"), (0, "S7CpuStatusUnknown"), (8, "S7CpuStatusRun")): + self.server.set_cpu_status(status) + self.assertEqual(self.client.get_cpu_state(), expected) + finally: + self.server.set_cpu_status(8) + + def test_szl_0424_record_layout(self) -> None: + """SZL 0x0424 is a partial-list header plus one 20-byte record with the mode at offset 3.""" + szl = self.client.read_szl(0x0424, 0) + data = bytes(b & 0xFF for b in szl.Data[: szl.Header.LengthDR]) + self.assertEqual(len(data), 24) + self.assertEqual(struct.unpack(">HH", data[:4]), (20, 1)) + self.assertEqual(data[4:8], bytes([0x43, 0x02, 0xFF, 0x08])) + + def test_read_without_address_is_refused(self) -> None: + """A read request with no address specification gets an error, not DB1 defaults.""" + request = self.client.protocol.build_cpu_state_request() + with self.assertRaises(S7ProtocolError): + self.client._send_receive(request) + # The connection survives the refusal + self.assertEqual(len(self.client.db_read(1, 0, 4)), 4) + + def test_heartbeat_probe(self) -> None: + """A refused probe still proves the link; a transport failure does not.""" + self.client._heartbeat_probe() + with patch.object(self.client, "get_cpu_state", side_effect=S7ProtocolError("refused")): + self.client._heartbeat_probe() + with patch.object(self.client, "get_cpu_state", side_effect=S7ConnectionError("gone")): + with self.assertRaises(S7ConnectionError): + self.client._heartbeat_probe() + + +@pytest.mark.server +class TestParseCpuStateSzl(unittest.TestCase): + """parse_cpu_state_szl maps the bzu-id byte of SZL 0x0424 to an S7CpuStatus string.""" + + @staticmethod + def _szl(mode: int) -> S7SZL: + payload = struct.pack(">HH", 20, 1) + bytes([0x43, 0x02, 0xFF, mode]) + bytes(16) + szl = S7SZL() + szl.Header.LengthDR = len(payload) + szl.Header.NDR = 1 + for i, b in enumerate(payload): + szl.Data[i] = b + return szl + + def test_modes(self) -> None: + cases = { + 0x00: "S7CpuStatusUnknown", + 0x08: "S7CpuStatusRun", + 0x58: "S7CpuStatusRun", # previous mode startup, requested mode RUN + 0x03: "S7CpuStatusStop", # STOP (self initialization), seen on older CPUs + 0x04: "S7CpuStatusStop", + 0x84: "S7CpuStatusStop", # previous mode RUN, requested mode STOP + 0x05: "S7CpuStatusStop", # startup + 0x0A: "S7CpuStatusStop", # HOLD + 0x0D: "S7CpuStatusStop", # DEFECT + } + for mode, expected in cases.items(): + with self.subTest(mode=hex(mode)): + self.assertEqual(parse_cpu_state_szl(self._szl(mode)), expected) + + def test_short_reply_is_an_error(self) -> None: + szl = self._szl(0x08) + szl.Header.LengthDR = 7 + with self.assertRaises(S7ProtocolError): + parse_cpu_state_szl(szl) @pytest.mark.server