fix(s7commplus): use explicit target types for scalar writes

CHANGES.md

@@ -9,6 +9,8 @@ Major release: new `s7commplus` package with S7CommPlus protocol support.
 * New `s7commplus` package for S7CommPlus protocol (S7-1200/1500)
 * S7CommPlus V1, V2 (TLS), and V3 support for S7-1200/1500
 * S7CommPlus area read/write (M, I, Q, counters, timers)
+* Require an explicit target datatype in S7CommPlus multi-write tuples; add
+  `datatype=` to sync/async symbolic and area writes for scalar PLC targets.
 * S7CommPlus PLC start/stop via INVOKE
 * S7CommPlus object browsing via EXPLORE
 * S7CommPlus live symbol browsing (`client.browse()`) and datablock listing (experimental)

doc/API/tags.rst

@@ -115,6 +115,7 @@ S7CommPlus tag API is not implemented yet; :class:`~snap7.tags.Tag` and the
    import struct
 
    from s7commplus import Client
+   from s7commplus.protocol import DataType
 
    client = Client()
    client.connect("192.168.1.10", use_tls=True)
@@ -126,7 +127,27 @@ S7CommPlus tag API is not implemented yet; :class:`~snap7.tags.Tag` and the
    # Symbolic values are currently exposed as raw wire bytes.
    raw = client.read_symbolic(path[0], path[1:])
    speed = struct.unpack(">f", raw)[0]
-   client.write_symbolic(path[0], path[1:], struct.pack(">f", 1500.0))
+   client.write_symbolic(path[0], path[1:], struct.pack(">f", 1500.0), datatype=DataType.REAL)
+
+The write datatype must match the PLC variable's ``data_type`` from browsing.
+For example, an INT requires ``struct.pack(">h", value)`` and
+``datatype=DataType.INT``. BLOB is not a generic scalar datatype. The
+``datatype=`` keyword is supported by both sync and async symbolic and area
+writes; omitting it preserves the legacy BLOB encoding.
+
+Multi-write requires four-element tuples with an explicit datatype. The
+three-element form from development versions is rejected before sending:
+
+.. code-block:: python
+
+   client.db_write_multi([
+       (7, 0, struct.pack(">f", 2.0), DataType.REAL),
+       (7, 4, struct.pack(">H", 512), DataType.WORD),
+   ])
+
+These DB/offset addresses must be validated for the target PLC; they are not
+interchangeable with browse-derived symbolic paths. Explicit BLOB remains
+available for targets that accept it, including the raw-byte emulator.
 
 API reference
 -------------

s7commplus/async_client.py

@@ -501,7 +501,7 @@ async def db_write(self, db_number: int, start: int, data: bytes, datatype: Data
         await self.db_write_multi([(db_number, start, data, datatype)])
 
     async def db_write_multi(self, items: list[DBWriteItem]) -> None:
-        """Write multiple regions, optionally adding a DataType as each tuple's fourth item."""
+        """Write (db_number, start_offset, data, datatype) tuples matching the PLC target types."""
         payload = _build_write_payload(items, self._protocol_version)
         response = await self._send_request(FunctionCode.SET_MULTI_VARIABLES, payload)
         _parse_write_response(response)
@@ -526,9 +526,9 @@ async def read_area(self, area_rid: int, start: int, size: int) -> bytes:
             raise RuntimeError("Area read failed")
         return results[0]
 
-    async def write_area(self, area_rid: int, start: int, data: bytes) -> None:
-        """Write raw bytes to a controller memory area (M, I, Q, counters, timers)."""
-        payload = _build_area_write_payload(area_rid, start, data, self._protocol_version)
+    async def write_area(self, area_rid: int, start: int, data: bytes, *, datatype: DataType = DataType.BLOB) -> None:
+        """Write a controller memory area, specifying the target datatype for scalar writes."""
+        payload = _build_area_write_payload(area_rid, start, data, self._protocol_version, datatype=datatype)
         response = await self._send_request(FunctionCode.SET_MULTI_VARIABLES, payload)
         _parse_write_response(response)
 
@@ -710,12 +710,17 @@ async def read_symbolic(self, access_area: int, lids: list[int], symbol_crc: int
             raise RuntimeError("Symbolic read failed")
         return results[0]
 
-    async def write_symbolic(self, access_area: int, lids: list[int], data: bytes, symbol_crc: int = 0) -> None:
+    async def write_symbolic(
+        self, access_area: int, lids: list[int], data: bytes, symbol_crc: int = 0, *, datatype: DataType = DataType.BLOB
+    ) -> None:
         """Write a variable using S7CommPlus symbolic (LID-based) access.
 
         .. warning:: This method is **experimental** and may change.
+
+        Set ``datatype`` to the target PLC datatype reported by browse().
+        The legacy BLOB default is not a generic replacement for scalar types.
         """
-        payload = _build_symbolic_write_payload(access_area, lids, data, symbol_crc, self._protocol_version)
+        payload = _build_symbolic_write_payload(access_area, lids, data, symbol_crc, self._protocol_version, datatype=datatype)
         response = await self._send_request(FunctionCode.SET_MULTI_VARIABLES, payload)
         _parse_write_response(response)
 

s7commplus/client.py

@@ -26,7 +26,6 @@
     decode_pvalue_to_bytes,
     encode_item_address,
     encode_object_qualifier,
-    encode_pvalue_blob,
     encode_pvalue_typed,
     parse_create_object_session_id,
 )
@@ -44,13 +43,12 @@
 logger = logging.getLogger(__name__)
 
 _T = TypeVar("_T")
-DBWriteItem: TypeAlias = tuple[int, int, bytes] | tuple[int, int, bytes, DataType]
+DBWriteItem: TypeAlias = tuple[int, int, bytes, DataType]
 
 
 def _normalize_write_item(item: DBWriteItem) -> tuple[int, int, bytes, DataType]:
-    if len(item) == 3:
-        db_number, start, data = item
-        return db_number, start, data, DataType.BLOB
+    if len(item) != 4:
+        raise ValueError("Write items require (db_number, start_offset, data, datatype); use the target PLC datatype")
     db_number, start, data, datatype = item
     return db_number, start, data, DataType(datatype)
 
@@ -245,15 +243,18 @@ def db_write_multi(self, items: list[DBWriteItem]) -> None:
         """Write multiple data block regions in a single request.
 
         Args:
-            items: ``(db_number, start_offset, data)`` tuples for BLOB writes,
-                or four-tuples adding an explicit :class:`DataType`.
+            items: ``(db_number, start_offset, data, datatype)`` tuples.
+                The datatype must match the target PLC variable. BLOB is not
+                a generic replacement for scalar datatypes.
         """
         if self._connection is None:
             raise RuntimeError("Not connected")
 
         if self._connection.requires_substreamed:
-            for item in items:
-                db_number, start, data, datatype = _normalize_write_item(item)
+            normalized = [_normalize_write_item(item) for item in items]
+            for _, _, data, datatype in normalized:
+                encode_pvalue_typed(datatype, data)
+            for db_number, start, data, datatype in normalized:
                 self._db_write_substreamed(db_number, start, data, datatype)
             return
 
@@ -322,25 +323,27 @@ def read_area(self, area_rid: int, start: int, size: int) -> bytes:
             raise RuntimeError("Area read failed")
         return results[0]
 
-    def write_area(self, area_rid: int, start: int, data: bytes) -> None:
+    def write_area(self, area_rid: int, start: int, data: bytes, *, datatype: DataType = DataType.BLOB) -> None:
         """Write raw bytes to a controller memory area (M, I, Q, counters, timers).
 
         Args:
             area_rid: Native object RID for the area.
             start: Start byte offset.
             data: Bytes to write.
+            datatype: Target PLC datatype. BLOB preserves legacy raw-byte calls;
+                use an explicit scalar datatype when writing a scalar target.
         """
         if self._connection is None:
             raise RuntimeError("Not connected")
 
         if self._connection.requires_substreamed:
             payload = _build_substreamed_write_payload(
-                self._connection.session_id, area_rid, Ids.CONTROLLER_AREA_VALUE_ACTUAL, [start + 1, len(data)], data
+                self._connection.session_id, area_rid, Ids.CONTROLLER_AREA_VALUE_ACTUAL, [start + 1, len(data)], data, datatype
             )
             self._connection.send_request(FunctionCode.SET_VAR_SUBSTREAMED, payload)
             return
 
-        payload = _build_area_write_payload(area_rid, start, data, self._connection.protocol_version)
+        payload = _build_area_write_payload(area_rid, start, data, self._connection.protocol_version, datatype=datatype)
         response = self._connection.send_request(FunctionCode.SET_MULTI_VARIABLES, payload)
         _parse_write_response(response)
 
@@ -389,7 +392,9 @@ def read_symbolic(self, access_area: int, lids: list[int], symbol_crc: int = 0)
             raise RuntimeError("Symbolic read failed")
         return results[0]
 
-    def write_symbolic(self, access_area: int, lids: list[int], data: bytes, symbol_crc: int = 0) -> None:
+    def write_symbolic(
+        self, access_area: int, lids: list[int], data: bytes, symbol_crc: int = 0, *, datatype: DataType = DataType.BLOB
+    ) -> None:
         """Write a variable using S7CommPlus symbolic (LID-based) access.
 
         .. warning:: This method is **experimental** and may change.
@@ -407,12 +412,14 @@ def write_symbolic(self, access_area: int, lids: list[int], data: bytes, symbol_
             lids: LID path through the symbol tree.
             data: Raw bytes to write.
             symbol_crc: Symbol CRC for layout validation (0 = skip check).
+            datatype: Target PLC datatype, as reported by browse(). BLOB preserves
+                legacy calls but is not a generic replacement for scalar types.
         """
         if self._connection is None:
             raise RuntimeError("Not connected")
 
         payload = _build_symbolic_write_payload(
-            access_area, lids, data, symbol_crc, protocol_version=self._connection.protocol_version
+            access_area, lids, data, symbol_crc, protocol_version=self._connection.protocol_version, datatype=datatype
         )
         response = self._connection.send_request(FunctionCode.SET_MULTI_VARIABLES, payload)
         _parse_write_response(response)
@@ -1072,7 +1079,9 @@ def _build_area_read_payload(area_rid: int, start: int, size: int, protocol_vers
     return bytes(payload)
 
 
-def _build_area_write_payload(area_rid: int, start: int, data: bytes, protocol_version: int = ProtocolVersion.V2) -> bytes:
+def _build_area_write_payload(
+    area_rid: int, start: int, data: bytes, protocol_version: int = ProtocolVersion.V2, *, datatype: DataType = DataType.BLOB
+) -> bytes:
     """Build a SetMultiVariables payload for controller memory area access."""
     addr_bytes, field_count = encode_item_address(
         access_area=area_rid,
@@ -1086,7 +1095,7 @@ def _build_area_write_payload(area_rid: int, start: int, data: bytes, protocol_v
     payload += encode_uint32_vlq(field_count)
     payload += addr_bytes
     payload += encode_uint32_vlq(1)  # item number 1
-    payload += encode_pvalue_blob(data)
+    payload += encode_pvalue_typed(datatype, data)
     payload += bytes([0x00])
     payload += encode_object_qualifier(protocol_version=protocol_version)
     payload += struct.pack(">I", 0)
@@ -1136,6 +1145,8 @@ def _build_symbolic_write_payload(
     data: bytes,
     symbol_crc: int = 0,
     protocol_version: int = ProtocolVersion.V2,
+    *,
+    datatype: DataType = DataType.BLOB,
 ) -> bytes:
     """Build a SetMultiVariables payload for symbolic (LID-based) access."""
     if access_area >= 0x8A0E0000:
@@ -1156,7 +1167,7 @@ def _build_symbolic_write_payload(
     payload += encode_uint32_vlq(field_count)
     payload += addr_bytes
     payload += encode_uint32_vlq(1)  # item number 1
-    payload += encode_pvalue_blob(data)
+    payload += encode_pvalue_typed(datatype, data)
     payload += bytes([0x00])
     payload += encode_object_qualifier(protocol_version=protocol_version)
     payload += struct.pack(">I", 0)

tests/test_s7_server.py

@@ -218,9 +218,9 @@ def test_multi_write(self, server: S7CommPlusServer) -> None:
         try:
             client.db_write_multi(
                 [
-                    (1, 0, b"first"),
-                    (1, 10, b"second"),
-                    (2, 20, b"third"),
+                    (1, 0, b"first", DataType.BLOB),
+                    (1, 10, b"second", DataType.BLOB),
+                    (2, 20, b"third", DataType.BLOB),
                 ]
             )
 
@@ -332,9 +332,9 @@ async def test_multi_write(self, server: S7CommPlusServer) -> None:
             await client.connect("127.0.0.1", port=TEST_PORT)
             await client.write_multi(
                 [
-                    (1, 0, b"alpha"),
-                    (1, 10, b"beta"),
-                    (2, 20, b"gamma"),
+                    (1, 0, b"alpha", DataType.BLOB),
+                    (1, 10, b"beta", DataType.BLOB),
+                    (2, 20, b"gamma", DataType.BLOB),
                 ]
             )
 

tests/test_s7_unit.py

@@ -156,13 +156,13 @@ def test_error_items(self) -> None:
 
 class TestBuildWritePayload:
     def test_single_item(self) -> None:
-        payload = _build_write_payload([(1, 0, bytes([1, 2, 3, 4]))])
+        payload = _build_write_payload([(1, 0, bytes([1, 2, 3, 4]), DataType.BLOB)])
         assert isinstance(payload, bytes)
         assert len(payload) > 0
 
     def test_data_appears_in_payload(self) -> None:
         data = bytes([0xDE, 0xAD, 0xBE, 0xEF])
-        payload = _build_write_payload([(1, 0, data)])
+        payload = _build_write_payload([(1, 0, data, DataType.BLOB)])
         # The raw data should appear in the payload (inside the BLOB PValue)
         assert data in payload
 
@@ -196,7 +196,7 @@ def test_read_payload_roundtrip(self) -> None:
     def test_write_read_consistency(self) -> None:
         """Build write and read payloads for same address, verify both compile."""
         read_payload = _build_read_payload([(1, 0, 4)])
-        write_payload = _build_write_payload([(1, 0, bytes([1, 2, 3, 4]))])
+        write_payload = _build_write_payload([(1, 0, bytes([1, 2, 3, 4]), DataType.BLOB)])
         assert isinstance(read_payload, bytes)
         assert isinstance(write_payload, bytes)
 
@@ -216,7 +216,7 @@ def test_read_payload_has_no_static_integrity_id(self) -> None:
         assert self._has_only_trailing_fill(payload)
 
     def test_write_payload_has_no_static_integrity_id(self) -> None:
-        payload = _build_write_payload([(1, 0, bytes([1, 2, 3, 4]))])
+        payload = _build_write_payload([(1, 0, bytes([1, 2, 3, 4]), DataType.BLOB)])
         assert self._has_only_trailing_fill(payload)
 
     def test_area_read_payload_has_no_static_integrity_id(self) -> None:
@@ -585,20 +585,20 @@ def test_db_read_multi_not_connected(self) -> None:
     def test_db_write_multi_not_connected(self) -> None:
         client = S7CommPlusClient()
         with pytest.raises(RuntimeError, match="Not connected"):
-            client.db_write_multi([(1, 0, b"data")])
+            client.db_write_multi([(1, 0, b"data", DataType.BLOB)])
 
     def test_write_multi_not_connected(self) -> None:
         client = S7CommPlusClient()
         with pytest.raises(RuntimeError, match="Not connected"):
-            client.write_multi([(1, 0, b"data")])
+            client.write_multi([(1, 0, b"data", DataType.BLOB)])
 
     def test_db_write_multi_uses_one_substreamed_request_per_item(self) -> None:
         client = S7CommPlusClient()
         connection = MagicMock()
         connection.requires_substreamed = True
         connection.session_id = 0x70000001
         client._connection = connection
-        items = [(1, 0, b"first"), (2, 10, b"second")]
+        items = [(1, 0, b"first", DataType.BLOB), (2, 10, b"second", DataType.BLOB)]
 
         client.db_write_multi(items)
 
@@ -614,7 +614,7 @@ def test_db_write_multi_uses_one_substreamed_request_per_item(self) -> None:
                         data,
                     ),
                 )
-                for db_number, start, data in items
+                for db_number, start, data, _ in items
             ]
         )