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

nav2_costmap_2d plugin container layer #4781

Merged

Conversation

alexanderjyuen
Copy link
Contributor

@alexanderjyuen alexanderjyuen commented Dec 4, 2024


Basic Info

Info Please fill out this column
Ticket(s) this addresses N/A
Primary OS tested on Ubuntu
Robotic platform tested on gazebo simulation of turtlebot)
Does this PR contain AI generated software? No

Description of contribution in a few bullet points

  • Added a new costmap layer plugin to nav2_costmap_2d called plugin container layer which allows users to specify different submaps within a costmap. For example if one would like to apply a certain set of inflation layer settings to the static layer, and a different set of inflation layer settings to an obstacle layer, in the same overall costmap, one could group those different layers under different plugin container layers

  • Modified nav2_costmap_2d integrated testing to also run tests for the plugin container layer

  • included nav2_params_plugin_container_layer.yaml to run turtlebot example

Description of documentation updates required from your changes

  • Configuring Costmap needs to include plugin container layer in the Plugin Parameters section
  • A new page has been added to outline how to configure the plugin container layer
  • Navigation Plugins need to include plugin container layer in Costmap Layer section
  • Tuning Guide needs to include plugin container layer in the Costmap2D Plugins section

Future work that may be required in bullet points

  • This could be possible extended to be used with costmap filters, although filters don't necessarily rely on each other

For Maintainers:

  • Check that any new parameters added are updated in docs.nav2.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

Signed-off-by: alexander <alex@polymathrobotics.com>
Signed-off-by: alexander <alex@polymathrobotics.com>
Signed-off-by: alexander <alex@polymathrobotics.com>
Signed-off-by: alexander <alex@polymathrobotics.com>
Signed-off-by: alexander <alex@polymathrobotics.com>
Signed-off-by: alexander <alex@polymathrobotics.com>
Signed-off-by: alexander <alex@polymathrobotics.com>
…ruction, changed addPlugin to take layer name as an argument

Signed-off-by: alexander <alex@polymathrobotics.com>
Signed-off-by: alexander <alex@polymathrobotics.com>
Signed-off-by: alexander <alex@polymathrobotics.com>
Signed-off-by: alexander <alex@polymathrobotics.com>
…unecessary members

Signed-off-by: alexander <alex@polymathrobotics.com>
Signed-off-by: alexander <alex@polymathrobotics.com>
Signed-off-by: alexander <alex@polymathrobotics.com>
Signed-off-by: alexander <alex@polymathrobotics.com>
Signed-off-by: alexander <alex@polymathrobotics.com>
Signed-off-by: alexander <alex@polymathrobotics.com>
Copy link

codecov bot commented Dec 4, 2024

Codecov Report

Attention: Patch coverage is 96.62921% with 3 lines in your changes missing coverage. Please review.

Files with missing lines Patch % Lines
nav2_costmap_2d/src/costmap_2d_ros.cpp 50.00% 2 Missing ⚠️
nav2_costmap_2d/plugins/plugin_container_layer.cpp 98.82% 1 Missing ⚠️
Files with missing lines Coverage Δ
nav2_costmap_2d/plugins/plugin_container_layer.cpp 98.82% <98.82%> (ø)
nav2_costmap_2d/src/costmap_2d_ros.cpp 88.67% <50.00%> (-0.47%) ⬇️

... and 9 files with indirect coverage changes

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.

I didn't call out everything, since I think much of other comments are moot if the costmap_ disappears so its better to wait for the revised version :-)

Check out the codecov annotations in the Code UX of the PR to see some gaps in your testing. Luckily, you have examples of alot of other Nav2 code of how to fill them (most are pretty easy pickings)

Signed-off-by: alexander <alex@polymathrobotics.com>
Signed-off-by: alexander <alex@polymathrobotics.com>
…nknownOverwrite case

Signed-off-by: alexander <alex@polymathrobotics.com>
Signed-off-by: alexander <alex@polymathrobotics.com>
Signed-off-by: alexander <alex@polymathrobotics.com>
Signed-off-by: alexander <alex@polymathrobotics.com>
Signed-off-by: alexander <alex@polymathrobotics.com>
…test coverage

Signed-off-by: alexander <alex@polymathrobotics.com>
Signed-off-by: alexander <alex@polymathrobotics.com>
@alexanderjyuen alexanderjyuen force-pushed the feature-plugin-container-layer branch from 5ee05cf to b25f2a3 Compare December 5, 2024 20:52
@alexanderjyuen
Copy link
Contributor Author

I didn't call out everything, since I think much of other comments are moot if the costmap_ disappears so its better to wait for the revised version :-)

Check out the codecov annotations in the Code UX of the PR to see some gaps in your testing. Luckily, you have examples of alot of other Nav2 code of how to fill them (most are pretty easy pickings)

Latest version should have code coverage up to 99%'

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.

Ah man, see how much more clean / concise this is now!

Signed-off-by: alexander <alex@polymathrobotics.com>
Signed-off-by: alexander <alex@polymathrobotics.com>
Signed-off-by: alexander <alex@polymathrobotics.com>
@SteveMacenski
Copy link
Member

Also, your tests are failing 🙃

Signed-off-by: alexander <alex@polymathrobotics.com>
Signed-off-by: alexander <alex@polymathrobotics.com>
Signed-off-by: alexander <alex@polymathrobotics.com>
Signed-off-by: alexander <alex@polymathrobotics.com>
Signed-off-by: alexander <alex@polymathrobotics.com>
@alexanderjyuen alexanderjyuen force-pushed the feature-plugin-container-layer branch from 51798af to 64f0b51 Compare December 9, 2024 19:21
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.

Then pretty much done!

On the docs, I still need the updated image for multiple inflation settings (or another example) to show how it can be used to create differing behavior -- but otherwise that is good too!

Signed-off-by: alexander <alex@polymathrobotics.com>
Signed-off-by: alexander <alex@polymathrobotics.com>
Signed-off-by: alexander <alex@polymathrobotics.com>
Signed-off-by: alexander <alex@polymathrobotics.com>
Signed-off-by: alexander <alex@polymathrobotics.com>
Signed-off-by: alexander <alex@polymathrobotics.com>
@SteveMacenski SteveMacenski merged commit 16abf9b into ros-navigation:main Dec 13, 2024
11 checks passed
@SteveMacenski
Copy link
Member

Thanks! All good and merged the pair!

SteveMacenski pushed a commit that referenced this pull request Dec 13, 2024
* updated CMakeLists.txt to include plugin_container_layer

Signed-off-by: alexander <alex@polymathrobotics.com>

* added plugin container layer to costmap_plugins.xml

Signed-off-by: alexander <alex@polymathrobotics.com>

* initial commit of plugin container layer

Signed-off-by: alexander <alex@polymathrobotics.com>

* fixed plugin namespace

Signed-off-by: alexander <alex@polymathrobotics.com>

* fixed message_filter

Signed-off-by: alexander <alex@polymathrobotics.com>

* linting

Signed-off-by: alexander <alex@polymathrobotics.com>

* modified addPlugin method to also take layer name

Signed-off-by: alexander <alex@polymathrobotics.com>

* reverted default plugins to be empty, free costmap_ during layer destruction, changed addPlugin to take layer name as an argument

Signed-off-by: alexander <alex@polymathrobotics.com>

* CMake changes to test plugin container layer

Signed-off-by: alexander <alex@polymathrobotics.com>

* added helper method to add plugin container layer

Signed-off-by: alexander <alex@polymathrobotics.com>

* added initial implementation of plugin container tests

Signed-off-by: alexander <alex@polymathrobotics.com>

* added enable to dynamic params, removed unecessary comments, removed unecessary members

Signed-off-by: alexander <alex@polymathrobotics.com>

* cleaned up and added additional tests

Signed-off-by: alexander <alex@polymathrobotics.com>

* added Apache copyrights

Signed-off-by: alexander <alex@polymathrobotics.com>

* linting for ament_cpplint

Signed-off-by: alexander <alex@polymathrobotics.com>

* added example file for plugin_container_layer to nav2_bringup

Signed-off-by: alexander <alex@polymathrobotics.com>

* removed unused rolling_window_ member variable

Signed-off-by: alexander <alex@polymathrobotics.com>

* removed default plugins and plugin types

Signed-off-by: alexander <alex@polymathrobotics.com>

* switched to using CombinationMethod enum, added updateWithMaxWithoutUnknownOverwrite case

Signed-off-by: alexander <alex@polymathrobotics.com>

* removed combined_costmap_

Signed-off-by: alexander <alex@polymathrobotics.com>

* fixed layer naming and accomodating tests

Signed-off-by: alexander <alex@polymathrobotics.com>

* removed nav2_params_plugin_container_layer.yaml

Signed-off-by: alexander <alex@polymathrobotics.com>

* added more comprehensive checks for checking if layers are clearable

Signed-off-by: alexander <alex@polymathrobotics.com>

* added dynamics parameter handling, fixed current_ setting, increased test coverage

Signed-off-by: alexander <alex@polymathrobotics.com>

* removed unnecessary locks, added default value

Signed-off-by: alexander <alex@polymathrobotics.com>

* removed unecessary resetMap

Signed-off-by: alexander <alex@polymathrobotics.com>

* added layer resetting when reset method is called

Signed-off-by: alexander <alex@polymathrobotics.com>

* swapped logic for isClearable

Signed-off-by: alexander <alex@polymathrobotics.com>

* fixed breaking tests, removed unnecessary combined_costmap_

Signed-off-by: alexander <alex@polymathrobotics.com>

* consolidated initialization for loops

Signed-off-by: alexander <alex@polymathrobotics.com>

* switched default_value_ to NO_INFORMATION

Signed-off-by: alexander <alex@polymathrobotics.com>

* added clearArea function

Signed-off-by: alexander <alex@polymathrobotics.com>

* added clearArea test

Signed-off-by: alexander <alex@polymathrobotics.com>

* removed TODO message

Signed-off-by: alexander <alex@polymathrobotics.com>

* removed constructor and destructors since they do nothing

Signed-off-by: alexander <alex@polymathrobotics.com>

* added check on costmap layer to see if it is clearable first

Signed-off-by: alexander <alex@polymathrobotics.com>

* fixed tests for clearing functionality

Signed-off-by: alexander <alex@polymathrobotics.com>

* added try catch around initialization of plugins

Signed-off-by: alexander <alex@polymathrobotics.com>

* fixed indents

Signed-off-by: alexander <alex@polymathrobotics.com>

---------

Signed-off-by: alexander <alex@polymathrobotics.com>
SteveMacenski added a commit that referenced this pull request Dec 13, 2024
* Pass IDLE to on_tick, use that for initialize condition (#4744)

* Pass IDLE to on_tick, use that for initialize condition

Signed-off-by: redvinaa <redvinaa@gmail.com>

* Fix battery sub creation bug

Signed-off-by: redvinaa <redvinaa@gmail.com>

---------

Signed-off-by: redvinaa <redvinaa@gmail.com>

* nav2_costmap_2d: add missing default_value_ copy in Costmap2D operator= (#4753)

default_value_ is an attribute of the Costmap2D class and should be
copied along with the other attributes.

Signed-off-by: Dylan De Coeyer <dylan.decoeyer@quimesis.be>

* nav2_costmap_2d plugin container layer (#4781)

* updated CMakeLists.txt to include plugin_container_layer

Signed-off-by: alexander <alex@polymathrobotics.com>

* added plugin container layer to costmap_plugins.xml

Signed-off-by: alexander <alex@polymathrobotics.com>

* initial commit of plugin container layer

Signed-off-by: alexander <alex@polymathrobotics.com>

* fixed plugin namespace

Signed-off-by: alexander <alex@polymathrobotics.com>

* fixed message_filter

Signed-off-by: alexander <alex@polymathrobotics.com>

* linting

Signed-off-by: alexander <alex@polymathrobotics.com>

* modified addPlugin method to also take layer name

Signed-off-by: alexander <alex@polymathrobotics.com>

* reverted default plugins to be empty, free costmap_ during layer destruction, changed addPlugin to take layer name as an argument

Signed-off-by: alexander <alex@polymathrobotics.com>

* CMake changes to test plugin container layer

Signed-off-by: alexander <alex@polymathrobotics.com>

* added helper method to add plugin container layer

Signed-off-by: alexander <alex@polymathrobotics.com>

* added initial implementation of plugin container tests

Signed-off-by: alexander <alex@polymathrobotics.com>

* added enable to dynamic params, removed unecessary comments, removed unecessary members

Signed-off-by: alexander <alex@polymathrobotics.com>

* cleaned up and added additional tests

Signed-off-by: alexander <alex@polymathrobotics.com>

* added Apache copyrights

Signed-off-by: alexander <alex@polymathrobotics.com>

* linting for ament_cpplint

Signed-off-by: alexander <alex@polymathrobotics.com>

* added example file for plugin_container_layer to nav2_bringup

Signed-off-by: alexander <alex@polymathrobotics.com>

* removed unused rolling_window_ member variable

Signed-off-by: alexander <alex@polymathrobotics.com>

* removed default plugins and plugin types

Signed-off-by: alexander <alex@polymathrobotics.com>

* switched to using CombinationMethod enum, added updateWithMaxWithoutUnknownOverwrite case

Signed-off-by: alexander <alex@polymathrobotics.com>

* removed combined_costmap_

Signed-off-by: alexander <alex@polymathrobotics.com>

* fixed layer naming and accomodating tests

Signed-off-by: alexander <alex@polymathrobotics.com>

* removed nav2_params_plugin_container_layer.yaml

Signed-off-by: alexander <alex@polymathrobotics.com>

* added more comprehensive checks for checking if layers are clearable

Signed-off-by: alexander <alex@polymathrobotics.com>

* added dynamics parameter handling, fixed current_ setting, increased test coverage

Signed-off-by: alexander <alex@polymathrobotics.com>

* removed unnecessary locks, added default value

Signed-off-by: alexander <alex@polymathrobotics.com>

* removed unecessary resetMap

Signed-off-by: alexander <alex@polymathrobotics.com>

* added layer resetting when reset method is called

Signed-off-by: alexander <alex@polymathrobotics.com>

* swapped logic for isClearable

Signed-off-by: alexander <alex@polymathrobotics.com>

* fixed breaking tests, removed unnecessary combined_costmap_

Signed-off-by: alexander <alex@polymathrobotics.com>

* consolidated initialization for loops

Signed-off-by: alexander <alex@polymathrobotics.com>

* switched default_value_ to NO_INFORMATION

Signed-off-by: alexander <alex@polymathrobotics.com>

* added clearArea function

Signed-off-by: alexander <alex@polymathrobotics.com>

* added clearArea test

Signed-off-by: alexander <alex@polymathrobotics.com>

* removed TODO message

Signed-off-by: alexander <alex@polymathrobotics.com>

* removed constructor and destructors since they do nothing

Signed-off-by: alexander <alex@polymathrobotics.com>

* added check on costmap layer to see if it is clearable first

Signed-off-by: alexander <alex@polymathrobotics.com>

* fixed tests for clearing functionality

Signed-off-by: alexander <alex@polymathrobotics.com>

* added try catch around initialization of plugins

Signed-off-by: alexander <alex@polymathrobotics.com>

* fixed indents

Signed-off-by: alexander <alex@polymathrobotics.com>

---------

Signed-off-by: alexander <alex@polymathrobotics.com>

* bumping to 1.3.3 for jazzy sync

Signed-off-by: Steve Macenski <stevenmacenski@gmail.com>

* Fix backporting error: renamed header files

Signed-off-by: Steve Macenski <stevenmacenski@gmail.com>

---------

Signed-off-by: redvinaa <redvinaa@gmail.com>
Signed-off-by: Dylan De Coeyer <dylan.decoeyer@quimesis.be>
Signed-off-by: alexander <alex@polymathrobotics.com>
Signed-off-by: Steve Macenski <stevenmacenski@gmail.com>
Co-authored-by: Vince Reda <60265874+redvinaa@users.noreply.github.com>
Co-authored-by: DylanDeCoeyer-Quimesis <102609575+DylanDeCoeyer-Quimesis@users.noreply.github.com>
Co-authored-by: alexanderjyuen <103065090+alexanderjyuen@users.noreply.github.com>
masf7g pushed a commit to quasi-robotics/navigation2 that referenced this pull request Dec 19, 2024
* updated CMakeLists.txt to include plugin_container_layer

Signed-off-by: alexander <alex@polymathrobotics.com>

* added plugin container layer to costmap_plugins.xml

Signed-off-by: alexander <alex@polymathrobotics.com>

* initial commit of plugin container layer

Signed-off-by: alexander <alex@polymathrobotics.com>

* fixed plugin namespace

Signed-off-by: alexander <alex@polymathrobotics.com>

* fixed message_filter

Signed-off-by: alexander <alex@polymathrobotics.com>

* linting

Signed-off-by: alexander <alex@polymathrobotics.com>

* modified addPlugin method to also take layer name

Signed-off-by: alexander <alex@polymathrobotics.com>

* reverted default plugins to be empty, free costmap_ during layer destruction, changed addPlugin to take layer name as an argument

Signed-off-by: alexander <alex@polymathrobotics.com>

* CMake changes to test plugin container layer

Signed-off-by: alexander <alex@polymathrobotics.com>

* added helper method to add plugin container layer

Signed-off-by: alexander <alex@polymathrobotics.com>

* added initial implementation of plugin container tests

Signed-off-by: alexander <alex@polymathrobotics.com>

* added enable to dynamic params, removed unecessary comments, removed unecessary members

Signed-off-by: alexander <alex@polymathrobotics.com>

* cleaned up and added additional tests

Signed-off-by: alexander <alex@polymathrobotics.com>

* added Apache copyrights

Signed-off-by: alexander <alex@polymathrobotics.com>

* linting for ament_cpplint

Signed-off-by: alexander <alex@polymathrobotics.com>

* added example file for plugin_container_layer to nav2_bringup

Signed-off-by: alexander <alex@polymathrobotics.com>

* removed unused rolling_window_ member variable

Signed-off-by: alexander <alex@polymathrobotics.com>

* removed default plugins and plugin types

Signed-off-by: alexander <alex@polymathrobotics.com>

* switched to using CombinationMethod enum, added updateWithMaxWithoutUnknownOverwrite case

Signed-off-by: alexander <alex@polymathrobotics.com>

* removed combined_costmap_

Signed-off-by: alexander <alex@polymathrobotics.com>

* fixed layer naming and accomodating tests

Signed-off-by: alexander <alex@polymathrobotics.com>

* removed nav2_params_plugin_container_layer.yaml

Signed-off-by: alexander <alex@polymathrobotics.com>

* added more comprehensive checks for checking if layers are clearable

Signed-off-by: alexander <alex@polymathrobotics.com>

* added dynamics parameter handling, fixed current_ setting, increased test coverage

Signed-off-by: alexander <alex@polymathrobotics.com>

* removed unnecessary locks, added default value

Signed-off-by: alexander <alex@polymathrobotics.com>

* removed unecessary resetMap

Signed-off-by: alexander <alex@polymathrobotics.com>

* added layer resetting when reset method is called

Signed-off-by: alexander <alex@polymathrobotics.com>

* swapped logic for isClearable

Signed-off-by: alexander <alex@polymathrobotics.com>

* fixed breaking tests, removed unnecessary combined_costmap_

Signed-off-by: alexander <alex@polymathrobotics.com>

* consolidated initialization for loops

Signed-off-by: alexander <alex@polymathrobotics.com>

* switched default_value_ to NO_INFORMATION

Signed-off-by: alexander <alex@polymathrobotics.com>

* added clearArea function

Signed-off-by: alexander <alex@polymathrobotics.com>

* added clearArea test

Signed-off-by: alexander <alex@polymathrobotics.com>

* removed TODO message

Signed-off-by: alexander <alex@polymathrobotics.com>

* removed constructor and destructors since they do nothing

Signed-off-by: alexander <alex@polymathrobotics.com>

* added check on costmap layer to see if it is clearable first

Signed-off-by: alexander <alex@polymathrobotics.com>

* fixed tests for clearing functionality

Signed-off-by: alexander <alex@polymathrobotics.com>

* added try catch around initialization of plugins

Signed-off-by: alexander <alex@polymathrobotics.com>

* fixed indents

Signed-off-by: alexander <alex@polymathrobotics.com>

---------

Signed-off-by: alexander <alex@polymathrobotics.com>
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