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 note on multirobot refactor to migration guide #609

Open
wants to merge 11 commits into
base: master
Choose a base branch
from

Conversation

luca-della-vedova
Copy link

Add migration for PR ros-navigation/navigation2#4715

migration/Jazzy.rst Outdated Show resolved Hide resolved
@SteveMacenski
Copy link
Member

One more thing and this should be done https://docs.nav2.org/tuning/index.html --> remove the use_namespace parameter description if we're removing it

Signed-off-by: Luca Della Vedova <[email protected]>
Signed-off-by: Luca Della Vedova <[email protected]>
luca-della-vedova and others added 2 commits November 26, 2024 19:24
Signed-off-by: Steve Macenski <[email protected]>
@SteveMacenski
Copy link
Member

I fixed some formatting for you, LGTM

@luca-della-vedova
Copy link
Author

It shows that I only wrote markdown in my life 😅, noticed that the highlighting of snippets wasn't working because it's supposed to be double backtick so fixed that in 3cc1e2e

Signed-off-by: Luca Della Vedova <[email protected]>
@@ -29,6 +29,14 @@ This costmap layer implements a plugin that processes sonar, IR, or other 1-D se
Description
Range topics to subscribe to.

Relative topics will be relative to the node's parent namespace.
Copy link
Member

Choose a reason for hiding this comment

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

Missed the entry for map_topic now 😉

Sorry to be a stickler about docs, I know its frustrating :(

Copy link
Author

Choose a reason for hiding this comment

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

Apologies not sure I follow, I thought map_topic is only a parameter in static_layer? Why is this comment on range_sensor_layer?

lucadv@noble:~/ionic_ws/src/nav2/navigation2/nav2_costmap_2d/plugins$ grep -r . -e map_topic
./static_layer.cpp:    map_topic_.c_str(),
./static_layer.cpp:    map_topic_, map_qos,
./static_layer.cpp:      map_topic_ + "_updates",
./static_layer.cpp:  declareParameter("map_topic", rclcpp::ParameterValue("map"));
./static_layer.cpp:  node->get_parameter(name_ + "." + "map_topic", map_topic_);
./static_layer.cpp:  map_topic_ = joinWithParentNamespace(map_topic_);
./static_layer.cpp:      param_name == name_ + "." + "map_topic" ||

Copy link
Member

Choose a reason for hiding this comment

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

I mean that i wanted you to add this block wrt clarification on the relative topic namespaces into the static layer's configuration guide for the map_topic now that it has the same behavior profile as topic for range / obstacle / voxel layers

@SteveMacenski
Copy link
Member

Otherwise, I can merge

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.

2 participants