cyberbotics / urdf2webots

Utility to convert URDF files to Webots PROTO nodes
Apache License 2.0
132 stars 43 forks source link

Treat collada.material.Effect when returns collada.material.Map rather than Tuple #103

Closed jgvictores closed 3 years ago

jgvictores commented 3 years ago

Hello!

I've been using urdf2webots to port some humanoid robots to Webots (e.g. https://github.com/roboticslab-uc3m/teo-webots-models lacks textures and dynamics but serves its purpose). While porting PAL Robotics REEM-C robot, I ran into TypeError: 'Map' object does not support indexing upon passing through visual.material.specular = colorVector2Instance(data.material.effect.specular). Here are the robot model repositories:

It turns out a Python tupleis expected; however, https://pycollada.readthedocs.io/en/latest/reference/generated/collada.material.Effect.html#collada.material.Effect as documented can alternatively return an instance of collada.material.Map, which is the case for this robot. The shameless (and textureless) hack at https://github.com/cyberbotics/urdf2webots/blob/master/urdf2webots/parserURDF.py#L553 (perma) was the following:

-                    if data.material.effect.specular:
+                    if data.material.effect.specular and isinstance(data.material.effect.specular, tuple):

I understand the expected behavior would be to treat the different situations where https://pycollada.readthedocs.io/en/latest/reference/generated/collada.material.Effect.html#collada.material.Effect returns a collada.material.Map (which is not restricted to specular), and/or at least prevent the TypeError. Thank you!

omichel commented 3 years ago

I believe we could implement your patch until we support the collada.material.Map type. Could you open a PR for that?

jgvictores commented 3 years ago

Sure! Here it goes: https://github.com/cyberbotics/urdf2webots/pull/105

omichel commented 3 years ago

Fixed in #105.