Skip to content

added namespace to controller manager#77

Closed
ahcorde wants to merge 1 commit intomasterfrom
ahcorde/add_cm_namespace
Closed

added namespace to controller manager#77
ahcorde wants to merge 1 commit intomasterfrom
ahcorde/add_cm_namespace

Conversation

@ahcorde
Copy link
Copy Markdown
Collaborator

@ahcorde ahcorde commented Jul 28, 2022

Signed-off-by: ahcorde ahcorde@gmail.com

Minor fix to allow CM to be run in the defined namespace, ref ros-controls/gazebo_ros2_control#122

Related PR ros-controls/gazebo_ros2_control#147

Signed-off-by: ahcorde <ahcorde@gmail.com>
@ahcorde ahcorde requested a review from destogl July 28, 2022 09:38
@ahcorde ahcorde self-assigned this Jul 28, 2022
@ahcorde ahcorde requested a review from bmagyar July 28, 2022 09:41
@moutalibbadr
Copy link
Copy Markdown

moutalibbadr commented Jul 29, 2022

@ahcorde @destogl It seems after looking around in the code that adding the namespace while creating the controller_manager is not enough. This should maybe be done also for the ignition_ros_control node

  std::string node_name = "ignition_ros_control";
  this->dataPtr->node_ = rclcpp::Node::make_shared(node_name, this->dataPtr->node_->get_namespace());

And also for rclcpp::init(static_cast<int>(argv.size()), argv.data()); we should take away the namespace argument

  std::string ns_arg = std::string("__ns:=") + ns;
  arguments.push_back(RCL_REMAP_FLAG);
  arguments.push_back(ns_arg);

since this will initialize our context inside the namespace and then whenever we try to create a new controller_manage it will do it inside the same context and thus the same namespace.

Testing this seems to work and will give us separated controller_manager instances in different namespaces but when we try to spawn our controllers they will be without namespace.

Copy link
Copy Markdown
Member

@destogl destogl left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Can you also please remove other obsolete CM namespace parameter and address comment above?

@moutalibbadr
Copy link
Copy Markdown

Hi @ahcorde, I don't know if you had time to look at my last comment. Having tested those changes locally seemed to fix the namespacing for the create3 with multiple instances of the robot in the same simulation. We had however to prefix our controllers in the config files
e.g:

/robot_0:
  controller_manager:
    ros__parameters:
      update_rate: 1000  # Hz

      robot_0_joint_state_broadcaster:
        type: joint_state_broadcaster/JointStateBroadcaster

      robot_0_diffdrive_controller:
        type: diff_drive_controller/DiffDriveController   

/robot_1:
  controller_manager:
    ros__parameters:
      update_rate: 1000  # Hz

      robot_1_joint_state_broadcaster:
        type: joint_state_broadcaster/JointStateBroadcaster

      robot_1_diffdrive_controller:
        type: diff_drive_controller/DiffDriveController

and our controller nodes appear like this :

/robot_0_diffdrive_controller
/robot_0_joint_state_broadcaster
/robot_1_diffdrive_controller
/robot_1_joint_state_broadcaster

And in the case of no namespacing it seemed to be working fine so i suppose these changes should not cause problems for normal applications that do not involve namespacing.

Tell me if i should put this in a separate PR.

@christophfroehlich
Copy link
Copy Markdown
Member

Seems to be fixed with #92 and #128

@christophfroehlich christophfroehlich deleted the ahcorde/add_cm_namespace branch January 3, 2025 19:40
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

Successfully merging this pull request may close these issues.

4 participants