Partial reads cause RTU framer to discard whole frame with asyncio

[marcosatti]

Versions

• Python: 3.10
• OS: Windows 10
• Pymodbus: 2.5.3
• Modbus Hardware (if used): USB RS-485 adapter to custom hardware

Pymodbus Specific

• Client: rtu - async

Description

Using the asyncio client, partial reads will cause the RTU framer to discard the whole incoming packet, complaining about the unit ID not being valid:

(Logs have been scrubbed of sensitive data.)

2021-12-01 09:11:28,424 - pymodbus.client.asynchronous.async_io - DEBUG - send: 0x1 [...]
2021-12-01 09:11:28,424 - pymodbus.transaction - DEBUG - Adding transaction 1
2021-12-01 09:11:28,456 - pymodbus.client.asynchronous.async_io - DEBUG - recv: 0x1 [...]
2021-12-01 09:11:28,456 - pymodbus.framer.rtu_framer - DEBUG - Frame - [b'\x01 ...'] not ready
2021-12-01 09:11:28,471 - pymodbus.client.asynchronous.async_io - DEBUG - recv: 0x65 [...]
2021-12-01 09:11:28,471 - pymodbus.framer.rtu_framer - DEBUG - Frame - [b'e ...'] not ready
2021-12-01 09:11:28,499 - pymodbus.client.asynchronous.async_io - DEBUG - recv: 0x6c [...]
2021-12-01 09:11:28,499 - pymodbus.framer.rtu_framer - DEBUG - Frame - [b'l ...'] not ready
2021-12-01 09:11:28,514 - pymodbus.client.asynchronous.async_io - DEBUG - recv: 0xfe [...]
2021-12-01 09:11:28,514 - pymodbus.framer.rtu_framer - DEBUG - Not a valid unit id - 1, ignoring!!
2021-12-01 09:11:28,514 - pymodbus.framer.rtu_framer - DEBUG - Resetting frame - Current Frame in buffer - 0x1 [...] 0xfb 0x7a

The problem is a combination of the following code:

client/asynchronous/async_io/__init__.py:

    def _dataReceived(self, data):
        _logger.debug("recv: " + hexlify_packets(data))
        unit = self.framer.decode_data(data).get("unit", 0)
        self.framer.processIncomingPacket(data, self._handleResponse, unit=unit)

framer/rtu_framer.py:

    def processIncomingPacket(self, data, callback, unit, **kwargs):
        if not isinstance(unit, (list, tuple)):
            unit = [unit]
        self.addToFrame(data)
        single = kwargs.get("single", False)
        if self.isFrameReady():
            if self.checkFrame():
                if self._validate_unit_id(unit, single):
                    self._process(callback)
                else:
                    _logger.debug("Not a valid unit id - {}, ignoring!!".format(self._header['uid']))
                    self.resetFrame()
            else:
                _logger.debug("Frame check failed, ignoring!!")
                self.resetFrame()
        else:
            _logger.debug("Frame - [{}] not ready".format(data))

framer/__init__.py:

    def _validate_unit_id(self, units, single):
        if single:
            return True
        else:
            if 0 in units or 0xFF in units:
                return True
            return self._header['uid'] in units

If an incoming packet is constructed from partial reads, the following happens:

1. On the initial partial read, the header is extracted from self.framer.decode_data(data), and the correct unit ID is read (in my case 1).
2. On subsequent partial reads, the header is extracted from self.framer.decode_data(data) and the unit parameter is passed through as the first byte of the partial read which is wrong - however self.isFrameReady() returns False until the full packet is received, so this is still ok.
3. On the final partial read, the wrong unit parameter is passed through and _validate_unit_id() gets called and fails (compared for equality against self._header['uid'], leading to the Not a valid unit id - 1, ignoring!! message and discarding the frame, even though it is complete and valid.

Adding single=True when calling processIncomingPacket() fixes the problem in my case, but I don't know if thats an appropriate fix - I can submit a pull request if it is.

Thanks.

[marcosatti]

Additionally, you aren't able to control pyserial-asyncio very much in regards to buffering, which I tried to do as a workaround (it would be fine if the data was all received at once).

[dhoomakethu]

partial data is a pain point and we are not yet able to come to a solution which can be generic. Please feel free to raise a PR if its working for you so that others can review and comment.

[marcosatti]

Is there prior discussion on it?

I'm guessing RTU framing is different to TCP framing etc without having a look, but for RTU framing simply not checking the subsequent partial frame data would be ok... By spec no device is meant to be replying except the addressed device.

I am interested in fixing it but I'm only experienced with how RTU works.

[nikito7]

By spec no device is meant to be replying except the addressed device.

But it will fail CRC check

Modbus TCP don't have CRC in modbus payload. Because TCP stream already have CRC