Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Fix string-valued launch argument substitution #5

Merged
merged 1 commit into from Oct 2, 2023

Conversation

okvik
Copy link
Contributor

@okvik okvik commented Sep 28, 2023

Consider a launch file that declares an argument meant to be used as a string,
in this example an IP address.

from simple_launch import SimpleLauncher

sl = SimpleLauncher()

sl.declare_arg('robot_ip', '10.0.30.112')


def launch():
    print(sl.arg('robot_ip'))
    return sl.launch_description()


generate_launch_description = sl.launch_description(opaque_function=launch)

Performing the substitution as above inside the opaque function context results
in an SyntaxError being thrown since the given literal value 10.0.30.112 --
as seen by ast.literal_eval -- cannot be interpreted as valid Python.

My suggested fix is simply catching this exception and returning any such
"invalid" inputs as strings.

It might be worth noting that one can bodge a fix on the user side as follows:

sl.declare_arg('robot_ip', '"10.0.30.112"')

ros2 launch ... robot_ip:=\"10.0.30.112\"

Note the additional quotes to force interpretation as a Python string.

P.S. Thank you for making this wonderful library. With it, I no longer feel existential
dread when faced with launching off a few ROS 2 nodes...

@oKermorgant
Copy link
Owner

Hi,

Good catch!

As you have guessed, the whole point of simple_launch is to focus on robotics so yeah, it is much better to hide this complexity inside the try_perform function than going into details on adding quotes around IPs as launch arguments.

Thanks for the PR, I'll merge it and prepare a new release.

@oKermorgant oKermorgant merged commit 88f5d5b into oKermorgant:devel Oct 2, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

2 participants