Add planner configurations to CHOMP and PILZ #1522
Merged
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Codecov Report
@@ Coverage Diff @@
## main #1522 +/- ##
==========================================
+ Coverage 50.97% 50.98% +0.01%
==========================================
Files 380 380
Lines 31758 31768 +10
==========================================
+ Hits 16186 16194 +8
- Misses 15572 15574 +2
Help us with your feedback. Take ten seconds to tell us how you rate us. Have a feature suggestion? Share it here. |
tylerjw
approved these changes
Aug 23, 2022
tylerjw
force-pushed
the
pr-add_missing_planner_configs
branch
from
cd8f9fc to
386dd9a
Compare
sjahr
force-pushed
the
pr-add_missing_planner_configs
branch
from
386dd9a to
2a3c773
Compare
sjahr
force-pushed
the
pr-add_missing_planner_configs
branch
from
2a3c773 to
a4c9caa
Compare
6 tasks
tylerjw
added
the
backport-humble
mergify bot
pushed a commit
that referenced
this pull request
Oct 28, 2022
(cherry picked from commit 888fc53)
tylerjw
pushed a commit
that referenced
this pull request
Oct 28, 2022
(cherry picked from commit 888fc53) Co-authored-by: Sebastian Jahr <[email protected]>
rhaschke
added a commit
to rhaschke/moveit2
that referenced
this pull request
Nov 10, 2022
This reverts commit 888fc53.
rhaschke
added a commit
to rhaschke/moveit2
that referenced
this pull request
Nov 10, 2022
This attempts to cleanup the mess in configuring multiple planning pipelines, which exists since the very beginning and was changed back and forth since then, see moveit#1096, moveit#1114, moveit#1522. This PR removes MoveItCpp::getPlanningPipelineNames(), which was obviously intended initially to provide a planning-group-based filter for all available planning pipelines: A pipeline was discarded for a group, if there were no `planner_configs` defined for that group on the parameter server. As pointed out in moveit#1522, only OMPL actually explicitly declares planner_configs on the parameter server. To enable all other pipelines as well (and thus circumventing the original filter mechanism), moveit#1522 introduced empty dummy planner_configs for all other planners as well (CHOMP + Pilz). This, obviously, renders the whole filter mechanism useless. Thus, here we just remove the function getPlanningPipelineNames().
rhaschke
added a commit
to rhaschke/moveit2
that referenced
this pull request
Nov 10, 2022
This reverts commit 888fc53.
rhaschke
added a commit
to rhaschke/moveit2
that referenced
this pull request
Nov 10, 2022
This attempts to cleanup the mess in configuring multiple planning pipelines, which exists since the very beginning and was changed back and forth since then, see moveit#1096, moveit#1114, moveit#1522. This PR removes MoveItCpp::getPlanningPipelineNames(), which was obviously intended initially to provide a planning-group-based filter for all available planning pipelines: A pipeline was discarded for a group, if there were no `planner_configs` defined for that group on the parameter server. As pointed out in moveit#1522, only OMPL actually explicitly declares planner_configs on the parameter server. To enable all other pipelines as well (and thus circumventing the original filter mechanism), moveit#1522 introduced empty dummy planner_configs for all other planners as well (CHOMP + Pilz). This, obviously, renders the whole filter mechanism useless. Thus, here we just remove the function getPlanningPipelineNames().
rhaschke
added a commit
to rhaschke/moveit2
that referenced
this pull request
Nov 11, 2022
This reverts commit 888fc53.
rhaschke
added a commit
to rhaschke/moveit2
that referenced
this pull request
Nov 11, 2022
This attempts to cleanup the mess in configuring multiple planning pipelines, which exists since the very beginning and was changed back and forth since then, see moveit#1096, moveit#1114, moveit#1522. This PR removes MoveItCpp::getPlanningPipelineNames(), which was obviously intended initially to provide a planning-group-based filter for all available planning pipelines: A pipeline was discarded for a group, if there were no `planner_configs` defined for that group on the parameter server. As pointed out in moveit#1522, only OMPL actually explicitly declares planner_configs on the parameter server. To enable all other pipelines as well (and thus circumventing the original filter mechanism), moveit#1522 introduced empty dummy planner_configs for all other planners as well (CHOMP + Pilz). This, obviously, renders the whole filter mechanism useless. Thus, here we just remove the function getPlanningPipelineNames().
sjahr
pushed a commit
to rhaschke/moveit2
that referenced
this pull request
Nov 15, 2022
This reverts commit 888fc53.
sjahr
pushed a commit
to rhaschke/moveit2
that referenced
this pull request
Nov 15, 2022
This attempts to cleanup the mess in configuring multiple planning pipelines, which exists since the very beginning and was changed back and forth since then, see moveit#1096, moveit#1114, moveit#1522. This PR removes MoveItCpp::getPlanningPipelineNames(), which was obviously intended initially to provide a planning-group-based filter for all available planning pipelines: A pipeline was discarded for a group, if there were no `planner_configs` defined for that group on the parameter server. As pointed out in moveit#1522, only OMPL actually explicitly declares planner_configs on the parameter server. To enable all other pipelines as well (and thus circumventing the original filter mechanism), moveit#1522 introduced empty dummy planner_configs for all other planners as well (CHOMP + Pilz). This, obviously, renders the whole filter mechanism useless. Thus, here we just remove the function getPlanningPipelineNames().
sjahr
pushed a commit
to rhaschke/moveit2
that referenced
this pull request
Nov 15, 2022
This reverts commit 888fc53.
sjahr
pushed a commit
to rhaschke/moveit2
that referenced
this pull request
Nov 15, 2022
This attempts to cleanup the mess in configuring multiple planning pipelines, which exists since the very beginning and was changed back and forth since then, see moveit#1096, moveit#1114, moveit#1522. This PR removes MoveItCpp::getPlanningPipelineNames(), which was obviously intended initially to provide a planning-group-based filter for all available planning pipelines: A pipeline was discarded for a group, if there were no `planner_configs` defined for that group on the parameter server. As pointed out in moveit#1522, only OMPL actually explicitly declares planner_configs on the parameter server. To enable all other pipelines as well (and thus circumventing the original filter mechanism), moveit#1522 introduced empty dummy planner_configs for all other planners as well (CHOMP + Pilz). This, obviously, renders the whole filter mechanism useless. Thus, here we just remove the function getPlanningPipelineNames().
rhaschke
added a commit
that referenced
this pull request
Nov 15, 2022
* Revert "Add planner configurations to CHOMP and PILZ (#1522)" * Cleanup lookup of planning pipelines Remove MoveItCpp::getPlanningPipelineNames(), which was obviously intended initially to provide a planning-group-based filter for all available planning pipelines: A pipeline was discarded for a group, if there were no `planner_configs` defined for that group on the parameter server. As pointed out in #1522, only OMPL actually explicitly declares planner_configs on the parameter server. To enable all other pipelines as well (and thus circumventing the original filter mechanism), #1522 introduced empty dummy planner_configs for all other planners as well (CHOMP + Pilz). This, obviously, renders the whole filter mechanism useless. Thus, here we just remove the function getPlanningPipelineNames() and the corresponding member groups_pipelines_map_.
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Description
Right now it is not possible to use planners other than OMPL with the moveit_cpp interface. The reason for this is the following:
When the plan() method of the planning component gets called, all available planning pipelines are searched based on their name [1]. These names are provided by moveit_cpp [2]. The moveit_cpp function that provides the planning pipeline names reads them from an internal map [3]. A planning pipelines needs to have a planner plugin that has a PlannerConfiguration in order to be added to this map [4]. This PR adds the missing configuations to CHOMP and PILZ to make them usable in moveit_cpp and in addition to that it makes the PlannerManager's
setPlannerConfigurationsfunction pure virtual to remind futur planning plugin implementations to add a planner configuationChecklist