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

Draft: Namespace Support #189

Draft
wants to merge 44 commits into
base: main
Choose a base branch
from

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 namespacing of the create3 robot to be able to spawn multiple instances in the same simulation space without resorting to isolated networks

This PR is a reboot of this one: #176. Sorry for the duplicate.
@hugo-tardiou has transitioned out of our team and I am taking over the project. Since the previous PR was linked to his personnal account, we are creating a new one here so that we can continue to update it correctly. We'll do our best to summarize past discussions in the checklist below

Type of change: new feature. This change does not fix any open issue

How has this been tested?

We have created a project with dependencies pointing to specific commits (submodules) here: https://github.com/sp-sophia-labs/turtlebot4_multi_sim (details in the dependency section below). Reviewers may clone the project and replicate exactly what we use for testing. You can follow the readme from the project or use the following:

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, launching the robot without namespaces is still available:
ros2 launch irobot_create_ignition_bringup create3_ignition.launch.py

In a separate terminal, spawn another create3 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
You can change the x, y, z param to spawn the robot where needed

Once the gazebo simulation has started, you can modify the Create3 HMI field to reflect on the new robot name. It defaults to "create3"
This will correctly map the Create3 simulated button to one of your robots

Dependencies

We have modified other packages outside of create3_sim to be able to spawn multiple create3 in simulation with namespacing. Here is the list with a short description of the reasons for the changes:

We'll update the dependencies above according to how the other PRs are progressing

Checklist

  • Create PR for dependencies and update this description
  • Remove rclcpp dependency (wildcard support has been backported to galactic)
  • Test namespacing on Gazebo classic (will probably require some work)
  • Create dedicated file for the OffsetParser utility
  • Evaluate if "robot_name"(ignition/gazebo name) should be the same as the namespace
  • Find a more elegant solution to the duplicate namespace entries in launch files
  • Remove error messages (non-breaking) from scene_manager when spawning additional robots

alsora and others added 30 commits November 4, 2022 11:20
Signed-off-by: Alberto Soragna <[email protected]>
unified robot spawn file and clean up

xacro fix

revert change in config files

offset_parser module moved to irobot_create_common_bringup

gazebo spawner launch

offset parser separate pkg

OffsetParser in irobot_create_common_bringup

Lint / testing / cleaning

linting more

small fixes to namespaces arguments

Signed-off-by: Alberto Soragna <[email protected]>

remove not needed rclcpp and rclpy dependencies

Signed-off-by: Alberto Soragna <[email protected]>

rviz namespace fix for robot description

dock and cmd_vel gui namespaced

lint import order

namespace create3_nodes

fix robot_description in rviz
@sp-sophia-labs sp-sophia-labs marked this pull request as draft December 5, 2022 14:16
@sp-sophia-labs sp-sophia-labs changed the title Namespace Support Draft: Namespace Support Dec 5, 2022
@@ -5,6 +5,7 @@

<!-- Gazebo version -->
<xacro:arg name="gazebo" default="classic" />
<xacro:arg name="namespace" default=""/>
Copy link
Contributor

Choose a reason for hiding this comment

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

you replaced

<namespace>/</namespace>

with

<namespace>$(arg namespace)</namespace>

should the default namespace be / ?
your change seems correct to me, but I want to double check that we are not breaking existing functionalities

private:
ignition::transport::Node node_;
ignition::transport::Node::Publisher create3_button_pub_;
std::string create3_button_topic_ = "/create3/buttons";
std::string robot_name_ = "create3";
std::string create3_button_topic_ = "/model/create3/buttons";
Copy link
Contributor

Choose a reason for hiding this comment

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

why is this topic changed?

Copy link
Author

@sp-sophia-labs sp-sophia-labs Dec 5, 2022

Choose a reason for hiding this comment

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

@alsora I believe this was an attempt on my part to standardize topic names on the ign side. Looking at it now, I would probably change it to /gui/create3/buttons instead... What do you think? I can revert it to previous naming

/clock
/cmd_vel
/gazebo/resource_paths
/gui/camera/pose
/gui/record_video/stats
*/create3/buttons -> /model/create3/buttons*
/model/create3/cmd_vel
/model/create3/pose
/model/create3_standard_dock/pose
/stats
/world/depot/clock
/world/depot/dynamic_pose/info
/world/depot/model/create3/link/base_link/sensor/cliff_front_left/scan
/world/depot/model/create3/link/base_link/sensor/cliff_front_left/scan/points
...

@alsora
Copy link
Contributor

alsora commented Dec 5, 2022

Hi, I gave a quick look at the PR and besides some comments it looks good to me.
We will need to test it thoroughly though.

@sp-sophia-labs
Copy link
Author

sp-sophia-labs commented Dec 5, 2022

@alsora Thank you for the review! I agree, it's a big one. Happy to lend a hand on the testing if it needs implementation. I'm still waiting for my changes to be merged to gz_ros2_control, and back-ported to galactic before we can test it properly. Don't hesitate to try this: https://github.com/sp-sophia-labs/turtlebot4_multi_sim in the meanwhile

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