rqt_controller_manager: Error in logic for controller activation

christophfroehlich commented 1 month ago

Background

If the controller is unconfigured (e.g, after unloading with the GUI), the item "configure and activate" is shown. https://github.com/ros-controls/ros2_control/blob/0433960580c5834b11af9703e3e34ace86824d01/rqt_controller_manager/rqt_controller_manager/controller_manager.py#L236-L238 But then, the wrong actions are performed. Instead of load+activate it should configure+activate https://github.com/ros-controls/ros2_control/blob/0433960580c5834b11af9703e3e34ace86824d01/rqt_controller_manager/rqt_controller_manager/controller_manager.py#L262-L264

[ruby $(which gz) sim-1] [INFO] [1728827696.045253385] [controller_manager]: Loading controller 'joint_trajectory_controller'
[ruby $(which gz) sim-1] [ERROR] [1728827696.045393950] [controller_manager]: A controller named 'joint_trajectory_controller' was already loaded inside the controller manager
[ruby $(which gz) sim-1] [INFO] [1728827696.060470338] [controller_manager]: Activating controllers: [ joint_trajectory_controller ]
[ruby $(which gz) sim-1] [WARN] [1728827696.060527518] [controller_manager]: Controller with name 'joint_trajectory_controller' is not inactive so its following controllers do not have to be checked, because it cannot be activated.
[ruby $(which gz) sim-1] [WARN] [1728827696.060534672] [controller_manager]: Could not activate controller with name 'joint_trajectory_controller'. Check above warnings for more details. Check the state of the controllers and their required interfaces using `ros2 control list_controllers -v` CLI to get more information.
[ruby $(which gz) sim-1] [ERROR] [1728827696.060540763] [controller_manager]: Aborting, no controller is switched! (::STRICT switch)

Instructions

Hi, this is a good-first-issue issue. This means we've worked to make it more legible to people who either haven't contributed to our codebase before, or even folks who haven't contributed to open source before.

We're interested in helping you take the first step, and can answer questions and help you out along the way. Note that we're especially interested in contributions from underrepresented groups!

We know that creating a pull request is the biggest barrier for new contributors. This issue is for you 💝

If you have contributed before, consider leaving this PR for someone new, and looking through our general bug issues. Thanks!

🤔 What you will need to know.

Nothing. This issue is meant to welcome you to Open Source :) We are happy to walk you through the process.

📋 Step by Step

[ ] 🙋 Claim this issue: Comment below. If someone else has claimed it, ask if they've opened a pull request already and if they're stuck -- maybe you can help them solve a problem or move it along!
[ ] 🗄️ Create a local workspace for making your changes and testing following these instructions, for Step 3 use "Download Source Code" section with these instructions.
[ ] 🍴 Fork the repository using the handy button at the top of the repository page and clone it into ~/ws_ros2_control/src/ros-controls/ros2_control, here is a guide that you can follow (You will have to remove or empty the existing ros2_control folder before cloning your own fork)
[ ] Checkout a new branch using git checkout -b <branch_name>
[ ] 🤖 Apply pre-commit auto formatting, by running pip3 install pre-commit and running pre-commit install in the ros2_control repo.
[ ] 💾 Commit and Push your changes
[ ] 🔀 Start a Pull Request to request to merge your code into master. There are two ways that you can start a pull request:
1. If you are not familiar with GitHub or how to create a pull request, here is a guide you can follow on how GitHub works.
[ ] 🏁 Done Ask in comments for a review :)

Is someone else already working on this?

🔗- We encourage contributors to link to the original issue in their pull request so all users can easily see if someone's already started on it.

👥- If someone seems stuck, offer them some help!

🤔❓ Questions?

Don’t hesitate to ask questions or to get help if you feel like you are getting stuck. For example leave a comment below! Furthermore, you find helpful resources here:

Good luck with your first issue!

christophfroehlich commented 1 month ago

@SantoshGovindaraj you want to tackle this?

SantoshGovindaraj commented 1 month ago

Thank you @christophfroehlich for reaching out! I’d be happy to take on this issue. I believe this would be a great opportunity to contribute further. Just to mention, I’m still relatively new to open-source contributions, but I’m eager to learn and tackle the challenge. I’ll do my best to implement the required changes and would appreciate any guidance along the way. 🙌 Is there any starting point to look into?

christophfroehlich commented 1 month ago

Welcome! For this reasons we have the good-first-issue labels ;) Have a look at the code snippets I posted, in fact it is a single line of code to change.

christophfroehlich commented 1 month ago

You can use any of the ros2_control_demos or gz_ros2_control_demos for playing aroung with the GUI.

SantoshGovindaraj commented 1 month ago

Hi @christophfroehlich I just managed to look into this. As far as I understand, I think we need to just change method name from load_controller to configure_controller in line 263.

christophfroehlich commented 1 month ago

yes I think so, too.

SantoshGovindaraj commented 1 month ago

Many Thanks @christophfroehlich , I have amended the changes and I am trying to the test it with ros2_control_demo_example_2, to find any other potential bugs. Once verified, I create PR for to solve this issue. Is there anything more we might have to look into?

SantoshGovindaraj commented 1 month ago

@christophfroehlich I have a quick question that I hope you don’t mind addressing. I’ve been trying to resolve this issue for a while. As per the instructions, I forked the repo and built it on my system (Ubuntu 22, ROS Iron). In the same workspace, I cloned and successfully built ros2_control_demos. However, when running diffbot.launch.py, the joint_state_controller and diffbot_base_controller fail to load.

Should I be working with the iron branch instead? If so, would this create any conflicts when submitting a PR?

Apologies if this isn't the appropriate place to ask this question.

christophfroehlich commented 1 month ago

If you want to use the rolling version of ros2_control (the master branch) on an iron distro, you also have to compile the correct version of ros2_controllers. See this repos file for example.

SantoshGovindaraj commented 1 month ago

Thanks, @christophfroehlich . I'm still encountering issues building the ros2_control master branch with ROS Iron, especially building controllers. Based on the ros_control_ci, it seems that rolling compatibility with Iron is failing, I believe.

I will push a PR with the suggested changes. Please let me know if any additional modifications are needed.

christophfroehlich commented 1 month ago

what are the issues with building it? as far as I remember, the compatibility job only fails somewhere in the tests

SantoshGovindaraj commented 1 month ago

Yes, I agree. The build is failing in the tests. The following is the build error I am facing.

--- stderr: joint_state_broadcaster                                               
/home/santosh/open_source/ros2_control_ws/src/ros-controls/ros2_controllers/joint_state_broadcaster/test/test_joint_state_broadcaster.cpp: In member function ‘void JointStateBroadcasterTest::init_broadcaster_and_set_parameters(const std::vector<std::__cxx11::basic_string<char> >&, const std::vector<std::__cxx11::basic_string<char> >&)’:
/home/santosh/open_source/ros2_control_ws/src/ros-controls/ros2_controllers/joint_state_broadcaster/test/test_joint_state_broadcaster.cpp:72:47: error: no matching function for call to ‘FriendJointStateBroadcaster::init(const char [24])’
   72 |   const auto result = state_broadcaster_->init("joint_state_broadcaster");
      |                       ~~~~~~~~~~~~~~~~~~~~~~~~^~~~~~~~~~~~~~~~~~~~~~~~~~~
In file included from /home/santosh/open_source/ros2_control_ws/install/controller_interface/include/controller_interface/controller_interface/controller_interface.hpp:21,
                 from /home/santosh/open_source/ros2_control_ws/src/ros-controls/ros2_controllers/joint_state_broadcaster/include/joint_state_broadcaster/joint_state_broadcaster.hpp:24,
                 from /home/santosh/open_source/ros2_control_ws/src/ros-controls/ros2_controllers/joint_state_broadcaster/test/test_joint_state_broadcaster.cpp:28:
/home/santosh/open_source/ros2_control_ws/install/controller_interface/include/controller_interface/controller_interface/controller_interface_base.hpp:129:15: note: candidate: ‘controller_interface::return_type controller_interface::ControllerInterfaceBase::init(const string&, const string&, unsigned int, const string&, const rclcpp::NodeOptions&)’
  129 |   return_type init(
      |               ^~~~
/home/santosh/open_source/ros2_control_ws/install/controller_interface/include/controller_interface/controller_interface/controller_interface_base.hpp:129:15: note:   candidate expects 5 arguments, 1 provided
/home/santosh/open_source/ros2_control_ws/src/ros-controls/ros2_controllers/joint_state_broadcaster/test/test_joint_state_broadcaster.cpp: In member function ‘void JointStateBroadcasterTest::activate_and_get_joint_state_message(const string&, sensor_msgs::msg::JointState&)’:
/home/santosh/open_source/ros2_control_ws/src/ros-controls/ros2_controllers/joint_state_broadcaster/test/test_joint_state_broadcaster.cpp:673:11: error: ‘rclcpp::executors’ has not been declared
  673 |   rclcpp::executors::SingleThreadedExecutor executor;
      |           ^~~~~~~~~
/home/santosh/open_source/ros2_control_ws/src/ros-controls/ros2_controllers/joint_state_broadcaster/test/test_joint_state_broadcaster.cpp:674:3: error: ‘executor’ was not declared in this scope
  674 |   executor.add_node(test_node.get_node_base_interface());
      |   ^~~~~~~~
/home/santosh/open_source/ros2_control_ws/src/ros-controls/ros2_controllers/joint_state_broadcaster/test/test_joint_state_broadcaster.cpp: In member function ‘void JointStateBroadcasterTest::test_published_dynamic_joint_state_message(const string&)’:
/home/santosh/open_source/ros2_control_ws/src/ros-controls/ros2_controllers/joint_state_broadcaster/test/test_joint_state_broadcaster.cpp:749:11: error: ‘rclcpp::executors’ has not been declared
  749 |   rclcpp::executors::SingleThreadedExecutor executor;
      |           ^~~~~~~~~
/home/santosh/open_source/ros2_control_ws/src/ros-controls/ros2_controllers/joint_state_broadcaster/test/test_joint_state_broadcaster.cpp:750:3: error: ‘executor’ was not declared in this scope
  750 |   executor.add_node(test_node.get_node_base_interface());
      |   ^~~~~~~~
gmake[2]: *** [CMakeFiles/test_joint_state_broadcaster.dir/build.make:76: CMakeFiles/test_joint_state_broadcaster.dir/test/test_joint_state_broadcaster.cpp.o] Error 1
gmake[1]: *** [CMakeFiles/Makefile2:264: CMakeFiles/test_joint_state_broadcaster.dir/all] Error 2
gmake[1]: *** Waiting for unfinished jobs....
In file included from /usr/include/c++/11/memory:76,
                 from /opt/ros/iron/src/gmock_vendor/include/gmock/gmock-actions.h:46,
                 from /opt/ros/iron/src/gmock_vendor/include/gmock/gmock.h:59,
                 from /home/santosh/open_source/ros2_control_ws/src/ros-controls/ros2_controllers/joint_state_broadcaster/test/test_load_joint_state_broadcaster.cpp:15:
/usr/include/c++/11/bits/unique_ptr.h: In instantiation of ‘typename std::_MakeUniq<_Tp>::__single_object std::make_unique(_Args&& ...) [with _Tp = hardware_interface::ResourceManager; _Args = {const std::__cxx11::basic_string<char, std::char_traits<char>, std::allocator<char> >&}; typename std::_MakeUniq<_Tp>::__single_object = std::unique_ptr<hardware_interface::ResourceManager>]’:
/home/santosh/open_source/ros2_control_ws/src/ros-controls/ros2_controllers/joint_state_broadcaster/test/test_load_joint_state_broadcaster.cpp:33:58:   required from here
/usr/include/c++/11/bits/unique_ptr.h:962:30: error: no matching function for call to ‘hardware_interface::ResourceManager::ResourceManager(const std::__cxx11::basic_string<char>&)’
  962 |     { return unique_ptr<_Tp>(new _Tp(std::forward<_Args>(__args)...)); }
      |                              ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
In file included from /home/santosh/open_source/ros2_control_ws/install/controller_manager/include/controller_manager/controller_manager/controller_manager.hpp:44,
                 from /home/santosh/open_source/ros2_control_ws/src/ros-controls/ros2_controllers/joint_state_broadcaster/test/test_load_joint_state_broadcaster.cpp:18:
/home/santosh/open_source/ros2_control_ws/install/hardware_interface/include/hardware_interface/hardware_interface/resource_manager.hpp:69:12: note: candidate: ‘hardware_interface::ResourceManager::ResourceManager(const string&, rclcpp::node_interfaces::NodeClockInterface::SharedPtr, rclcpp::node_interfaces::NodeLoggingInterface::SharedPtr, bool, unsigned int)’
   69 |   explicit ResourceManager(
      |            ^~~~~~~~~~~~~~~
/home/santosh/open_source/ros2_control_ws/install/hardware_interface/include/hardware_interface/hardware_interface/resource_manager.hpp:69:12: note:   candidate expects 5 arguments, 1 provided
/home/santosh/open_source/ros2_control_ws/install/hardware_interface/include/hardware_interface/hardware_interface/resource_manager.hpp:51:12: note: candidate: ‘hardware_interface::ResourceManager::ResourceManager(rclcpp::node_interfaces::NodeClockInterface::SharedPtr, rclcpp::node_interfaces::NodeLoggingInterface::SharedPtr)’
   51 |   explicit ResourceManager(
      |            ^~~~~~~~~~~~~~~
/home/santosh/open_source/ros2_control_ws/install/hardware_interface/include/hardware_interface/hardware_interface/resource_manager.hpp:51:12: note:   candidate expects 2 arguments, 1 provided
gmake[2]: *** [CMakeFiles/test_load_joint_state_broadcaster.dir/build.make:76: CMakeFiles/test_load_joint_state_broadcaster.dir/test/test_load_joint_state_broadcaster.cpp.o] Error 1
gmake[1]: *** [CMakeFiles/Makefile2:235: CMakeFiles/test_load_joint_state_broadcaster.dir/all] Error 2
gmake: *** [Makefile:146: all] Error 2
---
Failed   <<< joint_state_broadcaster [5.19s, exited with code 2]

christophfroehlich commented 1 month ago

Yes, I agree. The build is failing in the tests. The following is the build error I am facing.

No, the test stage is failing, not the build. From your log it seems that you have mixed different versions of the repos, or you have sourced your iron workspace before the build and the overlay does not work. Do you have ros2_control, ros2_controllers, and ros2_control_demos on the master branch? And built every dependency, e.g, colcon build --packages-up-to ros2_control_demo_example_2?

SantoshGovindaraj commented 1 month ago

From your log it seems that you have mixed different versions of the repos, or you have sourced your iron workspace before the build and the overlay does not work. Oh, okay. I understand.

Yes, it solved the issue I was having. Now, I am able to successfully build and run the ros2_control_demo_example_2. Many thanks @christophfroehlich for the clarification and helping with my problem. This will be useful understanding for my future contribution to the repo. I also tested the changes too. 😃

ros-controls / ros2_control