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

Add documentation for SDFormat/Gazebo extension to URDF #88

Merged
merged 5 commits into from
Mar 23, 2023

Conversation

azeey
Copy link
Collaborator

@azeey azeey commented Mar 11, 2023

There is good documentation for the <gazebo> tag at https://classic.gazebosim.org/tutorials?tut=ros_urdf&cat=connect_ros, but it is presented in a tutorial format and is missing some concepts and explanations. This is an attempt to document the current behavior and provide an explanation about fixed joint lumping.

Preview

@azeey azeey requested a review from aaronchongth March 11, 2023 04:25
@azeey azeey self-assigned this Mar 11, 2023
Copy link
Collaborator

@aaronchongth aaronchongth left a comment

Choose a reason for hiding this comment

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

Thanks for the write-up! I've found some typos here and there, but overall it looks very informative.

urdf/sdf_extensions.md Outdated Show resolved Hide resolved
urdf/sdf_extensions.md Outdated Show resolved Hide resolved
urdf/sdf_extensions.md Outdated Show resolved Hide resolved
urdf/sdf_extensions.md Outdated Show resolved Hide resolved
Copy link
Member

@scpeters scpeters left a comment

Choose a reason for hiding this comment

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

it could be nice to add examples for inserting lights and sensors to a link, and a force-torque sensor to a joint using the gazebo tags

<static>true</static>
<plugin name='testPlugin' filename='testFileName'/>
</gazebo>
</robot>
Copy link
Member

Choose a reason for hiding this comment

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

how do you feel about using <include/> elements in our Markdown files instead of duplicating the example code?

for example:

I believe sdformat.org supports the same syntax, though I guess the links may have to point to your fork until the pull request is merged

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Thanks for the pointer. I was hoping there was a way to do that, but I didn't see any examples in sdf_tutorials. I've updated all the examples. 3beee03

<td><code>gravity</code></td>
</tr>
<tr>
<td>dampingFactor</td>
Copy link
Member

Choose a reason for hiding this comment

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

I'd like to see some padding between the columns of this table

Screen Shot 2023-03-13 at 4 21 27 PM

Copy link
Member

Choose a reason for hiding this comment

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

it could also be helpful to draw lines between the rows and columns

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

<tr>
<td>turnGravityOff</td>
<td>bool</td>
<td>A value of "true" turns gravity off. Alternatively, <code>gravity</code> (with opposite boolean value) can be used via blob insertion</td>
Copy link
Member

Choose a reason for hiding this comment

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

I think this is the only place blob insertion is mentioned in the document, but it is not defined, so we should explain what it means

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

I added a mention of blob insertion right before the table in 3beee03

<tr>
<td>maxVel</td>
<td>double</td>
<td>maximum contact correction velocity truncation term.</td>
Copy link
Member

Choose a reason for hiding this comment

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

consider linking to the following gazebo-classic tutorial for the relevant parameters:

https://classic.gazebosim.org/tutorials?tut=physics_params&cat=physics#Contactparameters

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Good idea! Done in 3beee03

<mass value='0.12' />
<inertia ixx='0.01' ixy='0' ixz='0' iyy='0.01' iyz='0' izz='0.01' />
</inertial>
<collision>
Copy link
Member

Choose a reason for hiding this comment

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

add another collision to show that all collisions get the parameter?

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Done in 3beee03

@azeey
Copy link
Collaborator Author

azeey commented Mar 16, 2023

I've addressed all the feedback. I've also added more changes regarding preserveFixedJoint since my previous understanding of the parameter was incorrect.

Copy link
Collaborator

@aaronchongth aaronchongth left a comment

Choose a reason for hiding this comment

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

Thanks for going into more detail about the various behaviors of fixed joint lumping. LGTM!

@azeey
Copy link
Collaborator Author

azeey commented Mar 21, 2023

@scpeters The web app has been deployed and the preview is showing the improved table formatting. Mind giving this another look?

Copy link
Member

@scpeters scpeters left a comment

Choose a reason for hiding this comment

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

I don't see a use of the material_visual_example.urdf


**Example:**

<include src='https://github.com/azeey/sdf_tutorials/raw/urdf_sdf_extension/urdf/examples/link_ref_example.urdf' />
Copy link
Member

Choose a reason for hiding this comment

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

Suggested change
<include src='https://github.com/azeey/sdf_tutorials/raw/urdf_sdf_extension/urdf/examples/link_ref_example.urdf' />
<include src='https://github.com/azeey/sdf_tutorials/raw/urdf_sdf_extension/urdf/examples/material_example.urdf' />

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

<origin xyz='0 0 1' rpy='0 0 0'/>
</joint>
<link name='end_effector'>
<inertial>
Copy link
Member

Choose a reason for hiding this comment

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

<inertia /> block is missing, which deviates from prior example

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Good catch. Changed in 8d1ad17. The resulting SDFormat now also has the end_effector_visual. Not sure why it was missing before.

@azeey
Copy link
Collaborator Author

azeey commented Mar 22, 2023

I don't see a use of the material_visual_example.urdf

Thanks for catching this. Added in 1fa5044

@azeey azeey merged commit 8e6c780 into gazebosim:master Mar 23, 2023
@azeey azeey deleted the urdf_sdf_extension branch March 23, 2023 14:36
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
Archived in project
Development

Successfully merging this pull request may close these issues.

3 participants