Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Make max_effort settable in the MoveIt Simple Controller Manager #2956

Closed
2 tasks done
Tracked by #209
rickstaa opened this issue Nov 11, 2021 · 8 comments
Closed
2 tasks done
Tracked by #209

Make max_effort settable in the MoveIt Simple Controller Manager #2956

rickstaa opened this issue Nov 11, 2021 · 8 comments

Comments

@rickstaa
Copy link
Contributor

rickstaa commented Nov 11, 2021

Is your feature request related to a problem? Please describe.

I think the moveit_simple_controller_manager simple can be improved such that users can easily specify the MaxEffort that is set on the GripperCommand message. This will allow users to grasp an object and move the gripper to the desired position (see frankaemika/franka_ros#173 (comment)).

TODOS

  • Specify the GripperCommand max effort as a ROS parameter in the GripperCommand declaration.
  • Validate: Allow switching controllers for different (types of) trajectories (see Allow specification of controller names for trajectories #1832).
  • Add effort field to motion_planning_rviz_plugin when group uses GripperCommand action.
    EDIT(rhaschke): That's a great idea!
    EDIT(v4hn): a group does not use an action, a set of joints do. So this feature would break abstractions.
@v4hn
Copy link
Contributor

v4hn commented Nov 11, 2021

For completeness, the max_effort field currently depends on the effort set for the last waypoint of the trajectory to be executed.
That behavior should definitely be retained and is more flexible than what you propose.
But it's very true that there is no interface to specify the effort for this waypoint when you plan through the MoveGroup action.
It could make sense to add a max_effort field to the generic action requests (or PlanningOptions) and use this field for all waypoints if specified.

@rickstaa
Copy link
Contributor Author

For completeness, the max_effort field currently depends on the effort set for the last waypoint of the trajectory to be executed.
That behavior should definitely be retained and is more flexible than what you propose.

@v4hn I was planning to read the default max_effort from the parameter server and only apply it when no effort is set anywhere in the send Trajectory. Meaning if somebody sets an effort for one of the waypoints the parameter is no longer used.

But it's very true that there is no interface to specify the effort for this waypoint when you plan through the MoveGroup action.
It could make sense to add a max_effort field to the generic action requests (or PlanningOptions) and use this field for all waypoints if specified.

This was how I was planning to implement it.

EDIT(v4hn): a group does not use an action, a set of joints do. So this feature would break abstractions.

Is there a way that we can add the max effort to the motion_planning_rviz_plugin without breaking the abstractions? Or would you suggest dropping this TODO and only providing it through the MoveGroup action?

@v4hn
Copy link
Contributor

v4hn commented Nov 11, 2021 via email

@rickstaa

This comment has been minimized.

@rickstaa
Copy link
Contributor Author

rickstaa commented Dec 7, 2021

@rhaschke and @v4hn, I will not implement the RVIZ effort GUI since this feature requires more work after inspection than I can currently dedicate to this pull request. This might be something to add in the future.

rickstaa added a commit to rickstaa/moveit that referenced this issue Dec 7, 2021
…rCommand action

This commit adds the `max_effort` parameter to the GripperCommand
declaration in the `controller_list` (see issue moveit#2956). This value is
only used when effort is set in the requested gripper trajectory.
rickstaa added a commit to rickstaa/moveit that referenced this issue Dec 7, 2021
…rCommand action

This commit adds the `max_effort` parameter to the GripperCommand
declaration in the `controller_list` (see issue moveit#2956). This value is
only used when effort is set in the requested gripper trajectory.
rickstaa added a commit to rickstaa/moveit that referenced this issue Dec 7, 2021
…rCommand action

This commit adds the `max_effort` parameter to the GripperCommand
declaration in the `controller_list` (see issue moveit#2956). This value is
only used when effort is set in the requested gripper trajectory.
@rickstaa
Copy link
Contributor Author

rickstaa commented Dec 7, 2021

@rhaschke I think the main functionality of frankaemika/franka_ros#173 (comment) was implemented in #2984. Please let me know if it is what you had in mind.

You also mentioned something about controller switching in frankaemika/franka_ros#173 (comment). I, however, was not sure what you had in mind. It looks like that is a distinct feature not related to the gripper control we requested frankaemika/franka_ros#173 (comment). Would you mind verifying how you envisioned this controller switching? #1832 (comment) states that it was implemented in the past but has been removed.

rickstaa added a commit to rickstaa/moveit that referenced this issue Dec 8, 2021
…rCommand action

This commit adds the `max_effort` parameter to the GripperCommand
declaration in the `controller_list` (see issue moveit#2956). This value is
only used when effort is set in the requested gripper trajectory.

Co-authored-by: Jafar Abdi <[email protected]>
rickstaa added a commit to rickstaa/moveit that referenced this issue Dec 8, 2021
…rCommand action

This commit adds the `max_effort` parameter to the GripperCommand
declaration in the `controller_list` (see issue moveit#2956). This value is
only used when effort is set in the requested gripper trajectory.

Co-authored-by: Jafar Abdi <[email protected]>
v4hn pushed a commit that referenced this issue Dec 15, 2021
…rCommand action (#2984)

This commit adds the `max_effort` parameter to the GripperCommand
declaration in the `controller_list` (see issue #2956). This value is
only used when effort is set in the requested gripper trajectory.

Co-authored-by: Jafar Abdi <[email protected]>
@rickstaa
Copy link
Contributor Author

@rhaschke Now that #2984 (comment) has been merged, I think we can close this issue. The only TODO that is open is the one about the controller switching (see #2956 (comment)).

@rickstaa rickstaa removed their assignment Jan 1, 2022
@rhaschke
Copy link
Contributor

Closed via #2984.
The controller switching I was referring to was already implemented in #1832.
The core idea is to explicitly specify which controller (names) to use for specific sub trajectories of a longer plan. This way, one can switch between e.g. GripperCommand and JointTrajectoryControl.

v4hn pushed a commit to v4hn/moveit that referenced this issue Mar 15, 2022
…rCommand action (moveit#2984)

This commit adds the `max_effort` parameter to the GripperCommand
declaration in the `controller_list` (see issue moveit#2956). This value is
only used when effort is set in the requested gripper trajectory.

Co-authored-by: Jafar Abdi <[email protected]>
JafarAbdi added a commit that referenced this issue Mar 16, 2022
…rCommand action (#2984) (#3091)

This commit adds the `max_effort` parameter to the GripperCommand
declaration in the `controller_list` (see issue #2956). This value is
only used when effort is set in the requested gripper trajectory.

Co-authored-by: Rick Staa <[email protected]>
Co-authored-by: Jafar Abdi <[email protected]>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

3 participants