From 55f65094b525d8d818b9f5f565e2e8c36ea96db4 Mon Sep 17 00:00:00 2001 From: jan iversen Date: Mon, 7 Oct 2024 13:45:58 +0200 Subject: [PATCH] frameProcessIncomingPacket removed (#2355) --- pymodbus/framer/old_framer_ascii.py | 24 ----------- pymodbus/framer/old_framer_base.py | 61 ++++++++++++++-------------- pymodbus/framer/old_framer_rtu.py | 19 --------- pymodbus/framer/old_framer_socket.py | 38 ----------------- pymodbus/framer/old_framer_tls.py | 21 ---------- pymodbus/server/async_io.py | 3 -- test/sub_current/test_transaction.py | 22 ---------- 7 files changed, 30 insertions(+), 158 deletions(-) diff --git a/pymodbus/framer/old_framer_ascii.py b/pymodbus/framer/old_framer_ascii.py index 753bc4633..61215d18e 100644 --- a/pymodbus/framer/old_framer_ascii.py +++ b/pymodbus/framer/old_framer_ascii.py @@ -1,7 +1,5 @@ """Ascii_framer.""" -from pymodbus.exceptions import ModbusIOException from pymodbus.framer.old_framer_base import BYTE_ORDER, FRAME_HEADER, ModbusFramer -from pymodbus.logging import Log from .ascii import FramerAscii @@ -37,25 +35,3 @@ def __init__(self, decoder, client=None): self._start = b":" self._end = b"\r\n" self.message_handler = FramerAscii(decoder, [0]) - - def frameProcessIncomingPacket(self, single, callback, slave, tid=None): - """Process new packet pattern.""" - while len(self._buffer): - used_len, data = self.message_handler.decode(self._buffer) - if not data: - if not used_len: - return - self._buffer = self._buffer[used_len :] - continue - self.dev_id = self.message_handler.incoming_dev_id - if not self._validate_slave_id(slave, single): - Log.error("Not a valid slave id - {}, ignoring!!", self.message_handler.incoming_dev_id) - self.resetFrame() - return - - if (result := self.decoder.decode(data)) is None: - raise ModbusIOException("Unable to decode response") - self.populateResult(result) - self._buffer = self._buffer[used_len :] - self.dev_id = 0 - callback(result) # defer this diff --git a/pymodbus/framer/old_framer_base.py b/pymodbus/framer/old_framer_base.py index 1b425f6f9..85509a1fd 100644 --- a/pymodbus/framer/old_framer_base.py +++ b/pymodbus/framer/old_framer_base.py @@ -1,10 +1,10 @@ """Framer start.""" -# pylint: disable=missing-type-doc from __future__ import annotations import time from typing import TYPE_CHECKING +from pymodbus.exceptions import ModbusIOException from pymodbus.factory import ClientDecoder, ServerDecoder from pymodbus.framer.base import FramerBase from pymodbus.logging import Log @@ -44,16 +44,14 @@ def __init__( self.tid = 0 self.dev_id = 0 - def _validate_slave_id(self, slaves: list, single: bool) -> bool: + def _validate_slave_id(self, slaves: list) -> bool: """Validate if the received data is valid for the client. :param slaves: list of slave id for which the transaction is valid :param single: Set to true to treat this as a single context :return: """ - if single: - return True - if 0 in slaves or 0xFF in slaves: + if not slaves or 0 in slaves or 0xFF in slaves: # Handle Modbus TCP slave identifier (0x00 0r 0xFF) # in asynchronous requests return True @@ -95,18 +93,7 @@ def resetFrame(self): self.dev_id = 0 self.tid = 0 - def populateResult(self, result): - """Populate the modbus result header. - - The serial packets do not have any header information - that is copied. - - :param result: The response packet - """ - result.slave_id = self.dev_id - result.transaction_id = self.tid - - def processIncomingPacket(self, data: bytes, callback, slave, single=False, tid=None): + def processIncomingPacket(self, data: bytes, callback, slave, tid=None): """Process new packet pattern. This takes in a new request packet, adds it to the current @@ -117,14 +104,6 @@ def processIncomingPacket(self, data: bytes, callback, slave, single=False, tid= The processed and decoded messages are pushed to the callback function to process and send. - - :param data: The new packet data - :param callback: The function to send results to - :param slave: Process if slave id matches, ignore otherwise (could be a - list of slave ids (server) or single slave id(client/server)) - :param single: multiple slave ? - :param tid: transaction id - :raises ModbusIOException: """ Log.debug("Processing: {}", data, ":hex") self._buffer += data @@ -132,12 +111,32 @@ def processIncomingPacket(self, data: bytes, callback, slave, single=False, tid= return if not isinstance(slave, (list, tuple)): slave = [slave] - self.frameProcessIncomingPacket(single, callback, slave, tid=tid) - - def frameProcessIncomingPacket( - self, _single, _callback, _slave, tid=None - ) -> None: - """Process new packet pattern.""" + while True: + if self._buffer == b'': + return + used_len, data = self.message_handler.decode(self._buffer) + self.dev_id = self.message_handler.incoming_dev_id + if used_len: + self._buffer = self._buffer[used_len:] + if not data: + return + self.dev_id = self.message_handler.incoming_dev_id + self.tid = self.message_handler.incoming_tid + if not self._validate_slave_id(slave): + Log.debug("Not a valid slave id - {}, ignoring!!", self.message_handler.incoming_dev_id) + self.resetFrame() + continue + if (result := self.decoder.decode(data)) is None: + self.resetFrame() + raise ModbusIOException("Unable to decode request") + result.slave_id = self.dev_id + result.transaction_id = self.tid + Log.debug("Frame advanced, resetting header!!") + self._buffer = self._buffer[used_len:] + if tid and result.transaction_id and tid != result.transaction_id: + self.resetFrame() + else: + callback(result) # defer or push to a thread? def buildPacket(self, message: ModbusRequest | ModbusResponse) -> bytes: """Create a ready to send modbus packet. diff --git a/pymodbus/framer/old_framer_rtu.py b/pymodbus/framer/old_framer_rtu.py index c7c65a0ea..68012eb7a 100644 --- a/pymodbus/framer/old_framer_rtu.py +++ b/pymodbus/framer/old_framer_rtu.py @@ -1,7 +1,6 @@ """RTU framer.""" import time -from pymodbus.exceptions import ModbusIOException from pymodbus.framer.old_framer_base import BYTE_ORDER, FRAME_HEADER, ModbusFramer from pymodbus.framer.rtu import FramerRTU from pymodbus.logging import Log @@ -55,24 +54,6 @@ def __init__(self, decoder, client=None): super().__init__(decoder, client) self.message_handler: FramerRTU = FramerRTU(self.decoder, [0]) - def frameProcessIncomingPacket(self, _single, callback, _slave, tid=None): - """Process new packet pattern.""" - while True: - if self._buffer == b'': - break - used_len, data = self.message_handler.decode(self._buffer) - self.dev_id = self.message_handler.incoming_dev_id - if used_len: - self._buffer = self._buffer[used_len:] - if not data: - break - if (result := self.decoder.decode(data)) is None: - raise ModbusIOException("Unable to decode request") - result.slave_id = self.dev_id - result.transaction_id = 0 - Log.debug("Frame advanced, resetting header!!") - callback(result) # defer or push to a thread? - def sendPacket(self, message: bytes) -> int: """Send packets on the bus with 3.5char delay between frames. diff --git a/pymodbus/framer/old_framer_socket.py b/pymodbus/framer/old_framer_socket.py index 520be9cdd..8923ccb38 100644 --- a/pymodbus/framer/old_framer_socket.py +++ b/pymodbus/framer/old_framer_socket.py @@ -1,11 +1,7 @@ """Socket framer.""" -from pymodbus.exceptions import ( - ModbusIOException, -) from pymodbus.framer.old_framer_base import ModbusFramer from pymodbus.framer.socket import FramerSocket -from pymodbus.logging import Log # --------------------------------------------------------------------------- # @@ -40,37 +36,3 @@ def __init__(self, decoder, client=None): super().__init__(decoder, client) self._hsize = 0x07 self.message_handler = FramerSocket(decoder, [0]) - - def frameProcessIncomingPacket(self, single, callback, slave, tid=None): - """Process new packet pattern. - - This takes in a new request packet, adds it to the current - packet stream, and performs framing on it. That is, checks - for complete messages, and once found, will process all that - exist. This handles the case when we read N + 1 or 1 // N - messages at a time instead of 1. - - The processed and decoded messages are pushed to the callback - function to process and send. - """ - while True: - if self._buffer == b'': - return - used_len, data = self.message_handler.decode(self._buffer) - if not data: - return - self.dev_id = self.message_handler.incoming_dev_id - self.tid = self.message_handler.incoming_tid - if not self._validate_slave_id(slave, single): - Log.debug("Not a valid slave id - {}, ignoring!!", self.message_handler.incoming_dev_id) - self.resetFrame() - return - if (result := self.decoder.decode(data)) is None: - self.resetFrame() - raise ModbusIOException("Unable to decode request") - self.populateResult(result) - self._buffer: bytes = self._buffer[used_len:] - if tid and tid != result.transaction_id: - self.resetFrame() - else: - callback(result) # defer or push to a thread? diff --git a/pymodbus/framer/old_framer_tls.py b/pymodbus/framer/old_framer_tls.py index 196ca7627..6cb4a3e23 100644 --- a/pymodbus/framer/old_framer_tls.py +++ b/pymodbus/framer/old_framer_tls.py @@ -1,8 +1,5 @@ """TLS framer.""" -from pymodbus.exceptions import ( - ModbusIOException, -) from pymodbus.framer.old_framer_base import ModbusFramer from pymodbus.framer.tls import FramerTLS @@ -31,21 +28,3 @@ def __init__(self, decoder, client=None): super().__init__(decoder, client) self._hsize = 0x0 self.message_handler = FramerTLS(decoder, [0]) - - def frameProcessIncomingPacket(self, _single, callback, _slave, tid=None): - """Process new packet pattern.""" - # no slave id for Modbus Security Application Protocol - - while True: - used_len, data = self.message_handler.decode(self._buffer) - if not data: - return - self.dev_id = self.message_handler.incoming_dev_id - self.tid = self.message_handler.incoming_tid - - if (result := self.decoder.decode(data)) is None: - self.resetFrame() - raise ModbusIOException("Unable to decode request") - self.populateResult(result) - self._buffer: bytes = self._buffer[used_len:] - callback(result) # defer or push to a thread? diff --git a/pymodbus/server/async_io.py b/pymodbus/server/async_io.py index ad1dc7758..40aaca12d 100644 --- a/pymodbus/server/async_io.py +++ b/pymodbus/server/async_io.py @@ -122,13 +122,10 @@ async def inner_handle(self): slaves.append(0) Log.debug("Handling data: {}", data, ":hex") - - single = self.server.context.single self.framer.processIncomingPacket( data=data, callback=lambda x: self.execute(x, *addr), slave=slaves, - single=single, ) async def handle(self) -> None: diff --git a/test/sub_current/test_transaction.py b/test/sub_current/test_transaction.py index 54bec29cc..0a647ff01 100755 --- a/test/sub_current/test_transaction.py +++ b/test/sub_current/test_transaction.py @@ -302,11 +302,6 @@ def callback(data): expected.slave_id = 0xFF msg = b"\x00\x01\x12\x34\x00\x06\xff\x02\x12\x34\x01\x02" self._tcp.processIncomingPacket(msg, callback, [0, 1]) - # assert self._tcp.checkFrame() - # actual = ModbusRequest() - # self._tcp.populateResult(actual) - # for name in ("transaction_id", "protocol_id", "slave_id"): - # assert getattr(expected, name) == getattr(actual, name) @mock.patch.object(ModbusRequest, "encode") def test_tcp_framer_packet(self, mock_encode): @@ -627,23 +622,6 @@ def callback(data): self._ascii.processIncomingPacket(msg_parts[1], callback, [0,1]) assert result - def test_ascii_framer_populate(self): - """Test a ascii frame packet build.""" - request = ModbusRequest(0, 0, False) - self._ascii.populateResult(request) - assert not request.slave_id - - @mock.patch.object(ModbusRequest, "encode") - def test_ascii_framer_packet(self, mock_encode): - """Test a ascii frame packet build.""" - message = ModbusRequest(0, 0, False) - message.slave_id = 0xFF - message.function_code = 0x01 - expected = b":FF0100\r\n" - mock_encode.return_value = b"" - actual = self._ascii.buildPacket(message) - assert expected == actual - def test_ascii_process_incoming_packets(self): """Test ascii process incoming packet.""" count = 0