Let read_tags and read_multi_vars read more than 20 items

CHANGES.md

@@ -12,6 +12,11 @@ Unreleased
   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.
+* `read_tags` and `read_multi_vars` with a list of dicts now accept more than 20 items, in both clients. Previously
+  they raised `ValueError` over `MAX_VARS`, although the read optimizer already splits a read over several requests.
+  The optimizer now also puts at most 20 items in one request; `optimizer.packetize` takes a `max_items` argument
+  for this. The ctypes form of `read_multi_vars` and `write_multi_vars` keep the limit.
+* `read_tags` now raises `NotImplementedError` for a symbolic tag, as `read_tag` does, in both clients.
 
 3.4.0
 -----

snap7/async_client.py

@@ -811,8 +811,10 @@ async def write_tag(self, tag: "Union[Tag, str]", value: Any, encoding: str = "l
         return await self.write_area(Area(resolved.area), resolved.db_number, resolved.byte_offset, buf)
 
     async def read_tags(self, tags: "Sequence[Union[Tag, str]]", encoding: str = "latin-1") -> list[Any]:
-        """Read multiple tags in a single optimized request, in input order."""
+        """Read any number of tags, returning the values in input order."""
         resolved = [Tag.from_string(t) if isinstance(t, str) else t for t in tags]
+        if any(t.is_symbolic for t in resolved):
+            raise NotImplementedError("Symbolic (LID-based) tag access is not supported by the classic S7 client.")
         items = [{"area": Area(t.area), "db_number": t.db_number, "start": t.byte_offset, "size": t.size} for t in resolved]
         _code, data_list = await self.read_multi_vars(items)
         return [decode_tag(t, bytearray(d), encoding=encoding) for t, d in zip(resolved, data_list)]
@@ -951,15 +953,14 @@ async def read_multi_vars(self, items: List[dict[str, Any]]) -> Tuple[int, list[
         """Read multiple variables (sequentially, one read_area per item).
 
         Args:
-            items: List of item dicts with keys: area, db_number, start, size
+            items: List of item dicts with keys: area, db_number, start, size.
+                Any number of items is accepted.
 
         Returns:
             Tuple of (result_code, list_of_bytearrays)
         """
         if not items:
             return (0, [])
-        if len(items) > self.MAX_VARS:
-            raise ValueError(f"Too many items: {len(items)} exceeds MAX_VARS ({self.MAX_VARS})")
 
         results: list[bytearray] = []
         for item in items:

snap7/client.py

@@ -107,7 +107,7 @@ class Client(ClientMixin):
         >>> client.disconnect()
     """
 
-    MAX_VARS = 20  # Max variables per multi-read/multi-write request
+    MAX_VARS = 20  # Max variables per multi-read/multi-write PDU
 
     def __init__(
         self,
@@ -728,10 +728,11 @@ def write_tag(self, tag: "Union[Tag, str]", value: Any, encoding: str = "latin-1
         return self.write_area(Area(resolved.area), resolved.db_number, resolved.byte_offset, buf)
 
     def read_tags(self, tags: "Sequence[Union[Tag, str]]", encoding: str = "latin-1") -> list[Any]:
-        """Read multiple tags in a single optimized request.
+        """Read multiple tags in as few requests as possible.
 
         Uses the multi-variable read optimizer when available to batch
-        reads into minimal PDU exchanges.
+        reads into minimal PDU exchanges. Any number of tags is accepted;
+        tags that do not fit in one request go into further requests.
 
         Args:
             tags: Sequence of :class:`~snap7.tags.Tag` instances or address strings.
@@ -741,6 +742,8 @@ def read_tags(self, tags: "Sequence[Union[Tag, str]]", encoding: str = "latin-1"
             List of decoded values in the same order as input.
         """
         resolved = [Tag.from_string(t) if isinstance(t, str) else t for t in tags]
+        if any(t.is_symbolic for t in resolved):
+            raise NotImplementedError("Symbolic (LID-based) tag access is not supported by the classic S7 client.")
         items = [{"area": Area(t.area), "db_number": t.db_number, "start": t.byte_offset, "size": t.size} for t in resolved]
         _code, data_list = self.read_multi_vars(items)
         return [decode_tag(t, bytearray(d), encoding=encoding) for t, d in zip(resolved, data_list)]
@@ -976,12 +979,14 @@ def build_chunk_request(o: int = chunk_offset, cd: bytes = bytes(chunk_data)) ->
 
     @_instrumented("read_multi_vars")
     def read_multi_vars(self, items: Union[List[dict[str, Any]], "Array[S7DataItem]"]) -> Tuple[int, Any]:
-        """Read multiple variables in a single request.
+        """Read multiple variables in as few requests as possible.
 
         When given a list of dicts with two or more items, uses the multi-variable
         read optimizer to merge adjacent reads and pack them into minimal PDU
         exchanges.  This significantly reduces the number of round-trips compared
-        to reading each variable individually.
+        to reading each variable individually.  A list of dicts may hold any
+        number of items: each request carries at most ``MAX_VARS`` address
+        specifications and the rest go into further requests.
 
         .. warning::
 
@@ -999,16 +1004,15 @@ def read_multi_vars(self, items: Union[List[dict[str, Any]], "Array[S7DataItem]"
             ctypes array or a list of bytearrays in the original item order.
 
         Raises:
-            ValueError: If more than MAX_VARS items are requested.
+            ValueError: If a ctypes array holds more than MAX_VARS items.
         """
         if not items:
             return (0, items)
 
-        if len(items) > self.MAX_VARS:
-            raise ValueError(f"Too many items: {len(items)} exceeds MAX_VARS ({self.MAX_VARS})")
-
         # Handle S7DataItem array (ctypes) -- unchanged legacy path
         if hasattr(items, "_type_") and hasattr(items[0], "Area"):
+            if len(items) > self.MAX_VARS:
+                raise ValueError(f"Too many items: {len(items)} exceeds MAX_VARS ({self.MAX_VARS})")
             s7_items = cast("Array[S7DataItem]", items)
             for s7_item in s7_items:
                 area = Area(s7_item.Area)
@@ -1152,7 +1156,7 @@ def _read_multi_vars_optimized(self, dict_items: List[dict[str, Any]]) -> Tuple[
             sorted_ri = sort_items(read_items)
             max_block = self._max_read_size()
             blocks = merge_items(sorted_ri, max_gap=self.multi_read_max_gap, max_block_size=max_block)
-            packets = packetize(blocks, self.pdu_length)
+            packets = packetize(blocks, self.pdu_length, max_items=self.MAX_VARS)
             self._opt_plan = _OptimizationPlan(cache_key, packets, read_items)
 
         # Deep-copy blocks from cached packets so we don't mutate cached state

snap7/optimizer.py

@@ -14,6 +14,10 @@
 
 logger = logging.getLogger(__name__)
 
+# Most address specifications a CPU accepts in one read request.  This is the
+# limit native Snap7 enforces as ``MaxVars``.
+MAX_ITEMS_PER_PDU = 20
+
 # Areas that support contiguous-block merging.  Counter (0x1C) and Timer
 # (0x1D) use element-based addressing, not byte-based, so merging them
 # as contiguous byte ranges would produce incorrect reads.
@@ -206,23 +210,31 @@ def _split_block(block: ReadBlock, max_block_size: int) -> list[ReadBlock]:
     return sub_blocks
 
 
-def packetize(blocks: list[ReadBlock], pdu_size: int) -> list[ReadPacket]:
+def packetize(blocks: list[ReadBlock], pdu_size: int, max_items: int = MAX_ITEMS_PER_PDU) -> list[ReadPacket]:
     """Pack blocks into PDU-sized packets.
 
-    Two budgets are enforced per packet:
+    Three limits are enforced per packet:
       - **Request budget**: ``12 (header) + 2 (func+count) + 12*N (address specs) <= pdu_size``
       - **Reply budget**: ``12 (header) + 2 (func+count) + sum(4 + ceil_even(length)) <= pdu_size``
+      - **Item count**: at most *max_items* blocks (address specs)
 
     Oversized blocks are first split at item boundaries, then blocks are
     greedily packed into packets.
 
     Args:
         blocks: Merged read blocks.
         pdu_size: Negotiated PDU size in bytes.
+        max_items: Maximum number of blocks per packet (default 20).
 
     Returns:
         List of ReadPackets.
+
+    Raises:
+        ValueError: If *max_items* is less than 1.
     """
+    if max_items < 1:
+        raise ValueError(f"max_items must be at least 1, got {max_items}")
+
     # First split any oversized blocks
     # Max data payload per block in a single-block packet
     max_single_block = pdu_size - 12 - 2 - 4  # header + param + data item header
@@ -248,8 +260,9 @@ def packetize(blocks: list[ReadBlock], pdu_size: int) -> list[ReadPacket]:
 
         fits_request = current_req_used + req_cost <= pdu_size
         fits_reply = current_reply_used + reply_cost <= pdu_size
+        fits_count = len(current_packet.blocks) < max_items
 
-        if current_packet.blocks and (not fits_request or not fits_reply):
+        if current_packet.blocks and (not fits_request or not fits_reply or not fits_count):
             # Start a new packet
             packets.append(current_packet)
             current_packet = ReadPacket()

tests/test_async_typed_access.py

@@ -119,6 +119,14 @@ async def test_symbolic_tag_not_supported(client: AsyncClient) -> None:
         await client.read_tag(tag)
     with pytest.raises(NotImplementedError):
         await client.write_tag(tag, 1)
+    with pytest.raises(NotImplementedError):
+        await client.read_tags(["DB1:400:INT", tag])
+
+
+async def test_read_tags_above_20(client: AsyncClient) -> None:
+    await client.db_write(1, 500, bytearray(range(25)))
+    tags = [f"DB1.DBB{500 + i}:BYTE" for i in range(25)]
+    assert await client.read_tags(tags) == list(range(25))
 
 
 async def test_is_alive(server: Server) -> None:

tests/test_client.py

@@ -1239,16 +1239,31 @@ class TestMaxVars:
     def test_max_vars_constant(self) -> None:
         assert Client.MAX_VARS == 20
 
-    def test_read_multi_vars_rejects_too_many(self) -> None:
+    def test_read_multi_vars_ctypes_rejects_too_many(self) -> None:
+        """The ctypes form keeps the MAX_VARS limit of the native API."""
         client = Client()
         client.connected = True
-        mock_conn = MagicMock()
-        client.connection = mock_conn
+        client.connection = MagicMock()
 
-        items = [{"area": Area.DB, "db_number": 1, "start": i, "size": 1} for i in range(21)]
+        items = (S7DataItem * 21)()
+        for item in items:
+            item.Area = Area.DB.value
         with pytest.raises(ValueError, match="Too many items.*21.*MAX_VARS.*20"):
             client.read_multi_vars(items)
 
+    def test_read_multi_vars_dicts_above_max(self) -> None:
+        """The dict form has no item limit when the optimizer is off either."""
+        client = Client()
+        client.connected = True
+        client.connection = MagicMock()
+        client.use_optimizer = False
+        client.read_area = MagicMock(return_value=bytearray(1))
+
+        items = [{"area": Area.DB, "db_number": 1, "start": i, "size": 1} for i in range(21)]
+        result_code, results = client.read_multi_vars(items)
+        assert result_code == 0
+        assert len(results) == 21
+
     def test_write_multi_vars_rejects_too_many(self) -> None:
         client = Client()
         client.connected = True

tests/test_optimizer.py

@@ -7,6 +7,7 @@
 from .conftest import get_free_tcp_port
 from ctypes import c_char
 from typing import TYPE_CHECKING
+from unittest.mock import patch
 
 import pytest
 
@@ -23,6 +24,7 @@
     packetize,
     extract_results,
 )
+from snap7.tags import Tag
 from snap7.type import Area, SrvArea
 
 
@@ -185,6 +187,18 @@ def test_reply_budget_limit(self) -> None:
         packets = packetize(blocks, pdu_size=240)
         assert len(packets) == 2
 
+    def test_item_count_limit(self) -> None:
+        # 45 two-byte blocks fit the byte budgets of a 960-byte PDU, but not the 20-item limit
+        blocks = [ReadBlock(area=0x84, db_number=i, start_offset=0, byte_length=2, items=[]) for i in range(45)]
+        packets = packetize(blocks, pdu_size=960)
+        assert [len(p.blocks) for p in packets] == [20, 20, 5]
+
+    def test_item_count_limit_is_configurable(self) -> None:
+        blocks = [ReadBlock(area=0x84, db_number=i, start_offset=0, byte_length=2, items=[]) for i in range(7)]
+        assert [len(p.blocks) for p in packetize(blocks, pdu_size=960, max_items=3)] == [3, 3, 1]
+        with pytest.raises(ValueError):
+            packetize(blocks, pdu_size=960, max_items=0)
+
     def test_oversized_block_split(self) -> None:
         # A block larger than pdu - overhead should be split at item boundaries
         items = [
@@ -284,6 +298,35 @@ def test_multi_read_basic(self) -> None:
         assert results[0] == bytearray(self.db1_data[0:4])
         assert results[1] == bytearray(self.db1_data[10:14])
 
+    def test_multi_read_above_20_blocks(self) -> None:
+        """26 reads that cannot be merged are spread over requests of at most 20 items."""
+        offsets = range(0, 100, 8)  # gaps of 7 bytes, wider than multi_read_max_gap
+        items = [{"area": Area.DB, "db_number": db, "start": o, "size": 1} for db in (1, 2) for o in offsets]
+        assert len(items) == 26
+        assert self.client.multi_read_max_gap < 7
+
+        build = self.client.protocol.build_multi_read_request
+        with patch.object(self.client.protocol, "build_multi_read_request", side_effect=build) as spy:
+            result_code, results = self.client.read_multi_vars(items)
+
+        assert result_code == 0
+        expected = [bytearray([o]) for o in offsets] + [bytearray([100 + o]) for o in offsets]
+        assert results == expected
+        spec_counts = [len(call.args[0]) for call in spy.call_args_list]
+        assert spec_counts and max(spec_counts) <= 20
+        assert sum(spec_counts) >= 25  # a lone block may go out as a plain read
+
+    def test_read_tags_above_20(self) -> None:
+        """read_tags accepts more than 20 tags and keeps their order."""
+        tags = [f"DB{db}.DBB{o}:BYTE" for db in (1, 2) for o in range(0, 100, 8)]
+        values = self.client.read_tags(tags)
+        assert values == list(range(0, 100, 8)) + [100 + o for o in range(0, 100, 8)]
+
+    def test_read_tags_refuses_symbolic(self) -> None:
+        tag = Tag(Area.DB, 1, 0, "INT", access_sequence=[1, 2])
+        with pytest.raises(NotImplementedError):
+            self.client.read_tags(["DB1.DBB0:BYTE", tag])
+
     def test_multi_read_different_dbs(self) -> None:
         """Read items from DB1 and DB2."""
         items = [