fix(s7commplus): decode struct PValues per the real wire format (packed + normal)

s7/codec.py

@@ -466,14 +466,45 @@ def decode_pvalue_to_bytes(data: bytes, offset: int) -> tuple[bytes, int]:
         consumed += length
         return bytes(raw), consumed
     elif datatype == DataType.STRUCT:
-        # Struct: read count, then nested PValues
-        count, c = decode_uint32_vlq(data, offset + consumed)
-        consumed += c
+        # Struct value. Mirrors ValueStruct.Deserialize in the C# reference driver
+        # (thomas-v2/S7CommPlusDriver, Core/PValue.cs).
+        #
+        # The leading struct id is a fixed UInt32 (not VLQ). Two transmission forms:
+        #
+        #  * Packed struct — for system datatypes (DTL, optimized-DB structs, ...) whose
+        #    members are sent as one opaque blob. Detected by the id falling in the ranges
+        #    0x90000000..0x9fffffff or 0x02000000..0x02ffffff. Layout: UInt64 interface
+        #    timestamp, VLQ transport flags, VLQ element count (a second count follows when
+        #    the Count2Present flag, bit 10, is set), then `count` raw bytes. We return those
+        #    raw bytes verbatim — the caller interprets them using the struct's member layout.
+        #
+        #  * Normal struct — members as [VLQ key][PValue], terminated by a key of 0.
+        struct_id = int.from_bytes(data[offset + consumed : offset + consumed + 4], "big")
+        consumed += 4
+
+        if (0x90000000 < struct_id < 0x9FFFFFFF) or (0x02000000 < struct_id < 0x02FFFFFF):
+            consumed += 8  # PackedStructInterfaceTimestamp (UInt64, fixed)
+            transport_flags, c = decode_uint32_vlq(data, offset + consumed)
+            consumed += c
+            count, c = decode_uint32_vlq(data, offset + consumed)
+            consumed += c
+            if transport_flags & 0x400:  # Count2Present: a second count follows
+                count, c = decode_uint32_vlq(data, offset + consumed)
+                consumed += c
+            raw = data[offset + consumed : offset + consumed + count]
+            consumed += count
+            return bytes(raw), consumed
+
+        # Normal struct: concatenate member values, stopping at the 0 key terminator.
         result = bytearray()
-        for _ in range(count):
+        key, c = decode_uint32_vlq(data, offset + consumed)
+        consumed += c
+        while key > 0:
             val_bytes, c = decode_pvalue_to_bytes(data, offset + consumed)
             consumed += c
             result += val_bytes
+            key, c = decode_uint32_vlq(data, offset + consumed)
+            consumed += c
         return bytes(result), consumed
     else:
         raise ValueError(f"Unsupported PValue datatype: {datatype:#04x}")

tests/test_s7_codec.py

@@ -538,14 +538,64 @@ def test_wstring(self) -> None:
         result, consumed = decode_pvalue_to_bytes(data, 0)
         assert result == text
 
-    def test_struct_nested(self) -> None:
-        # Struct with 2 USINT elements
-        vlq_count = encode_uint32_vlq(2)
-        elem1 = bytes([0x00, DataType.USINT, 0x0A])
-        elem2 = bytes([0x00, DataType.USINT, 0x14])
-        data = bytes([0x00, DataType.STRUCT]) + vlq_count + elem1 + elem2
+    def test_struct_normal(self) -> None:
+        # Normal struct: UInt32 id, then [VLQ key][PValue] members, 0-key terminated.
+        struct_id = struct.pack(">I", 0x00000001)
+        member1 = encode_uint32_vlq(1) + bytes([0x00, DataType.USINT, 0x0A])
+        member2 = encode_uint32_vlq(2) + bytes([0x00, DataType.USINT, 0x14])
+        terminator = encode_uint32_vlq(0)
+        data = bytes([0x00, DataType.STRUCT]) + struct_id + member1 + member2 + terminator
         result, consumed = decode_pvalue_to_bytes(data, 0)
         assert result == bytes([0x0A, 0x14])
+        assert consumed == len(data)
+
+    def test_struct_packed(self) -> None:
+        # Packed struct (system datatype): id in 0x90000000..0x9fffffff, then UInt64 interface
+        # timestamp, VLQ transport flags, VLQ element count, then raw member bytes returned
+        # verbatim. Mirrors a real S7-1500 read of a struct node with three Int members.
+        struct_id = struct.pack(">I", 0x91080009)
+        timestamp = bytes.fromhex("58936099d03e9bd4")
+        transport_flags = encode_uint32_vlq(0x03)
+        members = struct.pack(">3h", 100, 200, 300)  # three Int members
+        count = encode_uint32_vlq(len(members))
+        data = bytes([0x00, DataType.STRUCT]) + struct_id + timestamp + transport_flags + count + members
+        result, consumed = decode_pvalue_to_bytes(data, 0)
+        assert result == members
+        assert consumed == len(data)
+
+    def test_struct_packed_count2_present(self) -> None:
+        # When the Count2Present transport flag (bit 10) is set, a second count follows.
+        struct_id = struct.pack(">I", 0x91080009)
+        timestamp = bytes(8)
+        transport_flags = encode_uint32_vlq(0x402)  # AlwaysSet | Count2Present
+        members = bytes([0xAA, 0xBB])
+        data = (
+            bytes([0x00, DataType.STRUCT])
+            + struct_id
+            + timestamp
+            + transport_flags
+            + encode_uint32_vlq(999)  # first count (ignored when Count2Present)
+            + encode_uint32_vlq(len(members))  # second count is the real one
+            + members
+        )
+        result, consumed = decode_pvalue_to_bytes(data, 0)
+        assert result == members
+        assert consumed == len(data)
+
+    def test_struct_id_boundary_is_normal(self) -> None:
+        # The packed-struct ranges are *exclusive* of their bounds, matching the C# reference
+        # (ValueStruct.Deserialize: ``Value > 0x90000000 && Value < 0x9fffffff``). The bound
+        # values 0x90000000 and 0x02000000 are base sentinels — real type-ids are base + n
+        # (e.g. TI_BOOL = 0x02000000 + 1), so the bounds themselves are never packed structs
+        # and must be parsed as normal structs. Guards against a `<=` regression.
+        for boundary in (0x90000000, 0x9FFFFFFF, 0x02000000, 0x02FFFFFF):
+            struct_id = struct.pack(">I", boundary)
+            member = encode_uint32_vlq(1) + bytes([0x00, DataType.USINT, 0x07])
+            terminator = encode_uint32_vlq(0)
+            data = bytes([0x00, DataType.STRUCT]) + struct_id + member + terminator
+            result, consumed = decode_pvalue_to_bytes(data, 0)
+            assert result == bytes([0x07]), f"id {boundary:#010x} must parse as a normal struct"
+            assert consumed == len(data)
 
     def test_unsupported_type(self) -> None:
         data = bytes([0x00, 0xFF])