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

Install the URDF files in a system directory and search #74

Merged
merged 72 commits into from
Aug 8, 2020

Conversation

jbmouret
Copy link
Collaborator

@jbmouret jbmouret commented Jun 2, 2020

Currently only the search (WIP). See #52

@jbmouret
Copy link
Collaborator Author

jbmouret commented Jun 2, 2020

Question: do we need to use the same path prefix for the packages? how does that work?

@jbmouret jbmouret added this to the RobotDART 1.0.0 milestone Jun 3, 2020
@jbmouret jbmouret linked an issue Jun 3, 2020 that may be closed by this pull request
@jbmouret
Copy link
Collaborator Author

jbmouret commented Jun 3, 2020

I broke the test_robot!

 /Users/jmouret/git/resibots/robot_dart/build/src/tests/test_robot

[...]
RobotDART:: using: /Users/jmouret/git/resibots/robot_dart/res/robots/pendulum.urdf
../src/tests/test_robot.cpp:58: error: in "test_fix_free": check pendulum->fixed() has failed
../src/tests/test_robot.cpp:60: error: in "test_fix_free": check !pendulum->free() has failed
unknown location:0: fatal error: in "test_fix_free": signal: SIGSEGV, si_code: 0 (memory access violation at address: 0x00000000)
../src/tests/test_robot.cpp:60: last checkpoint

*** 3 failures are detected in the test module "test_robot"

=> apparently the pointer has a problem, but I do not see where it comes from (yet). I guess the file is not properly loaded?

@jbmouret
Copy link
Collaborator Author

jbmouret commented Jun 3, 2020

It seems that the pendulum is not fixed by default. It works better if I first fix it with pendulum->fix_to_world(); but it then segv when we call pendulum->free_from_world();. Sounds like a bug somewhere but this is unexpected.

`

@jbmouret
Copy link
Collaborator Author

jbmouret commented Jun 3, 2020

(the master fails too?)
Fixed in #76

@jbmouret
Copy link
Collaborator Author

jbmouret commented Jun 4, 2020

@costashatz What do we do for complex URDF (Talos, iCub, etc.)?

  • option 1: they have they urdf in their own repo
  • option 2: we store in this repo a good urdf for simulation
  • option 3: we use git submodule to insert the repo of the urdf

It might be interesting to use a repo for these complex robots because the urdf are also used in the control libraries (tsid, opensot, etc.). However:

  • having multiple repos is not very user-friendly (makes things a bit more complicated)
  • I suggest to have a Talos class, but this would mean that this class can only work if we have cloned the right urdf repo (=> the code would not work out of the box)
  • We might not use the same urdf for control and for simulation anyway
  • I don't like having many repos without some directory structure (which is not given by github...) to find things that work together

I find it interesting that you had to use a special URDF from talos (not exactly the one we used so far): this suggests that having a special URDF for simulation is needed, and suggests that we should have our own urdf library. Of course, the modified URDF might work for control too (and Gazebo). But is it the rule? the same urdf for dart, gazebo, tsid, openSOT? is it easy?

I guess your are leaning towards option 3 (using a submodule)? I would prefer option 1 but no option is perfect.

What do you think?

@costashatz
Copy link
Member

  • option 1: they have they urdf in their own repo
  • option 2: we store in this repo a good urdf for simulation
  • option 3: we use git submodule to insert the repo of the urdf

I would be okay for option 1 and option 2. Option 2 is a bit better for the users of robot_dart. Option 1 is a bit more generic.

  • I suggest to have a Talos class

If we do this (one class per robot supported in robot_dart), then we need to have the URDFs in the repo. Otherwise each time that we update the submodule or the separate repo, we would have to check if everything is okay in robot_dart and a lot of regression errors will appear.

I guess your are leaning towards option 3 (using a submodule)? I would prefer option 1 but no option is perfect.

I do not like option 3. I usually do not like submodules except if they are really needed.

I find it interesting that you had to use a special URDF from talos (not exactly the one we used so far): this suggests that having a special URDF for simulation is needed, and suggests that we should have our own urdf library. Of course, the modified URDF might work for control too (and Gazebo). But is it the rule? the same urdf for dart, gazebo, tsid, openSOT? is it easy?

I did not have to use a special URDF. There are two main issues with all URDFs and all the different loaders (DART loader, Gazebo loader, RaiSim loader):

  • The underlying assumptions of the URDF loaders. For example, the DART loader assumes that if one link does not have a mass defined in the xml, then it is equal to 1kg. I know that Gazebo assumes a very small mass.
  • The URDFs are not well-defined for physics simulations. It's wrong to create a link that has no inertia parameters, yet all "official" URDFs have these links.

If you combine the above, something unexpected and different happens in every combination of URDF and loader. What I usually do, I create a correct clean URDF where I define everything explicitly (the dummy links with almost zero mass, etc.). The "official" URDFs should do this by default, but they do not.

What do you think?

So overall I would say to go either with option 1 and have correct clean URDFs for each robot that we want to use or option 3 and then someone can either copy them or install them with robot_dart and use them from "public" paths.

@jbmouret
Copy link
Collaborator Author

jbmouret commented Jun 4, 2020

There is also option 4:
a urdf library repo that contains good urdfs for all the robots (all the urdf at the same place, but outside of robot_dart). But, as we said, either we use this as a submodule, or we cannot really create class for robots (as they would be meaningless without a particular urdf).

So... option 1: we simply store all the urdf in robot_dart?

@Aneoshun
Copy link
Contributor

Aneoshun commented Jun 4, 2020

With option 1, how do you plan to deal with situations such as we have different hexapod URDFs, because my new hexapod is slightly different from the Inria hexapod.
Obviously, I want to use the URDF of my robot, not the Inria one, but storing all the URDFs of all the variants of the robots is not realistic.

@jbmouret
Copy link
Collaborator Author

jbmouret commented Jun 4, 2020

You would still be able to use your own urdf (like, the Hexapod class has an optional URDF filename).

But because we have only a few users for now, it might be better to simply have your urdf in the library too.

The idea is that it should be easy to say "I want to simulate a 6-legged robot" and start right away (without digging for the right, validated urdf).

I think we might have several versions of Talos and there are many versions of iCub. Maybe we could sort them properly by version if needed.

At any rate, I think we need a curated library of URDF that are known to work with this specific simulator.

@jbmouret
Copy link
Collaborator Author

jbmouret commented Jun 4, 2020

@costashatz I added a Talos example. Questions:

  • It seems that the packages require absolute paths? how does that work?
  • I've put a URDF and meshes for Talos that I had, but they might not be your favorite version (please check or replace by yours)
  • Should we keep the xacro here? I guess they are useful if we need to regenerate some things? (I did not do this for a long long time...).

@jbmouret
Copy link
Collaborator Author

jbmouret commented Jun 4, 2020

@Aneoshun if you have hexapod urdf, can you send it to me?

@costashatz
Copy link
Member

  • It seems that the packages require absolute paths? how does that work?

If I remember well yes, they require absolute paths.

  • I've put a URDF and meshes for Talos that I had, but they might not be your favorite version (please check or replace by yours)

I will..

  • Should we keep the xacro here? I guess they are useful if we need to regenerate some things? (I did not do this for a long long time...).

I would say in principle no, but it might be a good idea to keep the xacros used to generate the URDF. This can be good for debugging or small improvements; easier also for other people to improve the files.

@jbmouret
Copy link
Collaborator Author

jbmouret commented Jun 4, 2020

Another question: what is the 'is_urdf_string' boolean?

@costashatz
Copy link
Member

@jbmouret I had a deeper look, and :

  • I cleaned the dae files of Talos
  • There are a few stl ones remaining for Talos
  • We need to clean the iCub files also

I'll do this as soon as possible.. After this, I think that we can merge..

@costashatz
Copy link
Member

@jbmouret can you make the visual meshes of iCub (dae fiels) to glb?

@jbmouret
Copy link
Collaborator Author

I will have a look. This sounds possible.

@jbmouret jbmouret closed this Jul 22, 2020
@jbmouret jbmouret reopened this Jul 22, 2020
@jbmouret
Copy link
Collaborator Author

I had to rotate the visual of the feet (I don't know why). Also, I am not fond of the color, but this seems to be embedded in the glb (not in the urdf).

@jbmouret
Copy link
Collaborator Author

  • There are a few stl ones remaining for Talos

Why can't we remove them? do we use them?

@costashatz
Copy link
Member

Why can't we remove them? do we use them?

I will remove them. I just have to do it one by one (because there are other stl that we use)...

@costashatz
Copy link
Member

I had to rotate the visual of the feet (I don't know why).

I guess we have to rotate the waist also.. I'll try to do it..

I am not fond of the color

You mean of the feet or other color? The rest seems the same to me..

@costashatz
Copy link
Member

Also, I am not fond of the color, but this seems to be embedded in the glb (not in the urdf).

The issue is the normals, not the color. They are different from the dae files. Can you tell me what you did? And I can try to do it myself?

@costashatz
Copy link
Member

@jbmouret now that I check the Talos meshes also, the glb meshes look a bit weird to me. Like much less triangles than the dae ones. Did you do any transformation of the meshes?

@jbmouret
Copy link
Collaborator Author

I did not remove any triangle but I smoothed the normals. Smoothing is done using a heuristics (< 12 degrees), so sometimes we lose details (the triangles are still there, but they are too smooth). I simply did:

 for obj in bpy.data.objects:
                if obj.type == 'MESH':
                    obj.data.use_auto_smooth = 1
                    obj.data.auto_smooth_angle = math.radians(5) # 12 degrees

For iCub, I do not see any difference with the DAE version except for the feet (which are smoother now) and the waist (as you noted). I do not like the green/blue color, but we will see this kind of detail later :)

@costashatz
Copy link
Member

For iCub, I do not see any difference with the DAE version except for the feet

For me the shading is different (I am assuming because of the normals), but seems a bit weird.

For Talos, the meshes seem a bit too smoothed.

Smoothing is done using a heuristics (< 12 degrees)

Can you try with smaller degrees?

@costashatz
Copy link
Member

@jbmouret I think that this is ready to be merged. Can you have a last look?

@costashatz
Copy link
Member

Merging...

@costashatz costashatz merged commit a2cf0c1 into master Aug 8, 2020
@costashatz costashatz deleted the urdf_library branch August 8, 2020 09:18
costashatz added a commit that referenced this pull request Dec 8, 2021
Install the URDF files in a system directory and search
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Install the robot URDF in some 'path'?
4 participants