From 9b67640442cdff3ec572cc4f42e125372a49f336 Mon Sep 17 00:00:00 2001 From: jan iversen Date: Sun, 4 Feb 2024 16:16:20 +0100 Subject: [PATCH] Test and correct receiving more than one packet (#1965) --- pymodbus/client/base.py | 2 +- pymodbus/framer/socket_framer.py | 21 ++++++------ pymodbus/transport/stub.py | 28 +++++++++++++--- test/test_network.py | 57 ++++++++++++++++++++++++++++++++ 4 files changed, 92 insertions(+), 16 deletions(-) diff --git a/pymodbus/client/base.py b/pymodbus/client/base.py index 189ab61b8..f24638b8f 100644 --- a/pymodbus/client/base.py +++ b/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 ) diff --git a/pymodbus/framer/socket_framer.py b/pymodbus/framer/socket_framer.py index 7b0a2dcaa..4acf23356 100644 --- a/pymodbus/framer/socket_framer.py +++ b/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.""" diff --git a/pymodbus/transport/stub.py b/pymodbus/transport/stub.py index e41b49748..2b81b8234 100644 --- a/pymodbus/transport/stub.py +++ b/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 diff --git a/test/test_network.py b/test/test_network.py index bed404ab6..eb3f59475 100644 --- a/test/test_network.py +++ b/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()