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.

Sign up for GitHub

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

Jump to bottom

[multirobot - Part2] Add namespacing to nav stack #1147

Merged
merged 1 commit into from
Sep 25, 2019
Merged

[multirobot - Part2] Add namespacing to nav stack #1147

merged 1 commit into from
Sep 25, 2019

Conversation

orduno
Copy link
Contributor

@orduno orduno commented Sep 20, 2019

Basic Info

Info Please fill out this column
Ticket(s) this addresses #
Primary OS tested on Ubuntu 18.04
Robotic platform tested on Gazebo simulation of TB3

Description of contribution in a few bullet points

Extends #1146 adding capability of namespacing the stack (nodes and topics).

  • Extending rewritten_yaml to also replace keys
  • Adding a node launcher that puts a condition to applying the remappings.
  • Add namespacing to launch files
  • Update namespace of costmap. Planner and controller now pass the correct namespace to use.

@orduno orduno mentioned this pull request Sep 20, 2019
Merged
@orduno orduno self-assigned this Sep 20, 2019
SteveMacenski
SteveMacenski reviewed Sep 20, 2019
View reviewed changes
Copy link
Member

@SteveMacenski SteveMacenski left a comment

Choose a reason for hiding this comment

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

Side note there appears to be a rebasing problem, I'm seeing some other PRs stuff leaking into the github view.

nav2_bringup/bringup/launch/nav2_bringup_launch.py Show resolved Hide resolved
@orduno
Copy link
Contributor Author

orduno commented Sep 22, 2019

Side note there appears to be a rebasing problem, I'm seeing some other PRs stuff leaking into the github view.

@SteveMacenski Yes, I'm working on rebasing with master and it's taking longer than I anticipated. Some recent change in nav2 breaks my multi-robot example and I haven't found the source of the problem. Will let you know once I'm done so you can continue reviewing.

@SteveMacenski
Copy link
Member

I also wouldn't spend too much time trying to fix just the rebase, what's an unideal commit or two between colleagues 😉

@orduno orduno force-pushed the multi-robot/spawn_robot branch from 072bcfc to f592145 Compare September 23, 2019 20:20
@orduno orduno force-pushed the multi-robot/namespace_stack branch from be4a43f to eebdc8a Compare September 23, 2019 20:21
@orduno
Copy link
Contributor Author

orduno commented Sep 24, 2019 •
edited
Loading

I agree on an upstream change to add a prefix to the tf broadcaster/listener. The broadcaster could even take the namespace off the node if not adding a separate constructor argument.

@bpwilcox Prefixing the transforms was actually previously supported and deprecated.

There is an ongoing discussion on a replacement:
ros/geometry2#32

@orduno orduno force-pushed the multi-robot/spawn_robot branch from 03f84fa to 9cb8638 Compare September 25, 2019 21:45
@orduno orduno changed the base branch from multi-robot/spawn_robot to master September 25, 2019 21:48
@orduno
Extending rewritten_yaml to also replace keys.
0dc160d 
Adding a node launcher that put a condition to
applying the remappings.
Add namespacing of launch files
Update namespace of costmap and passing of correct
namespace by planner & controller.
@orduno orduno force-pushed the multi-robot/namespace_stack branch from eebdc8a to 0dc160d Compare September 25, 2019 21:58
@orduno orduno merged commit 226f06c into master Sep 25, 2019
@SteveMacenski SteveMacenski deleted the multi-robot/namespace_stack branch September 25, 2019 22:06
@orduno orduno mentioned this pull request Sep 26, 2019
Closed
3 tasks
@mkhansenbot
Copy link
Collaborator

It looks like this PR introduced an uncrustify failure that broke the build on master. Can you submit a PR to fix? The details are here:
https://travis-ci.org/ros-planning/navigation2/builds/589669928

@crdelsey crdelsey mentioned this pull request Oct 1, 2019
Closed
@SteveMacenski SteveMacenski mentioned this pull request Apr 25, 2023
Closed
savalena pushed a commit to savalena/navigation2 that referenced this pull request Jul 5, 2024
@orduno
[multirobot] Add namespacing to nav stack. (ros-navigation#1147)
0354a76 
* Extending `rewritten_yaml` to also replace keys.

* Adding a node launcher that puts a condition to
applying the remappings.

* Add namespacing of launch files.

* Update namespace of costmap and passing of correct
namespace by planner & controller.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@bpwilcox bpwilcox bpwilcox approved these changes

@SteveMacenski SteveMacenski SteveMacenski approved these changes

Assignees

@orduno orduno

Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
4 participants
@orduno @SteveMacenski @mkhansenbot @bpwilcox