Skip to content

[controller_server] [FollowPath] Allow multiple progress checkers#3555

Merged
SteveMacenski merged 8 commits intoros-navigation:mainfrom
doisyg:allow_multiple_goal_checkers
May 17, 2023
Merged

[controller_server] [FollowPath] Allow multiple progress checkers#3555
SteveMacenski merged 8 commits intoros-navigation:mainfrom
doisyg:allow_multiple_goal_checkers

Conversation

@doisyg
Copy link
Contributor

@doisyg doisyg commented May 3, 2023


Basic Info

Info Please fill out this column
Ticket(s) this addresses
Primary OS tested on Ubuntu
Robotic platform tested on Dexory sim

Description of contribution in a few bullet points

The parameters goal_checker_plugins and controller_plugins are list of strings, allowing multiple plugin configuration, whereas progress_checker_plugin is a single string allowing the declaration of only one goal checker.

This PR initializes the progress checker plugin(s) in the same way as for the goal checker and controller plugins: it is now a list of string and was renamed from progress_checker_plugin to progress_checker_plugins. This allows the initialization of multiple goal checker that can be chosen from the added progress_checker_id field of the FollowPath action

This is a breaking change as the configuration yaml has to be updated from

 controller_server:
   ros__parameters:
     progress_checker_plugin: "progress_checker"

to

 controller_server:
   ros__parameters:
     progress_checker_plugins: ["progress_checker"]

Description of documentation updates required from your changes

  • Name of parameter and default configuration in all examples
  • Migration guide

Future work that may be required in bullet points

For Maintainers:

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

I generally approve, but I want to see the docs PR to match since there will be a bunch of places these new ports and vectorized plugins need to be set. Last time we did this, there were vestiges in docs of it for way too long causing problems with copy+pasting of YAML

Edit: I lied. See below.

@doisyg doisyg changed the title [controller_server] [FollowPath] Allow multiple goal checkers [controller_server] [FollowPath] Allow multiple progress checkers May 4, 2023
@mergify
Copy link
Contributor

mergify bot commented May 10, 2023

@doisyg, your PR has failed to build. Please check CI outputs and resolve issues.
You may need to rebase or pull in main due to API changes (or your contribution genuinely fails).

@doisyg doisyg force-pushed the allow_multiple_goal_checkers branch from 0fc13ce to 4bb1e20 Compare May 10, 2023 16:44
@SteveMacenski
Copy link
Member

Just waiting on CI

@SteveMacenski SteveMacenski merged commit 07a9127 into ros-navigation:main May 17, 2023
enricosutera pushed a commit to enricosutera/navigation2 that referenced this pull request May 19, 2024
…s-navigation#3555)

* allow multiple progress checker plugin

* merge conflict

* handle change of progress checker during a preempt

* reset pc on preempt

* fix default declaration

* Handle deprecated progress_checker_plugin

* improve readability

* Update nav2_controller/src/controller_server.cpp

---------

Co-authored-by: Guillaume Doisy <guillaume@dexory.com>
Co-authored-by: Steve Macenski <stevenmacenski@gmail.com>
Signed-off-by: enricosutera <enricosutera@outlook.com>
savalena pushed a commit to savalena/navigation2 that referenced this pull request Jul 5, 2024
…s-navigation#3555)

* allow multiple progress checker plugin

* merge conflict

* handle change of progress checker during a preempt

* reset pc on preempt

* fix default declaration

* Handle deprecated progress_checker_plugin

* improve readability

* Update nav2_controller/src/controller_server.cpp

---------

Co-authored-by: Guillaume Doisy <guillaume@dexory.com>
Co-authored-by: Steve Macenski <stevenmacenski@gmail.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