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

Deterministic usage not implemented #1

Open
rubimat opened this issue Oct 5, 2023 · 3 comments
Open

Deterministic usage not implemented #1

rubimat opened this issue Oct 5, 2023 · 3 comments

Comments

@rubimat
Copy link

rubimat commented Oct 5, 2023

I tried using the modbus_rtu library to communicate with some motors. I wanted to use it the way described in your README. However the functions holding_register_* seem only to be virtual functions yet. Maybe I have overlooked something.

I tried to use the provided service call API and the read seems to work or at least I am getting a response back. However the holding_register_write and the holding_register_write_multiple do not seem to work. Is it also because it is not implemented yet? At least I saw a comment on the last line of this README.

It would be nice to at least mention if it is implemented or not in the main README. Some more details on the API and on the code structure would also help to understand the code better. If the project is still running I would be interested in taking a look at it and contribute to it.

Thank you and keep up the good work!

Here is the stderr output when I tried to implement the deterministic way;

---- stderr: rl2_driver                              
/rl2_ws/src/rl2_driver/src/modbus_driver.cpp: In member function ‘bool ModbusDriver::initialize()’:
/rl2_ws/src/rl2_driver/src/modbus_driver.cpp:16:41: error: ‘virtual void remote_modbus::Implementation::holding_register_read(std::shared_ptr<remote_modbus::srv::HoldingRegisterRead_Request_<std::allocator<void> > >, std::shared_ptr<remote_modbus::srv::HoldingRegisterRead_Response_<std::allocator<void> > >)’ is private within this context
   16 |   modbus_rtu_impl->holding_register_read(req, res);
      |   ~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~^~~~~~~~~~
In file included from /rl2_ws/install/remote_modbus_rtu/include/remote_modbus_rtu/implementation.hpp:19,
                 from /rl2_ws/src/rl2_driver/include/rl2_driver/modbus_driver.hpp:8,
                 from /rl2_ws/src/rl2_driver/src/modbus_driver.cpp:1:
/rl2_ws/install/remote_modbus/include/remote_modbus/implementation.hpp:126:16: note: declared private here
  126 |   virtual void holding_register_read(
      |                ^~~~~~~~~~~~~~~~~~~~~
/rl2_ws/src/rl2_driver/src/modbus_driver.cpp:17:1: warning: no return statement in function returning non-void [-Wreturn-type]
   17 | }
      | ^
gmake[2]: *** [CMakeFiles/modbus_driver.dir/build.make:76: CMakeFiles/modbus_driver.dir/src/modbus_driver.cpp.o] Error 1
gmake[1]: *** [CMakeFiles/Makefile2:165: CMakeFiles/modbus_driver.dir/all] Error 2
gmake: *** [Makefile:146: all] Error 2
---
Failed   <<< rl2_driver [6.50s, exited with code 2]
@openvmp
Copy link
Owner

openvmp commented Oct 5, 2023

Thank you for reaching out.

'holding_register_write()' definitely works as it's moving joints of my OpenVMP robot via this driver.

Could you, please, share debug output? You will need to define 'DEBUG' at compile time and, then, change ROS log level to 'debug'.

I would love to see contribution to this module. It is definitely used. My own attention is focused on the robot itself and I didn't make any changes to this layer recently as it already works for all the hardware I need at the moment.

@openvmp
Copy link
Owner

openvmp commented Oct 6, 2023

It would help to understand better how your code is different from this one, especially in terms of class inheritance.

Do you plan to publish your code? Is there any chance to look at your code in some development branch on GitHub? If not, we should probably Zoom or Google Meet to dive deeper into your problem. If you want to set up a call, please reach out.

@rubimat
Copy link
Author

rubimat commented Oct 23, 2023

Finally I had some time to try it again. Taking a look at only the code snippets in the README at the bottom. The factory way compiles and the implementation way does not. I quickly checked your stepper motors code and you seem to be using the factory method.

So this compiles:

#include "remote_modbus_rtu/factory.hpp"

...
  auto modbus_rtu_impl = remote_modbus_rtu::Factory::New(this);
  modbus_rtu_impl->holding_register_read(...);
...

and this:

#include "remote_modbus_rtu/implementation.hpp"

...
  auto modbus_rtu_impl = std::make_shared<remote_modbus_rtu::Implementation>(this);
  modbus_rtu_impl->holding_register_read(...);
...

throws the following error message when compiling:

--- stderr: test_modbus                             
/test_ws/src/test_modbus/src/modbus_driver.cpp: In constructor ‘Bla::Bla()’:
/test_ws/src/test_modbus/src/modbus_driver.cpp:12:38: error: ‘virtual void remote_modbus::Implementation::holding_register_read(std::shared_ptr<remote_modbus::srv::HoldingRegisterRead_Request_<std::allocator<void> > >, std::shared_ptr<remote_modbus::srv::HoldingRegisterRead_Response_<std::allocator<void> > >)’ is private within this context
   12 |     modbus_rtu->holding_register_read(req, resp);
      |     ~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~^~~~~~~~~~~
In file included from /base_ws/install/remote_modbus_rtu/include/remote_modbus_rtu/implementation.hpp:19,
                 from /test_ws/src/test_modbus/include/modbus_driver.hpp:1,
                 from /test_ws/src/test_modbus/src/modbus_driver.cpp:1:
/base_ws/install/remote_modbus/include/remote_modbus/implementation.hpp:126:16: note: declared private here
  126 |   virtual void holding_register_read(
      |                ^~~~~~~~~~~~~~~~~~~~~
gmake[2]: *** [CMakeFiles/modbus_driver.dir/build.make:76: CMakeFiles/modbus_driver.dir/src/modbus_driver.cpp.o] Error 1
gmake[1]: *** [CMakeFiles/Makefile2:137: CMakeFiles/modbus_driver.dir/all] Error 2
gmake: *** [Makefile:146: all] Error 2
---

Do you use only native API calls somewhere?

Also I am curious about following:

  • Did you ever notice any performance difference in the factory method vs the implementation method?
  • Did you try using libmodbus at some point and notice any performance difference in comparison with your implementation?

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

2 participants