frameProcessIncomingPacket removed (#2355)

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

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,27 +104,39 @@ 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
         if self._buffer == b'':
             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.

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.
 

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?

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?

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:

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