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

Create option for automatic bringup of lifecycle nodes in launch_ros API #418

Open
SteveMacenski opened this issue Oct 8, 2024 · 6 comments · May be fixed by #430
Open

Create option for automatic bringup of lifecycle nodes in launch_ros API #418

SteveMacenski opened this issue Oct 8, 2024 · 6 comments · May be fixed by #430

Comments

@SteveMacenski
Copy link

SteveMacenski commented Oct 8, 2024

Feature request

Feature description

Currently, transitioning up a lifecycle node can be done via an external node, lifecycle manager, or using Opaque functions with some rather nasty looking boilerplate with events. The event-based solution only works for LifecycleNode as that has the implementations for getting the states required. However, a ComponentNode can also be a lifecycle node and there is no way to my knowledge to perform lifecycle operations in launch_ros directly on the ComponentNode object.

It would be nice if there was a LifecycleNode and ComponentNode field for active_on_bringup (or eq.) that incorporated this boilerplate to make this easier as many users of lifecycle nodes don't truly care to transition up and control the lifecycle of all their programs with such granularity. Default false is good not to do it automatically unless specified, but this is a very common need that has alot of hacky solutions across the ecosystem.

Implementation considerations

I don't think there are any unique implementation considerations.

@mjcarroll
Copy link
Member

This would be an excellent addition. Do you have any bandwidth to work on it right now? Alternatively does it make sense to discuss at the PMC meeting?

@SteveMacenski
Copy link
Author

I’m already traveling for ROSCon / ROSI and board meetings, so I won’t be able to work on this for at least a month. I’m reporting as a ‘nice to have’ enhancement that’s come up in discussions in the Nav2 community enough times now I figure it has general community interest.

Maybe in December I could take a look, but if someone else has interest and capacity, I’m happy to hand it off.

@SteveMacenski
Copy link
Author

SteveMacenski commented Dec 19, 2024

@mjcarroll I'm looking at this again and could use some advise as to how to go about it.

I'm seeing that the lifecycle node creates a service client and creates an event handler to change on transition when an event is emitted [1]. I'm tracing that this is what is called when I do something like this and using the end-user EmitEvent API.

Well there's this neat service client already located conveniently inside of lifecycle_node.py that seems begging to be used, but the question is about the best way in the maintainer's view:

  • I can EmitEvent of the lifecycle transition and update the return to include those actions as things to execute
  • I can directly just use the __rclpy_change_state_client to call the transitions we're looking for in an auto-activate feature (my personal favorite, but I think is broken by declarative rules)
  • I can possibly imagine using the feature in LifecycleTransition to use the known Node's name and do the 2 transitions for inactive and active through configure and activate in order

Which (if any) do you think is the right move?

[1] https://github.com/ros2/launch_ros/blob/rolling/launch_ros/launch_ros/actions/lifecycle_node.py#L156-L163
[2] https://github.com/ros2/launch_ros/blob/rolling/launch_ros/launch_ros/actions/lifecycle_transition.py


More journaling: the next thing that I need to figure out is how to add and handle the autostart field in the Node base class (Note: ComponentNode and LifecycleNode also is able to use them; since both are also able to be lifecycle nodes) as an option alongside the node's name, namespace, remappings, and so on so that we can actually trigger the transitions when requested. I know this is probably obvious to maintainers, but very much not so at first glance as a first-time contributor.

As far as I can tell:

  1. Add in the autostart field in the contructor arguments [reference]
  2. Set its value as a class member self.__autostart
  3. Create a getter function for getting the autostart member's value
  4. Use the getter function inside of LifecycleNode.execute() to enact the behavior discussed in the previous section

Anything obvious I'm missing there?

@SteveMacenski
Copy link
Author

Journaling a bit at this point (leaving breadcrumbs should I or someone else need to do something like this again):

Adding in the new field was easy, from the bullet list 1-4. That was solved / as straight forward as I expected.

I'm working on adding the autotransition using LifecycleTransition, but running into a few stumbles on circular dependencies that are in the works currently. I expect to have a PR open in the next couple of days

@SteveMacenski
Copy link
Author

SteveMacenski commented Dec 20, 2024

PR #430 implements for Node and LifecycleNode. Composition node is TBD, but I have some design considerations

Design Considerations - Composition Node Auto-Bringup

The crux of the issue is two fold: (1) the composable node is not an action, so there's no execute() method to implement some behavior like in the lifecycle node and (2) the lifecycle 'stuff' we'd need to do for a composable node is pretty non-compact and copy+pasting that around in the load composable nodes and composable node container actions seems bad.

So, I think it would make sense to make the composable node a full-fledged Action like the others, just not derived from the ExecuteProcess class. The considerable options from there that I can think of:

  • If autostart: in ComposableNode.execute() create the same lifecycle event handlers, services, and so on so that we can use LifecycleTransition in the load composable node and composable node container actions like we do in LifecycleNode. This works, just alot of copy+pasting boilerplate around.
  • If autostart: we create an internal LifecycleNode in ComposableNode.execute() which is populated with the content from the composable node. When we return actions, return the internal node's actions as well. I think this is probably flawed since it derives from ExecuteProcess and is going to try to do some stuff we don't want
  • We break out the lifecycle interfaces and handlers into some utility functions or a utility class that the lifecycle node uses. Then, we make ComposableNode an action and use those utility(s) like the lifecycle node does when autostart in ComposableNode.execute(). This feels right, but involves big enough changes that deserves an "OK" from maintainers before I put in the time in case you prefer another solution (ie description composable node -> action; breaking out lifecycle into some TBD utilities; injecting those utilities into the composable node loading/container actions)

@SteveMacenski
Copy link
Author

SteveMacenski commented Dec 20, 2024

Continuing this afternoon - I'm just going to use my best judgement (would like this done before the holidays). I'm going with a lifecycle event manager utility that is used in the lifecycle node and component node 👍

I should have the PR updated to include composition nodes by EOD which will fully implement the feature

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 a pull request may close this issue.

2 participants