ros2 / launch

Tools for launching multiple processes and for writing tests involving multiple processes.
Apache License 2.0
124 stars 139 forks source link

Parse arguments into their relevant type #727

Closed nlamprian closed 11 months ago

nlamprian commented 1 year ago

Feature request

Feature description

Substitutions are meant to provide values for a dynamic launch configuration and thus they evaluate to strings. Substitutions though are also used to execute code (see PythonExpression). When that's performed, the result of a substitution must be wrapped in quotes as it needs to be defined as a string, and then it is compared to other strings. For example

PythonExpression(["'value1' if '", LaunchConfiguration("name"), "' == 'true' else 'value2'"])

That's a primitive and cumbersome way of working with arguments. It would be useful if it was possible for the launch configurations to be parsed into the relevant type of their argument.

Implementation considerations

We could hopefully have specialized launch configurations, {Bool,Int,Float,etc}LaunchConfiguration, that parse their value before returning it. Then, an expression like the above would become

PythonExpression(["'value1' if ", BoolLaunchConfiguration("name"), " else 'value2'"])

Do you think that's reasonable and would you be interested in a PR?

sloretz commented 12 months ago

Is this the feature Idea something like BooLaunchConfiguration("my_arg") takes yaml true/false (ex: my_arg:=true or my_arg:=no), and returns a string that evaluates to Python True/False (ex: "True" or "False") such that when used in PythonExpression() it can be included in a substitution without wrapping it in quotes or a type cast?

There are some substitutions that convert to yaml boolean strings, so making launch configurations that returned Python True/False strings would depart from yaml types being used.

Is there a way we could get the behavior without PythonExpression? I think it's ok if using PythonExpression is a little clunky as it's there to make all cases possible, but common cases should have a more specific substitution.

What about a substitution that returned different values abased on the yaml true-ness of the argument, such that my_arg:=true resulted in the substitution returning either "value1" or "value2" ?

IfThenElseSubstitution(if_=LaunchConfiguration("my_arg"), then="value1", else_="value2")
nlamprian commented 12 months ago

Although I have only come across boolean cases in my work so far, I would still hope to have typed substitutions so that I can directly compare any type. But yes, ultimately, the goal would be to provide such a conditional substitution, possibly in both the if and unless form, where you specify the first value and have the second one default to an empty string.

clalancette commented 11 months ago

We discussed this in the weekly meeting, and since we only have a known use case for booleans for now, we think we should stick to that.

So we think we should go with @sloretz's idea of a IfThenElseSubstitution; the goal should always be to get away from PythonExpression as much as possible. @nlamprian if you are good with that, we'd be happy to review a PR implementing it.