Add slam toolbox as exec dep for nav2_bringup

[Michael-Equi]

---

Basic Info

Info	Please fill out this column
Ticket(s) this addresses	(add tickets here #1827)
Primary OS tested on	Ubuntu 20.04 (docker)

---

Description of contribution in a few bullet points

Added exec depend to the package.xml for nav2 bringup to address issue #1827

Description of documentation updates required from your changes

---

Future work that may be required in bullet points

Handle ceres solver dependency to the docker container

[SteveMacenski]

please properly fill in the PR template.

[SteveMacenski]

This will also infinitely fail until slam toolbox is released or added to the ros2_dependencies.repos file. I think you need to add it.

[SteveMacenski]

https://github.com/ros-planning/navigation2/blob/master/tools/ros2_dependencies.repos

[SteveMacenski]

Well this is a fun issue. Basically what's happening is that we're building our deps workspace work gazebo ros plugins, bt.v3, and now slam toolbox. Its looking to install dependencies but requires nav2 map server to work. Nav2 map server is in the overlay workspace, not the dependencies workspace, so it doesn't know about nav2_map_server being built after this. To get this to build, we'd have to inject slam toolbox into the navigation2 overlay workspace rather than the underlay workspace. We could do this via the circle CI file.

Whoop whoop, circular dependencies. @ruffsl, I'm not asking you to do it, but what's your take on this? Technically we have a launch option to use slam:=true which launches slam toolbox in nav2_bringup but no other code dependencies on Slam Toolbox outside of launch files. My gut says that we really should support this since its in a launch file and is an implied dependency. My brain says "meh, leave as is". If we were to do it, the major options I see are to build nav2_map_server, nav2_utils, and nav2_common in the underlay workspace so we can make use of the caching or build slam toolbox in the overlay workspace. Either way, it requires a little bit of effort

[ruffsl]

Adding all of slam tool box to the overlay will probably really slow things down, while splitting the navigation packages across the underlay and overlay workspaces may add complexity.

I recall when using some of the original TurtleBot packages for simulation, I usually had to explicitly install missing dependencies is encounter when installing just the launch file packages. It added a little friction, but the docs would tell you what to completely install. I'm guessing it helped avoid circular dependencies like this. The turtlebot 3 packages are also pretty annoying, but instead because interdependencies are too strong and so you end up having to build the entire firmware driver stack even if you only just want to use the simulation side.

Perhaps like AMCL, the map server may deserve its own repo/release that is agnostic to core navigation repo. How much has it really evolved for navigation to beyond just being a generic service for saving and publishing PNG files? I think it was its own thing in ros1, so if we did that, it could be added to the underlay, and the exec depend for slam tool box could also be explicitly declared.

[SteveMacenski]

I usually had to explicitly install missing dependencies is encounter when installing just the launch file packages

I'm not sure the TB3 folks being lazy is good reason for us to do so as well. As you mention, they do make it quite challenging on users which I'd like to avoid.

How much has it really evolved for navigation to beyond just being a generic service for saving and publishing PNG files?

Actually a bit and it will continue to change a bit in the future. We have some passive work that will eventually bubble up into another map server for 3D maps and another for parsing and serving semantic map information. At the moment though, we also enabled a map saver server which is new.

Moving it would be a good long term decision with AMCL, I agree.

For the immediate future, I think we have 2 options

• Add another repos file for the overlay workspace and include slam toolbox into it, letting us explicitly include the rosdep key for it to be complete and not create some circular issues with releases
• Continue with status quo and add some documentation and notes about this known issue.

[SteveMacenski]

@Michael-Equi I think in our weekly meeting, @ruffsl had the best idea for how to proceed

• add Slam Toolbox as an exec depend
• In the docker images, add slam toolbox to the skip keys list

This way that if you build locally, you can rosdep slam toolbox when available, in docker you just need to compose images and add slam toolbox to it, and gets CI up and running.

Let me know if you think that's a good action.

[ruffsl]

In the docker images, add slam toolbox to the skip keys list

You'll also need to add the rosdep ignore argument to CircleCI as well.

See this previous commit for such an example: 55edf58

[Michael-Equi]

I implemented those changes which results in successful runs of the docker container build and build tests. I am not sure why the release and debug tests failed, it does not look to be related to the slam toolbox exec depend.

[SteveMacenski]

Rebase / pull in master to fix the mixin CI problem. Fixed in #1835

[SteveMacenski]

One last update and we just need to get CI to pass

[SteveMacenski]

Waiting on CI