-
Notifications
You must be signed in to change notification settings - Fork 566
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
Update controller_manager_plugin.cpp #3179
Conversation
Fixing the bug where the namespace is not properly applied when using Ros2ControlMultiManager
@wizard-coder - I reviewed the ticket as well - can you post the contents of |
As a note - this logic has been touched quite a bit in the past:
|
The contents of the config file are as follows: ros2_contollers.left.yaml/**/controller_manager:
ros__parameters:
update_rate: 100 # Hz
arm_controller:
type: joint_trajectory_controller/JointTrajectoryController
joint_state_broadcaster:
type: joint_state_broadcaster/JointStateBroadcaster
/**/arm_controller:
ros__parameters:
joints:
- left_joint0
- left_joint1
- left_joint2
- left_joint3
- left_joint4
- left_joint5
- left_joint6
command_interfaces:
- position
state_interfaces:
- position
allow_nonzero_velocity_at_trajectory_end: true
/**/joint_state_broadcaster:
ros__parameters:
joints:
- left_joint0
- left_joint1
- left_joint2
- left_joint3
- left_joint4
- left_joint5
- left_joint6
interfaces:
- position ros2_controllers.right.yaml/**/controller_manager:
ros__parameters:
update_rate: 100 # Hz
arm_controller:
type: joint_trajectory_controller/JointTrajectoryController
joint_state_broadcaster:
type: joint_state_broadcaster/JointStateBroadcaster
/**/arm_controller:
ros__parameters:
joints:
- right_joint0
- right_joint1
- right_joint2
- right_joint3
- right_joint4
- right_joint5
- right_joint6
command_interfaces:
- position
state_interfaces:
- position
allow_nonzero_velocity_at_trajectory_end: true
/**/joint_state_broadcaster:
ros__parameters:
joints:
- right_joint0
- right_joint1
- right_joint2
- right_joint3
- right_joint4
- right_joint5
- right_joint6
interfaces:
- position |
In the case of Ros2ControlManager, since ns_ is initialized as an empty string, I believe it should be initialized from the ros_control_namespace parameter. However, in the case of Ros2ControlMultiManager, it first finds the namespaces and then initializes ns_ through the Ros2ControlManager(const std::string& ns) initialization function. Therefore, since ns_ is already set at this point, I believe the initialization via ros_control_namespace should be ignored. |
There we go - that's the part I was missing as to how you were getting the ns_ set initially - thanks. |
Codecov ReportAttention: Patch coverage is
❗ Your organization needs to install the Codecov GitHub app to enable full functionality. Additional details and impacted files@@ Coverage Diff @@
## main #3179 +/- ##
==========================================
- Coverage 46.01% 45.62% -0.39%
==========================================
Files 714 714
Lines 62304 62279 -25
Branches 7532 7528 -4
==========================================
- Hits 28666 28406 -260
- Misses 33471 33706 +235
Partials 167 167 ☔ View full report in Codecov by Sentry. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thanks for the fix! I had a similar patch a while back, but I was waiting until I get time to add tests 🫠
It would be really good to add integration tests for this package, I think this's the 3 PR that try to fix the namespacing for Ros2ControlManager
😿
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thanks!
Does this need backports? I didn't look at this PR, so would appreciate @sjahr and @JafarAbdi if you guys could make that call here. I kicked off the backport PRs and you guys can review/merge/close them |
Fixing the bug where the namespace is not properly applied when using Ros2ControlMultiManager Co-authored-by: Sebastian Jahr <[email protected]> (cherry picked from commit 2566467) # Conflicts: # moveit_plugins/moveit_ros_control_interface/src/controller_manager_plugin.cpp
Fixing the bug where the namespace is not properly applied when using Ros2ControlMultiManager Co-authored-by: Sebastian Jahr <[email protected]> (cherry picked from commit 2566467)
Fixing the bug where the namespace is not properly applied when using Ros2ControlMultiManager Co-authored-by: Sebastian Jahr <[email protected]> (cherry picked from commit 2566467) Co-authored-by: Seohyeon Ryu <[email protected]>
* Update controller_manager_plugin.cpp (#3179) Fixing the bug where the namespace is not properly applied when using Ros2ControlMultiManager Co-authored-by: Sebastian Jahr <[email protected]> (cherry picked from commit 2566467) # Conflicts: # moveit_plugins/moveit_ros_control_interface/src/controller_manager_plugin.cpp * Address conflict --------- Co-authored-by: Seohyeon Ryu <[email protected]>
Fixing the bug where the namespace is not properly applied when using Ros2ControlMultiManager
Description
When using two ROS2 controls, the namespace is automatically detected, but the namespace is not properly applied when registering the action client.
For more details, please refer to #3150