pymodbus-dev__pymodbus-1923

The user is reporting an issue where `pymodbus` (specifically `AsyncModbusTcpClient`) fails to handle TCP segments that are split.
The log provided shows:
1. `recv: 0x0 0x1 0x0 0x0 0x0 0xb 0x1 0x3 0x8 0x0 0xb5 0x12 0x2f 0x37 0x21 0x0 old_data: addr=None`
2. `Processing: 0x0 0x1 0x0 0x0 0x0 0xb 0x1 0x3 0x8 0x0 0xb5 0x12 0x2f 0x37 0x21 0x0`
3. `Frame check failed, ignoring!!`
4. `Resetting frame - Current Frame in buffer - 0x0 0x1 0x0 0x0 0x0 0xb 0x1 0x3 0x8 0x0 0xb5 0x12 0x2f 0x37 0x21 0x0`
5. `recv: 0x3 old_data: addr=None`
6. `Processing: 0x3`
7. `Factory Response[ReadHoldingRegistersResponse': 3]`
8. `ERROR logging:115 General exception: index out of range`

The key is that the first segment was received, but it failed the frame check. Then a second segment arrived, and it was processed as a complete frame (which it wasn't).

In Modbus TCP, the MBAP header is 7 bytes long. The 5th through 7th bytes of the header (bytes 4, 5, 6 if 0-indexed) indicate the length of the remaining PDU.

The log shows:
`0x0 0x1 0x0 0x0 0x0 0xb 0x1 0x3 0x8 0x0 0xb5 0x12 0x2f 0x37 0x21 0x0`
MBAP Header:
- Transaction ID: `0x0 0x1` (2 bytes)
- Protocol ID: `0x0 0x0` (2 bytes)
- Length: `0x0 0xb` (2 bytes) -> 11 bytes
- Unit ID: `0x1` (1 byte)
Total MBAP header = 7 bytes.
The Length field says 11 bytes follow the length field.
The bytes following the length field are: `0x1 0x3 0x8 0x0 0xb5 0x12 0x2f 0x37 0x21 0x0`
Let's count them:
1. `0x1` (Unit ID)
2. `0x3` (Function Code 3 - Read Holding Registers)
3. `0x8` (Byte Count - 8 bytes of data)
4. `0x0` (Data 1)
5. `0xb5` (Data 2)
6. `0x12` (Data 3)
7. `0x2f` (Data 4)
8. `0x37` (Data 5)
9. `0x21` (Data 6)
10. `0x0` (Data 7)
There are only 10 bytes following the length field. But the length field `0x0 0xb` indicates 11 bytes.
So one byte is missing.
The second `recv` is `0x3`.
If we append `0x3` to the previous buffer, we have 11 bytes.

The issue is that the framer is attempting to process the packet as soon as some data arrives, rather than waiting until the amount of data specified in the MBAP header is received.

Relevant files:
- `pymodbus/framer/socket_framer.py`
- `pymodbus/framer/base.py`
- `pymodbus/client/base.py`
- `pymodbus/transport/transport.py`

Let's start by locating the `processIncomingPacket` and `frameProcessIncomingPacket` (or similar) methods in the framers.

The traceback provided:
```
  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")
```

Wait, the traceback says `socket_framer.py` line 136 calls `_process`, which at line 154 raises `ModbusIOException`.

Let's examine `pymodbus/framer/socket_framer.py` and `pymodbus/framer/base.py`.

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

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

Wait, only `base.py` has `processIncomingPacket`? No, the traceback mentioned `socket_framer.py` had `frameProcessIncomingPacket`.

Let's grep for `frameProcessIncomingPacket`.

{
  "command": "grep -rn --include=\"*.py\" \"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, so `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 look at `frameProcessIncomingPacket` 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

    def isFrameReady(self):
        """Check if we should continue decode logic.

        This is meant to be used in a while loop in the decoding phase to let
        the decoder factory know that there is still data in the buffer.

        :returns: True if ready, False otherwise
        """
        return len(self._buffer) > self._hsize
```

The logic in `frameProcessIncomingPacket`:

```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
            # ...
            self._process(callback, tid)
```

Wait! If `checkFrame()` returns `False`, it means the frame is not yet complete. But `frameProcessIncomingPacket` treats `checkFrame() == False` as a "Frame check failed" and calls `resetFrame()`.

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

If `checkFrame()` returns `False`, it just means we are waiting for more data. We should `break` the loop and wait for more data to arrive in `_buffer`, NOT `resetFrame()` and continue (which will likely lead to an infinite loop or discarding data).

Let's trace what happens with the provided logs:
1. First packet arrives. `self._buffer` has 16 bytes. `self._hsize` is 7.
2. `isFrameReady()` returns `True`.
3. `checkFrame()`:
    - `isFrameReady()` is True.
    - `struct.unpack` reads header. `len` = 11.
    - `len(self._buffer) - self._hsize + 1` = `16 - 7 + 1` = 10.
    - `10 >= 11` is `False`.
    - `checkFrame()` returns `False`.
4. Back in `frameProcessIncomingPacket`:
    - `if not self.checkFrame():` is `True`.
    - `Log.debug("Frame check failed, ignoring!!")` is logged.
    - `self.resetFrame()` is called.
    - `continue` to next iteration of `while True`.

This is exactly what happened in the logs!

The fix should be to `break` when `checkFrame()` is `False`, instead of calling `resetFrame()`.

Wait, but `checkFrame` also does:
```python
            # someone sent us an error? ignore it
            if self._header["len"] < 2:
                self.advanceFrame()
```
And `advanceFrame` does:
```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}
```
But `checkFrame` returns `None` (which is falsy) if it calls `advanceFrame()`.

Actually, if `checkFrame` didn't find a full frame, it should just return `False`. But `frameProcessIncomingPacket` should stop processing and wait for more data.

Let's confirm where `resetFrame` is.

{
  "command": "grep -rn \"def resetFrame\" pymodbus/framer/socket_framer.py"
}

{
  "status": "error",
  "error_type": "CommandError",
  "error_message": "",
  "details": {
    "stdout": "",
    "stderr": "",
    "exit_code": 1
  }
}

Ah, `resetFrame` might be in `base.py`.

{
  "command": "grep -rn \"def resetFrame\" pymodbus/framer/base.py"
}

{
  "status": "ok",
  "stdout": "81:    def resetFrame(self):\n",
  "stderr": "",
  "exit_code": 0
}

Let's read `pymodbus/framer/base.py` to see what `resetFrame` does.

{
  "end_line": 150,
  "filepath": "pymodbus/framer/base.py",
  "start_line": 70
}

{
  "status": "ok",
  "filepath": "pymodbus/framer/base.py",
  "content": "        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": 70,
  "end_line": 144,
  "total_lines": 144,
  "is_truncated": false
}

So `resetFrame` just clears the buffer.

Back to `pymodbus/framer/socket_framer.py`:

```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
            # ...
```

The `if not self.checkFrame():` block is definitely the problem. If `checkFrame()` returns `False`, it means the frame is incomplete. We should stop and wait for more data.

Wait, let's check `checkFrame()` again.

```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
```

If `self._header["len"] < 2`, it calls `self.advanceFrame()` and then returns `None` (which is falsy). This will lead to `resetFrame()` and `continue` in `frameProcessIncomingPacket`.

And if the frame is incomplete, it returns `False`. This also leads to `resetFrame()` and `continue` in `frameProcessIncomingPacket`.

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 it was an "error" (len < 2), `checkFrame` would have already called `advanceFrame()`. If we just `break`, we stop processing and wait for more data. But if it was an error and we already `advanceFrame()`, we might want to try to process the next frame in the buffer immediately without waiting for new data.

Wait, `advanceFrame()` modifies `self._buffer`.

```python
    def advanceFrame(self):
        """Skip over the current framed message.
        ...
        """
        length = self._hsize + self._header["len"] - 1
        self._buffer = self._buffer[length:]
        self._header = {"tid": 0, "pid": 0, "len": 0, "uid": 0}
```

If `checkFrame()` returns `False` because `isFrameReady()` was `True` but the frame was incomplete, we MUST `break`.

If `checkFrame()` returns `False` because `self._header["len"] < 2`, then `advanceFrame()` was called, so `self._buffer` has been advanced. In this case, we should `continue` (or just let the loop run) to see if there is another frame.

So `checkFrame()` probably should distinguish between "incomplete frame" and "invalid frame".

Alternatively, `frameProcessIncomingPacket` should only `break` if no frame was found AND no data was discarded.

Let's look at `checkFrame` again. It returns `True` if a complete frame is ready. It returns `False` (or `None`) otherwise.

If I change the logic to:

```python
            if not self.checkFrame():
                if not self.isFrameReady():
                    break
                # If it was a real error (len < 2) advanceFrame was called, 
                # so we should try again if isFrameReady is still true.
                # But if checkFrame’s isFrameReady was true, and it didn't return True,
                # and it didn't call advanceFrame, then it just returned False (incomplete).
```

Wait, `checkFrame` return values:
1. `isFrameReady()` is `False` -> returns `False`.
2. `isFrameReady()` is `True`, `self._header["len"] < 2` -> calls `advanceFrame()`, returns `None` (falsy).
3. `isFrameReady()` is `True`, `self._header["len"] >= 2`, `len(self._buffer) - self._hsize + 1 < self._header["len"]` -> returns `False`.
4. `isFrameReady()` is `True`, `self._header["len"] >= 2`, `len(self._buffer) - self._hsize + 1 >= self._header["len"]` -> returns `True`.

In case 3, we should `break`.
In case 2, we should `continue`.
In case 1, we should `break`.

But `checkFrame` doesn't tell us which one happened.

If we change `checkFrame` to return something else when it's just incomplete.

Or, we can modify `checkFrame` to be:

```python
    def checkFrame(self):
        if not self.isFrameReady():
            return False, False # (ready, complete)

        (
            self._header["tid"],
            self._header["pid"],
            self._header["len"],
            self._header["uid"],
        ) = struct.unpack(">HHHB", self._buffer[0 : self._hsize])

        if self._header["len"] < 2:
            self.advanceFrame()
            return True, False # Still ready (maybe), but not a complete frame

        if len(self._buffer) - self._hsize + 1 >= self._header["len"]:
            return True, True # Ready and complete

        return True, False # Ready (header received), but incomplete frame
```

This seems too complex for a small fix.

What if we just change `frameProcessIncomingPacket` to:

```python
    def frameProcessIncomingPacket(self, single, callback, slave, tid=None, **kwargs):
        while True:
            if not self.isFrameReady():
                # ... (keep this block)
                break
            
            if not self.checkFrame():
                # checkFrame returned False or None.
                # If it called advanceFrame, we should continue.
                # If it just returned False, it means the frame is incomplete.
                # How to know if advanceFrame was called? 
                # advanceFrame resets self._header['len'] to 0.
                # But we can't easily know if it was just called.
                break
            # ...
```

Wait, if `checkFrame` returns `False` because the frame is incomplete, we MUST `break`. If we `continue`, we just enter the loop again, `isFrameReady()` is still `True`, `checkFrame()` is still `False`... infinite loop (if we don't `resetFrame()`).

So the only way to `continue` is if some data was removed from `self._buffer`.

If `checkFrame` returns `False`, it either means:
A) `isFrameReady()` was `False`.
B) `self._header["len"] < 2` and `advanceFrame()` was called.
C) Frame is incomplete.

In case A and C, we must `break`.
In case B, we can `continue`.

Wait, let's look at the current `frameProcessIncomingPacket` again.

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

The problem is `self.resetFrame()` is called every time `checkFrame()` returns `False`.

If we just remove `self.resetFrame()` and `continue`, and replace it with `break`:

```python
            if not self.checkFrame():
                break
```

What happens in case B (`self._header["len"] < 2`)?
`advanceFrame()` is called, `self._buffer` is modified. Then we `break`.
We wait for more data to arrive. When more data arrives, `processIncomingPacket` is called again, and we start the `while True` loop again. Since `self._buffer` was already advanced, we'll start from the next potential frame.
This seems correct! We don't *need* to `continue` immediately if we just wait for the next `data_received` event.

But wait, what if the buffer already contained multiple frames, and the first one was an "error" (len < 2)? If we `break`, we don't process the remaining frames in the buffer until more data arrives. That's not ideal, but not fatal.

Actually, if `checkFrame` returns `False` and we `break`, we might miss frames that are already in the buffer.

Can we modify `checkFrame` to return `True` for "we made progress (either found a frame or skipped an invalid one)" and `False` for "we are waiting for more data"?

No, `checkFrame` currently return `True` ONLY if a complete frame is ready.

Let's redefine `checkFrame` to return:
- `True`: Complete frame ready.
- `False`: Frame incomplete, wait for more data.
- `None`: Invalid frame skipped, continue checking the buffer.

```python
    def checkFrame(self):
        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])

        if self._header["len"] < 2:
            self.advanceFrame()
            return None # Progress made, but no frame yet
        
        if len(self._buffer) - self._hsize + 1 >= self._header["len"]:
            return True # Frame complete
        
        return False # Frame incomplete, wait for more data
```

And in `frameProcessIncomingPacket`:

```python
            res = self.checkFrame()
            if res is True:
                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)
            elif res is False:
                break
            elif res is None:
                continue
```

This looks more robust.

Wait, let's check if `_validate_slave_id` is used elsewhere.

Actually, I should check if I can just do a simpler fix.
If I change `checkFrame` to return `True` for complete frame, and `False` for anything else.
And in `frameProcessIncomingPacket`:

```python
            if not self.checkFrame():
                # We must determine if we should break (incomplete) or continue (invalid frame skipped)
                # If isFrameReady is still True, and the buffer hasn't changed, we'd loop forever.
                # But checkFrame() didn't return True.
                # If we check if any bytes were removed from the buffer...
                break
```

Actually, the simplest fix that addresses the reported bug is just replacing `self.resetFrame()` and `continue` with `break`.

```python
            if not self.checkFrame():
                break
```

Let's see if that's sufficient. The user's log shows exactly this failure:
- Packet arrives, incomplete.
- `checkFrame` returns `False`.
- `resetFrame` is called (buffer cleared!).
- `continue` (loop again).
- `isFrameReady` is now `False` (buffer was cleared).
- `break` (loop ends).
- Next packet arrives (the missing byte).
- `processIncomingPacket` is called.
- `_buffer` now only contains the missing byte `0x3`.
- `isFrameReady` is `False` (len 1 < 7).
- `if len(self._buffer):` is `True`.
- `if self._header["len"] < 2:` (self._header was reset to 0).
- `self._process(callback, tid, error=True)` is called with data `0x3`.
- `_process` tries to decode `0x3` and fails with `ModbusIOException("Unable to decode request")`.

Yes, replacing `self.resetFrame()` and `continue` with `break` will fix this. The buffer will be preserved, and when the next packet arrives, `self._buffer` will be complete, `checkFrame()` will return `True`, and it will be processed.

Let's verify if there are other places where `checkFrame` is used.
grep for `checkFrame`.

{
  "command": "grep -rn \"checkFrame\" pymodbus/framer/socket_framer.py"
}

{
  "status": "ok",
  "stdout": "50:    def checkFrame(self):\n138:            if not self.checkFrame():\n",
  "stderr": "",
  "exit_code": 0
}

Only used in `frameProcessIncomingPacket`.

Now let's create a reproduction script.
The user provided a script and logs. I can simulate the split packet by creating a mock transport or just a small test case.

Actually, the easiest way to reproduce this is to call `frameProcessIncomingPacket` twice with split data.

{
  "command": "python3 - <<'PY'\nimport struct\nfrom pymodbus.framer.socket_framer import ModbusSocketFramer\nfrom pymodbus.logging import Log\nimport logging\n\n# Setup logging to capture the \"Frame check failed\" message\nlogging.basicConfig(level=logging.DEBUG)\n\nclass MockClient:\n    def __init__(self):\n        self.response_called = False\n    def handle_response(self, response):\n        self.response_called = True\n        print(f\"Response received: {response}\")\n\ndecoder = None # Not used for this test\nclient = MockClient()\nframer = ModbusSocketFramer(decoder, client)\n\n# Modbus TCP Response:\n# tid=1, pid=0, len=11, uid=1, fcode=3, bcount=8, data=[0, 0xb5, 0x12, 0x2f, 0x37, 0x21, 0x0, 0x3]\n# Total bytes: 7 + 11 - 1 = 17 bytes? \n# No, length field in MBAP is UID + PDU. \n# UID (1) + FCode (1) + ByteCount (1) + Data (8) = 11 bytes.\n# Total packet: 6 (tid, pid, len) + 11 = 17 bytes.\n\npacket = struct.pack(\">HHHB B B BBBBBBBB\", 1, 0, 11, 1, 3, 8, 0, 0xb5, 0x12, 0x2f, 0x37, 0x21, 0x0, 0x3)\nprint(f\"Full packet: {packet.hex()}\")\n\n# Split packet: first 16 bytes, then the last 1 byte.\npart1 = packet[:16]\npart2 = packet[16:]\n\nprint(f\"Part 1: {part1.hex()}\")\nprint(f\"Part 2: {part2.hex()}\")\n\n# Process Part 1\nframer._buffer = b\"\"\n# In a real scenario, processIncomingPacket is called.\n# We can call the internal frameProcessIncomingPacket directly.\n# since we don't have the base.py processIncomingPacket setup fully here.\n# But wait, we can just use the real class if we instantiate it.\n# Actually, let's just manually add to buffer and call frameProcessIncomingPacket.\n\nframer._buffer = part1\nframer.frameProcessIncomingPacket(single=False, callback=client.handle_response, slave=[1])\nprint(f\"After Part 1, buffer: {framer._buffer.hex()}, response_called: {client.response_called}\")\n\n# Process Part 2\nframer._buffer += part2\nframer.frameProcessIncomingPacket(single=False, callback=client.handle_response, slave=[1])\nprint(f\"After Part 2, buffer: {framer._buffer.hex()}, response_called: {client.response_called}\")\n\nif client.response_called:\n    print(\"SUCCESS: Response was processed!\")\nelse:\n    print(\"FAILURE: Response was not processed!\")\nPY\n"
}

{
  "status": "error",
  "error_type": "CommandError",
  "error_message": "DEBUG:pymodbus.logging:Frame check failed, ignoring!!\nDEBUG:pymodbus.logging:Resetting frame - Current Frame in buffer - 0x0 0x1 0x0 0x0 0x0 0xb 0x1 0x3 0x8 0x0 0xb5 0x12 0x2f 0x37 0x21 0x0\nTraceback (most recent call last):\n  File \"<stdin>\", line 51, in <module>\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 152, in _process\n    if (result := self.decoder.decode(data)) is None:\nAttributeError: 'NoneType' object has no attribute 'decode'\n",
  "details": {
    "stdout": "Full packet: 00010000000b01030800b5122f37210003\nPart 1: 00010000000b01030800b5122f372100\nPart 2: 03\nAfter Part 1, buffer: , response_called: False\n",
    "stderr": "DEBUG:pymodbus.logging:Frame check failed, ignoring!!\nDEBUG:pymodbus.logging:Resetting frame - Current Frame in buffer - 0x0 0x1 0x0 0x0 0x0 0xb 0x1 0x3 0x8 0x0 0xb5 0x12 0x2f 0x37 0x21 0x0\nTraceback (most recent call last):\n  File \"<stdin>\", line 51, in <module>\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 152, in _process\n    if (result := self.decoder.decode(data)) is None:\nAttributeError: 'NoneType' object has no attribute 'decode'\n",
    "exit_code": 1
  }
}