= API review = Proposer: '''kwc''' Present at review: * bhaskara * roland == Proposal == I did a quick survey at dinner to see what was most pressing for ROS 1.2, and roslaunch vars seems up there. I'd like to get a start on this ASAP, so the proposal is below. It's largely based on experience with ant properties. 1. `$(arg foo)` substitution arg `$(arg foo)` evaluates to the value specified by an `` tag. There must be a corresponding `` tag for a `$(arg)` usage. If an $(arg) fails to resolve, an error is printed the user, similar to the error with unresolved $(env) values. 2. `` tag There are three ways an `` can be declared: `` Declares the existence of `foo`. `foo` must be passed in either as a command-line argument (if top-level) or via `` passing (if included). `` Declares `foo` with a default value. `foo` can be overriden by command-line argument (if top-level) or via `` passing (if included). `` Declares `foo` with constant value. The value for `foo` cannot be overridden. This usage enables internal parameterization of a launch file without exposing that parameterization at higher levels. With the exception of `` tags inside of `` tags (see next), `` tag has no effect outside of the file it is declared in. 3. arg passing in `` An include tag can include `` tags, which are passed into the included file. These `` are evaluated according to the rules specified above. If an included file refers to an arg that has an unset value, this is a fatal error, e.g.: "roslaunch file foo.launch requires the argument 'robot' to be set." {{{ }}} 4. `if` and `unless` attributes With args in place, a powerful extension is 'if' and 'unless' attributes for all tags. This will include/exclude a tag based on the evaluation of a value. This exercises the power of args more strongly as a single launch file can organize multiple configurations more easily. (''already implemented'') {{{ }}} === Proposal Updates === Removed `location` attribute option for `` Removed `$(arg bar DEFAULT)` syntax as this leads to confusion of conflicting default values in use. Added `` as replacement for `$(arg bar DEFAULT)`. More declarative, more human-readable. === Analysis === This proposal conflates the idea of command-line arguments and roslaunch-declared variables/properties. I originally wrote these up as separate and tags. This proposal also takes a more functional approach to the args, i.e. each launch file is viewed as a separate entity that takes in args and spits back a configuration. This is in contrast to a more global dictionary of args, in which each launch file is acting against a global arg space that is being written to by the aggregate files. It's possible to migrate to a global model by adding namespace rules to the args, e.g. if an include/group tags set a namespace, the arg names are resolved against it. This means that higher-level files can "dig down" and pull out parameter settings from included files. I discussed this with jfaust and we agreed it was powerful, but that it's dangerous and limits the easy composition of the files. In a global model, it's much easier to break the entire file with an accidental arg conflict, aliasing, or any other sort of naming issue on a larger scale. My general experience with feature requests with roslaunch is that people generally doesn't understand the entirety of the larger launch files we deal with, and this would only exacerbate this by increasing the scope of parameterization. == Question / concerns / comments == Enter your thoughts on the API and any questions / concerns you have here. Please sign your name. Anything you want to address in the API review should be marked down here before the start of the meeting. Bhaskara * Rather than having a special case for location in arg tags, saying something like `` doesn't seem too onerous. * kwc: location seems unpopular, nixed * Could there be a special notation for the case above of an overridable argument with a default value, such as ``? At the time of writing a large and widely used file (such as one for move_base), it may not be clear which parameters users would want to override, so it should ideally not be too difficult to declare many parameters overridable. * kwc: I like this. This is what I implemented, with some minor rule clarification above. It's nice because it's very declarative about what is an expected parameter. I also added the no value/default case to represent require args. * I added an example from move_base_stage in [[attachment:move_base_fake_localization_2.5cm.launch]], which depends on [[attachment:move_base_stage.launch]]. Overall the format is pretty convenient. One thing which could have been useful in this example is to have a switch statement rather than just if-unless. For the example, there are two possible localization nodes, so it suffices to use a single flag `fake_localization`, but if it was also possible to have, say, a vslam localization node, we'd need another flag, and there's the possible inconsistency when they're both set to true. Similarly, when there are groups of related parameters (for example, the resolution affects both the resolution parameter and the world file name), a switch statement would allow them all to be set with a single argument. Roland * Awesome idea, I'll be able to delete 80% of my launch files. * kwc: yay! * Why not allow override in `` by default? That would seem (to me anyway) the most useful case. Or at least provide a way to say "I know what I'm doing and I want to override that arg no matter what". * kwc: the concern is that args are not globally unique. In our pr2 launch files, the tree of files is so large that people rarely understand the whole mess. On top of that, the same launch file may get included multiple times in different namespaces, and thus would have the same arg names. kwc * `--args` needs to be renamed (tick-tock style), now that 'arg' has a meaning. `--command`? `--print`, `--nodeargs` * Right now if/unless consider the following to be valid boolean values: 0, 1, true, and false (not case sensitive). It uses the exact same code as the tag for deciding boolean truth. Do we want a different rule, e.g "empty, 0, and false are FALSE, everything else is TRUE". * I just looked at pr2_bringup and the use cases there are pretty powerful. You can completely remount /etc/ros with a small parameter tweak, if you're willing to parameterize everything. * I'm still inclined to add a 'location' value in the future, but we can mull it over. jfaust pointed out that I should probably use the same URI syntax that's been cooked up elsewhere. The reason that ant has it is because it supports platform independence better (i.e. Windows, which has the wrong dir separator) by stating to the system that it's resolving a path. == Meeting agenda == To be filled out by proposer based on comments gathered during API review period == Conclusion == Package status change mark change manifest) * /!\ Action items that need to be taken. * {X} Major issues that need to be resolved ---- ## PackageReviewCategory