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

Use move in gripper_action when max_effort is 0 #209

Merged
merged 4 commits into from
Apr 26, 2022

Conversation

marcbone
Copy link
Contributor

@marcbone marcbone commented Dec 2, 2021

This PR implements the my suggested workaround from #173.

This PR solves #172.

Now the gripper_action can close the gripper to a smaller width when max_effort is 0. However, since this is a breaking change we want to wait until the following issue is resolved in moveit (@rickstaa):

@rickstaa
Copy link
Contributor

rickstaa commented Dec 2, 2021

@marcbone Thanks for implementing this. I am currently still trying to fix the gravity compensator (i.e. #160) but I will jump right on it after that is done.

@rickstaa
Copy link
Contributor

rickstaa commented Dec 7, 2021

@marcbone I implemented the requested functionality in Moveit (see moveit/moveit#2984). It is currently under review. I will let you know when it is merged.

@rickstaa
Copy link
Contributor

rickstaa commented Dec 8, 2021

@marcbone I tested your pull request, and I think your solution is very intuitive. It works perfectly when the gripper hand is positioned horizontally. Please be aware that the move action is currently broken for non-horizontal gripper hand orientations (see #173 (comment) and #173 (comment)).

rickstaa added a commit to rickstaa/panda-gazebo that referenced this pull request Dec 10, 2021
This commit simplifies the logic inside the `set_gripper_width` action.
Previously we used both the `franka_gripper/grasp` and
`franka_gripper/move` services but after
frankaemika/franka_ros#209 has been merged we
can simply use the `franka_gripper/gripper_action` service.
rickstaa added a commit to rickstaa/panda-gazebo that referenced this pull request Dec 10, 2021
This commit simplifies the logic inside the `set_gripper_width` action.
Previously we used both the `franka_gripper/grasp` and
`franka_gripper/move` services but after
frankaemika/franka_ros#209 has been merged we
can simply use the `franka_gripper/gripper_action` service.
rickstaa added a commit to rickstaa/panda-gazebo that referenced this pull request Dec 10, 2021
This commit simplifies the logic inside the `set_gripper_width` action.
Previously we used both the `franka_gripper/grasp` and
`franka_gripper/move` services but after
frankaemika/franka_ros#209 has been merged we
can simply use the `franka_gripper/gripper_action` service.
rickstaa added a commit to rickstaa/panda-gazebo that referenced this pull request Dec 10, 2021
This commit simplifies the logic inside the `set_gripper_width` action.
Previously we used both the `franka_gripper/grasp` and
`franka_gripper/move` services but after
frankaemika/franka_ros#209 has been merged we
can simply use the `franka_gripper/gripper_action` service.
rickstaa added a commit to rickstaa/panda-gazebo that referenced this pull request Dec 10, 2021
This commit simplifies the logic inside the `set_gripper_width` action.
Previously we used both the `franka_gripper/grasp` and
`franka_gripper/move` services but after
frankaemika/franka_ros#209 has been merged we
can simply use the `franka_gripper/gripper_action` service.
rickstaa added a commit to rickstaa/panda-gazebo that referenced this pull request Dec 10, 2021
This commit simplifies the logic inside the `set_gripper_width` action.
Previously we used both the `franka_gripper/grasp` and
`franka_gripper/move` services but after
frankaemika/franka_ros#209 has been merged we
can simply use the `franka_gripper/gripper_action` service.
rickstaa added a commit to rickstaa/panda-gazebo that referenced this pull request Dec 10, 2021
This commit simplifies the logic inside the `set_gripper_width` action.
Previously we used both the `franka_gripper/grasp` and
`franka_gripper/move` services but after
frankaemika/franka_ros#209 has been merged we
can simply use the `franka_gripper/gripper_action` service.
@rickstaa
Copy link
Contributor

@marcbone Just a heads-up. moveit/moveit#2984 (comment) has been merged.

@rickstaa
Copy link
Contributor

@marcbone, @gollth Just wondering, do you need me to implement something else on the MoveIt side before this can be merged?

Also, did you already manage to find good PID gains that can control the franka_gripper in the vertical mode that does not break the velocity checking logic in the grasp server (see #173 (comment))?

@gollth
Copy link
Contributor

gollth commented Jan 10, 2022

Also, did you already manage to find good PID gains that can control the franka_gripper in the vertical mode that does not break the velocity checking logic in the grasp server (see #173 (comment))?

Not yet unfortunately but we have it on the radar

@rickstaa
Copy link
Contributor

Great! Thanks a lot!

@marcbone
Copy link
Contributor Author

Just wondering, do you need me to implement something else on the MoveIt side before this can be merged?

No, basically I am just waiting for the moveit release, since I guess nobody is going to build moveit from source.

@rickstaa
Copy link
Contributor

@marcbone Ha good point! I overlooked that fact. Let's wait for the release then.

@rickstaa
Copy link
Contributor

rickstaa commented Jan 29, 2022

@marcbone Just a heads up, I tried to rebase this branch on the develop branch but git runs into conflicts of which two are not flagged by git. The following lines are injected by the git merge tool on line 354-355 of the franka_gripper_sim.cpp file without git flagging them:

double width = this->finger1_.getPosition() + this->finger2_.getPosition();  // recalculate
bool ok = goal->width - eps.inner < width and width < goal->width + eps.outer;

This will throw a error, further the following line will be changed from:

result.success = static_cast<decltype(result.success)>(true);

to

result.success = static_cast<decltype(result.success)>(ok);

Which also throws an error. After fixing these lines and solving the other flagged conflicts, the pull request works again (see https://github.com/rickstaa/franka_ros/tree/use-move-in-gripper-action-rebased).

@marcbone marcbone merged commit becb3b8 into develop Apr 26, 2022
@gollth gollth deleted the use-move-in-gripper-action branch April 26, 2022 12:52
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants