Allow socket frames to be split in multiple packets (#1923)

Author: janiversen

pymodbus/framer/socket_framer.py

@@ -52,20 +52,21 @@ def checkFrame(self):
 
         Return true if we were successful.
         """
-        if self.isFrameReady():
-            (
-                self._header["tid"],
-                self._header["pid"],
-                self._header["len"],
-                self._header["uid"],
-            ) = struct.unpack(">HHHB", self._buffer[0 : self._hsize])
-
-            # someone sent us an error? ignore it
-            if self._header["len"] < 2:
-                self.advanceFrame()
-            # we have at least a complete message, continue
-            elif len(self._buffer) - self._hsize + 1 >= self._header["len"]:
-                return True
+        if not self.isFrameReady():
+            return False
+        (
+            self._header["tid"],
+            self._header["pid"],
+            self._header["len"],
+            self._header["uid"],
+        ) = struct.unpack(">HHHB", self._buffer[0 : self._hsize])
+
+        # someone sent us an error? ignore it
+        if self._header["len"] < 2:
+            self.advanceFrame()
+        # we have at least a complete message, continue
+        elif len(self._buffer) - self._hsize + 1 >= self._header["len"]:
+            return True
         # we don't have enough of a message yet, wait
         return False
 
@@ -128,23 +129,15 @@ def frameProcessIncomingPacket(self, single, callback, slave, tid=None, **kwargs
         The processed and decoded messages are pushed to the callback
         function to process and send.
         """
-        while True:
-            if not self.isFrameReady():
-                if len(self._buffer):
-                    # Possible error ???
-                    if self._header["len"] < 2:
-                        self._process(callback, tid, error=True)
-                break
-            if not self.checkFrame():
-                Log.debug("Frame check failed, ignoring!!")
-                self.resetFrame()
-                continue
-            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()
-                continue
-            self._process(callback, tid)
+        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."""

test/test_framers.py

@@ -8,9 +8,12 @@
 from pymodbus.client.base import ModbusBaseClient
 from pymodbus.exceptions import ModbusIOException
 from pymodbus.factory import ClientDecoder
-from pymodbus.framer.ascii_framer import ModbusAsciiFramer
-from pymodbus.framer.binary_framer import ModbusBinaryFramer
-from pymodbus.framer.rtu_framer import ModbusRtuFramer
+from pymodbus.framer import (
+    ModbusAsciiFramer,
+    ModbusBinaryFramer,
+    ModbusRtuFramer,
+    ModbusSocketFramer,
+)
 from pymodbus.transport import CommType
 from pymodbus.utilities import ModbusTransactionState
 
@@ -26,6 +29,10 @@ def fixture_rtu_framer():
     """RTU framer."""
     return ModbusRtuFramer(ClientDecoder())
 
+@pytest.fixture(name="socket_framer")
+def fixture_socket_framer():
+    """Socket framer."""
+    return ModbusSocketFramer(ClientDecoder())
 
 @pytest.fixture(name="ascii_framer")
 def fixture_ascii_framer():
@@ -359,3 +366,24 @@ def test_decode_ascii_data(ascii_framer, data):
         assert data.get("fcode") == 1
     else:
         assert not data
+
+def test_recv_split_packet():
+    """Test receive packet."""
+    response_ok = False
+
+    def _handle_response(_reply):
+        """Handle response."""
+        nonlocal response_ok
+        response_ok = True
+
+    message = bytearray(b"\x00\x01\x00\x00\x00\x0b\x01\x03\x08\x00\xb5\x12\x2f\x37\x21\x00\x03")
+    for i in range(0, len(message)):
+        part1 = message[:i]
+        part2 = message[i:]
+        response_ok = False
+        framer = ModbusSocketFramer(ClientDecoder())
+        if i:
+            framer.processIncomingPacket(part1, _handle_response, slave=0)
+            assert not response_ok, "Response should not be accepted"
+        framer.processIncomingPacket(part2, _handle_response, slave=0)
+        assert response_ok, "Response is valid, but not accepted"