ros / xacro

Xacro is an XML macro language. With xacro, you can construct shorter and more readable XML files by using macros that expand to larger XML expressions.
http://www.ros.org/wiki/xacro
BSD 3-Clause "New" or "Revised" License
88 stars 98 forks source link

including a macro does not work anymore in Jade #97

Closed vrabaud closed 9 years ago

vrabaud commented 9 years ago

So I see a lot of changes in Jade but I don't see anything that affects the following: you include a file with a macro (and no namespace is provided) and that macro cannot be found. Should it be used with filename.macro now ? No doc on that on http://wiki.ros.org/xacro#Including_other_xacro_files

To replicate

git clone https://github.com/ros-naoqi/pepper_robot.git
cd pepper_robot/pepper_description/urdf/pepper1.0_generated_urdf/
rosrun xacro xacro pepper_robot.xacro

Error

unknown macro name: xacro:insert_visu_Neck
when processing file: pepper_robot.xacro

While pepper_robot.xacro includes pepper_visual_collisions.xacro which defines that macro.

On Jade and xacro 1.10.4. Thx

codebot commented 9 years ago

I'll have a look right now.

codebot commented 9 years ago

it looks like the problem is due to the colons in the macro names, which are no longer processing correctly. Line 233 in naoqi_tools.generate_urdf.py is prepending "xacro:" to the macro names; if the colon in the prefix is removed, the macros should work.

I have created a (failing) test case in daedba0 and will make some kind of workaround so that these types of files can continue to work. It is printing the warning, as expected, but we can probably substitute some string (say, a few underscores or COLON or something) as we parse the macro name so that it works, after printing the warning.

codebot commented 9 years ago

looks like the problem was that the "xacro:" prefix in the names was getting removed in some places, but not all. This used to work in previous ROS distros, right? I just committed 6b4f6d0, which I think is the minimal fix to this issue. @rhaschke how does this look to you?

vrabaud commented 9 years ago

good catch, I had not seen the names starting with xacro: It indeed works with your fix, and if I remove it by hand. I'll fix our scripts right up. Thx ! You should punish @keulYSMB who's sitting right next to you :)

codebot commented 9 years ago

dude, @keulYSMB is 100x smarter than I am, there is no way he deserves punishment :)

On Thu, Jun 25, 2015 at 12:12 PM, Vincent Rabaud notifications@github.com wrote:

good catch, I had not seen the names starting with xacro: It indeed works with your fix, and if I remove it by hand. I'll fix our scripts right up. Thx ! You should punish @keulYSMB https://github.com/keulYSMB who's sitting right next to you :)

— Reply to this email directly or view it on GitHub https://github.com/ros/xacro/issues/97#issuecomment-115367136.

vrabaud commented 9 years ago

@codebot , that is possible: after all, the only thing I really know you for is that little thing called ROS, which is really unimpressive compared to the amount of beer @keulYSMB can drink :) Damn kids ... they're smart.

codebot commented 9 years ago

lol i'm just dead weight, ROS so big nowadays that nobody can ever claim it ! Community-built projects are so much fun to participate in.

I'll close this issue now; feel free to re-open it if this fix has problems