osrf / drcsim

Repository for the DRC project.
18 stars 7 forks source link

Model for left arm is wrong in v4 (and has always been wrong) #492

Closed osrf-migration closed 9 years ago

osrf-migration commented 9 years ago

Original report (archived issue) by Twan Koolen (Bitbucket: tkoolen).

The original report had attachments: atlas_v4_simple_shapes_after_xacro.urdf, atlas_v4_simple_shapes_after_xacro_and_fixes.urdf, r_hand_rotated_pi_about_y.dae, rotate_180_y.mlx, atlas_v5_simple_shapes_after_xacro_and_fixes.urdf


While doing Vicon experiments with markers on the hands and torso, we (the MIT team) noticed that the kinematic model for the right arm was much more accurate than the one for the left arm.

In the current URDF, the kinematic model for the left arm is (mostly, I believe) mirrored from that of the right arm. However on the physical robot, the left arm and right arm are the same, except for the fact that some parts are mounted with a pi rotation offset.

We figured out the correct model for the left arm by simply copying a subset of the right arm properties to the left and subsequently inverting some of the joint axes and adding pi rotation offsets based on the way the physical links were mounted.

After making these changes, we found that the match between the model and the Vicon measurements was much better (magnitude of residuals similar to what was observed for the right arm).

A detailed summary of the changes we made to the URDF (in order):

Note that these modifications imply that the mesh tags for the left arm links now point to the right arm mesh files as we believe they should be since they're actually the same on the physical robot, just mounted differently.

See attachment for:

A caveat is that the urdf we use for simulation and control is actually further modified w.r.t. the one I attached, so we haven't run any simulations with this model. However, I'm pretty confident that these are the right changes since they were done by running the same Python code in both cases.

osrf-migration commented 9 years ago

Original comment by Twan Koolen (Bitbucket: tkoolen).


osrf-migration commented 9 years ago

Original comment by Steve Peters (Bitbucket: Steven Peters, GitHub: scpeters).


Thanks for posting this. I'm going through some of the changes, and I currently have one question. I see two references to joints named [lr]_arm_shy:

$ grep -rn shy atlas_v4*
atlas_v4_simple_shapes_after_xacro_and_fixes.urdf:861:  <joint name="l_arm_shy" type="revolute">
atlas_v4_simple_shapes_after_xacro_and_fixes.urdf:988:  <joint name="r_arm_shy" type="revolute">

I thought there were no _arm_shy joints in the v4.

osrf-migration commented 9 years ago

Original comment by Steve Peters (Bitbucket: Steven Peters, GitHub: scpeters).


I just made a diff of those files and put it in a gist:

The _arm_shy changes are here for l_arm and here for r_arm.

osrf-migration commented 9 years ago

Original comment by Steve Peters (Bitbucket: Steven Peters, GitHub: scpeters).


It looks like it's mainly some sign changes in poses, joint axes, and off-axis inertia matrix terms and using some right-side meshes on the left side.

osrf-migration commented 9 years ago

Original comment by Twan Koolen (Bitbucket: tkoolen).


Sorry about the shy joint names. I thought I was being smart by applying our joint name mapping, running our modification script, and then applying the inverse of the joint name mapping, but it turns out our joint name mapping was out of date. The shy's should be renamed back to shz.

osrf-migration commented 9 years ago

Original comment by Twan Koolen (Bitbucket: tkoolen).


The sign changes in the link origin poses and joint origin poses (except for shz), the off-axis inertia matrix changes, and the meshes are explained by copying the right arm properties to the left. We then found the joint axes pretty much experimentally (just checking on the real robot which direction was positive) after copying those properties over from right to left and adding the pi rotation offsets.

osrf-migration commented 9 years ago

Original comment by Stefan Kohlbrecher (Bitbucket: Stefan_Kohlbrecher).


@BillBlank Can you comment on this? This is a major issue for all Atlas teams. I think it is unclear at the moment what the proper setup for v5 is and it would be good to have a correct model as soon as possible.

osrf-migration commented 9 years ago

Original comment by Bill Blank (Bitbucket: BillBlank).


I agree with everything Twan has said above. He's right, the arms aren't exactly symmetric, and the changes he proposes above to resolve it look good. Boston Dynamics endorses Twan's changes.

osrf-migration commented 9 years ago

Original comment by DouglasS (Bitbucket: DouglasS).


@BillBlank @scpeters @tkoolen @Stefan_Kohlbrecher has anybody taken a look at V5 to see if this is the same?

osrf-migration commented 9 years ago

Original comment by Bill Blank (Bitbucket: BillBlank).


The arms on v5 are identical except for the wryx,wry2, and wry joint/links which are of course completely different. The electric forearms are much more symmetric than the upper arm links, so one should only need to apply the above non-wrx/y changes to v5, and just leave the electric forearms as is, ie, symmetric.

osrf-migration commented 9 years ago

Original comment by Steve Peters (Bitbucket: Steven Peters, GitHub: scpeters).


I'll start by copying @tkoolen 's changes into a branch for the v4 and will probably need some guidance on what exactly to do for the v5.

osrf-migration commented 9 years ago

Original comment by Bill Blank (Bitbucket: BillBlank).


Sounds like you might need Twan to share the revised r_lfarm.dae file w/ you to apply his changes.

osrf-migration commented 9 years ago

Original comment by Twan Koolen (Bitbucket: tkoolen).


I actually just added an 'origin' tag to the visual with a pi rotation instead of modifying the mesh file, i.e. line 500 in the file with our fixes:

<origin rpy="0 3.14159265359 0" xyz="0 0 0"/>

But it would be easy to do the mesh rotation in meshlab if you prefer.

osrf-migration commented 9 years ago

Original comment by Twan Koolen (Bitbucket: tkoolen).


By the way, I assume you mean r_hand.dae, which is the mesh file for the r_lfarm link.

osrf-migration commented 9 years ago

Original comment by Twan Koolen (Bitbucket: tkoolen).


Here's the rotated mesh if you prefer it. Created by running

meshlabserver -i r_hand.dae -o r_hand_rotated_pi_about_y.dae -om vn -s rotate_180_y.mlx

rotate_180_y.mlx is also attached.

osrf-migration commented 9 years ago

Original comment by David Conner (Bitbucket: dcconner).


@scpeters is there any remaining confusion that preventing the change to v5? Is should be the same for the sh and el joints.

We need this change, and would prefer to stand in step with the main branch. Any idea when this will be available.

osrf-migration commented 9 years ago

Original comment by DouglasS (Bitbucket: DouglasS).


@scpeters @dcconner We second this. The sooner we can have a non-vendored and correct V5 model, the better.

osrf-migration commented 9 years ago

Original comment by Steve Peters (Bitbucket: Steven Peters, GitHub: scpeters).


osrf-migration commented 9 years ago

Original comment by Steve Peters (Bitbucket: Steven Peters, GitHub: scpeters).


osrf-migration commented 9 years ago

Original comment by Steve Peters (Bitbucket: Steven Peters, GitHub: scpeters).


Since @tkoolen 's patch was between xacro'd versions of atlas_v4_simple_shapes.urdf, I first passed this file through xacro in 1e0e52d95be568bee0c34b67fb98d62ff0312e4b to get the attributes sorted the same way and whitespace aligned. I then applied his patch in 24d4800437c49942bd0add11fb9ba17fc67506ef and followed it up with another xacro run to clean up any whitespace. So I would look at specific commits rather than a complete branch comparison, which has a lot of noise in it.

How does this look?

I imagine these v4 changes should be propagated to other v4 urdf's, and then we need to address the v5.

osrf-migration commented 9 years ago

Original comment by Steve Peters (Bitbucket: Steven Peters, GitHub: scpeters).


I also recommend opening a browser in full screen and using the side-by-side diff comparison, which does a better job highlighting the actual changes.

osrf-migration commented 9 years ago

Original comment by Steve Peters (Bitbucket: Steven Peters, GitHub: scpeters).


pull request #535 for atlas_v4_simple_shapes.urdf

osrf-migration commented 9 years ago

Original comment by Steve Peters (Bitbucket: Steven Peters, GitHub: scpeters).


I just repeated the effort for atlas_v4_no_wry2_simple_shapes.urdf:

osrf-migration commented 9 years ago

Original comment by Steve Peters (Bitbucket: Steven Peters, GitHub: scpeters).


@billblank so the wry2, wrx, and wry joints are symmetric. The wry joint connects the larm to the ufarm. Can I assume that the ufarm is symmetric, while the larm is not? I'm currently working under that assumption.

osrf-migration commented 9 years ago

Original comment by Steve Peters (Bitbucket: Steven Peters, GitHub: scpeters).


I just made an attempt at atlas_v5_simple_shapes in branch issue_492_v5:

osrf-migration commented 9 years ago

Original comment by Steve Peters (Bitbucket: Steven Peters, GitHub: scpeters).


There's something wrong with the v5 urdf I just committed. I'm looking into it...

EDIT: I could use some guidance.

osrf-migration commented 9 years ago

Original comment by DouglasS (Bitbucket: DouglasS).


@scpeters I also made an attempt to simply apply everything above the l_arm_wry from @tkoolen and it didn't work at all… there were a bunch of reflections I couldn't sort out. So you're in good company.

The kinematics are definitely off on the left arm for v5, we can verify this by simply observing arm positioning in the real world vs. in our user interface which uses the URDF/SDF for visualization. And it's a very large discrepancy. But we haven't established what's wrong, exactly. So yes some guidance would be nice.

osrf-migration commented 9 years ago

Original comment by Twan Koolen (Bitbucket: tkoolen).


@scpeters, here's a modified version of atlas_v5_simple_shapes.urdf from the revision after you ran xacro on it but before your modifications (456dd5d0b84d6a75026af5f8f7357c3770e34973). I generated it by running the same python script we used for atlas_v4. A further xacro'd version of the file (adding the head and such) looks good to me in our visualizer, but I can't compare it to the real robot since we don't have the new arms yet.

osrf-migration commented 9 years ago

Original comment by DouglasS (Bitbucket: DouglasS).


@tkoolen @scpeters I'll give it a try on the real robot this afternoon.

osrf-migration commented 9 years ago

Original comment by Twan Koolen (Bitbucket: tkoolen).


The top things I'm not certain about and I can't check are:

osrf-migration commented 9 years ago

Original comment by DouglasS (Bitbucket: DouglasS).


@tkoolen @scpeters Apparently my comments are being auto-flagged as spam (hence that grey background, so I don't think anybody can read what I'm writing until an OSRF person approves the comment) but we tried Twan's new model on the real robot and it wasn't quite right, but I'm really confused as to why it doesn't work.

We have a slightly tweaked xacro for adding the Robotiq hands to our model because we use the simplified pin-joint model instead of the complex four-bar-linkage model. We've also modified our real robot mounting plates so that the mounting when the robot is at the zero pose has both of the hands with their thumbs up. Using Twan's V5 .URDF, we can generate a model with this configuration in the zero pose. But when we convert to .sdf and load this model in to our visualization software and run the real robot, there is a ~90º offset on the left hand. We're not sure where this offset is coming from since the zero pose looks correct.

osrf-migration commented 9 years ago

Original comment by Steve Peters (Bitbucket: Steven Peters, GitHub: scpeters).


We had some spam flagging that has been resolved, sorry about that.

osrf-migration commented 9 years ago

Original comment by Steve Peters (Bitbucket: Steven Peters, GitHub: scpeters).


Ok, I'm testing @tkoolen 's urdf right now...

osrf-migration commented 9 years ago

Original comment by Steve Peters (Bitbucket: Steven Peters, GitHub: scpeters).


Ok, @tkoolen 's urdf looks much better than the one I had posted in branch issue_492_v5. I've merged his changes for atlas_v5_simple_shapes.urdf into branch issue_492_v5_twan in eb29fe8a691eb4a0f7285f74b70f5916639d8f7b.

It sounds like there are still some issues with the hand mounting, so I will hold off on making a pull request.

osrf-migration commented 9 years ago

Original comment by Twan Koolen (Bitbucket: tkoolen).


Taylor will be happy about that :-)

osrf-migration commented 9 years ago

Original comment by Steve Peters (Bitbucket: Steven Peters, GitHub: scpeters).


whoops, fixed :)

osrf-migration commented 9 years ago

Original comment by Mathew DeDonato (Bitbucket: mdedonato NA).


The new v5 model and I am assuming the new v4 model has the axis flipped on the left hand. Twan mentioned this in his summary of changes. Is there any plan to fix this, or is this change going to be permanent?

osrf-migration commented 9 years ago

Original comment by Steve Peters (Bitbucket: Steven Peters, GitHub: scpeters).


@mdedonato can you comment in pull request #535 and/or submit a patch?

osrf-migration commented 9 years ago

Original comment by Twan Koolen (Bitbucket: tkoolen).


We have the new forearms now, and at first glance everything seems OK for us using the new v5 model. The zero pose and the direction in which the joints move (i.e. the joint axes) match between the physical robot and our visualization. Unfortunately everybody does their own modifications to the urdf, so it's hard to figure out what's wrong for the others.

As for the top things that I thought might be wrong:

@dljsjr,

Something in the v5 arms is definitely wrong, we just aren't sure what yet. The hand mounting looks fine in the zero pose but as the robot moves around visualization is incorrect. So there's probably a flipped axis or something.

Have you figured out what the cause of the visualization weirdness was?

@dljsjr,

Using Twan's V5 .URDF, we can generate a model with this configuration in the zero pose. But when we convert to .sdf and load this model in to our visualization software and run the real robot, there is a ~90º offset on the left hand.

This sounds like a different issue, what's the status on this one?

osrf-migration commented 9 years ago

Original comment by DouglasS (Bitbucket: DouglasS).


@tkoolen They're actually the same issue; I just worded it poorly. We don't consume .urdf at all, everything (including our visualizations) uses .sdf. We have no update on this, we're in the process of setting up our own MOCAP experiments to mimic yours and see if we can't find the root of the problem. We do have a pipeline that allows us to visualize our state using RViz, though, so we may want to run that experiment using your modified files as well.

osrf-migration commented 9 years ago

Original comment by Steve Peters (Bitbucket: Steven Peters, GitHub: scpeters).


Let me know when you need anything more from me. I'm still waiting for review of pull request #535 (v4 left arm) and consensus on the v5 (branch issue_492_v5_twan)

osrf-migration commented 9 years ago

Original comment by Twan Koolen (Bitbucket: tkoolen).


If you're waiting for a review from me, I say #535 is ready to go. I suppose fewer and fewer people are going to care about any atlas_v4 changes anyway.

osrf-migration commented 9 years ago

Original comment by Steve Peters (Bitbucket: Steven Peters, GitHub: scpeters).


Thanks, do you mind clicking the Approve button in the pull request? (pull request #535)

osrf-migration commented 9 years ago

Original comment by Twan Koolen (Bitbucket: tkoolen).


Done.

osrf-migration commented 9 years ago

Original comment by Steve Peters (Bitbucket: Steven Peters, GitHub: scpeters).


I just merged pull request #535, which fixed atlas_v4_simple_shapes.urdf. I've created two new pull requests:

osrf-migration commented 9 years ago

Original comment by DouglasS (Bitbucket: DouglasS).


@tkoolen @scpeters I'm beginning to suspect that the problem isn't with the model but with a codepath on our end. I have some tests I'm running this afternoon using the v5 with Twan's fixes.

osrf-migration commented 9 years ago

Original comment by Steven Gray (Bitbucket: stgray).


What's the status on this? I'm trying out pull request #536 and the left hand ends up inside the elbow. pull_536_v5_simple_shapes.png

osrf-migration commented 9 years ago

Original comment by Steve Peters (Bitbucket: Steven Peters, GitHub: scpeters).


@stgray confirmed; the hand mounting points are wrong.

osrf-migration commented 9 years ago

Original comment by Twan Koolen (Bitbucket: tkoolen).


As I mentioned before, it works for us but unfortunately everybody does their own modifications to the urdf to add the various hands. It might be better for somebody who is experiencing these problems to propose a solution, especially since I don't have a lot of cycles to spend on this.

osrf-migration commented 9 years ago

Original comment by Steve Peters (Bitbucket: Steven Peters, GitHub: scpeters).


Ok, thanks for the work you've done already!