search

ros / urdfdom

URDF parser
http://ros.org/wiki/urdf
Other
96 stars 132 forks source link

assign correct MaterialPtr to all visuals #114

Closed rhaschke closed 5 years ago

rhaschke commented 6 years ago

So far, only the first visual in visual_array got the correct MaterialPtr (from the model) assigned. All other visual had a default-constructed Material with black color. This required a work around in rviz, to figure out the correct material colors (#1079 or https://github.com/ros-visualization/rviz/pull/1294/commits/ed3b0b3726fd10783b558c31371ebeb054b00375)

eisoku9618 commented 5 years ago

+1

Could you merge and release this?

clalancette commented 5 years ago

Sorry @clalancette, I should have explained the situation in more detail. The old parsing was less picky in the sense that it didn't recognized references to undefined materials (see inline github comments for details). As an example consider this example urdf, which defines materials Black and Orange, but also uses Grey. The old parser doesn't complain about the missing Grey definition - which can be easily validated with check_urdf. The new parser, however, complains about the issue. Because this more picky behaviour might break many existing urdf files (for example the Shadow Robot hand), I turned the error into a warning and don't bail out.

This makes sense to me. Thanks for the detailed explanation.

rhaschke commented 5 years ago

Ping @scpeters, @clalancette. Can we merge this bug fix and get that a new release anytime soon? I think this is also related to Chris' cleanup in #104. It's so sad that the latter PR is already dangling there for more than a year.

rhaschke commented 5 years ago

Huh. That was a quick response! Thanks. What is your release schedule for this non-ROS lib? Is there a chance to see a new release in Bionic already?

scpeters commented 5 years ago

I made a 1.0.3 release yesterday which should be able to get into the new debian without trouble, but it's hard to get releases into bionic that aren't security fixes. cc @j-rivero

rhaschke commented 5 years ago

should be able to get into the new debian

What do you mean by this exactly? A .deb package or the Debian distribution? How would the newly released lib enter my ROS ecosystem? Is it released via the ROS release / distribution mechanism, i.e. bloom too?