Do We Still Need lifecycle_publisher->on_activate()?

[alanxuefei]

The following code (from 5 years ago) may be outdated because the lifecycle of lifecycle_publisher has been managed by the LifecycleNode for the past 2 years.

We built a publisher without on_activate() and it works.

https://github.com/ros-navigation/navigation2/blob/main/nav2_amcl/src/amcl_node.cpp#L263

  pose_pub_->on_activate();
  particle_cloud_pub_->on_activate();
  ....
  pose_pub_->on_deactivate();
  particle_cloud_pub_->on_deactivate();

The following steps outline the management of the lifecycle_publisher within the LifecycleNode:

1. The LifecycleNode::create_publisher method registers a publisher as a managed entity within the LifecycleNode.

   // lifecycle_node_impl.hpp
   
   LifecycleNode::create_publisher(
     const std::string & topic_name,
     const rclcpp::QoS & qos,
     const rclcpp::PublisherOptionsWithAllocator<AllocatorT> & options)
   {
     .......
     this->add_managed_entity(pub);
     return pub;
   }

2. When the LifecycleNode is activated, it invokes the on_activate() method of each managed entity, including publishers.

   // lifecycle_node_interface_impl.cpp
   
   void LifecycleNode::LifecycleNodeInterfaceImpl::add_managed_entity(
     std::weak_ptr<rclcpp_lifecycle::ManagedEntityInterface> managed_entity)
   {
     weak_managed_entities_.push_back(managed_entity);
   }
   
   void LifecycleNode::LifecycleNodeInterfaceImpl::on_activate() const
   {
     for (const auto & weak_entity : weak_managed_entities_) {
       auto entity = weak_entity.lock();
       if (entity) {
         entity->on_activate();
       }
     }
   }

3. The LifecycleNode calls the on_activate() method for each of its managed entities, such as publishers.

   // lifecycle_publisher.hpp
   
   void on_activate() override
   {
     SimpleManagedEntity::on_activate();
     should_log_ = true;
   }

[SteveMacenski]

I don't see any on_activate method in lifecycle_node_impl.hpp like you posted: https://github.com/ros2/rclcpp/blob/rolling/rclcpp_lifecycle/include/rclcpp_lifecycle/lifecycle_node_impl.hpp -- nor does the class have the member weak_managed_entities_ [1]

[1] https://github.com/ros2/rclcpp/blob/rolling/rclcpp_lifecycle/include/rclcpp_lifecycle/lifecycle_node.hpp

[alanxuefei]

weak_managed_entities_ is located in rclcpp_lifecycle/src/lifecycle_node_interface_impl.cpp#L563

The sequnce of adding publishers to managed_entities is as below.

1. LifecycleNode::create_publisher in lifecycle_node_impl.hpp#L64 call this->add_managed_entity(pub).
2. LifecycleNode::add_managed_entity in lifecycle_node.cpp#L708 call impl_->add_managed_entity(managed_entity);
3. weak_managed_entities_.push_back(managed_entity) is triggered in lifecycle_node_interface_impl.cpp#L566

The sequence for triggering on_activate of publishers via managed_entities is as follows:

1. LifecycleNode::on_activate(const State &) calls impl_->on_activate() at lifecycle_node.cpp#L696
2. The on_activate functions of each managed_entieis are triggered via lifecycle_node_interface_impl.cpp#L579

[SteveMacenski]

This is also starting to jog my memory. It looks like those are implemented in on_activate and on_deactivate within the lifecycle node, which are overridable by the application for its own activation / deactivation routines https://github.com/ros2/rclcpp/blob/ee94bc63e4ce47a502891480a2796b53d54fcdfc/rclcpp_lifecycle/include/rclcpp_lifecycle/lifecycle_node.hpp#L1055-L1057

I don't see where the base class LifecycleNode::on_activate() is called within our overridden on_activate's in Nav2 https://github.com/ros-navigation/navigation2/blob/main/nav2_controller/src/controller_server.cpp#L249-L273

Wouldn't we need to call the base class instance of on_activate in our final derived class's?

[alanxuefei]

We need to call LifecycleNode::on_activate(state); as shown here.

Therefore,

pose_pub_->on_activate();
particle_cloud_pub_->on_activate();

can be replaced with:

LifecycleNode::on_activate(state);

The pull request #1863 added the feature of "Automatically transition lifecycle entities when node transitions."

This feature was demonstrated in an example node. We can also call the default on_activate() within a customized on_activate(), or simply use the default on_activate() without customizing it.

[alanxuefei]

Thanks for explaining the code. It helped us gain a clear understanding of the ROS 2 normal publisher (rclcpp::Publisher) and lifecycle publisher (rclcpp_lifecycle::LifecyclePublisher). A normal publisher is used in my codebase, which is why it does not need to be activated or deactivated.

The key points are summarized below for closing the ticket.

1. Normal Publisher:

   rclcpp::Publisher<std_msgs::msg::String>::SharedPtr normal_pub_;

   • A normal publisher does not require activation and can publish messages as soon as it is created.

2. Lifecycle Publisher:

   std::shared_ptr<rclcpp_lifecycle::LifecyclePublisher<std_msgs::msg::String>> pub_;

   • A lifecycle publisher, like the one shown above, can be managed using:

     1. The default LifecycleNode::on_activate(state) method.

     2. A custom on_activate method that calls LifecycleNode::on_activate(state).

     3. Manually by calling pub_->on_activate().

[SteveMacenski]

@alanxuefei If you're open to a PR, you could add in the LifecycleNode::on_activate(state) at the start of each method (and deactivate too) and remove the publisher activation / deactivations.

I'm pretty ambivalent on it, if you like it better, then happy to merge it!

[alanxuefei]

@alanxuefei If you're open to a PR, you could add in the LifecycleNode::on_activate(state) at the start of each method (and deactivate too) and remove the publisher activation / deactivations.

I'm pretty ambivalent on it, if you like it better, then happy to merge it!

Sure, I will open PRs and refactor some nodes later. It’s a minor enhancement, but it’s worth trying.

[SteveMacenski]

Definitely!

[SteveMacenski]

@alanxuefei any chance you've had a few moments to work on this for a PR? 😄 I'm working to get Nav2's ticket count below 60 this week

[alanxuefei]

@alanxuefei any chance you've had a few moments to work on this for a PR? 😄 I'm working to get Nav2's ticket count below 60 this week

My computer is currently occupied by an experiment that converts images into a 3D voxel grid using deep learning. Later, I plan to integrate it with the voxel layer in the Navigation2 costmap.

Since this ticket is low priority, we can close it for now. Once my computer is free and I have more time, I will re-open the ticket and refactor the code.