-
-
Notifications
You must be signed in to change notification settings - Fork 1.8k
Don't wait for reply on broadcast write_* operations. #467
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
base: master
Are you sure you want to change the base?
Conversation
I think the behaviour of these should be controlled by "quirks flags" as has been proposed for other areas like non-standard addressing too. (node addresses 247-255) |
Could you elaborate on these quirk flags and how they would work? (or reference a discussion). I've seen some discussion about using broadcast to a serial network with one client as a convenience for not needing to know the client's address. For that to work, the client needs to respond to the broadcast and the server should be expecting a response to a broadcast. Both of those behaviors are deviations from the modbus standard (quoted by stephane in #379). I think the standard should be the default behavior of the library, but I don't see any problem with using a flag or other mechanism to "quirk" the library into the non-standard behavior described above. Is that what you are suggesting, and if so, is the quirk flag system defined enough for me to make the code modifications right now? |
Yes, I was suggesting that allowing non-default things should be optional, and not be forced to only do things in the standard. There's been some discussion and code linked here #38 (there's a few other things that would be covered by quirks) |
But if the library no longer waits for a reply, will it wait at all? What I mean is: is there still a mechanism to wait for what is called the "turnaround delay" (p10/44 spec & impl guide V1.02)? They suggest to set that turnaround delay to 100..200ms. |
@JoelStienlet I see your point. I would also expect that the turnaround delay should be about the same as the response timeout (maybe accounting for transmit time). A given slave is either ready for a new frame or not, and whether a response is transmitted before hand doesn't affect that. My PR as it stands now does not implement this turnaround delay (unless such a function is already implemented in libmodbus, but it sounds like it is not), so a user of the library could send off broadcast messages as frequently as he wishes, and it would be up to him to limit the frequency in such a way so that all slaves can keep up. I think that's a reasonable expectation. If the turnaround delay were added to libmodbus (debatable whether it should be), my guess is that the right place to put it would be a timer that is checked at the beginning of every master transmission to check that the correct turnaround time has elapsed if the previous transmission was a serial broadcast, and wait if it has not. |
Indeed that looks the right place, inserting a test before the master transmission still allows the master to perform some useful task during the turnaround delay, instead of just waiting (if it were placed after the broadcast). |
I implemented a TCP/RTU relay using libmodbus, and handled turnaroudn delay in that layer, yes, delaying RTU sends if it hadn't been long enough since the last one. It only applies to RTU. "compliant" is a bit special for modbus anyway, given how many devices out there aren't. |
I would really like this to be merged.. the delay while waiting for a response to broadcast commands is causing delays in my timing sensitive project. |
This PR addresses #379. It adds a static
is_serial_broadcast
check to the modbus client functions that only write so that a response is not expected when broadcasting over serial. Otherwise, broadcast write requests result in Connection Timed Out errors because modbus serial servers (correctly) do not respond to broadcast write operations.All "make check" tests still pass. Also the Connection Timed Out problem in my write_registers application is gone. However, I did not run or expand the RTU tests. I wanted to open the discussion on what the right approach here would be because my only available slave does not match the hard-coded behaviors in the RTU tests.
Another thing to consider is whether to add a new error type that is returned whenever the broadcast address is used for a client function requesting a response, like read_*. I erred on the side of minimalism and didn't add one so that there would be no change to existing behavior. If anyone is trying to do this now, it either doesn't work or their server device has special behavior, and they are happy with the way things are.