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

Add support for multi-robot nav2 simulation #1058

Closed
wants to merge 12 commits into from

Conversation

orduno
Copy link
Contributor

@orduno orduno commented Aug 23, 2019


Basic Info

Info Please fill out this column
Ticket(s) this addresses #971
Primary OS tested on Ubuntu 18.04
Robotic platform tested on Turtlebot3

Description of contribution in a few bullet points

Enabling multiple robots running nav2 in simulation. It's not ready for prime time, the purpose is to provide interested parties a glance into the work done so far.

The branch should be built against ROS2 master:

Once the tasks mentioned below are completed, I will create ~3 PRs to merge the work and get more detailed feedback.

For a video, click the image below.

image


Work-in-progress

To do before submitting the work for review:

  • Fix launching a single robot using nav2_simulation_launch.py.
  • Organize nav2_bringup into /launch, /map, /world, /rviz.
  • Rebase, build and test agaist ROS master (Eloquent).
  • Move spawn_turtlebot3 to nav2_bringup
  • Set correct topic namespace on the RVIZ config file.
  • Extend launch_ros.actions.Node to conditionalize remappings -- remappings are only applied when launching multiple robots.

Future work

To do after the initial solution is merged:

  • Update docs, add launch instructions
  • Move /map & /cmd_vel to relative namespaces
  • Singular view of the system through RVIZ.
  • Fix limitations preventing from launching successfully more than 3 robots.
  • Create a RobotsLayer in Costmap2D to avoid bumping into each other.
  • Create a multi-robot system test.
  • Enabling a PushNodeRemapping action. upstream ticket to prevent defining remaps without context.

@orduno orduno self-assigned this Aug 23, 2019
@orduno orduno changed the title Add support for multi-robot simulation Add support for multi-robot nav2 simulation Aug 23, 2019
@@ -34,7 +34,8 @@ DwbController::DwbController()
RCLCPP_INFO(get_logger(), "Creating");

// The costmap node is used in the implementation of the DWB controller
costmap_ros_ = std::make_shared<nav2_costmap_2d::Costmap2DROS>("local_costmap");
costmap_ros_ = std::make_shared<nav2_costmap_2d::Costmap2DROS>(
Copy link
Member

Choose a reason for hiding this comment

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

The local namespace + name seems to be the same, is that something you expect to be true for the costmaps? Maybe we can reduce this to a single parameter?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Yes, we could just pass one argument for both and keep the current namespacing:

Robot1/global_costmap/global_costmap
Robot1/local_costmap/local_costmap

However, I'm thinking that probably better naming could be:

Robot1/dwb/costmap
Robot2/world_model/costmap

In which case the local namespace and node's name are now different.

Copy link
Contributor Author

@orduno orduno Aug 23, 2019

Choose a reason for hiding this comment

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

However, namespacing the costmaps differently is not a change I'm planning to do now.

Copy link
Member

Choose a reason for hiding this comment

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

I think that might be a pain if you want to do something like having another server subbed to the costmap to run some calculation, since then the costmap will depend on the controller type. I just think its nicer to have the first version, but I don't have a strong opinion either way

nav2_world_model/src/world_model.cpp Outdated Show resolved Hide resolved
@orduno
Copy link
Contributor Author

orduno commented Aug 30, 2019

Multi-robot bring-up is currently broken; I'm in the middle to rebasing against Eloquent and Nav2/Master.

This branch is on a working state if built against Dashing.

@orduno
Copy link
Contributor Author

orduno commented Sep 3, 2019

Submitted issue ros2/launch#325 and proposed fix preventing from executing GroupAction s correctly on nav2_multirobot_simulation_launch.py.

@orduno
Copy link
Contributor Author

orduno commented Sep 3, 2019

Submitted issue ros2/rviz#442 and found fix preventing from namespacing RVIZ and remapping topics.

@codecov
Copy link

codecov bot commented Sep 3, 2019

Codecov Report

Merging #1058 into master will increase coverage by 6.62%.
The diff coverage is n/a.

Impacted file tree graph

@@            Coverage Diff             @@
##           master    #1058      +/-   ##
==========================================
+ Coverage    28.8%   35.43%   +6.62%     
==========================================
  Files         241      241              
  Lines       11201    11205       +4     
  Branches     3689     4382     +693     
==========================================
+ Hits         3227     3970     +743     
+ Misses       5681     4495    -1186     
- Partials     2293     2740     +447
Flag Coverage Δ
#project 35.43% <ø> (+6.62%) ⬆️
Impacted Files Coverage Δ
...gation2/nav2_costmap_2d/src/observation_buffer.cpp 39.6% <0%> (ø) ⬆️
...controller/dwb_core/include/dwb_core/publisher.hpp 0% <0%> (ø) ⬆️
...avigation2/nav2_util/src/map_loader/map_loader.cpp 54% <0%> (ø) ⬆️
...ion2/nav2_util/include/nav2_util/line_iterator.hpp 0% <0%> (ø) ⬆️
src/navigation2/nav2_amcl/src/main.cpp 33.33% <0%> (ø) ⬆️
.../src/sensors/laser/likelihood_field_model_prob.cpp
...ation2/nav2_util/src/motion_model/motion_model.cpp
...2/nav2_util/src/motion_model/omni_motion_model.cpp
src/navigation2/nav2_util/src/pf/pf.c
...il/include/nav2_util/motion_model/motion_model.hpp
... and 98 more

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 16fda6a...1e4d48d. Read the comment docs.

@orduno orduno force-pushed the 971_multi_robot_namespaces branch from 4e5e1f3 to 1e4d48d Compare September 17, 2019 22:54
@orduno
Copy link
Contributor Author

orduno commented Sep 20, 2019

PR is too big to review, closing in favor of stacked PRs.

@orduno orduno closed this Sep 20, 2019
@orduno orduno deleted the 971_multi_robot_namespaces branch October 1, 2019 17:51
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants