Can one of the admins verify this patch?

@ros-pull-request-builder

Test passed.
Refer to this link for build results: http://jenkins.ros.org/job/_pull_request-indigo-ros_comm/3/

Hi Dirk,

Thanks for the review!

Sorry about the delay, I was travelling when I got the notification and completely forgot to go back to it.

Friendly ping.

@dirk-thomas What is preventing this to be merged? I think this is a good feature and I'd be happy to help with the missing part if any/needed.

While the patch implements an interesting feature it significantly changes the internal structure of roslaunch. The current tests are not sufficient to ensure that this big change does not break existing launch files. Therefore the PRs hasn't been merged.

There are a few other PRs regarding new features in roslaunch and they are all pending for the same reason.

I am aware that this is not a satisfactory answer. But stability for Indigo is very important.

To move forward on this (and other similar PRs) I think one option would be to get more user with plenty of (complex) roslaunch files to try these changes and confirm that it doesn't break anything for them.

Could we accomplish the same thing using the pyparsing module? E.g., this simple parser:

from pyparsing import *
d =  '$(find $(arg robot)_description)/robots/upload_$(arg robot).launch'
OneOrMore(nestedExpr(opener='$(') + Word(printables, excludeChars=['$'])).parseString(d)

yields:

([(['find', (['arg', 'robot'], {}), '_description'], {}), '/robots/upload_', (['arg', 'robot'], {}), '.launch'], {})

+1, to the nested substitution feature. This would make a big difference in our use cases (very much along the same lines as the '$(robot_model)_description' examples discussed above). Not having this has meant we have had to make launch files (robot) platform specific, and install/devel specific, which really shouldn't be.

@dirk-thomas Considering the original substitution xml test launch file is still part of the tests, I'm curious what else you see is missing to ensure this feature doesn't break existing launch files?

(also, to @gerkey 's point - I agree it would be good improvement to leverage a dedicated Parsing library for parsing in roslaunch. However, at this point I think it would be better to add this valuable feature first, incrementally upon the implementation that already exists. Then, doing a library refactor could be tested against stably reproducing the same set of features / behavior.)

+1 from me as well. We spent a while searching for any documentation or ROS answers regarding nested args today, and I was surprised this hasn't come up more often.

The python evaluation addition to xacro was a huge boost to its usefulness, which @rhaschke appears to have brought to roslaunch with #707 . I was able to construct an incredibly configurable gripper xacro with 8x different swappable fingers and fingertips with this feature ros/xacro#54

I will close this in favor if the current PR #762 (which is a follow up of #707 mentioned in the previous comment).