Unit ID not handled properly in async clients

[dhoomakethu]

@dhoomakethu, sorry for long response. Below is the log file of failing unit id validation.

2021-03-25 23:47:13,091 MainThread      DEBUG    selector_events:53       Using selector: SelectSelector
2021-03-25 23:47:13,097 MainThread      DEBUG    __init__       :789      Connecting.
2021-03-25 23:47:13,106 MainThread      DEBUG    __init__       :106      Client connected to modbus server
2021-03-25 23:47:13,106 MainThread      INFO     __init__       :806      Protocol made connection.
2021-03-25 23:47:13,106 MainThread      INFO     __init__       :798      Connected to COM57
2021-03-25 23:47:13,107 MainThread      DEBUG    mdbs_master    :27       Read input registers
2021-03-25 23:47:13,107 MainThread      DEBUG    __init__       :140      send: 0x1 0x4 0x0 0x1 0x0 0xe 0x20 0xe
2021-03-25 23:47:13,107 MainThread      DEBUG    transaction    :448      Adding transaction 1
2021-03-25 23:47:13,148 MainThread      DEBUG    __init__       :149      recv: 0x1 0x4 0x1c 0x55 0x55 0x55
2021-03-25 23:47:13,149 MainThread      DEBUG    rtu_framer     :240      Frame - [b'\x01\x04\x1cUUU'] not ready
2021-03-25 23:47:13,164 MainThread      DEBUG    __init__       :149      recv: 0x55 0x55 0x55 0x55 0x55 0x55 0x55 0x55
2021-03-25 23:47:13,164 MainThread      DEBUG    rtu_framer     :240      Frame - [b'UUUUUUUU'] not ready
2021-03-25 23:47:13,181 MainThread      DEBUG    __init__       :149      recv: 0x55 0x55 0x55 0x55 0x55 0x55 0x55
2021-03-25 23:47:13,181 MainThread      DEBUG    rtu_framer     :240      Frame - [b'UUUUUUU'] not ready
2021-03-25 23:47:13,196 MainThread      DEBUG    __init__       :149      recv: 0x55 0x55 0x55 0x55 0x55 0x55 0x55 0x55
2021-03-25 23:47:13,196 MainThread      DEBUG    rtu_framer     :240      Frame - [b'UUUUUUUU'] not ready
2021-03-25 23:47:13,213 MainThread      DEBUG    __init__       :149      recv: 0x55 0x55 0xf3 0xbf
2021-03-25 23:47:27,496 MainThread      DEBUG    rtu_framer     :234      Not a valid unit id - 1, ignoring!!
2021-03-25 23:47:32,495 MainThread      DEBUG    rtu_framer     :120      Resetting frame - Current Frame in buffer - 0x1 0x4 0x1c 0x55 0x55 0x55 0x55 0x55 0x55 0x55 0x55 0x55 0x55 0x55 0x55 0x55 0x55 0x55 0x55 0x55 0x55 0x55 0x55 0x55 0x55 0x55 0x55 0x55 0x55 0x55 0x55 0xf3 0xbf

If in the line 230 in pymodbus/framer/rtu_framer.py remove unit validation:

                if True: #if self._validate_unit_id(unit, single):

messages will start to receive correctly.
The reason, as I said, in pymodbus/client/asynchronous/async_io/init.py code:

        # here decode_data assigns unit to the first byte of last part of message, 0x55 in the example above
        unit = self.framer.decode_data(data).get("unit", 0)  
        self.framer.processIncomingPacket(data, self._handleResponse, unit=unit)

Originally posted by @olddudealex in #608 (comment)

[grungy-ado]

Hi, I encounter the same problem. It seems that problem occurs in asynchronous modbus client when modbus frame is received in multiple chunks, so that unit is decoded (unit = self.framer.decode_data(data).get("unit", 0)) for each chunk, but only first chunk contains unit info. The other chunks of this frame does not contain unit and so the validation fails.

I would suggest resolving this issue by completely removing unit decoding part:

< unit = self.framer.decode_data(data).get("unit", 0)  
< self.framer.processIncomingPacket(data, self._handleResponse, unit=unit)
---
> self.framer.processIncomingPacket(data, self._handleResponse, unit=0)

because it seems that unit argument is used only for validation and since it is validated against same source (received frame chunk) it will always be valid.

I`m willing to help by providing merge request if this solution is ok.

[mdavidsaver]

I've bumped into this issue as well. The doc comment for processIncomingPacket() describes unit= as:

https://github.com/riptideio/pymodbus/blob/731b080df0d277f0c6c32c949667a24a69a6bd7b/pymodbus/framer/rtu_framer.py#L219-L220

This matches what is (I think) happening with a synchronous client, where unit=request.unit_id.

https://github.com/riptideio/pymodbus/blob/731b080df0d277f0c6c32c949667a24a69a6bd7b/pymodbus/transaction.py#L208-L210

For an asynchronous client to pass what is intended to be the response unit ID seems wrong.

https://github.com/riptideio/pymodbus/blob/731b080df0d277f0c6c32c949667a24a69a6bd7b/pymodbus/client/asynchronous/async_io/__init__.py#L150-L151

I've tried to fix this, for asyncio at least, with #665

[janiversen]

Solved on dev.