Conversation
mikeferguson
left a comment
There was a problem hiding this comment.
I'll take a closer look later today
moveit_plugins/moveit_simple_controller_manager/src/moveit_simple_controller_manager.cpp
Outdated
Show resolved
Hide resolved
|
Codecov ReportAttention: Patch coverage is
❗ Your organization needs to install the Codecov GitHub app to enable full functionality. Additional details and impacted files@@ Coverage Diff @@
## main #3246 +/- ##
==========================================
- Coverage 45.65% 45.57% -0.07%
==========================================
Files 714 716 +2
Lines 62299 62373 +74
Branches 7532 7541 +9
==========================================
- Hits 28437 28422 -15
- Misses 33694 33784 +90
+ Partials 168 167 -1 ☔ View full report in Codecov by Sentry. |
There was a problem hiding this comment.
Left some minor comments.
The major comment is a totally new thing that needs to happen here, which is ensuring that this type of controller is included in the MoveIt Setup Assistant.
There's probably a few places in the moveit_setup_assistant/moveit_setup_controllers package to edit -- for example, this and this
Looking at the code, it seems MSA never added the effort_controllers/GripperActionController either, but... eh, probably not needed since that's also deprecated in Jazzy.
moveit_plugins/moveit_ros_control_interface/src/parallel_gripper_command_controller_plugin.cpp
Outdated
Show resolved
Hide resolved
moveit_plugins/moveit_ros_control_interface/src/parallel_gripper_command_controller_plugin.cpp
Show resolved
Hide resolved
...ager/include/moveit_simple_controller_manager/parallel_gripper_command_controller_handle.hpp
Outdated
Show resolved
Hide resolved
...ager/include/moveit_simple_controller_manager/parallel_gripper_command_controller_handle.hpp
Outdated
Show resolved
Hide resolved
...ager/include/moveit_simple_controller_manager/parallel_gripper_command_controller_handle.hpp
Outdated
Show resolved
Hide resolved
moveit_plugins/moveit_simple_controller_manager/src/moveit_simple_controller_manager.cpp
Outdated
Show resolved
Hide resolved
moveit_plugins/moveit_simple_controller_manager/src/moveit_simple_controller_manager.cpp
Outdated
Show resolved
Hide resolved
|
@sea-bass thanks for the detailed review, I appreciate the feedback! I will try to get to the MSA stuff as soon as I can, but if you are okay to push this off to a different PR then I think this is good to go. I personally don't use the MSA because it makes messy configs. |
sea-bass
left a comment
There was a problem hiding this comment.
You can do the MSA stuff after -- would definitely wait for @JafarAbdi and @mikeferguson to comment though, since they said they would.
...ager/include/moveit_simple_controller_manager/parallel_gripper_command_controller_handle.hpp
Outdated
Show resolved
Hide resolved
...ager/include/moveit_simple_controller_manager/parallel_gripper_command_controller_handle.hpp
Outdated
Show resolved
Hide resolved
...ager/include/moveit_simple_controller_manager/parallel_gripper_command_controller_handle.hpp
Outdated
Show resolved
Hide resolved
...ager/include/moveit_simple_controller_manager/parallel_gripper_command_controller_handle.hpp
Outdated
Show resolved
Hide resolved
* Add ParallelGripperCommand controller * only use max velocity/effor when specified * else if * clean up author and copyright comments * address review * single joint control, remove allow_stalling --------- Co-authored-by: Jafar Uruç <cafer.abdi@gmail.com> (cherry picked from commit f47ecb5)
* Add ParallelGripperCommand controller * only use max velocity/effor when specified * else if * clean up author and copyright comments * address review * single joint control, remove allow_stalling --------- Co-authored-by: Jafar Uruç <cafer.abdi@gmail.com>
* Add ParallelGripperCommand controller * only use max velocity/effor when specified * else if * clean up author and copyright comments * address review * single joint control, remove allow_stalling --------- Co-authored-by: Jafar Uruç <cafer.abdi@gmail.com>
* Add ParallelGripperCommand controller * only use max velocity/effor when specified * else if * clean up author and copyright comments * address review * single joint control, remove allow_stalling --------- Co-authored-by: Jafar Uruç <cafer.abdi@gmail.com>
Description
This PR closes #3017 and adds the option to control grippers that use the ros2_control
ParallelGripperCommandinterface. I renamed all of the existing gripper controller files to include the namegripper_commandand created new files for theparallel_gripper_command_controller.To setup add the following to your
moveit_controllers.yamlWhen you start MoveIt you should see the following when adding the controllers