Skip to content

Check params_file arg, and use default if it has no slam_toolbox params#368

Merged
SteveMacenski merged 2 commits intoSteveMacenski:foxy-develfrom
pal-robotics-forks:check-provided-params
Mar 18, 2021
Merged

Check params_file arg, and use default if it has no slam_toolbox params#368
SteveMacenski merged 2 commits intoSteveMacenski:foxy-develfrom
pal-robotics-forks:check-provided-params

Conversation

@v-lopez
Copy link
Contributor

@v-lopez v-lopez commented Mar 17, 2021

Basic Info

Info Please fill out this column
Ticket(s) this addresses ros-navigation/navigation2#2240
Primary OS tested on Ubuntu 20.04
Robotic platform tested on simulated turtlebot, simulated TIAGo Base

Description

This is a workaround to ros-navigation/navigation2#2243 to keep foxy API stable by not changing the params_file name.

This is a workaround to
ros-navigation/navigation2#2243 to keep foxy API
stable by not changing the params_file name.
@v-lopez
Copy link
Contributor Author

v-lopez commented Mar 17, 2021

@SteveMacenski As discussed on ros-navigation/navigation2#2243 this is the workaround for foxy.

I've added a LogInfo when this workaround is applied, so users have some clue that their params_file is being ignored.

[INFO] [launch.user]: provided params_file /home/user/nav_ws/install/nav2_bringup/share/nav2_bringup/params/nav2_params.yaml does not contain slam_toolbox parameters. Using default: /home/user/nav_ws/install/slam_toolbox/share/slam_toolbox/config/mapper_params_online_sync.yaml

Copy link
Owner

@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.

Clever, works for me!

@@ -0,0 +1,15 @@
# Copyright (c) 2021 PAL Robotics S.L.
Copy link
Owner

Choose a reason for hiding this comment

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

This project is LGPLv2.1 due to the use of openkarto, please use the appropriate license header (or dont include like the others)

#
# Licensed under the Apache License, Version 2.0 (the "License");
# you may not use this file except in compliance with the License.
# You may obtain a copy of the License at
Copy link
Owner

Choose a reason for hiding this comment

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

We have a dep on nav2_map_server, you can make a dep on nav2_common and add this to that in foxy + use it from there.

@SteveMacenski
Copy link
Owner

SteveMacenski commented Mar 17, 2021

Just licensing / not copying files around too too much (and licensing is dealt with by just moving it back to nav2_common)

@v-lopez
Copy link
Contributor Author

v-lopez commented Mar 17, 2021

Moved the file to nav2_common as suggested. See ros-navigation/navigation2#2257.

Copy link
Owner

@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.

Did you test with the import from nav2_common?

@v-lopez
Copy link
Contributor Author

v-lopez commented Mar 18, 2021

Yes, even removed workspace and recompiled to make sure of it.

@SteveMacenski SteveMacenski merged commit 99e12e5 into SteveMacenski:foxy-devel Mar 18, 2021
maxlein pushed a commit to kin-dergarten/slam_toolbox that referenced this pull request Aug 5, 2021
…ms (SteveMacenski#368)

* Check params_file arg, and use default if it has no slam_toolbox params

This is a workaround to
ros-navigation/navigation2#2243 to keep foxy API
stable by not changing the params_file name.

* Use has_node_params in nav2_common
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