Skip to content

Conversation

vidurvij-Unity
Copy link
Contributor

@vidurvij-Unity vidurvij-Unity commented May 13, 2021

Proposed change(s)

Documentation update on primitive sizing

The original issue described the inability of the user to export the size of the primitive he created to the URDF file. He was trying to export the size of the box he created to a URDF file using the export feature but the URDF file always exported the size [1 1 1]. This is an expected behavior because URDF importer does not read or write size of the primitives in the BoxCollider, CapsuleCollider or the Spherical Collider. Instead it change the size of the visual and collider meshes using the scale of the parent gameObject of the mesh.

This is done because:

  1. To have consistency between the way the size of the mesh is changed between primitive collider meshes and other meshes.
  2. This API also works by changing the scale of the mesh under the hood.

Adding a list of supported URDF tags

Adding a list of supported URDF tags which are read from a file and attributed to an Articulation Body property in Unity.

Useful links (GitHub issues, JIRA tickets, forum threads, etc.)

https://jira.unity3d.com/browse/AIRO-623
https://jira.unity3d.com/browse/AIRO-642
Unity-Technologies/URDF-Importer#79

Types of change(s)

  • Bug fix
  • New feature
  • Code refactor
  • Documentation update
  • Other (please describe)

Testing and Verification

Test the bug by importing and exporting R2D2 URDF file which is made up box primitives.

Test Configuration:

  • Unity Version: [e.g. Unity 2020.2.0f1]
  • Unity machine OS + version: [e.g. Windows 10]
  • ROS machine OS + version: [e.g. Ubuntu 18.04, ROS Noetic]
  • ROS–Unity communication: [e.g. Docker]

Checklist

  • Ensured this PR is up-to-date with the dev branch
  • Created this PR to target the dev branch
  • Followed the style guidelines as described in the Contribution Guidelines
  • Added tests that prove my fix is effective or that my feature works
  • Updated the documentation as appropriate

Other comments

<collision>
```

The size of the primitive will be set in Unity by changing the scale of gameObject containing UrdfCollision script.
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This is passive voice, which is a bit confusing. Who will do the setting? Is it the URDF Importer? If so, here's a suggestion: "The URDF Importer will set the size of the primitive using the "scale" parameter of the GameObject that contains the UrdfCollsion script." [if the meaning is still correct]

| ![Collision gameObject in hierarchy](images/link_hierarchy.png) | ![Size of primitive set using scale](images/primitive_scale.png) |
|:---:|:---:|

This is done, as opposed to using the size API of the primitive collider, to have consistency across different mesh types. The sizing API is only available for primitive mesh colliders and not for complex collider meshes and visual meshes. Thats why we use the parent's scaling to change the shape of the primitive mesh. This scale is exported as the primitive size during URDF export.
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggestion to improve clarity:
"The reason we set the size via the "scale" of the parent GameObject as opposed to the "size" of the primitive collider is to have consistency across different mesh types; the size API is only available for primitive mesh colliders and not for complex collider meshes and visual meshes, whose size must be changed using their parent GameObject's "scale" parameter."

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I don't understand what the last sentence means - can you clarify?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Updated the language

@vidurvij-Unity vidurvij-Unity changed the title Adding primitive sizing documentation URDF documentation Update May 25, 2021
Copy link
Contributor

@sarah-gibson sarah-gibson left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Looks good, but since this is getting so long, it would be nice to put a TOC on top!

| ![Collision gameObject in hierarchy](images/link_hierarchy.png) | ![Size of primitive set using scale](images/primitive_scale.png) |
|:---:|:---:|

The reason we set the size via the "scale" of the parent GameObject as opposed to the "size" of the primitive collider is to have consistency across different mesh types; the size API is only available for primitive mesh colliders and not for complex collider meshes and visual meshes, whose size must be changed using their parent GameObject's "scale" parameter. The "scale" parameter of the gameObject is also used set the values of primitive "size" in the URDF when performing an URDF export.
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

  • ... is also used [to] set the values of [the] primitive['s] ...
  • ... performing [a] URDF export.

Copy link
Contributor

@hyounesy hyounesy left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

  • The example xml needs to be corrected.
  • Also some minor typos.

Comment on lines 388 to 393
<collision>
<geometry>
<cylinder size=".4 .3 .4" />
</geometry>
</collision>
<collision>
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

remove the extra tag at the bottom and reformat:

<collision>
  <geometry>
    <cylinder size=".4 .3 .4" />
  </geometry>
</collision>


## Disable Collision Support
URDF defines the robot visually using Visual Meshes, and its collision using Collision Meshes. Collision meshes define the physical volume of the links, and are used to calculate the inertia of the links and also to detect collisions between different physical objects. In Unity, rigidbodies cannot have concave collision meshes, so when importing a concave collision mesh, all concave regions are closed over to produce a convex outline. As a result, the convex shapes might intersect with each other, creating a hindrance in robot movement. To remedy this, we support a ```disable collision``` tag in URDF. To add an exception for collision detection in Unity:
URDF defines the robot visually using Visual Meshes, and its collision using Collision Meshes. Collision meshes define the physical volume of the links, and are used to calculate the inertia of the links and also to detect collisions between different physical objects. In Unity, RigidBodies cannot have concave collision meshes, so when importing a concave collision mesh, all concave regions are closed over to produce a convex outline. As a result, the convex shapes might intersect with each other, creating a hindrance in robot movement. To remedy this, we support a ```disable collision``` tag in URDF. To add an exception for collision detection in Unity:
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

nit: Rigidbody
Perhaps you can also link it to the documentation. e.g.:
In Unity, Rigidbody components cannot ...

@vidurvij-Unity vidurvij-Unity requested a review from hyounesy July 6, 2021 17:47

## Disable Collision Support
URDF defines the robot visually using Visual Meshes, and its collision using Collision Meshes. Collision meshes define the physical volume of the links, and are used to calculate the inertia of the links and also to detect collisions between different physical objects. In Unity, rigidbodies cannot have concave collision meshes, so when importing a concave collision mesh, all concave regions are closed over to produce a convex outline. As a result, the convex shapes might intersect with each other, creating a hindrance in robot movement. To remedy this, we support a ```disable collision``` tag in URDF. To add an exception for collision detection in Unity:
URDF defines the robot visually using Visual Meshes, and its collision using Collision Meshes. Collision meshes define the physical volume of the links, and are used to calculate the inertia of the links and also to detect collisions between different physical objects. In Unity, [RigidBodies](https://docs.unity3d.com/ScriptReference/Rigidbody.html) cannot have concave collision meshes, so when importing a concave collision mesh, all concave regions are closed over to produce a convex outline. As a result, the convex shapes might intersect with each other, creating a hindrance in robot movement. To remedy this, we support a ```disable collision``` tag in URDF. To add an exception for collision detection in Unity:
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

consistency nit: RigidB[b]odies, or better [Rigidbody components]

@vidurvij-Unity vidurvij-Unity merged commit 7253d64 into dev Jul 6, 2021
peifeng-unity pushed a commit that referenced this pull request Jul 16, 2021
* Adding primitive sizing documentation
* Add list of supported URDF tags
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