-
Notifications
You must be signed in to change notification settings - Fork 145
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
Plumb rclpy.init context to get_default_launch_description #193
Conversation
d1d17e5
to
a6d87b3
Compare
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
A question for @wjwwood - is it necessary to shut-down the context when we're done, or is it sufficient to just pass a context here?
No need to call shutdown explicitly (it is called automatically by the pycapsule destructor), but if you create a context, it's a nice thing to do when you know it no longer should be used. I'd change this:
launch/ros2launch/ros2launch/api/api.py
Lines 140 to 141 in 632ef1f
launch_service.include_launch_description( | |
launch_ros.get_default_launch_description(prefix_output_with_name=False)) |
To be something like this:
context = rclpy.context.Context()
rclpy.init(argv=..., context=context)
launch_service.include_launch_description(
launch_ros.get_default_launch_description(prefix_output_with_name=False, context=context))
# ...
ret = launch_service.run()
rclpy.shutdown(context=context)
return ret
d9dd5e2
to
f0ebd90
Compare
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
lgtm
The test failures look unrelated/flaky:
|
From #189
This lets you pass a launch context to get_default_launch_description into get_default_launch_description and plumbs it all the way down to the ROSSpecificLaunchStartup launch action.
I couldn't find where rclpy.shutdown is called, so I didn't hook that up anywhere.
I also had to change
rclpy.get_global_executor
to constructing my own SingleThreadedExecutor because the executor needs to be built with the same context as the node.This Incomplete!
This is about 50% of what's needed for issue 189. The other half is to make ros2 launch actually use this plumbing. A question for @wjwwood - is it necessary to shut-down the context when we're done, or is it sufficient to just pass a context here?