Skip to content

Conversation

@hardesh
Copy link
Contributor

@hardesh hardesh commented May 24, 2022

Addresses #2939

Tested the build with ROS2 Humble

@mergify
Copy link
Contributor

mergify bot commented May 24, 2022

@hardesh, please properly fill in PR template in the future. @SteveMacenski, use this instead.

  • Check that any new parameters added are updated in navigation.ros.org
  • Check that any significant change is added to the migration guide
  • Check that any new features OR changes to existing behaviors are reflected in the tuning guide
  • Check that any new functions have Doxygen added
  • Check that any new features have test coverage
  • Check that any new plugins is added to the plugins page
  • If BT Node, Additionally: add to BT's XML index of nodes for groot, BT package's readme table, and BT library lists
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.

LGTM conceptually, just a couple of tiny changes.

The biggest thing that also needs updated are

  • Migration guide entry that we made this change
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.

LGTM, please add a section to the migration guide about the API change to the goal checkers https://navigation.ros.org/migration/Galactic.html, it'll make it into Humble

@hardesh
Copy link
Contributor Author

hardesh commented May 27, 2022

I've opened a PR in the docs repository. Missed it yesterday

@SteveMacenski
Copy link
Member

You have a linting error but then can merge


-  const std::shared_ptr<nav2_costmap_2d::Costmap2DROS> /*costmap_ros*/)
+  const std::shared_ptr<nav2_costmap_2d::Costmap2DROS>/*costmap_ros*/)
@SteveMacenski SteveMacenski merged commit 32d1004 into ros-navigation:main May 27, 2022
redvinaa pushed a commit to EnjoyRobotics/navigation2 that referenced this pull request Jun 30, 2022
* Added costmap in the goal checker

* Added costmap in goal_checker

* Adressed comments in PR

* Fixed linting error
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

2 participants