support multiple material for one link

[k-okada]

This is patches for setting multiple materials for one link. I understand this is not clean patch, but anyway send PR and hoping someone will find more better way.

c.f: http://answers.ros.org/question/154816/color-issue-with-urdf-and-multiple-visual-tags/

you need following patch to urdf_parser.

diff --git a/urdf_parser/src/link.cpp b/urdf_parser/src/link.cpp
index b69824a..e324651 100644
--- a/urdf_parser/src/link.cpp
+++ b/urdf_parser/src/link.cpp
@@ -380,7 +380,8 @@ bool parseVisual(Visual &vis, TiXmlElement *config)
     }
   }

-  vis.group_name = std::string("default");
+  //vis.group_name = std::string("default");
+  vis.group_name = vis.material_name;
   const char *group_name_char = config->Attribute("group");
   if (group_name_char)
     logWarn("The notion of a group name for visual tags is not supported by URDF.");

[mmoerdijk]

what is the status on this PR? Is this already implemented?

[wjwwood]

It's in the untargeted milestone: https://github.com/ros-visualization/rviz/milestone/102

It's probably this way because it requires (or required) a patch in urdf_parser.

[mmoerdijk]

Thanks for the quick reply @wjwwood , I looked at urdf_parser and it is a bit different now than at the time @k-okada submitted this patch. I think the corresponding code is now here.

If I understand the patch that @k-okada submitted correctly it changes the vis.group_name from default to the material name. But if I look at the current code of the urdf_paser it already uses the matrial name for vis.name (group_name does not exist any more).

I don't have experience with the rviz or de urdfdom code base, but could it be that the urdf_parser already implements the behavior required for this PR? Or am I missing something?

[k-okada]

@mmoerdijk thank you for your research, See #1079 or #1080 for rewrited version of this PR using material name for vis.name. Please check if this works for you. Note that I totally forget what I did a year ago, so the code is not so confident ,

[mmoerdijk]

@k-okada Thanks! I somehow totally missed this comment, I will test it on Monday.

[mmoerdijk]

@k-okada I tested the indigo PR and can confirm it works for me. Thanks again.
@wjwwood could you say what you would need to merge these PR's? And if it is possible to cherry pick it to jade as well.

[wjwwood]

@mmoerdijk just time on my end. I need to test them out and merge other pr's before doing a release. I'm going to do a round of releases in preparation for ROS Lunar (which should be soonish).

[wjwwood]

closing in favor of #1079 and #1080