pymodbus-dev__pymodbus-1923

The user is reporting an issue where `pymodbus` fails to reassemble TCP segments that are split over multiple frames.
Specifically, the user provided logs showing that a Modbus response is split into two frames.
- First frame: `0x0 0x1 0x0 0x0 0x0 0xb 0x1 0x3 0x8 0x0 0xb5 0x12 0x2f 0x37 0x21 0x0`
- Second frame: `0x3`

The logs indicate:
1. The first frame is processed: `Processing: 0x0 0x1 0x0 0x0 0x0 0xb 0x1 0x3 0x8 0x0 0xb5 0x12 0x2f 0x37 0x21 0x0`
2. `Frame check failed, ignoring!!`
3. Then the second frame is processed: `Processing: 0x3`
4. `Factory Response[ReadHoldingRegistersResponse': 3]`
5. `General exception: index out of range`
6. `pymodbus.exceptions.ModbusIOException: Modbus Error: [Input/Output] Unable to decode request`

The breakdown of the Modbus TCP frame (MBAP header):
- Transaction Identifier: 2 bytes
- Protocol Identifier: 2 bytes
- Length: 2 bytes
- Unit Identifier: 1 byte
- PDU: variable length

Looking at the first frame: `0x0 0x1 0x0 0x0 0x0 0xb 0x1 0x3 0x8 0x0 0xb5 0x12 0x2f 0x37 0x21 0x0`
- Transaction ID: `0x00 0x01`
- Protocol ID: `0x00 0x00`
- Length: `0x00 0x0b` (11 bytes)
- Unit ID: `0x01`
- PDU: `0x03 0x08 0x00 0xb5 0x12 0x2f 0x37 0x21 0x0` (9 bytes)

Wait, if length is 11, then Unit ID (1) + PDU (10) = 11.
The PDU should be 10 bytes.
The first frame has Unit ID (1) + 9 bytes of PDU.
Total bytes after length field = 1 + 9 = 10. But length was 11.
So 1 byte is missing.
The second frame is `0x3`.
If the second frame is appended to the first, we have:
Unit ID (1) + PDU (10 bytes: `0x03 0x08 0x00 0xb5 0x12 0x2f 0x37 0x21 0x0 0x3`) = 11 bytes.
This matches the length.

The problem is that `pymodbus` is trying to process the first frame as a complete frame and failing the frame check, instead of waiting for the remaining byte.

From the traceback:
```
  File "/home/simon/tools/pymodbus/pymodbus/transport/transport.py", line 352, in datagram_received
    cut = self.callback_data(self.recv_buffer, addr=addr)
  File "/home/simon/tools/pymodbus/pymodbus/client/base.py", line 186, in callback_data
    self.framer.processIncomingPacket(data, self._handle_response, slave=0)
  File "/home/simon/tools/pymodbus/pymodbus/framer/base.py", line 139, in processIncomingPacket
    self.frameProcessIncomingPacket(single, callback, slave, **kwargs)
  File "/home/simon/tools/pymodbus/pymodbus/framer/socket_framer.py", line 136, in frameProcessIncomingPacket
    self._process(callback, tid, error=True)
  File "/home/simon/tools/pymodbus/pymodbus/framer/socket_framer.py", line 154, in _process
    raise ModbusIOException("Unable to decode request")
```

It seems `socket_framer.py` is responsible for this.

Let's locate the relevant files.
`pymodbus/framer/socket_framer.py`
`pymodbus/framer/base.py`
`pymodbus/client/base.py`

I'll start by examining `pymodbus/framer/socket_framer.py`.

{
  "command": "grep -rn --include=\"*.py\" -e \"processIncomingPacket\" pymodbus/framer/"
}

{
  "status": "ok",
  "stdout": "pymodbus/framer/base.py:115:    def processIncomingPacket(self, data, callback, slave, **kwargs):\n",
  "stderr": "",
  "exit_code": 0
}

Wait, `grep` only found it in `base.py`. But the traceback mentioned `socket_framer.py:136` calling `frameProcessIncomingPacket`.
Ah, `processIncomingPacket` seems to be a wrapper that calls `frameProcessIncomingPacket`.

Let's check `pymodbus/framer/socket_framer.py`.

{
  "command": "grep -rn --include=\"*.py\" -e \"frameProcessIncomingPacket\" pymodbus/framer/"
}

{
  "status": "ok",
  "stdout": "pymodbus/framer/binary_framer.py:121:    def frameProcessIncomingPacket(self, single, callback, slave, _tid=None, **kwargs):\npymodbus/framer/base.py:139:        self.frameProcessIncomingPacket(single, callback, slave, **kwargs)\npymodbus/framer/base.py:141:    def frameProcessIncomingPacket(\npymodbus/framer/socket_framer.py:119:    def frameProcessIncomingPacket(self, single, callback, slave, tid=None, **kwargs):\npymodbus/framer/tls_framer.py:91:    def frameProcessIncomingPacket(self, single, callback, slave, _tid=None, **kwargs):\npymodbus/framer/ascii_framer.py:111:    def frameProcessIncomingPacket(self, single, callback, slave, _tid=None, **kwargs):\npymodbus/framer/rtu_framer.py:208:    def frameProcessIncomingPacket(self, single, callback, slave, _tid=None, **kwargs):\n",
  "stderr": "",
  "exit_code": 0
}

Okay, `socket_framer.py` implements `frameProcessIncomingPacket`.
Let's read `pymodbus/framer/socket_framer.py`.

{
  "filepath": "pymodbus/framer/socket_framer.py"
}

{
  "status": "ok",
  "filepath": "pymodbus/framer/socket_framer.py",
  "content": "\"\"\"Socket framer.\"\"\"\n# pylint: disable=missing-type-doc\nimport struct\n\nfrom pymodbus.exceptions import (\n    InvalidMessageReceivedException,\n    ModbusIOException,\n)\nfrom pymodbus.framer.base import SOCKET_FRAME_HEADER, ModbusFramer\nfrom pymodbus.logging import Log\n\n\n# --------------------------------------------------------------------------- #\n# Modbus TCP Message\n# --------------------------------------------------------------------------- #\n\n\nclass ModbusSocketFramer(ModbusFramer):\n    \"\"\"Modbus Socket Frame controller.\n\n    Before each modbus TCP message is an MBAP header which is used as a\n    message frame.  It allows us to easily separate messages as follows::\n\n        [         MBAP Header         ] [ Function Code] [ Data ] \\\n        [ tid ][ pid ][ length ][ uid ]\n          2b     2b     2b        1b           1b           Nb\n\n        while len(message) > 0:\n            tid, pid, length`, uid = struct.unpack(\">HHHB\", message)\n            request = message[0:7 + length - 1`]\n            message = [7 + length - 1:]\n\n        * length = uid + function code + data\n        * The -1 is to account for the uid byte\n    \"\"\"\n\n    method = \"socket\"\n\n    def __init__(self, decoder, client=None):\n        \"\"\"Initialize a new instance of the framer.\n\n        :param decoder: The decoder factory implementation to use\n        \"\"\"\n        super().__init__(decoder, client)\n        self._hsize = 0x07\n\n    # ----------------------------------------------------------------------- #\n    # Private Helper Functions\n    # ----------------------------------------------------------------------- #\n    def checkFrame(self):\n        \"\"\"Check and decode the next frame.\n\n        Return true if we were successful.\n        \"\"\"\n        if self.isFrameReady():\n            (\n                self._header[\"tid\"],\n                self._header[\"pid\"],\n                self._header[\"len\"],\n                self._header[\"uid\"],\n            ) = struct.unpack(\">HHHB\", self._buffer[0 : self._hsize])\n\n            # someone sent us an error? ignore it\n            if self._header[\"len\"] < 2:\n                self.advanceFrame()\n            # we have at least a complete message, continue\n            elif len(self._buffer) - self._hsize + 1 >= self._header[\"len\"]:\n                return True\n        # we don't have enough of a message yet, wait\n        return False\n\n    def advanceFrame(self):\n        \"\"\"Skip over the current framed message.\n\n        This allows us to skip over the current message after we have processed\n        it or determined that it contains an error. It also has to reset the\n        current frame header handle\n        \"\"\"\n        length = self._hsize + self._header[\"len\"] - 1\n        self._buffer = self._buffer[length:]\n        self._header = {\"tid\": 0, \"pid\": 0, \"len\": 0, \"uid\": 0}\n\n    def isFrameReady(self):\n        \"\"\"Check if we should continue decode logic.\n\n        This is meant to be used in a while loop in the decoding phase to let\n        the decoder factory know that there is still data in the buffer.\n\n        :returns: True if ready, False otherwise\n        \"\"\"\n        return len(self._buffer) > self._hsize\n\n    def getFrame(self):\n        \"\"\"Return the next frame from the buffered data.\n\n        :returns: The next full frame buffer\n        \"\"\"\n        length = self._hsize + self._header[\"len\"] - 1\n        return self._buffer[self._hsize : length]\n\n    # ----------------------------------------------------------------------- #\n    # Public Member Functions\n    # ----------------------------------------------------------------------- #\n    def decode_data(self, data):\n        \"\"\"Decode data.\"\"\"\n        if len(data) > self._hsize:\n            tid, pid, length, uid, fcode = struct.unpack(\n                SOCKET_FRAME_HEADER, data[0 : self._hsize + 1]\n            )\n            return {\n                \"tid\": tid,\n                \"pid\": pid,\n                \"length\": length,\n                \"slave\": uid,\n                \"fcode\": fcode,\n            }\n        return {}\n\n    def frameProcessIncomingPacket(self, single, callback, slave, tid=None, **kwargs):\n        \"\"\"Process new packet pattern.\n\n        This takes in a new request packet, adds it to the current\n        packet stream, and performs framing on it. That is, checks\n        for complete messages, and once found, will process all that\n        exist.  This handles the case when we read N + 1 or 1 // N\n        messages at a time instead of 1.\n\n        The processed and decoded messages are pushed to the callback\n        function to process and send.\n        \"\"\"\n        while True:\n            if not self.isFrameReady():\n                if len(self._buffer):\n                    # Possible error ???\n                    if self._header[\"len\"] < 2:\n                        self._process(callback, tid, error=True)\n                break\n            if not self.checkFrame():\n                Log.debug(\"Frame check failed, ignoring!!\")\n                self.resetFrame()\n                continue\n            if not self._validate_slave_id(slave, single):\n                header_txt = self._header[\"uid\"]\n                Log.debug(\"Not a valid slave id - {}, ignoring!!\", header_txt)\n                self.resetFrame()\n                continue\n            self._process(callback, tid)\n\n    def _process(self, callback, tid, error=False):\n        \"\"\"Process incoming packets irrespective error condition.\"\"\"",
  "start_line": 1,
  "end_line": 150,
  "total_lines": 182,
  "is_truncated": true
}

Let's read the rest of `pymodbus/framer/socket_framer.py` to see `_process` and `resetFrame`.

{
  "filepath": "pymodbus/framer/socket_framer.py",
  "start_line": 151
}

{
  "status": "ok",
  "filepath": "pymodbus/framer/socket_framer.py",
  "content": "        data = self._buffer if error else self.getFrame()\n        if (result := self.decoder.decode(data)) is None:\n            self.resetFrame()\n            raise ModbusIOException(\"Unable to decode request\")\n        if error and result.function_code < 0x80:\n            raise InvalidMessageReceivedException(result)\n        self.populateResult(result)\n        self.advanceFrame()\n        if tid and tid != result.transaction_id:\n            self.resetFrame()\n        else:\n            callback(result)  # defer or push to a thread?\n\n    def buildPacket(self, message):\n        \"\"\"Create a ready to send modbus packet.\n\n        :param message: The populated request/response to send\n        \"\"\"\n        data = message.encode()\n        packet = struct.pack(\n            SOCKET_FRAME_HEADER,\n            message.transaction_id,\n            message.protocol_id,\n            len(data) + 2,\n            message.slave_id,\n            message.function_code,\n        )\n        packet += data\n        return packet\n\n\n# __END__",
  "start_line": 151,
  "end_line": 182,
  "total_lines": 182,
  "is_truncated": false
}

Wait, I missed `resetFrame` in the `read_file` output? Let me check again.
It's not in the `read_file` output. Let me see if it's in the base class.

{
  "filepath": "pymodbus/framer/base.py"
}

{
  "status": "ok",
  "filepath": "pymodbus/framer/base.py",
  "content": "\"\"\"Framer start.\"\"\"\n# pylint: disable=missing-type-doc\nfrom __future__ import annotations\n\nfrom typing import Any\n\nfrom pymodbus.factory import ClientDecoder, ServerDecoder\nfrom pymodbus.logging import Log\n\n\n# Unit ID, Function Code\nBYTE_ORDER = \">\"\nFRAME_HEADER = \"BB\"\n\n# Transaction Id, Protocol ID, Length, Unit ID, Function Code\nSOCKET_FRAME_HEADER = BYTE_ORDER + \"HHH\" + FRAME_HEADER\n\n# Function Code\nTLS_FRAME_HEADER = BYTE_ORDER + \"B\"\n\n\nclass ModbusFramer:\n    \"\"\"Base Framer class.\"\"\"\n\n    name = \"\"\n\n    def __init__(\n        self,\n        decoder: ClientDecoder | ServerDecoder,\n        client=None,\n    ) -> None:\n        \"\"\"Initialize a new instance of the framer.\n\n        :param decoder: The decoder implementation to use\n        \"\"\"\n        self.decoder = decoder\n        self.client = client\n        self._header: dict[str, Any] = {\n            \"lrc\": \"0000\",\n            \"len\": 0,\n            \"uid\": 0x00,\n            \"tid\": 0,\n            \"pid\": 0,\n            \"crc\": b\"\\x00\\x00\",\n        }\n        self._buffer = b\"\"\n\n    def _validate_slave_id(self, slaves: list, single: bool) -> bool:\n        \"\"\"Validate if the received data is valid for the client.\n\n        :param slaves: list of slave id for which the transaction is valid\n        :param single: Set to true to treat this as a single context\n        :return:\n        \"\"\"\n        if single:\n            return True\n        if 0 in slaves or 0xFF in slaves:\n            # Handle Modbus TCP slave identifier (0x00 0r 0xFF)\n            # in asynchronous requests\n            return True\n        return self._header[\"uid\"] in slaves\n\n    def sendPacket(self, message):\n        \"\"\"Send packets on the bus.\n\n        With 3.5char delay between frames\n        :param message: Message to be sent over the bus\n        :return:\n        \"\"\"\n        return self.client.send(message)\n\n    def recvPacket(self, size):\n        \"\"\"Receive packet from the bus.\n\n        With specified len\n        :param size: Number of bytes to read\n        :return:\n        \"\"\"\n        return self.client.recv(size)\n\n    def resetFrame(self):\n        \"\"\"Reset the entire message frame.\n\n        This allows us to skip ovver errors that may be in the stream.\n        It is hard to know if we are simply out of sync or if there is\n        an error in the stream as we have no way to check the start or\n        end of the message (python just doesn't have the resolution to\n        check for millisecond delays).\n        \"\"\"\n        Log.debug(\n            \"Resetting frame - Current Frame in buffer - {}\", self._buffer, \":hex\"\n        )\n        self._buffer = b\"\"\n        self._header = {\n            \"lrc\": \"0000\",\n            \"crc\": b\"\\x00\\x00\",\n            \"len\": 0,\n            \"uid\": 0x00,\n            \"pid\": 0,\n            \"tid\": 0,\n        }\n\n    def populateResult(self, result):\n        \"\"\"Populate the modbus result header.\n\n        The serial packets do not have any header information\n        that is copied.\n\n        :param result: The response packet\n        \"\"\"\n        result.slave_id = self._header.get(\"uid\", 0)\n        result.transaction_id = self._header.get(\"tid\", 0)\n        result.protocol_id = self._header.get(\"pid\", 0)\n\n    def processIncomingPacket(self, data, callback, slave, **kwargs):\n        \"\"\"Process new packet pattern.\n\n        This takes in a new request packet, adds it to the current\n        packet stream, and performs framing on it. That is, checks\n        for complete messages, and once found, will process all that\n        exist.  This handles the case when we read N + 1 or 1 // N\n        messages at a time instead of 1.\n\n        The processed and decoded messages are pushed to the callback\n        function to process and send.\n\n        :param data: The new packet data\n        :param callback: The function to send results to\n        :param slave: Process if slave id matches, ignore otherwise (could be a\n               list of slave ids (server) or single slave id(client/server))\n        :param kwargs:\n        :raises ModbusIOException:\n        \"\"\"\n        Log.debug(\"Processing: {}\", data, \":hex\")\n        self._buffer += data\n        if not isinstance(slave, (list, tuple)):\n            slave = [slave]\n        single = kwargs.pop(\"single\", False)\n        self.frameProcessIncomingPacket(single, callback, slave, **kwargs)\n\n    def frameProcessIncomingPacket(\n        self, _single, _callback, _slave, _tid=None, **kwargs\n    ):\n        \"\"\"Process new packet pattern.\"\"\"",
  "start_line": 1,
  "end_line": 144,
  "total_lines": 144,
  "is_truncated": false
}

Okay, `resetFrame` is in `base.py`.

Now let's look at `ModbusSocketFramer.frameProcessIncomingPacket` in `socket_framer.py`:

```python
    def frameProcessIncomingPacket(self, single, callback, slave, tid=None, **kwargs):
        # ... (docstring)
        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)
```

And `checkFrame`:

```python
    def checkFrame(self):
        """Check and decode the next frame.

        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
        # we don't have enough of a message yet, wait
        return False
```

Wait, `checkFrame` is called inside the `while True` loop of `frameProcessIncomingPacket`.
If `checkFrame()` returns `False`, it means we don't have enough data yet for a complete frame.
BUT, in `frameProcessIncomingPacket`, if `checkFrame()` returns `False`, it does:

```python
            if not self.checkFrame():
                Log.debug("Frame check failed, ignoring!!")
                self.resetFrame()
                continue
```

`self.resetFrame()` CLEARS THE BUFFER!

```python
    def resetFrame(self):
        # ...
        Log.debug(
            "Resetting frame - Current Frame in buffer - {}", self._buffer, ":hex"
        )
        self._buffer = b""
        self._header = { ... }
```

This is definitely the bug. If `checkFrame()` returns `False`, it means "not enough data yet", but `frameProcessIncomingPacket` treats it as "invalid frame" and wipes the buffer.

It should just `break` the loop and wait for more data.

Wait, let's verify.
`isFrameReady()` checks if `len(self._buffer) > self._hsize`.
`checkFrame()`:
1. If `isFrameReady()` is true, it unpacks the header.
2. If `header["len"]` is too small, it advances the frame.
3. If we have enough data for the specified length, it returns `True`.
4. Otherwise, it returns `False`.

If `checkFrame()` returns `False`, it means either:
a) `isFrameReady()` was false (buffer too short for header).
b) `isFrameReady()` was true, but not enough data for the PDU part of the frame.

In both cases, we should just stop processing and wait for more data.

The current implementation of `frameProcessIncomingPacket` does:

```python
            if not self.checkFrame():
                Log.debug("Frame check failed, ignoring!!")
                self.resetFrame()
                continue
```

This is wrong. If `checkFrame()` is `False`, it should `break` the loop.

Wait, let's check `isFrameReady()` call at the beginning of the loop:

```python
        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 the buffer has some data but less than `self._hsize` (7 bytes), `isFrameReady()` returns `False`.
Then it checks if `len(self._buffer)` is non-zero.
If it is, it checks if `self._header["len"] < 2`.
Wait, `self._header` might be from a previous attempt to read a frame. But if we are here, it means we don't even have enough for a new header.

Actually, the logic in `frameProcessIncomingPacket` seems very confused.

Let's see how it should behave.
A Modbus TCP frame is: Header (7 bytes) + PDU (length bytes).
Wait, the `length` field in MBAP header is for Unit ID + PDU.
So total frame length = 6 + length.
Hold on, `self._hsize` is 7.
Header is `tid` (2), `pid` (2), `len` (2), `uid` (1). Total 7 bytes.
The `len` field is "Unit ID + PDU length".
So the PDU length is `len - 1`.
Total frame length = 7 + (len - 1) = 6 + len.

In `checkFrame()`:
```python
            elif len(self._buffer) - self._hsize + 1 >= self._header["len"]:
                return True
```
`len(self._buffer) - 7 + 1` is `len(self._buffer) - 6`.
If `len(self._buffer) - 6 >= header["len"]`, then `len(self._buffer) >= 6 + header["len"]`. This is correct.

Now look at `frameProcessIncomingPacket`:
```python
        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 `isFrameReady()` is True (buffer > 7 bytes), but `checkFrame()` is False (buffer < 6 + len), then it resets the frame. This is exactly what's happening in the bug report.

The user's first frame was 16 bytes.
`_hsize` = 7. `isFrameReady()` is True.
`checkFrame()`:
`header["len"]` was `0x0b` (11).
Expected total length = 6 + 11 = 17.
`len(self._buffer)` is 16.
`16 - 7 + 1` = 10.
`10 >= 11` is False.
So `checkFrame()` returns `False`.
Then `frameProcessIncomingPacket` calls `resetFrame()` and clears the 16 bytes.
Then the second frame arrives: `0x3` (1 byte).
`self._buffer` = `b'\x03'`.
`isFrameReady()`: `len(b'\x03') > 7` is False.
The loop breaks.
Wait, if the loop breaks, why did the log say:
`2024-01-04 11:16:16,719 DEBUG logging:103 Processing: 0x3`
`2024-01-04 11:16:16,719 DEBUG logging:103 Factory Response[ReadHoldingRegistersResponse': 3]`
`2024-01-04 11:16:16,719 ERROR logging:115 General exception: index out of range`

Wait, `processIncomingPacket` is called for each `data` received.

First call: `data` = 16 bytes.
`_buffer` = 16 bytes.
`isFrameReady()` is True.
`checkFrame()` is False.
`resetFrame()` clears `_buffer`.
Loop breaks (next iteration: `isFrameReady()` is False).

Second call: `data` = 1 byte (`0x3`).
`_buffer` = 1 byte.
`isFrameReady()` is False.
Inside `if not self.isFrameReady()`:
`len(self._buffer)` is 1.
`self._header["len"]` is 0 (reset by `resetFrame`).
`0 < 2` is True.
`self._process(callback, tid, error=True)` is called!

Inside `_process(callback, tid, error=True)`:
`data = self._buffer if error else self.getFrame()`
So `data` = `b'\x3'`.
`self.decoder.decode(data)` is called.
The decoder probably tries to decode `0x3` as a response.
Since it's only 1 byte, it might return something or throw an exception.
The log says `Factory Response[ReadHoldingRegistersResponse': 3]`.
Then `self.populateResult(result)` is called.
Then `self.advanceFrame()` is called.
`advanceFrame()`:
```python
    def advanceFrame(self):
        length = self._hsize + self._header["len"] - 1
        self._buffer = self._buffer[length:]
        self._header = {"tid": 0, "pid": 0, "len": 0, "uid": 0}
```
`length = 7 + 0 - 1 = 6`.
`self._buffer` was 1 byte, so `self._buffer[6:]` is empty.
Then `callback(result)` is called.
The callback is `_handle_response` in `client/base.py`.
The error `General exception: index out of range` likely happens inside the callback or when returning the response because the response was incorrectly decoded from 1 byte.

Anyway, the root cause is clearly that `checkFrame()` returning `False` should not trigger `resetFrame()`. It should just `break` the loop.

Wait, if `checkFrame()` returns `False`, the buffer contains a partial frame. We should just wait for more data.

So the fix is to change:
```python
            if not self.checkFrame():
                Log.debug("Frame check failed, ignoring!!")
                self.resetFrame()
                continue
```
to:
```python
            if not self.checkFrame():
                break
```

Wait, if we `break`, then the next time `frameProcessIncomingPacket` is called, `_buffer` will already contain the partial frame. `isFrameReady()` will be True if it's > 7 bytes, and `checkFrame()` will be called again.

Is there any case where `checkFrame()` returning `False` SHOULD result in `resetFrame()`?
`checkFrame()`:
```python
    def checkFrame(self):
        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
        # we don't have enough of a message yet, wait
        return False
```

If `isFrameReady()` is True, it unpacks the header.
If `len < 2` (which is an invalid length for a Modbus TCP frame, since Unit ID + PDU should be at least 2 bytes: 1 for Unit ID, 1 for Function Code), it calls `advanceFrame()` and then returns `False`.
`advanceFrame()` skips `7 + len - 1` bytes. If `len < 2`, it's at least `7+0-1=6` bytes.
So if it's an invalid length, it skips some bytes and returns `False`.
In `frameProcessIncomingPacket`, if it returns `False`, it currently calls `resetFrame()`.
If it was an invalid length and `advanceFrame()` was called, it already skipped the garbage. Calling `resetFrame()` would then wipe any *remaining* data that might be a valid start of the next frame.

So `resetFrame()` is almost always wrong if we just want to wait for more data or have already handled an invalid header.

Wait, there is one danger: what if we are really out of sync and we keep accumulating data in the buffer because `checkFrame()` always returns `False`?
But `checkFrame()` only returns `False` if:
1. Buffer too small for header.
2. Buffer too small for specified length.

If we have a corrupted header with a huge `len`, we will wait until the buffer reaches that size. If we never do, the buffer grows.
But `ModbusFramer` doesn't seem to have a maximum buffer size.

However, the reported issue is specifically about split frames.

Let's check if there are other framers that behave similarly.
`binary_framer.py` (RTU) might have different logic.

In `socket_framer.py`, the fix seems to be:

```python
    def frameProcessIncomingPacket(self, single, callback, slave, tid=None, **kwargs):
        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
                break  # Wait for more data
```

Wait, what about the "Possible error ???" section?

```python
        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 `isFrameReady()` is False, `len(self._buffer) <= 7`.
If `len(self._buffer)` is 1, and `self._header["len"]` is 0 (default), then `0 < 2` is True, and `_process` is called with `error=True`.
`_process(callback, tid, error=True)`:
`data = self._buffer` (which is 1 byte).
It tries to decode it. This is what happened in the bug report for the second frame.
This seems totally wrong. If `isFrameReady()` is False, we should just `break` and wait for more data. We shouldn't be calling `_process` just because the buffer is non-empty but too small for a header.

The "Possible error ???" block seems to be trying to handle the case where we have some trailing data that doesn't form a full frame. But it does it every time `isFrameReady()` is false!

If `isFrameReady()` is false, we should just `break`.

Let's see the original code again:

```python
    def frameProcessIncomingPacket(self, single, callback, slave, tid=None, **kwargs):
        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 I change `if not self.checkFrame():` to `break`, that solves the issue of split frames where the first part is > 7 bytes.
But the second part (the trailing byte) will still trigger the "Possible error ???" block.

Wait, the "Possible error ???" block is only executed once per call to `frameProcessIncomingPacket` (at the end of the `while True` loop, when `isFrameReady()` becomes False).

In the bug report:
1. First packet arrives (16 bytes). `isFrameReady()` is True. `checkFrame()` is False. `resetFrame()` is called. Buffer is now empty. Loop ends.
2. Second packet arrives (1 byte). `isFrameReady()` is False. `len(self._buffer)` is 1. `self._header["len"]` is 0. `self._process(..., error=True)` is called.

If I fix `checkFrame()` result:
1. First packet arrives (16 bytes). `isFrameReady()` is True. `checkFrame()` is False. `break` is called. Buffer remains 16 bytes.
2. Second packet arrives (1 byte). Buffer becomes 17 bytes.
3. `isFrameReady()` is True.
4. `checkFrame()` is now True! (17 bytes is enough for `len=11`).
5. `_process` is called (with `error=False`). It decodes the 17 bytes correctly.
6. `advanceFrame()` is called. Buffer becomes empty.
7. Next iteration: `isFrameReady()` is False.
8. `

{
  "command": "python3 - <<'PY'\nimport asyncio\nimport struct\nfrom pymodbus.exceptions import ModbusIOException\nfrom pymodbus.framer.socket_framer import ModbusSocketFramer\n\nclass MockDecoder:\n    def decode(self, data):\n        if len(data) == 10: # UID + PDU\n            return struct.unpack(\">B B\", data[0:2]) # Just a dummy return\n        return None\n\nasync def main():\n    # We need a decoder that doesn't crash.\n    # Actually, let's just mock it more simply.\n    class SimpleDecoder:\n        def decode(self, data):\n            if len(data) == 10:\n                class Result:\n                    def __init__(self):\n                        self.function_code = 0x03\n                        self.transaction_id = 1\n                return Result()\n            return None\n\n    framer = ModbusSocketFramer(SimpleDecoder())\n    \n    # Frame: TID=1, PID=0, LEN=11, UID=1, PDU=...\n    # Total length = 6 + 11 = 17 bytes.\n    # We send 16 bytes first.\n    packet1 = struct.pack(\">HHHB\", 1, 0, 11, 1) + b'\\x03' + b'\\x00' * 9 \n    # MBAP: 1, 0, 11, 1 (7 bytes). PDU: 0x03 + 9 bytes = 10 bytes. \n    # Total = 17. Let's make packet1 16 bytes.\n    packet1 = packet1[:16] \n    packet2 = b'\\x00' # The 17th byte\n    \n    def callback(result):\n        print(\"Callback received result!\")\n\n    print(\"Sending packet 1...\")\n    framer.processIncomingPacket(packet1, callback, slave=[1])\n    print(\"Done packet 1. Buffer len:\", len(framer._buffer))\n    \n    print(\"Sending packet 2...\")\n    framer.processIncomingPacket(packet2, callback, slave=[1])\n    print(\"Done packet 2. Buffer len:\", len(framer._buffer))\n\nif __name__ == \"__main__\":\n    asyncio.run(main())\nPY\n"
}

{
  "status": "error",
  "error_type": "CommandError",
  "error_message": "Traceback (most recent call last):\n  File \"<stdin>\", line 48, in <module>\n  File \"/usr/local/lib/python3.10/asyncio/runners.py\", line 44, in run\n    return loop.run_until_complete(main)\n  File \"/usr/local/lib/python3.10/asyncio/base_events.py\", line 649, in run_until_complete\n    return future.result()\n  File \"<stdin>\", line 44, in main\n  File \"/workspace/pymodbus/framer/base.py\", line 139, in processIncomingPacket\n    self.frameProcessIncomingPacket(single, callback, slave, **kwargs)\n  File \"/workspace/pymodbus/framer/socket_framer.py\", line 136, in frameProcessIncomingPacket\n    self._process(callback, tid, error=True)\n  File \"/workspace/pymodbus/framer/socket_framer.py\", line 154, in _process\n    raise ModbusIOException(\"Unable to decode request\")\npymodbus.exceptions.ModbusIOException: Modbus Error: [Input/Output] Unable to decode request\n",
  "details": {
    "stdout": "Sending packet 1...\nDone packet 1. Buffer len: 0\nSending packet 2...\n",
    "stderr": "Traceback (most recent call last):\n  File \"<stdin>\", line 48, in <module>\n  File \"/usr/local/lib/python3.10/asyncio/runners.py\", line 44, in run\n    return loop.run_until_complete(main)\n  File \"/usr/local/lib/python3.10/asyncio/base_events.py\", line 649, in run_until_complete\n    return future.result()\n  File \"<stdin>\", line 44, in main\n  File \"/workspace/pymodbus/framer/base.py\", line 139, in processIncomingPacket\n    self.frameProcessIncomingPacket(single, callback, slave, **kwargs)\n  File \"/workspace/pymodbus/framer/socket_framer.py\", line 136, in frameProcessIncomingPacket\n    self._process(callback, tid, error=True)\n  File \"/workspace/pymodbus/framer/socket_framer.py\", line 154, in _process\n    raise ModbusIOException(\"Unable to decode request\")\npymodbus.exceptions.ModbusIOException: Modbus Error: [Input/Output] Unable to decode request\n",
    "exit_code": 1
  }
}