libmodbus
libmodbus copied to clipboard
Add modbus_reply_callback() and modbus_set_reply_callback()
Based on @frodete's proposal (#319) this implementation adds a a callback-based reply-functionality.
The old mapping-implementation is now based on this version. As a nice side-effect some DRY code optimizations could have been done.
Unit tests are OK (locally, not on travis though). And my RTU-multi-slave server works.
EDIT: My motivation for needing a callback-based reply is that I need to implement a modbus-tcp-server and a modbus-rtu-slave. Both will handle the data within a cache. The rtu-slave implementation has to be able to handle multiple slaves. On one RS485-port multiple slave-id will be handled.
More details to what I have done: I took the modbus_reply
-function and changed its name to modbus_reply_callback
. Then, each time where something was checked against the start_bits/start_registers
-adresses and tab_*
-counts I replaced it with a call to the callback verify
. And each time the payload-part of the request or response-buffer was accesses I replaced it with a call to the callback read
or write
respectively.
Then I implemented the previous modbus_reply
-function in a way that it uses the modbus_reply_callback
function by providing callbacks for verify
, read
and write
. In these callback-functions I put the code which previously was in charge to fill in the buffer or to verify the boundaries. By doing so, some code-duplicates have been removed, especially accesses to the buffers in the mapping and its verification.
Also adds a MODBUS_SLAVE_ACCEPT_ALL "address", which can be used with modbus_set_slave(). This makes the modbus_receive()-function accept all requests, independently of the slave-address found in the decoded-request. Useful for multi-slave-implementation using libmodbus.
I updated the man-pages and added one for modbus_reply_callback(), please review and comment and merge
Not sure why travis is failing, it works locally.
Endorsement: using this PR in production code; I have read some of the code, it looks good to me. Maybe the interface is a bit too low-level, e.g. I don't like that I need to handle both WRITE_SINGLE_REGISTER and WRITE_MULTIPLE_REGISTERS when the callback signature actually covers both cases, but maybe this flexibility can be useful for some cases. I like the fact that it comes with docu, which seems complete and accurate, although it could profit from being translated into "simple English". I have started doing that, and will finish it if there is interest.
Happy to hear that. Same here. Used happily in two different production environments. One of it really uses it all, especially the key-feature: multiple RTU-slaves.
Regarding WRITE_SINGLE_REGISTER
and WRITE_MULTIPLE_REGISTERS
distinction it is needed. Some slaves/clients might not want to handle the MULTIPLE_REGISTERS in order to be "backward"-compatible for replaced devices (my case).
Thanks for your documentation efforts. Describing all functionalities in a man-pages took me an enormous amount of time if you can simplify it, would be highly appreciated. Happy to pull that in. In my fork at least.
+1 for this PR as well, would be a good point adding support for custom command codes
Why wasn't the pull request merged? Can i help it somehow?
Has there been any more progress on this PR?