Gepetto / example-robot-data

Set of robot URDFs for benchmarking and developed examples.
BSD 3-Clause "New" or "Revised" License
143 stars 51 forks source link

UR robot doesn't work with latest release #17

Closed cmastalli closed 4 years ago

cmastalli commented 4 years ago

This is the code

import example_robot_data
model = example_robot_data.loadUR()

and it appears

/usr/local/lib/python2.7/dist-packages/pinocchio/shortcuts.py in buildModelsFromUrdf(filename, package_dirs, root_joint, verbose, meshLoader, geometry_types)
     43     for geometry_type in geometry_types:
     44         if meshLoader is None or (not WITH_HPP_FCL and not WITH_HPP_FCL_BINDINGS):
---> 45             geom_model = pin.buildGeomFromUrdf(model, filename, geometry_type, package_dirs)
     46         else:
     47             geom_model = pin.buildGeomFromUrdf(model, filename, geometry_type, package_dirs, meshLoader)

ValueError: Mesh package://example-robot-data/robots/ur_description/meshes/ur5/collision/base.stl could not be found.

I don't know what it is the source of the problem, maybe pinocchio? This issue was reported by @mfocchi

nim65s commented 4 years ago

those package:// stuff are usually loaded by resource-retriever, by looking at ROS_PACKAGE_PATH, or CMAKE_PREFIX_PATH.

I think this loadUR() method is ran in unit tests, and unit tests are passing, so I guess this is just about the configuration on your side.

But yes, we should probably advertise that we have a dependency on something able to understand thesepackage:// url, or that some configuration is needed.

cmastalli commented 4 years ago

This was not necessary before, indeed it is not required for the other robots.

It should fix the issue by returning back the old way of including the meshes file, i.e.

        <mesh filename="package://ur_description/meshes/ur5/visual/base.dae"/>

There is a reason to this change, could we return it back?


I really don't like to depend on ROS setup :(

cmastalli commented 4 years ago

This was not necessary before, indeed it is not required for the other robots.

It should fix the issue by returning back the old way of including the meshes file, i.e.

        <mesh filename="package://ur_description/meshes/ur5/visual/base.dae"/>

There is a reason to this change, could we return it back?


I really don't like to depend on ROS setup :(

wxmerkt commented 4 years ago

Pinocchio recently made changes to the URDF loaders. Perhaps it's related? It appears unlikely the issue is in example-robot-data as there haven't been changes to the URDFs themselves?!

cmastalli commented 4 years ago

@wxmerkt this could happen. @jcarpent do you know if this issue is related with Pinocchio changes?

florent-lamiraux commented 4 years ago

At first glance, the test line 44 seems to indicate that pinocchio should not look for meshes.

For each path in ROS_PACKAGE_PATH environment variable, do:

  1. check that there is a package.xml file in path,
  2. check that path + '/example-robot-data/robots/ur_description/meshes/ur5/collision/base.stl' exist. If 1 and 2 are true for at least one path, it is probably a problem in pinocchio.
nim65s commented 4 years ago

@cmastalli : Looking at the history of this file: https://github.com/Gepetto/example-robot-data/commits/master/robots/ur_description/urdf/ur5_robot.urdf nothing ever changed about that, apart from 2545adcf0bbecd43ed40d6be03820cb53f3a16d7 from @florent-lamiraux

You can try to revert this commit and check if it fix the issue for you.

mfocchi commented 4 years ago

Hi guys, I have ubuntu 16, robotpkg-py27-pinocchio 2.3.1, robotpkg-py27-qt4-gepetto-viewer-corba 5.3.2, and reverting example_robot_data back to the commit previous of 2545adc it loads the meshes, but the visualization is screwed up and spits a warning: "Warning: your locale convention uses ',' as decimal separator while DAE expects '.'." I am sure you have seen this before

cmastalli commented 4 years ago

@florent-lamiraux thanks for your feedback. I reverted the commit that @nim65s suggested us, and it fixed my problem as well as @mfocchi. I will create soon a PR!

@mfocchi I think this is related with an issue in 16.04, and @jmirabel or @florent-lamiraux might know the solution (probably installing by source).

In Ubuntu 18.04 works well. See the screenshot of my desktop

Screenshot from 2020-04-03 10-05-33

wxmerkt commented 4 years ago

We should not revert https://github.com/Gepetto/example-robot-data/commit/2545adcf0bbecd43ed40d6be03820cb53f3a16d7 as it fixed a previous issue with URDFs in this package.

Is this really a different behaviour between 16.04 and 18.04 using the latest release? [Based on the console output from Carlos, it appears to be?]

cmastalli commented 4 years ago

We should not revert 2545adc as it fixed a previous issue with URDFs in this package.

Is this really a different behaviour between 16.04 and 18.04 using the latest release? [Based on the console output from Carlos, it appears to be?]

@wxmerkt that commit changed the way we include mesh files in the UR robot, and I couldn't find the reason of this change. Furthermore, if you check other robot URDFs, you will observe that we include differently the meshes (different to the proposition of 2545adc). @florent-lamiraux do you remember what was the reason of this modification?

On the other hand, my screenshot shows the robot display after reverting 2545adc in Ubuntu 18.04. @mfocchi reported another problem in Ubuntu 16.04. If I remember correctly it's related to gepetto-viewer-corba binaries for 16.04 (i.e. another issue).

jmirabel commented 4 years ago

Is example-robot-data a subfolder of the folders in ROS_PACKAGE_PATH ?

If you don't want this, another fix is to modify the example_robot_data.loadUR so that it adds a path to example-robot-data to the list of searched paths. This is very easy to do and will not hurt anyone.

cmastalli commented 4 years ago

Is example-robot-data a subfolder of the folders in ROS_PACKAGE_PATH ?

IHMO it shouldn't be.

If you don't want this, another fix is to modify the example_robot_data.loadUR so that it adds a path to example-robot-data to the list of searched paths. This is very easy to do and will not hurt anyone.

I agree with this proposition.

wxmerkt commented 4 years ago

I think to remember that the conversation was to fix the other URDFs following the UR example as well.

If you don't want this, another fix is to modify the example_robot_data.loadUR so that it adds a path to example-robot-data to the list of searched paths. This is very easy to do and will not hurt anyone.

This sounds like a good proposition.

Is example-robot-data a subfolder of the folders in ROS_PACKAGE_PATH ?

example-robot-data can be built and found as a main package on the ROS_PACKAGE_PATH. As thus, the root of the package example-robot-data is the root of the repository, not example-robot-data/robots/ROBOT_NAME.

wxmerkt commented 4 years ago

I am trying to reproduce this locally and cannot (latest devel on Pinocchio, with and without HPP-FCL, latest devel on example-robot-data, Ubuntu 18.04 - all locally compiled). I noticed in the error message above that your Pinocchio has no meshLoader or not compiled with hpp-fcl, is that correct?

@mfocchi Could you please provide the versions for Pinocchio (and with/without HPP-FCL)?

florent-lamiraux commented 4 years ago

@florent-lamiraux do you remember what was the reason of this modification?

The objective was to remove dependency to ur_description. It was agreed that this change should be generalized to the other robots.

mfocchi commented 4 years ago

@wxmerkt I am using pinocchio installed from debian package robotpkg-py27-pinocchio, version 2.3.1, robotpkg-py27-qt4-gepetto-viewer-corba 5.3.2, I do not have HPP-FCL

in gepetto viewer I see somthing like this:

image

florent-lamiraux commented 4 years ago

@cmastalli : using ROS_PACKAGE_PATH does not constitute a dependency to ROS. It is a way to tell URDF parser where to find files starting with "package://". We used this variable name to follow ROS convention, but we could have used "URDF_PACKAGE_PATH" in pinocchio parser. In the same way you use PYTHONPATH to tell python where to find a module when using command import, or LD_LIBRARY_PATH to tell the dynamic loader where to find libraries and so on.

cmastalli commented 4 years ago

@cmastalli : using ROS_PACKAGE_PATH does not constitute a dependency to ROS. It is a way to tell URDF parser where to find files starting with "package://". We used this variable name to follow ROS convention, but we could have used "URDF_PACKAGE_PATH" in pinocchio parser. In the same way you use PYTHONPATH to tell python where to find a module when using command import, or LD_LIBRARY_PATH to tell the dynamic loader where to find libraries and so on.

Yes, I agree. I guess the main issue was to do not continue in other robots and to do not document this action in Readme file.

jcarpent commented 4 years ago

This seems to be solved.