ReadDeviceInformationRequest fails with struct error

[dhoomakethu]

Versions

• Python: 2.7/3.x
• OS: NA
• Pymodbus: 1.5.2
• Modbus Hardware (if used): Veris-8036 / Schenier EM-6400

Pymodbus Specific

• Client: rtu-sync

Description

Trying to execute mei message (ReadDeviceInformationRequest) and ReportSlaveIdRequest results in struct error. This is partly because these requests do not get_response_pdu_size methods implemented and expected response length is passed as None . The _wait_for_data method in client.sync.ModbusSerialClient class for some devices does not wait for the complete data to come in to the read buffer, thereby returning partial data .

Code and Logs

error: unpack requires a string argument of length 2
(7 additional frame(s) were not displayed)
...
  File "pymodbus/transaction.py", line 164, in execute
    request.unit_id)
  File "pymodbus/framer/rtu_framer.py", line 217, in processIncomingPacket
    if self.checkFrame():
  File "pymodbus/framer/rtu_framer.py", line 89, in checkFrame
    self.populateHeader()
  File "pymodbus/framer/rtu_framer.py", line 152, in populateHeader
    size = pdu_class.calculateRtuFrameSize(data)
  File "pymodbus/mei_message.py", line 118, in calculateRtuFrameSize
    _, object_length = struct.unpack('>BB', buffer[size:size+2])

unpack requires a string argument of length 2

[chintal]

This should be fixed in ModbusRtuFramer.checkFrame(), which should catch the struct error from populateHeader() and return False, as it already does with IndexError and KeyError. There should also be some kind of timeout which ensures advanceFrame() is called if the packet is irretrievably broken or will never come, but I expect that's already there since you're already handling IndexError and KeyError in exactly the same way.

A time.sleep() timeout is not the best thing to add here.

[dhoomakethu]

Fixed with v2.1.0

[aharrah]

Hello,

I've run into a curious issue in sync.py's _wait_for_data().

I have two USB to RS485 converters. One behaves well, but the other, which unfortunately I am strapped with, does not.

Specifically the poorly behaving converter will send along about 9 bytes, then buffer the remaining bytes. This buffering takes longer than the 0.01 second threshold in _wait_for_data(), such that _wait_for_data() breaks out of its while loop since it sees no new data for more than 10ms.

The response is approximately 119 bytes long (ReadDeviceInformationRequest)

Changing
time.sleep(0.01)
to
time.sleep(0.03)

works around the problem, at least for the length of data I'm expecting. Do you think increasing that timeout is wise? Is there a better approach?

Here is a link to the code in question:
https://github.com/riptideio/pymodbus/blob/31c165026f94dccb851a05f0fb32adcbf39003eb/pymodbus/client/sync.py#L536

Thank you for your time,
Andy

[dhoomakethu]

@aharrah can you give some more info on your setup (os, pymodbus version etc?)

[aharrah]

OS: Windows 10
Python version: 3.6.8
pymodbus version: 2.2.0

I am communicating with sensors from www.vaisala.com, specifically their temperature and CO2 sensors.

The only issue is with ReadDeviceInformationRequest. All read_holding_register requests work w/o any issues.

The USB to RS485 adapter provided by Vaisala behaves well, but the generic USB to RS485 adapter does not behave well.

[aharrah]

I was also able to reproduce the behavior on Linux. We are using a PLC (https://revolution.kunbus.com/revpi-connect/) that runs Linux.

uname -a:
Linux still-boat 4.9.76-rt60-v7+ #1 SMP PREEMPT RT Tue, 12 Mar 2019 15:19:36 +0100 armv7l GNU/Linux

[dhoomakethu]

On Windows you probably will have to use strict=False as one of the kwarg while creating the client. You can also increase read timeout to see if that helps. In any case debug logs would help to identify the issue.

[aharrah]

The issue is with the while loop on line 546. This loop will loop as long as new data arrives. When 10ms goes by without any new data the loop will exit.

In some cases 10ms is too short and the loop exits prematurely. Increasing the wait on line 553 to 30ms will work around the problem, at least for my particular situation.

--

There is a related issue in _send() around line 525 which is:
result = self.socket.read(waitingbytes)

On both Linux and Windows waitingbytes is less than the actual number of waiting bytes. This causes the code that cleans up the recv buffer to not completely clean up, which causes issues later. On line 524, changing the if to a while might be the first step in resolving this issue.

[dkristell]

OS: Windows 10
Python version: 3.6.8
pymodbus version: 2.2.0

I am communicating with sensors from www.vaisala.com, specifically their temperature and CO2 sensors.

The only issue is with ReadDeviceInformationRequest. All read_holding_register requests work w/o any issues.

The USB to RS485 adapter provided by Vaisala behaves well, but the generic USB to RS485 adapter does not behave well.

Hi! I am also using Vaisala Temp sensor and expereince the same issue using FTDI based USB to RS485 adapter and talking Modbus. Do you know about any other solution except increasing sleep to 30ms? I would prefer not to customize sync.py.

[aharrah]

I had the same preference. I patch sync.py at runtime. To use the code below just execute hot_fix_sync() in your code. This code will verify the old code (just in case sync is updated) and then replace the _wait_for_data function with a new one with a longer timeout.

import logging
import os
import time
from functools import partial

from pymodbus.client.sync import ModbusSerialClient

LOGGER = logging.getLogger('rl.sync_hotfixer')


OLD_CODE = '''    def _wait_for_data(self):
        size = 0
        more_data = False
        if self.timeout is not None and self.timeout != 0:
            condition = partial(lambda start, timeout:
                                (time.time() - start) <= timeout,
                                timeout=self.timeout)
        else:
            condition = partial(lambda dummy1, dummy2: True, dummy2=None)
        start = time.time()
        while condition(start):
            avaialble = self._in_waiting()
            if (more_data and not avaialble) or (more_data and avaialble == size):
                break
            if avaialble and avaialble != size:
                more_data = True
                size = avaialble
            time.sleep(0.01)
        return size
'''


def _new_wait_for_data(self):
    size = 0
    more_data = False
    if self.timeout is not None and self.timeout != 0:
        condition = partial(lambda start, timeout:
                            (time.time() - start) <= timeout,
                            timeout=self.timeout)
    else:
        condition = partial(lambda dummy1, dummy2: True, dummy2=None)
    start = time.time()
    while condition(start):
        available = self._in_waiting()
        if (more_data and not available) or (more_data and available == size):
            break
        if available and available != size:
            more_data = True
            size = available
        time.sleep(0.03)
    return size


def hot_fix_sync():
    # noinspection PyProtectedMember
    fn = ModbusSerialClient._wait_for_data

    if OLD_CODE == inspect.getsource(fn):
        LOGGER.info('Code matches, applying hotfix')
        ModbusSerialClient._wait_for_data = _new_wait_for_data
    else:
        if os.name == 'nt':
            LOGGER.info('pymodbus Code does not match (patching aborted)')
        else:
            LOGGER.error('pymodbus Code does not match (patching aborted)')

[dkristell]

aharrah.
Interesting. I found patch branch patch-307 that I merged into master (locally). That works as well! But perhaps your suggestion with 0.03 is more stable....