Cleanup lookup of planning pipelines in MoveItCpp

[rhaschke]

This PR attempts to cleanup the mess in configuring multiple planning pipelines in MoveItCpp, which exists since the very beginning and was changed back and forth since then, see #1096, #1114, #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 #1522, only OMPL actually explicitly declares planner_configs on the parameter server.
To enable all other pipelines as well, #1522 introduced empty dummy planner_configs for all other planners (CHOMP + Pilz) - and thus, obviously, circumventing the original filter mechanism!

As the filter mechanism is useless now, we can just remove the function getPlanningPipelineNames() and revert #1522 instead.
@sjahr, this is what I was thinking of for moveit/moveit#3244 (actually the PR here simplifies even more).

[henningkayser]

Are you suggesting to remove the PlannerConfigurationSettings as well? I think this would be the direct consequence of omitting the interface, and I would agree that it would make a lot of sense to do so. Also, if we don't require a mapping, the groups_pipelines_map_ should also be removed with this PR.

Otherwise, a simple PlannerManager::canServiceGroup() would be a more elegant solution if we do want to filter for groups.

[rhaschke]

Are you suggesting to remove the PlannerConfigurationSettings as well? I think this would be the direct consequence of omitting the interface.

No, not at all. I think it makes a lot of sense to allow different planner configs for different groups!
The PR here just provides a fallback in case a planner plugin doesn't setup any config settings. Most user's assumption is that the plugin will serve all groups will all its planning capabilities = planners.

Also, if we don't require a mapping, the groups_pipelines_map_ should also be removed with this PR.

I agree with that and pushed another commit.

[rhaschke]

Sorry for using GHA as a compiler. I don't have a ROS2 system running right now.

[codecov]

Codecov Report

Base: 50.99% // Head: 50.97% // Decreases project coverage by -0.02% ⚠️

Coverage data is based on head (f2288a9) compared to base (4c26cbe).
Patch has no changes to coverable lines.

❗ Current head f2288a9 differs from pull request most recent head 5025cc4. Consider uploading reports for the commit 5025cc4 to get more accurate results

Additional details and impacted files

@@            Coverage Diff             @@
##             main    #1710      +/-   ##
==========================================
- Coverage   50.99%   50.97%   -0.01%     
==========================================
  Files         378      378              
  Lines       31653    31639      -14     
==========================================
- Hits        16138    16125      -13     
+ Misses      15515    15514       -1     

Impacted Files	Coverage Δ
...al_motion_planner/pilz_industrial_motion_planner.h	100.00% <ø> (ø)
...ion_planner/src/pilz_industrial_motion_planner.cpp	100.00% <ø> (ø)
moveit_ros/moveit_servo/src/servo_calcs.cpp	66.45% <0.00%> (-1.49%)	⬇️
moveit_ros/moveit_servo/src/pose_tracking.cpp	76.78% <0.00%> (-0.47%)	⬇️
...anning_scene_monitor/src/current_state_monitor.cpp	75.51% <0.00%> (-0.09%)	⬇️
...veit_servo/include/moveit_servo/servo_parameters.h	100.00% <0.00%> (ø)
...ssistant/moveit_setup_framework/src/rviz_panel.cpp	0.00% <0.00%> (ø)
...g/rdf_loader/src/synchronized_string_parameter.cpp	32.82% <0.00%> (ø)
...nclude/moveit/robot_state/cartesian_interpolator.h	50.00% <0.00%> (ø)
...include/moveit/robot_trajectory/robot_trajectory.h	98.72% <0.00%> (+0.02%)	⬆️
... and 4 more

Help us with your feedback. Take ten seconds to tell us how you rate us. Have a feature suggestion? Share it here.

☔ View full report at Codecov.
📢 Do you have feedback about the report comment? Let us know in this issue.

[sjahr]

I tested this with OMPL and PILZ and multiple pipelines are usable. CI failure is not cause by this PR but unrelated clang-tidy errors that are probably fixed by #1706.

[rhaschke]

@sjahr, if this gets approved here, can you please adapt moveit/moveit#3244 correspondingly?

[sjahr]

@sjahr, if this gets approved here, can you please adapt ros-planning/moveit#3244 correspondingly?

Yes, thanks a lot for contributing these changes 🙏