Lifecycle helper interface proposal

[mjeronimo]

Feature request

Feature description

LifecycleNodes (and Nodes) sometimes have helper classes that aren't themselves nodes, but that use the parent node handle to create publishers, subscribers, services, and so on. When walking a LifecycleNode through its various states, these helper classes should be walked along through the states as well. For example,

CallbackReturn
Parent::on_activate(const rclcpp_lifecycle::State & state)
{
  // Parent does its own activation

  // Delegates the on_activate message to is contained classes
  // Helpers could throw an exception upon failure
  helper1__->on_activate(state);
  helper2__->on_activate(state);

  return nav2_lifecycle::CallbackReturn::SUCCESS;
}

One could use the LifecycleNode interface (https://github.com/ros2/rclcpp/blob/master/rclcpp_lifecycle/include/rclcpp_lifecycle/node_interfaces/lifecycle_node_interface.hpp) for the helper classes but it seems to me that the on_error and on_shutdown callbacks are appropriate for nodes but not the helper classes; a helper class doesn't really "shut down" and the parent would be the one to recover, if possible, from an error state. So, using the LifecycleNodeInterface for this purpose wouldn't be very elegant - the helper classes would have to override these methods, but would likely not provide any useful implementation

Instead, I propose a LifecycleHelperInterface that has a subset of the methods from LifecycleNodeInterface: on_configure, on_activate, on_deactivate, and on_cleanup. Helper classes would implement these methods, allowing them to be used by a LifecycleNode and track with the state changes.

[jacobperron]

Let me preface this by saying I'm not that familiar with the lifecycle implementation or its current state, so some of my comments are ill-informed and may be incorrect.

---

I think something like this would be very useful. I see a similar pattern in the lifecycle talker demo where the LifecyclePublisher's on_activate() is called when the parent node is activated:

https://github.com/ros2/demos/blob/3ba4549306b2c8cf702114f96789e6ca3501ccc6/lifecycle/src/lifecycle_talker.cpp#L146-L151

The same for on_deactivate().

It seems to me LifecyclePublisher acting like your definition of a "helper class" in that it manages a publisher (but is itself not a node). Although, given that the default LifecycleNodeInterface implementation does "nothing" (returns true), why couldn't LifecyclePublisher extend LifecycleNodeInterface and only implement the subset of callbacks it cares about? I can only find the motivation that the LifecyclePublisherInterface is "more convenient":

rclcpp/rclcpp_lifecycle/include/rclcpp_lifecycle/lifecycle_publisher.hpp

Lines 27 to 41 in b352d45

	/// base class with only
	/**
	* pure virtual functions. A managed
	* node can then deactivate or activate
	* the publishing.
	* It is more a convenient interface class
	* than a necessary base class.
	*/
	class LifecyclePublisherInterface
	{
	public:
	virtual void on_activate() = 0;
	virtual void on_deactivate() = 0;
	virtual bool is_activated() = 0;
	};

But is it really? @Karsten1987 Is there a strong reason not the have LifecyclePublisher extend LifecycleNodeInterface?

---

I think the proposed helper class would be even more valuable if there were a mechanism in place to automatically trigger all helper children transitions when the parent transitions. For example, either the user could explicitly trigger the transition of all children it is managing with a library call:

CallbackReturn
Parent::on_activate(const rclcpp_lifecycle::State & state)
{
  // Parent does its own activation
  // ...
  // Explicitly trigger children activation
  return children_on_activate();
}

or optionally, if no explicit trigger is done, then all children are implicitly transitioned after the parent succeeds:

CallbackReturn
Parent::on_activate(const rclcpp_lifecycle::State & state)
{
  // Parent does its own activation
  // ...
  // Children are implicitly activated after this callback returns
  return nav2_lifecycle::CallbackReturn::SUCCESS;
}

---

Following up from our discussion on using actions with lifecycle nodes:

Whether rclcpp_lifecycle provides a helper class or not, an equivalent to LifecyclePublisher will have exist for action clients and actions servers in order for them to be managed by lifecycle nodes in a way that they can be enabled/disabled.

[mjeronimo]

@jacobperron I agree that a helper class could simply dervive from the LifecycleNodeInterface. However, I'd prefer that this interface be renamed "LifecycleInterface" though as it wouldn't strictly be used for nodes; any class could be "lifecycle-aware."

I like your suggestion of automatically triggering all helper children when the parent transitions. Are you thinking that the parent lifecycle node would add each of the helper objects to a collection, such that any objects in this collection support the LifecycleInterface? Then, the Lifecycle automatically transitions each of these upon its own transitions? Something like that? How about when there are helpers that are used by a rclcpp::Node? Would such a collection be in the Node class as well?

So far, I've used a simplistic approach w/ an autoinit flag, using true when used by normal nodes and false (manual) when used by lifecycle nodes:

class SomeHelperClass : public rclcpp_lifecycle::LifecycleInterface
{
  SomeHelperClass(<required node interfaces>, bool autoinit = false)
  {
    if (autoinit_) {
      rclcpp_lifecycle::State state0(
        lifecycle_msgs::msg::State::PRIMARY_STATE_ACTIVE, "unconfigured");
      on_configure(state0);

      rclcpp_lifecycle::State state1(
        lifecycle_msgs::msg::State::PRIMARY_STATE_INACTIVE, "inactive");
      on_activate(state1);
    }
  }
}

[jacobperron]

Are you thinking that the parent lifecycle node would add each of the helper objects to a collection, such that any objects in this collection support the LifecycleInterface? Then, the Lifecycle automatically transitions each of these upon its own transitions

Yes, that is what I was thinking.

How about when there are helpers that are used by a rclcpp::Node?

I guess this depends on the use-case. Your autoinit flag approach looks fine as long as the logic of the helper class is decoupled from any lifecyle state transition logic. Depending on how the helper class is used, it might be more appropriate to create a class without any lifecycle logic and then extend it for a lifecycle variant, e.g.

class LifecycleSomeHelperClass : public rclcpp_lifecycle::LifecycleInterface, public SomeHelperClass
{
  ...
};

Or, define an interface, and implement lifecycle and non-lifecycle concrete types:

class SomeHelperClass : public SomeHelperClassInterface
{
  ...
};

class LifecycleSomeHelperClass : public rclcpp_lifecycle::LifecycleInterface, public SomeHelperClassInterface
{
  ...
};

Then a runtime flag can select whether we instantiate the lifecycle or non-lifecycle variant of the helper.

I don't think maintaining a collection in rclcpp::Node is necessary.