-
Notifications
You must be signed in to change notification settings - Fork 86
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
Support tele-op for Franka Kitchen environments ("vel" mode) #142
base: dev
Are you sure you want to change the base?
Conversation
I haven't tested in cases you are using it now. This might be a good chance for me to test those. I'm traveling this week. I'll be able to take a look next week. |
Bump on this PR. Even though the teleop works, it would be good to know if the implementation is correct, especially since it doesn't work when I normalize the velocities. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thanks for your contributions and appologies for the delay in reviews. I have been traveling.
Let me know when its ready for a merge and I'll do a thorough review. |
Ready for review! |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Added one comment, rest looks good.
@@ -47,14 +47,13 @@ | |||
<geom type="box" group="3" pos='0 0 .142' size="0.02 0.10 0.03" contype="0" conaffinity="0" rgba=".9 .7 .95 1" euler="0 0 -.785"/> | |||
</body> | |||
|
|||
<site name='target' pos='0 0 0' size='0.1' rgba='0 2 0 .2'/> | |||
<site name='target' type='box' size='.03 .07 .04' pos='-0.4 0.4 2.1' group='1' rgba='0 1 .4 0' euler="0 3.14 3.14"/> |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
To make it a general solution
- we can keep the site in its default position.
- When teleOp is initialized, we should programmatically move this site to where the end effector is. That will avoid any initial jump in teleOp during connection to the robot.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Done! It's working as expected, but I'm seeing some weird behaviour from the rpFrankaRobotiqData-v0
environment:
The target site isn't placed exactly at the end effector. It's placed a little bit above + rotated compared to the actual location of the end effector. For the Franka Kitchen environment, the goal site is centered exactly on the end effector.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This seems like an issue with conventions. Double-check where all the site frames are (both position and orientations)
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
It looks like we see this issue because the end_effector
site is defined for the Franka gripper, and the Robotiq gripper doesn't redefine it. Because of the grippers' different dimensions, it doesn't align perfectly for the Robotiq gripper.
- The overall difference is fairly small (see images below) and probably doesn't matter much for teleop.
- This issue can also be observed in the main branch and is not introduced in this PR.
Changes Summary:
ee_target
to Franka Kitchen xmlnormalize_actions
Motivation:
Currently, the teleop code only supports "pos" mode environments, where the expected input action is the qpos coordinates. Other environments, like the Franka Kitchen envs, are "vel" mode - where input action is the velocity of each joint.
NOTE: Normalizing the qvel does not work, the values are too small and the end effector never reaches the teleop target. Is there some issue with how the normalization is called?