Test and correct receiving more than one packet (#1965)

Author: janiversen

pymodbus/client/base.py

@@ -157,13 +157,13 @@ async def async_execute(self, request=None):
 
         count = 0
         while count <= self.retries:
+            req = self.build_response(request.transaction_id)
             if not count or not self.no_resend_on_retry:
                 self.transport_send(packet)
             if self.broadcast_enable and not request.slave_id:
                 resp = b"Broadcast write sent - no response expected"
                 break
             try:
-                req = self.build_response(request.transaction_id)
                 resp = await asyncio.wait_for(
                     req, timeout=self.comm_params.timeout_connect
                 )

pymodbus/framer/socket_framer.py

@@ -77,7 +77,7 @@ def advanceFrame(self):
         it or determined that it contains an error. It also has to reset the
         current frame header handle
         """
-        length = self._hsize + self._header["len"]
+        length = self._hsize + self._header["len"] -1
         self._buffer = self._buffer[length:]
         self._header = {"tid": 0, "pid": 0, "len": 0, "uid": 0}
 
@@ -129,15 +129,16 @@ def frameProcessIncomingPacket(self, single, callback, slave, tid=None, **kwargs
         The processed and decoded messages are pushed to the callback
         function to process and send.
         """
-        if not self.checkFrame():
-            Log.debug("Frame check failed, ignoring!!")
-            return
-        if not self._validate_slave_id(slave, single):
-            header_txt = self._header["uid"]
-            Log.debug("Not a valid slave id - {}, ignoring!!", header_txt)
-            self.resetFrame()
-            return
-        self._process(callback, tid)
+        while True:
+            if not self.checkFrame():
+                Log.debug("Frame check failed, ignoring!!")
+                return
+            if not self._validate_slave_id(slave, single):
+                header_txt = self._header["uid"]
+                Log.debug("Not a valid slave id - {}, ignoring!!", header_txt)
+                self.resetFrame()
+                return
+            self._process(callback, tid)
 
     def _process(self, callback, tid, error=False):
         """Process incoming packets irrespective error condition."""

pymodbus/transport/stub.py

@@ -1,29 +1,47 @@
 """ModbusProtocol network stub."""
 from __future__ import annotations
 
-from pymodbus.transport.transport import ModbusProtocol
+from typing import Callable
+
+from pymodbus.transport.transport import CommParams, ModbusProtocol
 
 
 class ModbusProtocolStub(ModbusProtocol):
     """Protocol layer including transport."""
 
+    def __init__(
+        self,
+        params: CommParams,
+        is_server: bool,
+        handler: Callable[[bytes], bytes] | None = None,
+    ) -> None:
+        """Initialize a stub instance."""
+        self.stub_handle_data = handler if handler else self.dummy_handler
+        super().__init__(params, is_server)
+
+
     async def start_run(self):
         """Call need functions to start server/client."""
         if  self.is_server:
             return await self.transport_listen()
         return await self.transport_connect()
 
+
     def callback_data(self, data: bytes, addr: tuple | None = None) -> int:
         """Handle received data."""
         if (response := self.stub_handle_data(data)):
             self.transport_send(response)
         return len(data)
 
+    def callback_new_connection(self) -> ModbusProtocol:
+        """Call when listener receive new connection request."""
+        new_stub = ModbusProtocolStub(self.comm_params, False)
+        new_stub.stub_handle_data = self.stub_handle_data
+        return new_stub
+
     # ---------------- #
     # external methods #
     # ---------------- #
-    def stub_handle_data(self, data: bytes) -> bytes | None:
+    def dummy_handler(self, data: bytes) -> bytes | None:
         """Handle received data."""
-        if len(data) > 5:
-            return data
-        return None
+        return data

test/test_network.py

@@ -1,4 +1,7 @@
 """Test transport."""
+from __future__ import annotations
+
+import asyncio
 
 import pytest
 
@@ -24,5 +27,59 @@ async def test_stub(self, use_port, use_cls):
         stub = ModbusProtocolStub(use_cls, True)
         assert await stub.start_run()
         assert await client.connect()
+        test_data = b"Data got echoed."
+        client.transport.write(test_data)
+        client.transport_close()
+        stub.transport_close()
+
+    async def test_double_packet(self, use_port, use_cls):
+        """Test double packet on network."""
+        old_data = b''
+        client = AsyncModbusTcpClient(NULLMODEM_HOST, port=use_port, retries=0)
+
+        def local_handle_data(data: bytes) -> bytes | None:
+            """Handle server side for this test case."""
+            nonlocal old_data
+
+            addr = int(data[9])
+            response = data[0:5] + b'\x05\x00\x03\x02\x00' + (addr*10).to_bytes(1, 'big')
+
+            # 1, 4, 7 return correct data
+            # 2, 5 return NO data
+            # 3 return 2 + 3
+            # 6 return 6 + 6 (wrong order
+            # 8 return 7 + half 8
+            # 9 return second half 8 + 9
+            if addr in {2, 5}:
+                old_data = response
+                response = None
+            elif addr == 3:
+                response = old_data + response
+                old_data = b''
+            elif addr == 6:
+                response = response + old_data
+                old_data = b''
+            elif addr == 8:
+                x =  response
+                response = response[:7]
+                old_data = x[7:]
+            elif addr == 9:
+                response = old_data + response
+                old_data = b''
+            return response
+
+        async def local_call(addr: int) -> bool:
+            """Call read_holding_register and control."""
+            nonlocal client
+            reply = await client.read_holding_registers(address=addr, count=1)
+            assert not reply.isError(), f"addr {addr} isError"
+            assert reply.registers[0] == addr * 10, f"addr {addr} value"
+
+        stub = ModbusProtocolStub(use_cls, True, handler=local_handle_data)
+        stub.stub_handle_data = local_handle_data
+        await stub.start_run()
+
+        assert await client.connect()
+        await asyncio.gather(*[local_call(x) for x in range(1, 10)])
         client.transport_close()
         stub.transport_close()