Pass along move_group_capabilities parameters#2587
Conversation
Codecov ReportAttention:
Additional details and impacted files@@ Coverage Diff @@
## main #2587 +/- ##
==========================================
- Coverage 50.74% 43.02% -7.72%
==========================================
Files 392 692 +300
Lines 32553 56297 +23744
Branches 0 7272 +7272
==========================================
+ Hits 16517 24218 +7701
- Misses 16036 31917 +15881
- Partials 0 162 +162 ☔ View full report in Codecov by Sentry. |
| @@ -228,6 +228,7 @@ def generate_move_group_launch(moveit_config): | |||
| move_group_params = [ | |||
| moveit_config.to_dict(), | |||
There was a problem hiding this comment.
This line should already include the capabilities. Could you verify this?
There was a problem hiding this comment.
The capabilities parameter is not currently included in the to_dict() function:
https://github.com/ros-planning/moveit2/blob/8d2eb900d222f6609ee2588f6f5ebb97986635bd/moveit_configs_utils/moveit_configs_utils/moveit_configs_builder.py#L119C5-L119C23
I tried adding the capabilities parameter to the to_dict() function but that didn't seem to work. It seems that the additional parameter needs to added it to move_group_params after move_group_configuration, but I'm not sure why.
There was a problem hiding this comment.
@henningkayser, did you have additional thoughts on this PR or are you able to approve it?
There was a problem hiding this comment.
sorry for late response... Params override each other, and move_group_configuration provides empty capabilities by default (lines 200 , 214). So I think the best solution would be to set the default to moveit_config.move_group_capabilities in line 200. That way, the launch file would still allow using the launch argument if necessary.
There was a problem hiding this comment.
@henningkayser, with a slight modification because moveit_config.move_group_capabilities is a Dict, your suggestion works. It still seems odd that the capabilities parameter is handled separately instead as part of moveit_config.to_dict() like the other moveit_config parameters.
There was a problem hiding this comment.
@forrest-rm You're right. Do you mind opening a brief issue so we can track fixing this in a more appropriate way in the future?
* Pass along move_group_capabilities parameters * fix lint check * Use move_group_capabilities as default launch argument (cherry picked from commit f47b4c0)
* Pass along move_group_capabilities parameters * fix lint check * Use move_group_capabilities as default launch argument (cherry picked from commit f47b4c0)
…oveit#2696)" This reverts commit 65970f0.
* Pass along move_group_capabilities parameters * fix lint check * Use move_group_capabilities as default launch argument (cherry picked from commit f47b4c0) Co-authored-by: Forrest Rogers-Marcovitz <39061824+forrest-rm@users.noreply.github.com>
The move_group_capabilities is currently unused in launches.py even though it is listed as a parameter of MoveItConfigs.
This fix is necessary to support MoveIt Task Constructor (MTC). In the move_group.lauch.py generated by the MoveIt Setup Assistant, the following line should be added when needed by MTC:
moveit_config.move_group_capabilities = {"capabilities": "move_group/ExecuteTaskSolutionCapability"}I am open to suggestions if there's a better way of handling the move_group_capabilities parameter or to add the previous note as a comment in the auto generated launch scripts.
This was tested on the main and humble branches.