Skip to content
This repository

reg-writes return written value instead of returned value #12

Closed
zeeed opened this Issue · 9 comments

3 participants

Jörg Ziegler Aleksey Timin Kelley Reynolds
Jörg Ziegler

Using the following code to write to a holding register:
result = slave.write_single_register (13, 108)

rmodbus writes 108 to holding reg 13:

Tx (8 bytes): [01][06][00][0d][00][6a][98][26]
but in the response, the modbus slave indicates that it has internally limited the value to 100
Rx (8 bytes): [01][06][00][0d][00][64][19][e2]

Intuitively, I would have expected the return value of the method to return the answer PDU. Looking at the gem code, I see that the response is purposefully(?) being overwriten with self. As a consequence, the same holds true for calls to the proxy method.

Is there any reason why the return PDU is not passed back to the caller?

Jörg Ziegler

I've created a fork, adding a humble attempt for your review. https://github.com/zeeed/rmodbus. I have only tried it on Windows and using a serial port - also, I didn't add the right unpacking for coils yet because I'm not using them. So far it seems to work for me with holding_regs.

Aleksey Timin
Owner

It is the design library. For ruby programmers is usually practices to return self object for making chains of calls when to return there is nothing. I think that for returning the PDU string is unnecessary becouse rmodbus has logger for monitoring pdu. What do you want to do with PDU string?

Jörg Ziegler

What do you want to do with PDU string?

when writing to a holding register, the slave can enforce boundaries for the value written to (min/max limits) and send the corrected/accepted value back in the same call. If the modified response is not available in the code, that means that an extra read call must be made after each write to verify the contents.

Aleksey Timin
Owner

Hmmm, I understand your fear, but changing external API is bad idea because it is damage of compatibility. This task need to resolve inside library. For example, exceptions is raised if response is changed.

Kelley Reynolds
Collaborator

If I may, page 4 of http://www.modbus.org/docs/Modbus_Application_Protocol_V1_1b.pdf states that for a non-exceptional response, the server is to echo the function code with a data response. This seems to say that the data response can be different than the data request and this is considered a non-exceptional response. To raise an exception in RModbus for this non-exceptional behavior I think would wrong. According to the specification, we should be returning the data response, not self. I understand that method chaining is all the rage these days, but it seems to be wrong in this case. Maybe we have to bump up another major version to account for it?

Jörg Ziegler

@flipback: true. breaking someone else's implementation is never a good idea. I like the idea of using the exception mechanism, I'll try it out.

@kreynolds: the protocol description says "The normal response is an echo of the request, returned after the register contents/coil states have been written". Emphasis normal. I would go so far as to say "if the response isn't normal, it's exceptional" ;)

Kelley Reynolds
Collaborator

The Specification appears to contradict itself as in the latter half it states what you pasted, and on page 4 it specifically refers to echoing only the 'function code' and allows for a different data response than request. I have not used enough devices to know whether or not it should be an application level exception or a library-level exception, but my gut feeling is that it should be an application-level exception and the library should return the response data if the slave doesn't think it's an exception to return something different. I'm going to try to get an answer from the Modbus working group to determine if that device is out of spec, or our implementation is out of spec.

Jörg Ziegler

From the protocol description:

When the server responds to the client, it uses the function code field to indicate either a normal (error-free) response or that some kind of error occurred (called an exception response). For a normal response, the server simply echoes to the request the original function code.

Is this the section you are referring to?

As stated in the pull request, I'd love to see either of the two (library-level exception OR passing the data through to the application) included, because I think it's potentially useful for others and my own timing would suffer badly if I had to make two requests write => read out of almost every write.

Kelley Reynolds
Collaborator

Referring to "For a normal response, the server simply echoes to the request the original function code." I interpret that to mean that it replies the original function code, not necessarily the original data, the two are distinct.

Aleksey Timin flipback referenced this issue from a commit
Aleksey Timin Fixed issue #12 44cbba5
Aleksey Timin flipback closed this
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Something went wrong with that request. Please try again.