Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Intermittent Incomplete Modbus Messages When Polling a Slave Device #2193

Closed
BhanuKiranChaluvadi opened this issue May 12, 2024 · 34 comments
Closed

Comments

@BhanuKiranChaluvadi
Copy link

I am attempting to log data from a Modbus slave device at a high rate, targeting one read per second. However, I am frequently encountering issues with incomplete Modbus messages. Occasionally, I receive complete messages, but the inconsistency is problematic.

Environment

  • Python Version: 3.12.0
  • Operating System: Windows
  • Pymodbus Version: 3.6.6
  • Modbus Hardware: Bronkhorst MFC

Configuration

  • Client Type: RTU - Sync

Problem Description

The goal is to consistently log information from the Modbus slave every second. However, the reception of incomplete messages is frequent, complicating the logging process. I need advice on optimizing the parameters or the loop rate to improve the reliability of the data acquisition.

Steps to Reproduce

  1. Connect to the Modbus slave using the specified configurations.
  2. Attempt to read from the slave device once every second.

Expected Behavior

Receive complete Modbus messages for every polling cycle without errors or incomplete data.

Actual Behavior

Frequent incomplete messages are received, with only occasional complete messages. This is illustrated in the debug logs provided below.

Code and Logs

from pymodbus.client.sync import ModbusSerialClient as ModbusClient
from pymodbus.transaction import ModbusRtuFramer as Framer

client = ModbusClient(
    framer=Framer.RTU,
    port="COM9",
    baudrate=19200,
    parity="E",
    stopbits=1,
    timeout=1,
)
client.connect()
response = client.read_holding_registers(address=41216, count=2, slave=1)
print(response)

Debug Logs

2024-04-29 09:28:40,078 DEBUG Reading Coils
2024-04-29 09:28:40,080 DEBUG Current transaction state - IDLE
2024-04-29 09:28:40,080 DEBUG Running transaction 1
2024-04-29 09:28:40,080 DEBUG SEND: 0x2 0x3 0xa1 0x0 0x0 0x2 0xe7 0xc4
2024-04-29 09:28:40,080 DEBUG Resetting frame - Current Frame in buffer
2024-04-29 09:28:40,080 DEBUG New Transaction state "SENDING"
2024-04-29 09:28:40,080 DEBUG Changing transaction state from "SENDING" to "WAITING FOR REPLY"
2024-04-29 09:28:40,103 DEBUG Incomplete message received, Expected 9 bytes Received 5 bytes !!!!
2024-04-29 09:28:40,103 DEBUG Changing transaction state from "WAITING FOR REPLY" to "PROCESSING REPLY"
2024-04-29 09:28:40,103 DEBUG RECV: 0x2 0x3 0x4 0x3b 0xc8
2024-04-29 09:28:40,103 DEBUG Processing: 0x2 0x3 0x4 0x3b 0xc8
2024-04-29 09:28:40,103 DEBUG Frame - not ready
2024-04-29 09:28:40,103 DEBUG Getting transaction 2
2024-04-29 09:28:40,210 DEBUG Changing transaction state from "PROCESSING REPLY" to "TRANSACTION_COMPLETE"

Questions

  1. How can I adjust the parameters to ensure more consistent receipt of complete messages?
  2. Is there a recommended loop rate or method for polling this type of Modbus device to minimize data loss?
@janiversen
Copy link
Collaborator

Look at ou4 examples !

  1. use async that i# a couple of factors faster

I ran a productions system with 3.6.1 in sync mode, and with a poll rate of 100ms, in essence 30 requests/second pr second without problem.

However I used tcp use seem to use serial, that have a lot more constraints:

  • Low baudrate will limit the throughput severely
  • your devices response time might be a limiting factor

With a request pr second the library is basically sleeping 80-90% of realtime, so you4 problem is somewhere else either your device is responding with partial packets (VERY unusual) or your usb converter is behaving badly (very normal, a lot of rs-485 usb converters handle the packets badly).

@janiversen
Copy link
Collaborator

we have no "polling" you issue a request and wait for the reply.

Copy link

This issue is stale because it has been open 30 days with no activity. Remove stale label or comment or this will be closed in 5 days.

@janiversen
Copy link
Collaborator

janiversen commented Jun 12, 2024

Closing due to inactivity.

@crash1013
Copy link

I have not been able to use pymodbus serial in Python version 11 or version 12. I see the same errors that are shown above, incomplete message received. I use the same code in python 9 and 10 and it works perfectly. It seems like the speed improvements that were made in python version 11 are not compatible with pymodbus serial.

@janiversen
Copy link
Collaborator

Please use the spply debug log call, and let's see a debug log.

@janiversen janiversen reopened this Jun 12, 2024
@janiversen
Copy link
Collaborator

our test harness uses pyserial heavily but with its socket option, I will try to setup a physical serial line and test.

@janiversen
Copy link
Collaborator

This is interesting! I just made a test, connected my mac and solar inverter with rs485 (via a usb hub) and it worked without errors with 9.600

@crash1013
Copy link

crash1013 commented Jun 14, 2024 via email

@janiversen
Copy link
Collaborator

OK, we have for other reasons decided to make a v3.6.9 that will be done this weekend, so I need your log fast to include a fix in that release.

@crash1013
Copy link

Here are the logs, one for each python version 3.9 through 3.12. 3.9 and 3.10 are perfect. 3.11 and 3.12 show the same error. All are running the same script. Only the interpreter is changed. They are all using 3.6.8 except python 3.12 is using pymodbus==3.6.7
test_logfile-V3.9.txt
test_logfile-V3.10.txt
test_logfile-V3.11.txt
test_logfile-V3.12.txt

@janiversen
Copy link
Collaborator

First of all let me say again, change to async will help you a lot ! pyserial remains the same, but the pymodbus transport layer is "hardened".

From what I see pyserial or your usb stick "eats" the last byte:

Incomplete message received, Expected 21 bytes Received 20 bytes !!!!
Changing transaction state from "WAITING FOR REPLY" to "PROCESSING REPLY"
RECV: 0xf8 0x3 0x10 0x0 0x0 0x0 0x0 0x0 0x0 0x0 0x0 0x0 0x0 0x0 0x0 0x0 0x0 0x0 0x0 0x48
Processing: 0xf8 0x3 0x10 0x0 0x0 0x0 0x0 0x0 0x0 0x0 0x0 0x0 0x0 0x0 0x0 0x0 0x0 0x0 0x0 0x48
Frame - not ready

Normally the failing byte would be part of the next receive, but its not:

RECV: 0xf8 0x3 0x2 0x0 0xb 0x65 0x97

So my experience tells me it's likely a cable problem or a missing pull-up resistor. I have today a serial connection to my solar inverter and read:
1 value every 50ms
10 values every 1s
90 values every 15minutes
that runs with python 3.12 on a RPI4, without problems (and the library isn't even sweating, so 1 read/second is slow).

You might try to lower the baud rate, since windows is known to have problem with usb-serial devices at higher rates.

I cannot tell you why its works with lower versions of pymodbus, since I do not have them installed.

@crash1013
Copy link

crash1013 commented Jun 17, 2024 via email

@janiversen
Copy link
Collaborator

Agreed but I have no idea on how to help, I do not have windows installed, and anyhow it seems to be a problem outside pymodbus.

There is a PR that to set rs485 parameters that might help you.

@jameshilliard
Copy link
Contributor

Is pymodbus supposed to throw an exception during the read if it's incomplete, I've been seeing this pattern a lot in our error reporting when using AsyncModbusSerialClient on pymodbus version 3.6.9 with python 3.12 on x86_64 Linux.

For code like this:

data = await c.read_holding_registers(58, 48, slave=self.address)
decoder = BinaryPayloadDecoder.fromRegisters(
    data.registers,
    byteorder=Endian.BIG,
    wordorder=Endian.BIG,
)
current_list = []
for i in range(0, 48):
    current_list.append(Decimal(decoder.decode_16bit_uint()) / Decimal(10))

I often see errors like these indicating incomplete reads(which is confirmed via sentry logs showing the payload decoder handle being empty or in come cases below the size of the expected read):

error: unpack requires a buffer of 2 bytes
  File "pymodbus/payload.py", line 373, in decode_16bit_uint

What I'm confused about is how it's possible for the read_holding_registers request to not thrown an exception or retry the request on an incomplete read? Some parts of the code are a little hard to follow but it looks like the expected response length is supposed to be getting validated but is apparently not under some conditions.

@janiversen
Copy link
Collaborator

You should test data, that do not always have registers as return value, but an error instead.

please see our examples.

@jameshilliard
Copy link
Contributor

You should test data, that do not always have registers as return value, but an error instead.

Well...sometimes there's actually register data there in the data.registers variable, but not the expected amount, like 2 registers(4 bytes) expected(i.e. decode_32bit_uint) with only 1 register(2 bytes) worth of data, I don't see how that error could be due to not checking the return here for data.registers if I'm also seeing these sort of unpack errors with actual register data(but truncated) in data.registers as well.

I was trying to go through the pymodbus register read response size validation code but some of it was a bit hard to follow for me(I keep getting mixed up which codepaths are handling async only operations and which parts are sync only) well enough to know if there might be a bug there somewhere allowing truncated reads rather than exceptions in some cases.

I also don't trust a lot of the hardware we're interfacing with to not be sending totally nonsense garbage over the rs485 lines fairly often or in some cases to just not respond at all when it should.

@janiversen
Copy link
Collaborator

Normally the error flag will be set in this condition, which you do not check.

@janiversen
Copy link
Collaborator

Remark if pymodbus receives a VALID message with only 1 register, that will be passed as legal. We do not check the amount you requested against the amount received that is for the app to do.

@jameshilliard
Copy link
Contributor

We do not check the amount you requested against the amount received that is for the app to do.

That's rather...unexpected behavior, what use case would there be for returning a partial read instead of an error? I mean one issue here seems to be that pymodbus implements retry logic so validating response length app side would be rather complex(as you would then likely need to re-implement some of the retry logic in the app as well). I also don't see the examples validating register read response sizes.

Is there anything else that doesn't get validated in the response handling like function codes? Are other requests like register write responses not fully validated?

@jameshilliard
Copy link
Contributor

jameshilliard commented Jul 13, 2024

Normally the error flag will be set in this condition, which you do not check.

Does this error flag only get set after all retries have been attempted for cases where no register data was received?

By the way is there a specific reason for requiring an error condition check like this vs just say having the read register call raise an exception on error instead?

@janiversen
Copy link
Collaborator

Does this error flag only get set after all retries have been attempted for cases where no register data was received?

The error flag get set when there are an error condition.

By the way is there a specific reason for requiring an error condition check like this vs just say having the read register call raise an exception on error instead?

Yes, that is how it's implemented.

@jameshilliard
Copy link
Contributor

jameshilliard commented Jul 13, 2024

The error flag get set when there are an error condition.

So it gets set if there is an initial error on the first request but a subsequent retry results in a valid read? Or does it only get set if all retries fail? It's not at all clear what the error flag actually means here. Like is it only set for recoverable errors or only for unrecoverable errors or for both?

Yes, that is how it's implemented.

I'm thinking it would make sense to change this to just raise an exception instead of setting a flag, especially since the docs for these read register functions indicate they can raise a ModbusException, which can happen AFAIU for other error conditions like timeouts and such. Having two entirely different ways of handling errors for the same function is rather surprising/unexpected behavior IMO.

Edit: I just noticed this example which has 3 entirely different ways of checking for errors, does this need to be done for all pymodbus calls?

@janiversen
Copy link
Collaborator

The error flag get set for unrecoverable error, retry is no an error, that is a perfect normal network situation.

ModbusException is, as documented, raised for internal errors, the error flag is for device errors. Of course that can be changed, but it would affect all apps using pymodbus, so that might be something for a 4.0 release.

Regarding the length expectation, there are a lot of devices that return less than requested for a number of good reasons, so implementing such a check would break perfectly ok apps.

@janiversen
Copy link
Collaborator

The 3 distinct ways you refer to, seems to be because you missed to read the comments...exceptionResponse can come anytime from a device, but are a valid response (indicating some problem in the device), e,g, try to write to a read only register.

We pointed it out in a number of examples, because a lot of users are unaware of this response,
.

@jameshilliard
Copy link
Contributor

ModbusException is, as documented, raised for internal errors, the error flag is for device errors. Of course that can be changed, but it would affect all apps using pymodbus, so that might be something for a 4.0 release.

Yeah I'm thinking it would be a good idea to unify the error handling so that all error conditions just raise different exceptions, maybe with all error being different error extending the ModbusException class, it's just very intuitive IMO to have so many different ways to check for types of errors for each call when in many cases one just cares that there is an error or not.

Regarding the length expectation, there are a lot of devices that return less than requested for a number of good reasons, so implementing such a check would break perfectly ok apps.

Hmm, this is not the norm though right or allowed by the modbus spec? It sounds to me that by default we should treat truncated responses as errors(and raise an exception after exhausting retries) but also have an escape hatch as a param like anallow_truncation boolean or something one passes to the read_holding_registers function to allow truncated responses for the specific request.

The 3 distinct ways you refer to, seems to be because you missed to read the comments...exceptionResponse can come anytime from a device, but are a valid response (indicating some problem in the device), e,g, try to write to a read only register.

I mean, even if it's technically a valid modbus response one can still make use of python's exception handling mechanisms in order to inform the application that the device received an exception response(like by raising a different variation of ModbusException and having the application catch the more specific exception before handling the generic case).

@janiversen
Copy link
Collaborator

Yeah I'm thinking it would be a good idea to unify the error handling so that all error conditions just raise different exceptions, maybe with all error being different error extending the ModbusException class, it's just very intuitive IMO to have so many different ways to check for types of errors for each call when in many cases one just cares that there is an error or not.

you are of course free to think as you like, the original and myself think differently! e.g. an exception response is perfectly legal and not considered an error in the modbus standard, then why should we.

You are making a very theoretical discussion, this library have a
one implementation, if you do not like make changes or use another library.

Hmm, this is not the norm though right or allowed by the modbus spec? It sounds to me that by default we should treat truncated responses as errors(and raise an exception after exhausting retries) but also have an escape hatch as a param like anallow_truncation boolean or something one passes to the read_holding_registers function to allow truncated responses for the specific request.

It is actually the norm ! the app specifies to read 10 registers from a number of devices, but one do not have all configured so it only returns what is has.

I mean, even if it's technically a valid modbus response one can still make use of python's exception handling mechanisms in order to inform the application that the device received an exception response(like by raising a different variation of ModbusException and having the application catch the more specific exception before handling the generic case).

All technically possible, but long time ago this library was created with some ground rules…..and I for one would never transform a legal modbus message (like exception) into a pymodbus exception.

@janiversen
Copy link
Collaborator

so to sum it up, the library is at it is, just like other libraries
, each with its own set of pros and cons…..

You are welcome to change that by submitting pull requests, which will be accepted when they are generic (e.g. have a defined function for all framers.

Discussing changes just for the sake of discussing it really not worth my time.

@jameshilliard
Copy link
Contributor

but one do not have all configured so it only returns what is has.

So when you have a partial read like this how do you know which specific registers were returned? Like if say you read register 1-10 but register 3 and 7 are not configured. Or I guess this probably only works if the truncated response starts at the start read offset?

All technically possible, but long time ago this library was created with some ground rules…..and I for one would never transform a legal modbus message (like exception) into a pymodbus exception.

There's def a bit of style weirdness here IMO, but I'm just trying to figure out the best way to handle what I would assume the be the normal case of reading some registers and returning either a complete response or raising an exception either by coming up with a pull request to pymodbus or some sort of wrapper in my code.

You are welcome to change that by submitting pull requests, which will be accepted when they are generic (e.g. have a defined function for all framers.

Part of the reason I'm discussing this is to see what pull requests may make sense here since I don't have a lot of background knowledge regarding reasons things are done certain ways. I'm trying to figure out how to properly implement retry logic for truncated reads in pymodbus.

@janiversen
Copy link
Collaborator

The app need to deal with exception response, since it is not an error but tells e.g. that the device is busy.

@crash1013
Copy link

crash1013 commented Jul 14, 2024 via email

@jameshilliard
Copy link
Contributor

The app need to deal with exception response, since it is not an error but tells e.g. that the device is busy.

I mean, wouldn't retrying be the normal way to handle a busy error response?

Could you please try installing python 10, not in the path. Then create a virtual environment for your application using Python 10. You can do pip freeze to create your requirements in order to duplicate you python 12 environment. Your problems should go away, they did for me.

Installing python10 with my setup is non-trivial since I'm not using a normal distro but rather buildroot to cross compile my entire operating system and userspace including the system python interpreter and python package dependencies, the version of buildroot I'm using only supports python12 and it would require significant changes to use an older version.

@crash1013
Copy link

crash1013 commented Jul 16, 2024 via email

@janiversen
Copy link
Collaborator

Seems the original issue, is no longer a concern, so closing.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

4 participants