LifecycleNodes - support `enable_communication_interface` constr parameter

[roncapat]

rclcpp_lifecycle::LifecycleNode features an additional constructor parameter called enable_communication_interface. (default: true).

However, in Nav2 the parameter is hidden:

ControllerServer::ControllerServer(const rclcpp::NodeOptions & options)
: nav2::LifecycleNode("controller_server", "", options),

should be:

ControllerServer::ControllerServer(const rclcpp::NodeOptions & options, bool enable_communication_interface)
: nav2::LifecycleNode("controller_server", "", options, enable_communication_interface),

... and then, into nav2::LifecycleNode it would need to be properly set into the constructor of the the base class rclcpp_lifecycle::LifecycleNode.

This is in support of ros2/rclcpp#2874 (comment) - of course is an advanced use case for SW integrators with very specific needs, but I think that it is a simple thing to add to Nav2 - basically not to hide a base class "toggle".

[SteveMacenski]

What's this feature do? From a quick glance, it looks like it enables or disables the lifecycle transition services used to transition nodes up and down. That is a core way that Nav2 transitions up and down the stack (and how an external lifecycle manager of any kind could control the state of the system), which cannot be disabled in Nav2's servers or they would be unable to be brought up or down.

Further, you cannot change the API of a Component and have it be loadable into the component container. It needs to contain only a single argument: rclcpp::NodeOptions (though there's support for const and & variants as well). So that suggestion I don't think is technically possible :-)

[roncapat]

What's this feature do? From a quick glance, it looks like it enables or disables the lifecycle transition services used to transition nodes up and down. That is a core way that Nav2 transitions up and down the stack (and how an external lifecycle manager of any kind could control the state of the system), which cannot be disabled in Nav2's servers or they would be unable to be brought up or down.

Not entirely true. I already manage to manually transition via C++ calls the nodes and suppressing bond usage, also removing nav2_lifecycle_manager usage. Basically, I am able to use nav2 lifecycle nodes without any kind of service-based interaction. I can provide snippets - works perfectly.

Further, you cannot change the API of a Component and have it be loadable into the component container. It needs to contain only a single argument: rclcpp::NodeOptions (though there's support for const and & variants as well). So that suggestion I don't think is technically possible :-)

I would need to check, but I'm quite sure that a constructor overload would do the trick. I can search maybe some Lifecycle Nodes outside nav2 that can be spawned as components.

[roncapat]

To add: the ultimate goal is allowing to reduce rmw graph congestion with unnecessary services (of course for users that don't need lifecycle management).

[roncapat]

Further, you cannot change the API of a Component and have it be loadable into the component container. It needs to contain only a single argument: rclcpp::NodeOptions (though there's support for const and & variants as well). So that suggestion I don't think is technically possible :-)

I would need to check, but I'm quite sure that a constructor overload would do the trick. I can search maybe some Lifecycle Nodes outside nav2 that can be spawned as components.

I did a quick test - adding a constructor like:

  explicit ControllerServer(const rclcpp::NodeOptions & options = rclcpp::NodeOptions());
  ControllerServer(const rclcpp::NodeOptions & options, bool enable_communication_interface); // NEW

and then in the implementation (sorry, quick test on Jazzy, I saw you recently worked around the Lifecycle class for the QoS rework):

// Contructor that allows broader flexibility when building node from C++ API
ControllerServer::ControllerServer(
  const rclcpp::NodeOptions & options,
  bool enable_communication_interface)
: nav2_util::LifecycleNode("controller_server", "", options, enable_communication_interface)
{
// Existing implementation
}

// Contructor that is used as component entrypoint
ControllerServer::ControllerServer(const rclcpp::NodeOptions & options)
: ControllerServer(options, true)
{
}

aaand works. It seems nobody else does this due to enable_communication_interface poorly documented. Nevertheless, it would be a good addition to nav2 and demonstrate to many others how to expose the flag while retaining component loadability IMHO.

[SteveMacenski]

I think perhaps I need some more context, can we take a step back and explain a little about what you're doing and trying to do that this is related to? Is this on a forked modified version of Nav2 or is this all from mainline just manually composed? Why not use dynamic composition (there's no computational difference and extremely marginally small memory overhead).

I don't have a particular issue with nav2::LifecycleNode having a constructor to expose this, but I'm more interested in what the right option is with the actual servers themselves. Exposing this within every single task server seems not optimal since that isn't really the 'right' way of working with the servers since Dynamic Composition has effectively no meaningful overhead, even on a RPi or similar embedded device.

I wouldn't oppose doing something like the autostart in the lifecycle node's constructor though so that it responds to a parameter instead and that way we don't need to modify the task server code itself (and comes for free in any additional servers made utilizing it - which seems like a nice feature for your application). We'd have to reconcile the rclcpp_lifecycle constructor call, but seems like there's probably a way around that.

[roncapat]

Sure, I can explain you a little bit more.

My objectives are:

• completely avoid Python
• reduce and control as much as possible the entities on the ROS graph
• bringup failry large system (70+ nodes) in a deterministic way via a custom C++ bringup logic

I used "vanilla" nav2, no forks! Just:

/**:
  ros__parameters:
    bond_heartbeat_period: 0.0
    bond_timeout: 0.0

in param file. Then, in code (NodeT as a generic LifecycleNode):

auto node = std::make_shared<NodeT>(options);
node->configure();
node->activate();
auto node_base = node->get_node_base_interface();
executor = std::make_shared<rclcpp::executors::SingleThreadedExecutor>();
executor->add_node(node_base);
executor->spin();
executor->remove_node(node_base); // Automatic shutdown transition on ctrl-c

and it just works. Additional note: for free, I can (must!) now remove the nav2_lifecycle_manager node.

Again, works with standard nav2, jazzy branch.

---

About dynamic composition: I used it at the beginning (3 years ago) but my personal experience drove me to the conclusion that it doesn't give me proper control on what I really want. The range of available Component Containers does not fit every need.

---

About coding different strategies, like an autostart parameter. I feel like if there is an underlying library facility, like the enable_communication_interface parameter, it would be better to expose it instead of thinking about something else.

If you do like you say and develop an "autostart" feature, you deal only with the manual call to configure()/activate(), but you don't disable Lifecycle services. Those are many entities on the ROS graph that the user won't be ever able to suppress (at most, only constrain to localhost with a not-so-intuitive RMW-dependent configuration). The user will continue to see them while calling ros2 service list and will be able to interact with them, even if that may not be desiderable by the developer, which instead wants to encode very specific handling logic inside the C++ bringup code.

Another way of seeing it is: if enable_communication_interface is there, but nobody uses it nor is allowed to do so, what's the point of that API? This is more a comment for the rclcpp developers. Why not passing it as an rclcpp::NodeOptions() attribute, so the downstream developers do not have to create an additional constructor? It is something that could be worked on on rclcpp side on the long run, but for the next one or even two "turtles" it would be faster to explicitly support the current API IMHO.

---

For the coding effort, I volounteer for opening a PR, as long as you are ok with it.
In parallel, I can try to convince rclcpp devs to move that parameter in rclcpp::NodeOptions or something like it, for the long term solution.

[SteveMacenski]

auto node = std::make_shared<NodeT>(options);
node->configure();
node->activate();
auto node_base = node->get_node_base_interface();
executor = std::make_shared<rclcpp::executors::SingleThreadedExecutor>();
executor->add_node(node_base);
executor->spin();
executor->remove_node(node_base); // Automatic shutdown transition on ctrl-c

You know if you use the composition launch, it adds all to their own executors in the same way. If you use autostart_node: true in their yaml file, then it automatically transitions to activated as well. You shouldn't need to write any of that code :-) That is how nearly all Nav2 users deploy and doesn't have any meaningful overhead. The Isolated Single Threaded Container was created to give each node their own executor so it acts like a multi-process node cluster since no node starves any other node, but with IPC and middleware optimization benefits due to being in the same context.

Thus, I think that strategy is unnecessary - we give you all that batteries included in our default bringup + autostart_node. Composition is mature and used in production by near all Nav2 users.

---

What I'm saying about the autostart node code is that we can parameterize it so it can be set via the yaml file rather than modifying the constructor of every Task Server. We can just have the parameter handled in the nav2::LifecycleNode constructor and all Task Servers + other nodes made with our Node get it for free, batteries included.

So, rather than modifying to add that parameter, you could just modify nav2::LifecycleNode's constructor to declare / get the enablement parameter (default on) so you can set it to off like you do with the bond timeout.

---

Why not passing it as an rclcpp::NodeOptions() attribute, so the downstream developers do not have to create an additional constructor?

To be honest, that's the real question. I don't see any value in it as a separate argument, other than the fact that NodeOptions is shared with non-lifecycle nodes as well. It would probably need to be renamed to enable_lifecycle_services or something so that its more contextually clear in the non-lifecycle case.

[roncapat]

Will that parameter allow to not instantiate the lifecycle topics and services?

[SteveMacenski]

You have to tell me, you pointed out that you wanted that flag exposed 👀 I'm providing you a clean way to do so

[roncapat]

What I'm saying about the autostart node code is that we can parameterize it so it can be set via the yaml file rather than modifying the constructor of every Task Server. We can just have the parameter handled in the nav2::LifecycleNode constructor and all Task Servers + other nodes made with our Node get it for free, batteries included.

So, rather than modifying to add that parameter, you could just modify nav2::LifecycleNode's constructor to declare / get the enablement parameter (default on) so you can set it to off like you do with the bond timeout.

I'm specifically referring to this paragraph, it is a bit obscure for me.
I mean - how a ROS parameter declared in nav2::LifecycleNode could set enable_communication_interface, if this flag is exposed by rclcpp only as constructor parameter?

I'm also going right now to check in depth the nav2::LifecycleNode code because maybe I'm missing something trivial 🤔

[SteveMacenski]

I think its pretty simple with something like:

  LifecycleNode(
    const std::string & node_name,
    const std::string & ns,
    const rclcpp::NodeOptions & options = rclcpp::NodeOptions())
  : rclcpp_lifecycle::LifecycleNode(node_name, ns, options, get_communication_interfaces())
  {..}

  bool get_communication_interfaces()
  {
    // Use declare / get API to obtain the parameter
  }

[roncapat]

From application/ROS perspective I would be 120% satisfied by this :smile

Doubt: are you sure it is possible to do so in C++? In the initialization list?

[SteveMacenski]

I.. think so? We do something not so dissimilar here

[roncapat]

Very nice! This would completely satisfy my request as it would allow to disable lifecycle I/Fs