(RFC-compliance: Issue813) A violation regarding the specifications for the Read Exception Status.#833
Conversation
|
We require contributors to sign our Contributor License Agreement. In order for us to review and merge your code, please fill https://forms.gle/5635zjphDo5JEJQSA to get added. Your document will be manually checked by the maintainer. Be patient... |
src/modbus.c
Outdated
| // fprintf(stderr, "FIXME Not implemented\n"); | ||
| // } | ||
| // errno = ENOPROTOOPT; | ||
| // return -1; |
There was a problem hiding this comment.
The commented out code should be removed.
src/modbus.c
Outdated
| MODBUS_EXCEPTION_ILLEGAL_FUNCTION, | ||
| rsp, | ||
| TRUE, | ||
| "FIXME Not implemented: 0x%0X\n", |
There was a problem hiding this comment.
Since we here know what function call is not implemented, I would use a human readable string eg. "READ EXCEPTION STATUS (0x07)" (or maybe re-use the libraries "MODBUS_FC_READ_EXCEPTION_STATUS") in the debug output, rather than only the numeric function id.
|
We require contributors to sign our Contributor License Agreement. In order for us to review and merge your code, please fill https://forms.gle/5635zjphDo5JEJQSA to get added. Your document will be manually checked by the maintainer. Be patient... |
src/modbus.c
Outdated
| rsp, | ||
| TRUE, | ||
| "FIXME Not implemented: READ EXCEPTION STATUS (0x07)\n", | ||
| function); |
There was a problem hiding this comment.
The function argument is not needed anymore. We can drop it.
I also recommend to squash all the commits into a single one and then force push to this branch.
There was a problem hiding this comment.
@mhei Thank you for your suggestion.
We have squashed all the commits into a single one and force-pushed to this branch.
|
We require contributors to sign our Contributor License Agreement. In order for us to review and merge your code, please fill https://forms.gle/5635zjphDo5JEJQSA to get added. Your document will be manually checked by the maintainer. Be patient... |
3 participants
We respectfully submit this pull request in order to improve the consistency between libmodbus and its specification.
Why is this PR needed?
In the previous implementation of libmodbus, when modbus clients send the valid but unimplemented command READ EXCEPTION STATUS, there is no response from the server. Therefore, clients will be confused. They are unable to distinguish between an unimplemented condition and a syntax error.
Why can this PR be safely merged?
We have only added the expected handling for unimplemented command types, without modifying any existing functionality. Therefore, our PR can be safely merged without affecting compatibility with actual clients.
We totally respect the priority: Real-world client compatibility > Theoretical RFC compliance, and are more than happy to assist in improving RFC compliance based on this foundation.
Thank you for your attention, and we look forward to your response : )