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

Automatic conversion tool + urdf library w/ meshes #74

Open
wants to merge 34 commits into
base: master
Choose a base branch
from

Conversation

Simon-Steinmann
Copy link
Contributor

Adds a automatic batch-conversion tool. How it works:

in the root directory, run:
python batch_conversion.py

This will take any URDF file in the automatic-conversion/urdf folder and convert it into a proto file and save it in a unique folder in the automatic-conversion/protos directory. It will have the same nested folder structure as in the automatic-conversion/urdf folder. The options for urdf conversion are defined in a .json file, which gets automatically created for any new urdf file added.

Once done converting, it will delete everything in the automatic-conversion/ExtraProjectTest folder, and add the newly converted files there. This directory can be added to Webots' Extra Project Path. This way, conversions can be easily tested, without manually copying them somewhere. Simply requires Webots restart.

@Simon-Steinmann
Copy link
Contributor Author

-added parsing options. Check with:
python batch_conversion.py --help

  • added a TestWorld generation. It takes every converted model and puts it into a world, spaced out in a grid. Very quick and easy way to see if models load into webots without errors

@DavidMansolino
Copy link
Member

That looks very nice and convenient, thank you for sharing this!

Note that the 'batch_conversion.py' file does not respect PEP8 (which is causing the automatic tests to fail): https://travis-ci.com/github/cyberbotics/urdf2webots/jobs/376314699#L303

@Simon-Steinmann
Copy link
Contributor Author

got it, didn't know that was a rule. Will keep in mind.

@Simon-Steinmann
Copy link
Contributor Author

Test the tool by following these steps:

  1. checkout to this branch an pull
  2. run python batch_conversion.py
  3. Once completed, open Webots and change the Extra projects path to urdf2webots/automatic_conversion/ExtraProjectTest (of course the full path in your system)
  4. Open the testworld located at urdf2webots/automatic_conversion/ExtraProjectTest/TestAllRobots.wbt
    This will take a moment to load, as it loads every converted model into a dynamically created world.

Copy link
Member

@DavidMansolino DavidMansolino left a comment

Choose a reason for hiding this comment

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

We discussed with the @cyberbotics/maintainers team and we believe it would be cleaner to put the content of the automatic_conversion folder in a root source folder in the https://github.com/cyberbotics/community-projects repository, this way, the source and result remain close.
This will also keep this repository clean and light (which is important as this repository is used in other projects as a submodule).

@Simon-Steinmann
Copy link
Contributor Author

That is a good point. I agree. Did you test it though? have you feedback on the code and process itself?

@DavidMansolino
Copy link
Member

I didn't test it yet, but I had a look at the code and it looks like a very nice and useful tool!

@@ -155,18 +155,39 @@ def URDFLink(proto, link, level, parentList, childList, linkList, jointList, sen
proto.write((level + 1) * indent + 'physics Physics {\n')
proto.write((level + 2) * indent + 'density -1\n')
proto.write((level + 2) * indent + 'mass %lf\n' % link.inertia.mass)
if link.inertia.ixx > 0.0 and link.inertia.iyy > 0.0 and link.inertia.izz > 0.0:
proto.write((level + 2) * indent + 'centerOfMass [ %lf %lf %lf ]\n' % (link.inertia.position[0],
# if link.inertia.position != [0.0, 0.0, 0.0]:
Copy link
Contributor Author

Choose a reason for hiding this comment

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

This check causes issues. A link with mass can have no origin-translation for the inertia. We already check if the link has mass. That is enough.

Copy link
Member

Choose a reason for hiding this comment

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

I am not sure to understand your point, we are using later link.inertia.position, so if it doesn't exist it will cause issues too, no?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

we dont check whether it exists, we check whether it is not [0, 0, 0]
Many urdf files have at least one link, that has a [0, 0, 0] translation for its inertia. In this case, we don't write the center of mass and Webots complains. As soon as we have a defined mass, we have to have a center of mass. We are already checking if we have mass, so this must be included, without additional checks.

Copy link
Member

Choose a reason for hiding this comment

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

This adds useless center of mass new lines in the test (which is probably the cause of the failure):

centerOfMass [ 0.000000 0.000000 0.000000 ]

Copy link
Contributor Author

Choose a reason for hiding this comment

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

We have to think about this critically!

@@ -473,8 +503,8 @@ def URDFShape(proto, link, level, normal=False):
if name is None:
if visualNode.geometry.name is not None:
name = computeDefName(visualNode.geometry.name)
if visualNode.geometry.defName is None:
name = robotNameMain + '_' + name if robotNameMain else name
name = robotNameMain + '_' + name if robotNameMain else name
Copy link
Contributor Author

Choose a reason for hiding this comment

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

We have to define the name before the condition, otherwise the adjusted path is not used, causing errors

@Simon-Steinmann
Copy link
Contributor Author

The two comments I left are the only changes specific to this, the rest is updates from recent changes (pulled the newest master just before and added these 2 changes)

I also added parsable input and output paths to the batch_conversion.py. You should call it now like this:
python batch_conversion.py --input=/home/simon/community-projects/sources --output=/home/simon/community-projects/converted

I pushed the urdf sources I'm currently using. You can pull them here:
https://github.com/cyberbotics/community-projects/tree/Simon-Steinmann-sources

@DavidMansolino
Copy link
Member

Can you please update with master and resolve conflicts?

@Simon-Steinmann
Copy link
Contributor Author

Simon-Steinmann commented Aug 28, 2020

I updated with master, but I will fix the pep8 errors.

@Simon-Steinmann
Copy link
Contributor Author

The two comments I left are the only changes specific to this, the rest is updates from recent changes (pulled the newest master just before and added these 2 changes)

I also added parsable input and output paths to the batch_conversion.py. You should call it now like this:
python batch_conversion.py --input=/home/simon/community-projects/sources --output=/home/simon/community-projects/converted

I pushed the urdf sources I'm currently using. You can pull them here:
https://github.com/cyberbotics/community-projects/tree/Simon-Steinmann-sources

@DavidMansolino
Copy link
Member

I updated with master, but I will fix the pep8 errors.

But from the diff it looks like it is not the case, because it contains changes from other PR (e.g. these changes https://github.com/cyberbotics/urdf2webots/pull/74/files#diff-68c204ae057e777029c1e7dbd77d1f03R393-R402 are from #76).

@Simon-Steinmann
Copy link
Contributor Author

These changes are compared to what this branch started out with. What is the proper way to do this? Using the updated master as the base?

@DavidMansolino
Copy link
Member

Yes, you should update your local master branch, then merge it with this branch and finally push this branch, here is how I do this (but that's not the only way):

git checkout master
git pull
git checkout Simon-Steinmann-batch-conversion
git merge master
git push origin Simon-Steinmann-batch-conversion

batch_conversion.py Outdated Show resolved Hide resolved
batch_conversion.py Outdated Show resolved Hide resolved
batch_conversion.py Outdated Show resolved Hide resolved
batch_conversion.py Outdated Show resolved Hide resolved
batch_conversion.py Outdated Show resolved Hide resolved
batch_conversion.py Outdated Show resolved Hide resolved
batch_conversion.py Outdated Show resolved Hide resolved
batch_conversion.py Outdated Show resolved Hide resolved
batch_conversion.py Outdated Show resolved Hide resolved
batch_conversion.py Outdated Show resolved Hide resolved
Simon-Steinmann and others added 9 commits August 28, 2020 16:09
Co-authored-by: David Mansolino <[email protected]>
Co-authored-by: David Mansolino <[email protected]>
Co-authored-by: David Mansolino <[email protected]>
Co-authored-by: David Mansolino <[email protected]>
Co-authored-by: David Mansolino <[email protected]>
Co-authored-by: David Mansolino <[email protected]>
Co-authored-by: David Mansolino <[email protected]>
Co-authored-by: David Mansolino <[email protected]>
Co-authored-by: David Mansolino <[email protected]>
@Simon-Steinmann
Copy link
Contributor Author

Did the test work for you? With conversion and loading the TestWorld.wbt?

Copy link
Member

@DavidMansolino DavidMansolino left a comment

Choose a reason for hiding this comment

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

Ahh I had to
git submodule update --init --recursive
to run the test.

The files are exactly the same, except for the line (in each file:)
# Extracted from: /home/david/urdf2webots/tests/sources/motoman/motoman_sia20d_support/urdf/sia20d.urdf

When I run it, I have the following diff:

diff --git a/tests/expected/Human.proto b/tests/expected/Human.proto
index b0fa3da..0564c41 100644
--- a/tests/expected/Human.proto
+++ b/tests/expected/Human.proto
@@ -2,7 +2,7 @@
 # license: Apache License 2.0
 # license url: http://www.apache.org/licenses/LICENSE-2.0
 # This is a proto file for Webots for the Human
-# Extracted from: /home/travis/urdf2webots/tests/sources/gait2392_simbody/urdf/human.urdf
+# Extracted from: /home/david/urdf2webots/tests/sources/gait2392_simbody/urdf/human.urdf
 
 PROTO Human [
   field  SFVec3f     translation     0 0 0
@@ -276,6 +276,7 @@ PROTO Human [
                                   physics Physics {
                                     density -1
                                     mass 0.216600
+                                    centerOfMass [ 0.000000 0.000000 0.000000 ]
                                     inertiaMatrix [
                                       0.000100 0.000200 0.001000
                                       0.000000 0.000000 0.000000
@@ -298,6 +299,7 @@ PROTO Human [
                             physics Physics {
                               density -1
                               mass 1.250000
+                              centerOfMass [ 0.000000 0.000000 0.000000 ]
                               inertiaMatrix [
                                 0.001400 0.003900 0.004100
                                 0.000000 0.000000 0.000000
@@ -320,6 +322,7 @@ PROTO Human [
                       physics Physics {
                         density -1
                         mass 0.100000
+                        centerOfMass [ 0.000000 0.000000 0.000000 ]
                         inertiaMatrix [
                           0.001000 0.001000 0.001000
                           0.000000 0.000000 0.000000
@@ -342,6 +345,7 @@ PROTO Human [
                 physics Physics {
                   density -1
                   mass 3.707500
+                  centerOfMass [ 0.000000 0.000000 0.000000 ]
                   inertiaMatrix [
                     0.050400 0.005100 0.051100
                     0.000000 0.000000 0.000000
@@ -364,6 +368,7 @@ PROTO Human [
           physics Physics {
             density -1
             mass 9.301400
+            centerOfMass [ 0.000000 0.000000 0.000000 ]
             inertiaMatrix [
               0.133900 0.035100 0.141200
               0.000000 0.000000 0.000000
@@ -601,6 +606,7 @@ PROTO Human [
                                   physics Physics {
                                     density -1
                                     mass 0.216600
+                                    centerOfMass [ 0.000000 0.000000 0.000000 ]
                                     inertiaMatrix [
                                       0.000100 0.000200 0.001000
                                       0.000000 0.000000 0.000000
@@ -623,6 +629,7 @@ PROTO Human [
                             physics Physics {
                               density -1
                               mass 1.250000
+                              centerOfMass [ 0.000000 0.000000 0.000000 ]
                               inertiaMatrix [
                                 0.001400 0.003900 0.004100
                                 0.000000 0.000000 0.000000
@@ -645,6 +652,7 @@ PROTO Human [
                       physics Physics {
                         density -1
                         mass 0.100000
+                        centerOfMass [ 0.000000 0.000000 0.000000 ]
                         inertiaMatrix [
                           0.001000 0.001000 0.001000
                           0.000000 0.000000 0.000000
@@ -667,6 +675,7 @@ PROTO Human [
                 physics Physics {
                   density -1
                   mass 3.707500
+                  centerOfMass [ 0.000000 0.000000 0.000000 ]
                   inertiaMatrix [
                     0.050400 0.005100 0.051100
                     0.000000 0.000000 0.000000
@@ -689,6 +698,7 @@ PROTO Human [
           physics Physics {
             density -1
             mass 9.301400
+            centerOfMass [ 0.000000 0.000000 0.000000 ]
             inertiaMatrix [
               0.133900 0.035100 0.141200
               0.000000 0.000000 0.000000
@@ -926,6 +936,7 @@ PROTO Human [
                                   physics Physics {
                                     density -1
                                     mass 0.457500
+                                    centerOfMass [ 0.000000 0.000000 0.000000 ]
                                     inertiaMatrix [
                                       0.000892 0.000547 0.001340
                                       0.000000 0.000000 0.000000
@@ -948,6 +959,7 @@ PROTO Human [
                             physics Physics {
                               density -1
                               mass 0.607500
+                              centerOfMass [ 0.000000 0.000000 0.000000 ]
                               inertiaMatrix [
                                 0.002962 0.000618 0.003213
                                 0.000000 0.000000 0.000000
@@ -970,6 +982,7 @@ PROTO Human [
                       physics Physics {
                         density -1
                         mass 0.607500
+                        centerOfMass [ 0.000000 0.000000 0.000000 ]
                         inertiaMatrix [
                           0.002962 0.000618 0.003213
                           0.000000 0.000000 0.000000
@@ -992,6 +1005,7 @@ PROTO Human [
                 physics Physics {
                   density -1
                   mass 2.032500
+                  centerOfMass [ 0.000000 0.000000 0.000000 ]
                   inertiaMatrix [
                     0.011946 0.004121 0.013409
                     0.000000 0.000000 0.000000
@@ -1186,6 +1200,7 @@ PROTO Human [
                                   physics Physics {
                                     density -1
                                     mass 0.457500
+                                    centerOfMass [ 0.000000 0.000000 0.000000 ]
                                     inertiaMatrix [
                                       0.000892 0.000547 0.001340
                                       0.000000 0.000000 0.000000
@@ -1208,6 +1223,7 @@ PROTO Human [
                             physics Physics {
                               density -1
                               mass 0.607500
+                              centerOfMass [ 0.000000 0.000000 0.000000 ]
                               inertiaMatrix [
                                 0.002962 0.000618 0.003213
                                 0.000000 0.000000 0.000000
@@ -1230,6 +1246,7 @@ PROTO Human [
                       physics Physics {
                         density -1
                         mass 0.607500
+                        centerOfMass [ 0.000000 0.000000 0.000000 ]
                         inertiaMatrix [
                           0.002962 0.000618 0.003213
                           0.000000 0.000000 0.000000
@@ -1252,6 +1269,7 @@ PROTO Human [
                 physics Physics {
                   density -1
                   mass 2.032500
+                  centerOfMass [ 0.000000 0.000000 0.000000 ]
                   inertiaMatrix [
                     0.011946 0.004121 0.013409
                     0.000000 0.000000 0.000000
@@ -1274,6 +1292,7 @@ PROTO Human [
           physics Physics {
             density -1
             mass 26.826600
+            centerOfMass [ 0.000000 0.000000 0.000000 ]
             inertiaMatrix [
               1.474500 0.755500 1.431400
               0.000000 0.000000 0.000000
@@ -1296,6 +1315,7 @@ PROTO Human [
     physics Physics {
       density -1
       mass 11.777000
+      centerOfMass [ 0.000000 0.000000 0.000000 ]
       inertiaMatrix [
         0.102800 0.087100 0.057900
         0.000000 0.000000 0.000000

batch_conversion.py Outdated Show resolved Hide resolved
batch_conversion.py Outdated Show resolved Hide resolved
batch_conversion.py Show resolved Hide resolved
@Simon-Steinmann
Copy link
Contributor Author

Can you handle the test files? For me only the extracted from was different. But it has to match the setup of the automated test by travis right?

@DavidMansolino
Copy link
Member

Can you handle the test files? For me only the extracted from was different. But it has to match the setup of the automated test by travis right?

Just did: 0c9e3e0

batch_conversion.py Outdated Show resolved Hide resolved
batch_conversion.py Outdated Show resolved Hide resolved
Simon-Steinmann and others added 2 commits August 28, 2020 16:37
Co-authored-by: David Mansolino <[email protected]>
Co-authored-by: David Mansolino <[email protected]>
batch_conversion.py Outdated Show resolved Hide resolved
@Simon-Steinmann
Copy link
Contributor Author

Added the descriptions.

@DavidMansolino
Copy link
Member

DavidMansolino commented Aug 28, 2020

image

It seems the robots are added twice to the test world, do you have the same behavior?

@Simon-Steinmann
Copy link
Contributor Author

no, only once for me

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement New feature or request
Development

Successfully merging this pull request may close these issues.

2 participants