Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Test and correct receiving more than one packet #1965

Merged
merged 12 commits into from
Feb 4, 2024
Merged
2 changes: 1 addition & 1 deletion pymodbus/client/base.py
Original file line number Diff line number Diff line change
Expand Up @@ -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
)
Expand Down
21 changes: 11 additions & 10 deletions pymodbus/framer/socket_framer.py
Original file line number Diff line number Diff line change
Expand Up @@ -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}

Expand Down Expand Up @@ -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."""
Expand Down
28 changes: 23 additions & 5 deletions pymodbus/transport/stub.py
Original file line number Diff line number Diff line change
@@ -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
57 changes: 57 additions & 0 deletions test/test_network.py
Original file line number Diff line number Diff line change
@@ -1,4 +1,7 @@
"""Test transport."""
from __future__ import annotations

import asyncio

import pytest

Expand All @@ -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()