Conversation
|
Thanks for your pull request! It looks like this may be your first contribution to a Google open source project. Before we can look at your pull request, you'll need to sign a Contributor License Agreement (CLA). View this failed invocation of the CLA check for more information. For the most up to date status, view the checks section at the bottom of the pull request. |
|
Please wait a few days until our company's agreement about CLA is approved. |
|
Our company's agreement about CLA has been approved, so please review this PR. |
|
Hi @mamoruoka, will get to this next week, a bit busy with finals this week. |
|
I found mistakes in the models and have corrected it. |
|
@kevinzakka |
kevinzakka
left a comment
kevinzakka
left a comment
There was a problem hiding this comment.
Hi @mamoruoka, thanks so much for your contribution and I apologize for the huge delay. I left some review comments. One they are addressed, can you add an entry in the main README linking to this model?
|
|
||
| Requires MuJoCo 3.1.0 or later. | ||
|
|
||
| ## Overview |
There was a problem hiding this comment.
Can you add the steps you took to derive the XML? Was this XML generated from a CAD model or from a URDF? In general, we strive to have a detailed reproduction paragraph, e.g. see here
There was a problem hiding this comment.
@kevinzakka
This XML is generated from URDF model, but the URDF model is not open-sourced yet.
What should I do in this case?
torobo/torobo.xml
Outdated
|
|
||
| <default> | ||
| <default class="torobo"> | ||
| <joint armature="0.1" stiffness="0" damping="0"/> |
There was a problem hiding this comment.
Can you remove attributes that are defaults already in MuJoCo? e.g. stiffness and damping are 0 so feel free to remove.
| </default> | ||
| </default> | ||
|
|
||
| <asset> |
There was a problem hiding this comment.
- Could you use more descriptive names for the materials instead of numeric values?
- Please round the numeric values to at most 2 or 3 decimal places.
- Feel free to remove the
specularandshininessattributes, I've found that they don't have a significant effect in our scenes.
torobo/torobo.xml
Outdated
| </asset> | ||
|
|
||
| <worldbody> | ||
| <body name="base_link_trans_x" pos="0 0 0"> |
| <joint joint1="left_hand/third_finger_base_joint" joint2="left_hand/second_finger_base_joint" polycoef="0 1.0 0 0 0"/> | ||
| </equality> | ||
|
|
||
| <contact> |
There was a problem hiding this comment.
This exclude list is quite bulky. What's the rationale? Do all these collisions occur upon loading the model?
There was a problem hiding this comment.
@kevinzakka
These excludes are copied from ones generated by moveit_setup_assistant.
They include non-collision links and adjacent links, which are unneeded for mujoco contact exclusions.
So, I fixed to remove these unneeded contact exclusions.
| @@ -0,0 +1,16 @@ | |||
| # Torobo Hand Description (MJCF) | |||
There was a problem hiding this comment.
Same comments apply for the hand.
|
Thank you for the review, @kevinzakka .
|
2 participants
This PR adds the Torobo and Torobo Hand model (issue #113).
Please feel free to suggest any suggestions to improve the quality of this PR.