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

Define meaningful tool frame #194

Merged
merged 1 commit into from
Nov 12, 2021
Merged

Conversation

rhaschke
Copy link
Contributor

@rhaschke rhaschke commented Nov 7, 2021

When a hand is mounted, the link8 frame is 45° rotated wrt. the hand.
Better define a tool frame that is nicely aligned with the hand, i.e.
x=normal, y=slide, z=approach, and nicely centered between the fingers.

Before After
image image

rhaschke added a commit to rhaschke/panda_moveit_config that referenced this pull request Nov 8, 2021
Introduced in frankaemika/franka_ros#194, this allows for a more convinient configuration of the gripper.
rhaschke added a commit to rhaschke/franka_ros that referenced this pull request Nov 8, 2021
@marcbone
Copy link
Contributor

marcbone commented Nov 8, 2021

I tested this together with the new panda_moveit_config in the noetic-branch. I really like that everything is now properly lined up due to changing the parent_link of the end-effector to "panda_tool". However, this would break all existing user tasks that rely on moveit for motion planning. Therefore, we have some objections, merging this.

@gavanderhoorn
Copy link
Contributor

Define meaningful tool frame

it's debatable whether tool is actually meaningful :)

@rhaschke
Copy link
Contributor Author

rhaschke commented Nov 8, 2021

it's debatable whether tool is actually meaningful :)

Gijs, I guess you want to debate the name, not the concept. Please suggest a better name 😉
Maybe, Franka already has a name for this logical frame internally? I could imagine tcp (tool center point) as well.

Marc, my proposal is fully backward-compatible. link8 still exists as before and existing user code (planning based on link8) still works for me. Did you observe concrete issues when testing this?
Switching to the new tool link, though, will probably simplify matters for them 😉

@rickstaa
Copy link
Contributor

rickstaa commented Nov 8, 2021

I used something similar in my own project and called it gripper_center since Franka calls their end-effectors parallel yaw-gripper and Vacuum gripper (see https://pkj-robotics.dk/wp-content/uploads/2020/09/Franka-Emika_Brochure_EN_April20_PKJ.pdf). Nevertheless, I think hand_center or tool_center_point would also be fine. 👍🏻

@gavanderhoorn
Copy link
Contributor

gripper_center might actually be ok.

tool_center_point is meaningless imo.

Ideally TCP frame names are application specific/defined, but I understand you're going for a generic reusable one.

@rhaschke
Copy link
Contributor Author

rhaschke commented Nov 8, 2021

Yes, my point was to have a generic name that applies to configs w/ and w/o a hand. Thus gripper_center isn't a good name imo. I see your point reserving tcp for applications. So, any more suggestions?

@rickstaa
Copy link
Contributor

rickstaa commented Nov 8, 2021

Yes, my point was to have a generic name that applies to configs w/ and w/o a hand. Thus gripper_center isn't a good name imo. I see your point reserving tcp for applications. So, any more suggestions?

What about ee_center or end_effector_center? Since when the hand is not attached, the arm head essentially becomes the end-effector?

@rhaschke
Copy link
Contributor Author

rhaschke commented Nov 8, 2021

Let's wait for Franka people to chime in...

@marcbone
Copy link
Contributor

marcbone commented Nov 9, 2021

So we definitely have to discuss the name. But this is not our main concern. The real problem is the changing of the end-effector link in the move group. The "panda_arm" move group behaves now differently since it does not use link8 (the flange) anymore. Therefore, every movement that previously worked is now rotated 45 degrees and about 10 cm away from the target, which is a huge breaking change, that we have to discuss internally

@rhaschke
Copy link
Contributor Author

rhaschke commented Nov 9, 2021

Changing the end-effector link is, of course, the crucial point of this PR.
But you are right: when not explicitly specifying the end_effector link to be used by the MoveGroupInterface, the default will be the chain tip, i.e. tool (or whatever name is finally chosen 😉).
However, users can easily change this default by calling MGI::setEndEffectorLink. If this is set to link8, the robot will behave exactly as before.

But I also understand your concerns regarding existing user code. I can think of these two mitigations to maintain compatibility:

  • Introduce a second end-effector (tool), which uses tool as its end effector link. An implementation can be found here, but resulting in two interactive markers being displayed. This is rather confusing.
  • Introduce a new group and end-effector implementing the new behaviour and leaving panda_arm as is, implemented here. I think that's the preferred way to handle backward compatibility. In rviz (and everywhere else) users can simply switch between old and new behavior by choosing the corresponding JMG.

In any case, the backward compatibility issues affect the panda_moveit_config package only. Thus the PR here can be merged independently of that. Particularly, existing MoveIt configs will be not be affected at all.

@rhaschke
Copy link
Contributor Author

rhaschke commented Nov 9, 2021

By the way, does Franka has a preference for a name?

@marcbone
Copy link
Contributor

marcbone commented Nov 9, 2021

So in our opinion the best way would be to leave panda_arm as it was for backward compatibility and add a new move group called "manipulator" (seems to be quite a common name in other moveit_configs) with the new ee link.

Our preferred name for the new link is "panda_hand_tcp" and we would like to only have it when the urdf contains a hand. To make this work the "manipulator" move group would only be present when the srdf is build with the hand argument. Without the hand argument you would still have the panda_arm group with the flange as the tip.

@rhaschke
Copy link
Contributor Author

rhaschke commented Nov 9, 2021

I will modify this PR and panda_moveit_config appropriately.

@rhaschke
Copy link
Contributor Author

@marcbone, do you have any more objections to merge this?

@marcbone
Copy link
Contributor

Yes. The panda_hand_tcp link should not be present when the hand is not used

@rhaschke
Copy link
Contributor Author

rhaschke commented Nov 11, 2021

Sure, forgot about that. Done now. I will move the definition to hand.xacro...

@rhaschke
Copy link
Contributor Author

Hm. Somehow I screwed my commits. 😞

@rhaschke
Copy link
Contributor Author

Ok. Should be fixed now.

rhaschke added a commit to rhaschke/franka_ros that referenced this pull request Nov 11, 2021
…frankaemika#191 (drop-joint-states-desired) and frankaemika#189 (rework-hand-collision) into develop
rhaschke added a commit to rhaschke/franka_ros that referenced this pull request Nov 12, 2021
…t-states-desired) and frankaemika#194 (tool-frame) into develop
rhaschke added a commit to rhaschke/franka_ros that referenced this pull request Nov 12, 2021
…t-states-desired) and frankaemika#194 (tool-frame) into develop
... in the center between and aligned with the fingers
rhaschke added a commit to rhaschke/franka_ros that referenced this pull request Nov 12, 2021
…t-states-desired) and frankaemika#194 (tool-frame) into develop
rhaschke added a commit to rhaschke/franka_ros that referenced this pull request Nov 12, 2021
…t-states-desired) and frankaemika#194 (tool-frame) into develop
@marcbone marcbone merged commit a741389 into frankaemika:develop Nov 12, 2021
@rhaschke rhaschke deleted the tool-frame branch November 28, 2021 16:32
gollth added a commit that referenced this pull request Jun 29, 2022
* commit 'e2cfe284e21f0ed8fd3af66665057acfd1856088':
  ADD: Accept `xacro_args` to pass down additional arguments to URDF
  ADD: `tcp_xyz` & `tcp_rpy` offsets to XACRO files
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.

4 participants