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

Context and Namespace Handling for Multi-Robot Sim #92

Merged
merged 14 commits into from
Mar 28, 2023

Conversation

sp-sophia-labs
Copy link

@sp-sophia-labs sp-sophia-labs commented Nov 4, 2022

Description

The goal of this PR is to enable namespaces and prevent segmentation faults when spawning multiple time the same robot

Type of change: fix. This change aims at fixing the following open issue: #49

An existing PR seems to be targeting the namespace issue: #77. But it does not solve the context and controller manager node name issues

How has this been tested?

We have tested this code on create3 and turtlebot4 robots. You can find our project here: https://github.com/sp-sophia-labs/turtlebot4_multi_sim. Reviewers may clone the project and replicate exactly what we use for testing. Here are the launch commands (you can find the details in the project's README)

Launch the world and the first create3 instance:
ros2 launch irobot_create_ignition_bringup create3_ignition.launch.py namespace:=robot_0 robot_name:=robot_zero

Alternatively, you can launch turtlebot4 instead:
ros2 launch turtlebot4_ignition_bringup ignition.launch.py namespace:=robot_0 robot_name:=robot_zero

In a separate terminal, spawn another create3/turtlebot4 instance with different name, namespace and pose:
ros2 launch irobot_create_ignition_bringup create3_spawn.launch.py namespace:=robot_1 robot_name:=robot_one y:=1
ros2 launch irobot_create_ignition_bringup turtlebot4_spawn.launch.py namespace:=robot_1 robot_name:=robot_one y:=1
You can change the x, y, z param to spawn the robot where needed

Checklist

  • Lint code with appropriate parser
  • Evaluate a more elegant way to get robot namespace (cf. the other pull request)
  • Rebase code on galactic branch

@Ronoman
Copy link

Ronoman commented Nov 7, 2022

I'm blocked on #49, so I'm excited to test these changes!

@bmagyar
Copy link
Member

bmagyar commented Nov 25, 2022

Could you please retarget this to master?

@destogl
Copy link
Member

destogl commented Nov 27, 2022

Is this overlapping with #77. Does #77 also fixes this issue, or should we close it?

@sp-sophia-labs
Copy link
Author

sp-sophia-labs commented Nov 28, 2022

@bmagyar Yes sure

@destogl Yes, this is overlapping with #77. I've tested #77 on my side and while it resolves some of the issues it does not cover multi-context creation

@sp-sophia-labs sp-sophia-labs changed the base branch from galactic to master November 29, 2022 13:37
@sp-sophia-labs sp-sophia-labs requested review from Ronoman and ahcorde and removed request for ahcorde and Ronoman December 7, 2022 17:52
@ArganMechatronic
Copy link

Hello,
Thank you very much for the help you are providing on the issues about multi robot spawning with ign_ros2_control. I also need this ability.
I'm using Ros2 Foxy with Ignition Citadel and a built version of ign_ros2_control 0.1.4 in which I have modified the ign_ros2_control_plugin.cpp file with your PR modification. Then, I can successfully load multiple controller_manager with their own namespace, however I can't load a controller with is namespace anymore.
It was possible before the PR' modification, for only one robot, but I don't understand yet which part of the code is responsible of that and why ...
Do you have any idea on how to make this plugin launch a namespace for the controller_manager and the controllers at once ? Could it be link to the commit "Namespace Loaded Controllers" you also participated to, @sp-sophia-labs and @bmagyar ?

For information, I use :

  • Ubuntu 20.04.5 LTS
  • ros2-control 0.11.0
  • ros2-controllers 0.8.2
  • ignition-citadel 1.0.2
  • The controllers from ros2_control, mainly the position controller.
  • I work with a workspace and a robot I built myself.

@sp-sophia-labs
Copy link
Author

@ArganMechatronic Thank you for your interest in this PR. Here is the description of what we are trying to achieve: iRobotEducation/create3_sim#189 you'll also find links to all the other modifications we have been pushing on different repos including ros-controls/ros2_control#852 (as you correctly guessed) and ros-controls/ros2_controllers#461

As for your issue, I would recommend trying to apply the content of the above PRs (what you describe is very close to what we fixed) . Sadly, we have only been testing this on ROS Galactic on our side so I cannot assure you that it would work in your case

PS. It turns out that I'm based in France too, and I'll be happy to help more in depth if you need: https://www.linkedin.com/in/hugo-borne-336539260/

@destogl
Copy link
Member

destogl commented Jan 5, 2023

Do you have any idea on how to make this plugin launch a namespace for the controller_manager and the controllers at once ?

Are you setting the controller manager parameter into spawners? It should be something like -c /<namespace>/controller_manager add as additional argument in the spawners call. We have this argument in examples (for sure in ros2_control_demos) repository.

If this is working, can you load the controller using service calls on specific controller manager?

@ArganMechatronic
Copy link

@destogl, @sp-sophia-labs,

Thank you for your answers !

Yes I'm using the -c /<namespace>/controller_manager tag in my commands to launch the controllers. I can see the multiple controller manager spawned but the simulation crash since it's the controllers turn. And, actually, I took the ros2_control_demos launchfile as a basis for my launchfile 👍.

I had the opportunity to discuss about it more precisly with @sp-sophia-labs and it seems that I will have to find the good modification to add in ros2_control and in the trajectory_controller. Some differences between Foxy and Galactic could be a factor too.

When I will find the solution I will keep you up to date !

@sp-sophia-labs
Copy link
Author

@ahcorde @bmagyar @destogl Hello, is there something blocking this code from being merged? Please tell me if I'm missing something

Copy link
Member

@bmagyar bmagyar left a comment

Choose a reason for hiding this comment

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

@sp-sophia-labs as far as I'm concerned, only my outstanding comment is missing for a merge

ign_ros2_control/src/ign_ros2_control_plugin.cpp Outdated Show resolved Hide resolved
@tonybaltovski
Copy link

@bmagyar can we get this merged please?

@roni-kreinin
Copy link
Contributor

Can this be added to the humble branch as well?

@roni-kreinin
Copy link
Contributor

@sp-sophia-labs Can you fix the merge conflicts? You might have to rebase your branch to ros-controls:master.

@roni-kreinin
Copy link
Contributor

I've made a duplicate PR for the master branch at #128 with merge conflict fixes. This PR can target humble instead.

@bmagyar bmagyar changed the base branch from master to humble March 28, 2023 16:16
@bmagyar bmagyar merged commit 10ec2cd into ros-controls:humble Mar 28, 2023
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.

None yet

8 participants