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 nav2_dwb_controller package #10

Merged
merged 2 commits into from
Aug 12, 2022

Conversation

biancabnd
Copy link

@biancabnd biancabnd commented Aug 4, 2022


Basic Info

Info Please fill out this column
Ticket(s) this addresses (add tickets here #1)
Primary OS tested on (Ubuntu, MacOS, Windows)
Robotic platform tested on (Steve's Robot, gazebo simulation of Tally, hardware turtlebot)

Description of contribution in a few bullet points

Add nav2_dwb_controller and its dependencies to umd nav2.
This package is needed to substitute the controller plugin PositionController used for Copters to a suitable controller plugin for AMRs. In this case, the dwb controller.

Description of documentation updates required from your changes


Future work that may be required in bullet points

@biancabnd biancabnd requested a review from a team August 4, 2022 14:54
Copy link

@vicmassy vicmassy left a comment

Choose a reason for hiding this comment

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

Overall good! Maybe I would suggest to merge this into feat/update_humble so that everything is updated once we merge humble. What do you think?

@@ -0,0 +1,7 @@
dwb_msgs

Choose a reason for hiding this comment

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

Why do we need this file? Maybe we can put it all together in minimal_repos.txt??

Copy link
Author

@biancabnd biancabnd Aug 12, 2022

Choose a reason for hiding this comment

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

Merging into feat/update_humble sounds good!
Regarding the file, that was my first intention the problem is that this file is used in two commands:
Here in the rosdep install:

 apt-get update && rosdep install -q -y \
  --from-paths $(awk '$0="src/navigation2/"$0' ./minimal_repos.txt) \ 
  --ignore-src \
  && rm -rf /var/lib/apt/lists/*

And in the colcon build:

  --symlink-install \
  --mixin \
  $OVERLAY_MIXINS \
  --packages-select $(cat ./nav2_pkgs.txt) $(cat ./minimal_repos.txt) \

The problem is that in the first command it looks for packages at path : src/navigation2 while these packages are located in src/navigation2/nav2_dwb_controller which makes this command to fail. And if you add the entire path in the minimal_repos.txt then it fails the colcon build because it expects only the name of the package so dwb_core for example and not the entire path. I solved this issue by adding all these pkgs in a second file, but if you have a better alternative I am open to discuss it.

Copy link
Author

Choose a reason for hiding this comment

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

The branch for humble is the feat/humble_main no? Because I cannot find feat/update_humble

Choose a reason for hiding this comment

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

Yes sorry, I meant feat/humble_main

I understand now the issue with the files. Lets leave it like this and if we think of a cleaner solution we can still do it in the PR of Humble. Thanks for the clarification Bianca!

@biancabnd biancabnd changed the base branch from master_umd to feat/humble_main August 12, 2022 07:08
@vicmassy vicmassy self-requested a review August 12, 2022 08:14
@vicmassy vicmassy merged commit cac9ef3 into feat/humble_main Aug 12, 2022
@vicmassy vicmassy deleted the feat/integrate_tiago_controller branch August 12, 2022 08:16
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