Possible build errors due to new xacro processing

[jwhendy]

I submitted #117, which built successfully, then updated to use the new ${radians(foo)} syntax and it failed. The error was:

<?xml version="1.0" encoding="UTF-8"?>
<testsuite tests="1" failures="1" time="1" errors="0" name="roslaunch-check_test_roslaunch_test_lbr_iiwa_7_r800.xml.xml">
  <testcase name="test_ran" status="run" time="1" classname="Results">
  <failure message="roslaunch check [/root/catkin_ws/src/kuka_experimental/kuka_lbr_iiwa_support/test/roslaunch_test_lbr_iiwa_7_r800.xml] failed" type=""/>
  </testcase>
  <system-out><![CDATA[[
[/root/catkin_ws/src/kuka_experimental/kuka_lbr_iiwa_support/test/roslaunch_test_lbr_iiwa_7_r800.xml]:
while processing /root/catkin_ws/src/kuka_experimental/kuka_lbr_iiwa_support/launch/load_lbr_iiwa_7_r800.launch:
Invalid <param> tag: Cannot load command parameter [robot_description]: command [/opt/ros/indigo/share/xacro/xacro.py '/root/catkin_ws/src/kuka_experimental/kuka_lbr_iiwa_support/urdf/lbr_iiwa_7_r800.xacro'] returned with code [1]. 
Param xml is <param command="$(find xacro)/xacro.py '$(find kuka_lbr_iiwa_support)/urdf/lbr_iiwa_7_r800.xacro'" name="robot_description"/>
]]></system-out>
</testsuite>

In comparing to other launch/load_robot.launch files, I noticed three differences:

• missing <?xml version="1.0"?> tag at the top
• xacro.py vs xacro
• no --inorder flag

That PR has had four build checks on Travis so far:

• 182: passed, none of the above
• 184: failed, none of the above, only change was from floats to ${radians(foo)}
• 187: failed, added xml tag and changed to xacro (no .py)
• 188: passed, added --inorder to the commit that ran in build test 187 above.

Looking at the documentation, I see:

Because --inorder processing is much more powerful, in future version beyond Jade, the new processing style will become default and you should check the compatibility of your xacro files. Usually, both processing styles should give identical results.

The docs suggest generating an xml for the .xacro with rosrun xacro xacro [--inorder] file.xacro, running diff on the output with and without the flag. For both the lbr_7_r800 (updated to new standards) and the existing lbr_14_r820 (with none of the three features bulleted above), I get no difference, so I'm confused on the build data.

In the end, it likely doesn't matter... if this is the new standard, I can update the ~10 grep hits I get for xacro.py and no --inorder flag. I'm documenting this for the benefit of others running into build errors around this.

Any input/suggestions?

[gavanderhoorn]

--inorder is only needed on Indigo when you want to use Jade+ version of xacro.

If you use the radians(..) expression and want to load the xacro on an Indigo installation, you'll have to add --inorder or xacro will fail, as you experienced.

The other support packages you found don't use Jade+ xacro constructs (with the exception of a few), so --inorder should not be needed.

[gavanderhoorn]

As your new xacro macro does use Jade+ xacro, --inorder is needed, as the last Travis build shows.

Does that clear things up? If not, please clarify, as I'm slightly confused as to what the issue is.

[jwhendy]

I'll preface this with admitting noob status... This was to raise the issue of an unexpected build failure that might be best to fix proactively vs. reactively. In my hunting, I didn't find a mention of --inorder applying to the new macros features. Your comment suggests that is a definite relationship, I just didn't see that stated in the wiki.

As another data point, after learning of ${radians()}, I made the exact same changes to the lbr_14_r820 macro.xacro and it's launch file doesn't use --inorder, yet the build succeeded. That suggested to me that --inorder and jade macros weren't causally related.

So... output of this (which I'm happy to do) could be various, but I brought it to ask the experts. To suggest two ideas:

• I can edit the wiki to add the following in the math expressions section:

note that these functionalities require the --inorder flag in launch files (mentioned below).

• if there's no harm in --inorder and there are other potential benefits, I could update existing files here.

I like fixing for the future. Someone else will come along to help, have a build fail, and spend an hour like me trying to figure out why. If the solution is known and could be added in places they will stumble on, why not do it?

Preface repeated: I'm a noob so if everyone already knows that --inorder is obvious when using jade macros, I'll just go away :)

[gavanderhoorn]

--inorder is not needed on any of the xacros that currently don't have it, but if you'd like to add it, I don't see any harm.

[jwhendy]

Thanks for the dialog. I'm still puzzled by lbr_4_r800 builds failing while lbr_14_r820 succeeded, both using Jade macros and neither having --inorder. The docs aren't very clear on how order processing relates to these macros, either.

Going by your input that --inorder is required for these macros, I decided to update the documentation as the best approach. Looking into these macros, it appears that various python module functions are brought into the global environment, making them available via ${function(arg)}. I tweaked the docs to better reflect what's going on, and pointed to the use of --inorder. Hopefully this helps other users avoid gotchas.

Since ROS Jade, Xacro employs python to evaluate expressions enclosed in dollared-braces, ${}. This allows for more complex arithmetic expressions. Functions and constants from the python math module (e.g. pi and trigonometric functions) are available for use. Note that this feature requires the use of the --inorder flag discussed below.

I'll close this, and appreciate the feedback!

[jwhendy]

I decided to also ask about this more generally on ROS Answers to see if someone can clarify the discrepancies and provided doc improvement suggestions.

[gavanderhoorn]

I have a feeling you're making this a much bigger "problem" than it really is.

--inorder on Jade ROS releases essentially enables "in order processing". You can read all about that on the xacro wiki page.

"in order processing" is completely orthogonal to all the other nice features that xacro was extended with on Jade and newer ROS (such as the Python expressions you're using in your PR).

As people wanted to use those features in their xacros on ROS Indigo as well, the xacro developers looked for a relatively simple and unobtrusive way to make that possible without having to resort to forking or creating a completely new xacro package. All to avoid the situation that those new features would break backwards compatibility with xacros created before xacro got 'upgraded'.

That's when someone figured out that they could use the --inorder option for this: if you're on Indigo and are creating (or updating) a xacro that you want to be compatible with how xacro works on Jade and newer, you're probably going to want to make sure it works with the new "in order processing" that's enabled by default there. You'd do that by adding --inorder to the command line args.

So the idea was to enable all the other new features in xacro on Indigo using the same flag. By adding --inorder to the command line of xacro on Indigo, it does some tricks internally when loading the relevant Python libraries to make this possible.

If you don't add --inorder, you get plain old Indigo xacro.

That is all there is to it really.

[gavanderhoorn]

I just checked your edits to the wiki. I believe those are not correct. --inorder is not needed to enable the new features, unless you are on Indigo.

[gavanderhoorn]

I've also answered your ROS Answers question with something similar to my earlier comment, but with a bit more context and links to PRs where xacro got upgraded and --inorder got 'abused' to enable Jade+ xacro on Indigo.

Hope that helps.

[jwhendy]

I have a feeling you're making this a much bigger "problem" than it really is.

Very possibly. That said, ROS is pretty complex and I've struggled with the documentation numerous times in my learning journey. I saw nothing on the xacro page (which is where people will mostly go to look for help on xacro) containing the excellent insights you've provided above. With some minor doc tweaks, there's the potential to spare contributors frustration and confusion. I think that's worth it.

I just checked your edits to the wiki. I believe those are not correct. --inorder is not needed to enable the new features, unless you are on Indigo.

Thank you for that. I just changed to specify Indigo.

I'm still unsure why #118 didn't fail the build.

[gavanderhoorn]

@jwhendy wrote:

Thank you for that. I just changed to specify Indigo.

I think your current edit is still not (completely) correct: all many Jade+ xacro features will require --inorder, not just the "math expressions".

[jwhendy]

Also good point. I guess I'd opt for a note at the top to that effect, then, and just ditch anything specific in math expressions. Then it would hopefully be more clear that most of the features in yellow highlights would require --inorder. Seem reasonable?

[gavanderhoorn]

You could probably add a section titled "Use of new features on Indigo" (or something similar) and explain the use of --inorder there, yes.

[jwhendy]

Done, and thanks for the guidance!

1. Use of new features on Indigo
Many of the features highlighted as "New in Jade" below are also accessible from Indigo. To use them, the --inorder flag is required. This flag is discussed below, and for some background information see this pull request. Put simply, this flag is used to trigger Jade-enabled xacro processing on Indigo.