Skip to content
Closed
6 changes: 3 additions & 3 deletions nav2_mppi_controller/test/trajectory_visualizer_tests.cpp
Copy link
Copy Markdown
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

File Purpose: Updated trajectory visualization test callback signatures to use SharedPtr for lifecycle subscription compatibility.

Summary:

  1. Updated multiple test callbacks from raw message to SharedPtr pattern
  2. Modified assignments to dereference SharedPtr (*msg)
  3. Consistent pattern across all visualization test cases

Original file line number Diff line number Diff line change
Expand Up @@ -45,7 +45,7 @@ TEST(TrajectoryVisualizerTests, VisOptimalTrajectory)
visualization_msgs::msg::MarkerArray received_msg;
auto my_sub = node->create_subscription<visualization_msgs::msg::MarkerArray>(
"~/candidate_trajectories",
[&](const visualization_msgs::msg::MarkerArray msg) {received_msg = msg;});
[&](const visualization_msgs::msg::MarkerArray::SharedPtr msg) {received_msg = *msg;});
rclcpp::executors::SingleThreadedExecutor executor;
executor.add_node(node->get_node_base_interface());

Expand Down Expand Up @@ -109,7 +109,7 @@ TEST(TrajectoryVisualizerTests, VisCandidateTrajectories)
visualization_msgs::msg::MarkerArray received_msg;
auto my_sub = node->create_subscription<visualization_msgs::msg::MarkerArray>(
"~/candidate_trajectories",
[&](const visualization_msgs::msg::MarkerArray msg) {received_msg = msg;});
[&](const visualization_msgs::msg::MarkerArray::SharedPtr msg) {received_msg = *msg;});

rclcpp::executors::SingleThreadedExecutor executor;
executor.add_node(node->get_node_base_interface());
Expand Down Expand Up @@ -142,7 +142,7 @@ TEST(TrajectoryVisualizerTests, VisOptimalPath)
nav_msgs::msg::Path received_path;
auto my_sub = node->create_subscription<nav_msgs::msg::Path>(
"~/optimal_path",
[&](const nav_msgs::msg::Path msg) {received_path = msg;});
[&](const nav_msgs::msg::Path::SharedPtr msg) {received_path = *msg;});

rclcpp::executors::SingleThreadedExecutor executor;
executor.add_node(node->get_node_base_interface());
Expand Down
9 changes: 9 additions & 0 deletions nav2_ros_common/CMakeLists.txt
Copy link
Copy Markdown
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

File Purpose: Added test executable for lifecycle subscription feature validation.

Change Summary:

  1. Added test_lifecycle_sub executable target
  2. Configure dependencies for lifecycle subscription testing
  3. Added installation directive for the test binary

Original file line number Diff line number Diff line change
Expand Up @@ -55,6 +55,15 @@ install(TARGETS ${PROJECT_NAME}
)

ament_export_include_directories(include/${PROJECT_NAME})
add_executable(test_lifecycle_sub test/test_lifecycle_sub.cpp)
ament_target_dependencies(test_lifecycle_sub
rclcpp
std_msgs
lifecycle_msgs
rclcpp_lifecycle
)
target_link_libraries(test_lifecycle_sub ${PROJECT_NAME})
install(TARGETS test_lifecycle_sub DESTINATION lib/${PROJECT_NAME})

if(BUILD_TESTING)
find_package(ament_lint_auto REQUIRED)
Expand Down
51 changes: 30 additions & 21 deletions nav2_ros_common/include/nav2_ros_common/interface_factories.hpp
100644 → 100755
Copy link
Copy Markdown
Author

@Prathmesh2931 Prathmesh2931 Jan 12, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

File Purpose: -Implements lifecycle-aware create_subscription and Provides overloads for rclcpp::Node and rclcpp_lifecycle::LifecycleNode.

Changes:
Replaced single complex create_subscription function with two overloads that automatically select appropriate behavior based on node type.

Original file line number Diff line number Diff line change
Expand Up @@ -59,21 +59,17 @@ inline rclcpp::SubscriptionOptions createSubscriptionOptions(
rclcpp::QOSLivelinessChangedCallbackType qos_liveliness_changed_callback = nullptr)
{
rclcpp::SubscriptionOptions options;
// Allow for all topics to have QoS overrides
if (allow_parameter_qos_overrides) {
options.qos_overriding_options = rclcpp::QosOverridingOptions(
{rclcpp::QosPolicyKind::Depth, rclcpp::QosPolicyKind::Durability,
rclcpp::QosPolicyKind::Reliability, rclcpp::QosPolicyKind::History});
}

// Set the callback group to use for this subscription, if given
options.callback_group = callback_group_ptr;

// ROS 2 default logs this already
options.event_callbacks.incompatible_qos_callback = requested_incompatible_qos_callback;
options.event_callbacks.incompatible_type_callback = incompatible_qos_type_callback;

// Set the event callbacks if given, else log
if (qos_message_lost_callback) {
options.event_callbacks.message_lost_callback =
qos_message_lost_callback;
Expand Down Expand Up @@ -187,32 +183,45 @@ inline rclcpp::PublisherOptions createPublisherOptions(
* @param node Node to create the subscription on
* @param topic_name Name of topic
* @param callback Callback function to handle incoming messages
* @param qos QoS settings for the subscription (default is nav2::qos::StandardTopicQoS())
* @param qos QoS settings for the subscription (default is nav2:: qos::StandardTopicQoS())
* @param callback_group The callback group to use (if provided)
* @return A shared pointer to the created subscription
* @return A shared pointer to the created lifecycle-enabled subscription
*/
template<typename MessageT, typename NodeT, typename CallbackT>
typename nav2::Subscription<MessageT>::SharedPtr create_subscription(
const NodeT & node,
// Function 1: For LifecycleNode (with lifecycle checking)
template<typename MessageT, typename CallbackT>
typename nav2::Subscription<MessageT>::SharedPtr
create_subscription(
const std::shared_ptr<rclcpp_lifecycle::LifecycleNode> & node,
const std::string & topic_name,
CallbackT && callback,
const rclcpp::QoS & qos = nav2::qos::StandardTopicQoS(),
const rclcpp::CallbackGroup::SharedPtr & callback_group = nullptr)
{
bool allow_parameter_qos_overrides = nav2::declare_or_get_parameter(
node, "allow_parameter_qos_overrides", true);

auto params_interface = node->get_node_parameters_interface();
auto topics_interface = node->get_node_topics_interface();
return rclcpp::create_subscription<MessageT, CallbackT>(
params_interface,
topics_interface,
topic_name,
qos,
std::forward<CallbackT>(callback),
createSubscriptionOptions(topic_name, allow_parameter_qos_overrides, callback_group));
(void)callback_group;
auto wrapped_callback = [node, callback = std::forward<CallbackT>(callback)]
(typename MessageT::SharedPtr msg) {
if (node->get_current_state().id() == lifecycle_msgs::msg::State::PRIMARY_STATE_ACTIVE) {
callback(msg);
}
};
return node->rclcpp_lifecycle::LifecycleNode::template create_subscription<MessageT>(
topic_name, qos, wrapped_callback);
}

// Function 2: For plain Node (no lifecycle, always active)
template<typename MessageT, typename CallbackT>
typename nav2::Subscription<MessageT>::SharedPtr
create_subscription(
const std::shared_ptr<rclcpp::Node> & node,
const std::string & topic_name,
CallbackT && callback,
const rclcpp::QoS & qos = nav2::qos::StandardTopicQoS(),
const rclcpp::CallbackGroup::SharedPtr & callback_group = nullptr)
{
(void)callback_group;
return node->template create_subscription<MessageT>(
topic_name, qos, std::forward<CallbackT>(callback));
}
/**
* @brief Create a publisher to a topic using Nav2 QoS profiles and PublisherOptions
* @param node Node to create the publisher on
Expand Down
51 changes: 47 additions & 4 deletions nav2_ros_common/include/nav2_ros_common/lifecycle_node.hpp
100644 → 100755
Copy link
Copy Markdown
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

File Purpose: Extended LifecycleNode with managed entity interface support for lifecycle management.

Additions:

  1. Managed Entity System: Track publishers, subscriptions, services, actions as lifecycle-managed entities
  2. Coordinated Activation and Deactivation: activateInterfaces() - activate all entities and deactivateInterfaces() - deactivate all entities in order.
  3. Entity Registration: add_managed_entity() - register entities for lifecycle management

Original file line number Diff line number Diff line change
Expand Up @@ -27,6 +27,7 @@
#include "rcl_interfaces/msg/parameter_descriptor.hpp"
#include "nav2_ros_common/node_thread.hpp"
#include "rclcpp_lifecycle/lifecycle_node.hpp"
#include "rclcpp_lifecycle/managed_entity.hpp"
#include "rclcpp/rclcpp.hpp"
#include "bondcpp/bond.hpp"
#include "bond/msg/constants.hpp"
Expand Down Expand Up @@ -145,11 +146,9 @@ class LifecycleNode : public rclcpp_lifecycle::LifecycleNode
* @param callback Callback function to handle incoming messages
* @param qos QoS settings for the subscription (default is nav2::qos::StandardTopicQoS())
* @param callback_group The callback group to use (if provided)
* @return A shared pointer to the created nav2::Subscription
* @return A shared pointer to the created nav2::LifecycleSubscription
*/
template<
typename MessageT,
typename CallbackT>
template<typename MessageT, typename CallbackT>
typename nav2::Subscription<MessageT>::SharedPtr
create_subscription(
const std::string & topic_name,
Expand Down Expand Up @@ -369,6 +368,47 @@ class LifecycleNode : public rclcpp_lifecycle::LifecycleNode
}
}

// ADDED: New methods for lifecycle interface management
/**
* @brief Add a managed entity (publisher, subscription, etc.) to be lifecycle-managed
* @param entity Shared pointer to the managed entity
*/
void add_managed_entity(std::shared_ptr<rclcpp_lifecycle::ManagedEntityInterface> entity)
{
managed_entities_.push_back(entity);
}

/**
* @brief Activate all managed interfaces in the correct order
* Order: Publishers -> Service Clients -> Action Clients ->
* Subscriptions -> Service Servers -> Action Servers
*/
void activateInterfaces()
{
RCLCPP_INFO(get_logger(), "Activating all managed interfaces for %s", get_name());
for (auto & entity : managed_entities_) {
if (entity) {
entity->on_activate();
}
}
}

/**
* @brief Deactivate all managed interfaces in reverse order
* Order: Action Servers -> Service Servers -> Subscriptions ->
* Action Clients -> Service Clients -> Publishers
*/
void deactivateInterfaces()
{
RCLCPP_INFO(get_logger(), "Deactivating all managed interfaces for %s", get_name());
// Deactivate in reverse order
for (auto it = managed_entities_.rbegin(); it != managed_entities_.rend(); ++it) {
if (*it) {
(*it)->on_deactivate();
}
}
}

protected:
/**
* @brief Print notifications for lifecycle node
Expand Down Expand Up @@ -427,6 +467,9 @@ class LifecycleNode : public rclcpp_lifecycle::LifecycleNode
double bond_heartbeat_period{0.1};
rclcpp::TimerBase::SharedPtr autostart_timer_;

// ADDED: Vector to store all managed entities (publishers, subscriptions, etc.)
Copy link
Copy Markdown
Member

@SteveMacenski SteveMacenski Dec 9, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This line and ones like it are making me think this is the output of an LLM. How did you validate, test, review, and compile this? We do not typically review AI outputs without you taking responsibility for quality control, we cannot review direct AI outputs - it is not a good use of maintainer time and resources. If you want to contribute a feature or fix, please put in your own legwork / effort to at least do QA and testing.

Copy link
Copy Markdown
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks for your feedback @SteveMacenski ,
The issue was on my side -I hadn’t fully verified the build environment and Docker setup before submitting the changes. That’s my mistake.
I will make sure to do Proper checks , validation and QC from my end .
I really appreciate your guidance and it means lot to me .
Sorry for inconvenience .

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Its ok - just for the future its important to do your own QC before involving other developers :-)

std::vector<std::shared_ptr<rclcpp_lifecycle::ManagedEntityInterface>> managed_entities_;

private:
/**
* @brief Get the enable_lifecycle_services parameter value from NodeOptions
Expand Down
6 changes: 1 addition & 5 deletions nav2_ros_common/include/nav2_ros_common/subscription.hpp
100644 → 100755
Copy link
Copy Markdown
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

File Purpose: Nav2 nodes rely on managed entities to ensure subscriptions follow lifecycle
transitions like activate and deactivate correctly.

Original file line number Diff line number Diff line change
Expand Up @@ -17,15 +17,11 @@

#include <memory>
#include "rclcpp/rclcpp.hpp"
#include "rclcpp_lifecycle/managed_entity.hpp"

namespace nav2
{

/**
* @brief A ROS 2 subscription for Nav2
* This is a convenience type alias to simplify the use of subscriptions in Nav2
* which may be further built up on in the future with custom APIs.
*/
template<typename MessageT>
using Subscription = rclcpp::Subscription<MessageT>;

Expand Down
22 changes: 11 additions & 11 deletions nav2_ros_common/test/test_actions.cpp
Copy link
Copy Markdown
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

File Purpose: Updated action server test to use LifecycleNode and SharedPtr callbacks for lifecycle subscription compatibility.

Change Summary:

  1. Class Inheritance: Changed from rclcpp::Node to nav2::LifecycleNode
  2. Callback Signatures: Updated from UniquePtr to SharedPtr pattern
  3. Constructor Update: Updated base class initialization
  4. QoS Syntax: Fixed subscription parameter order

Original file line number Diff line number Diff line change
Expand Up @@ -27,11 +27,11 @@ using GoalHandle = rclcpp_action::ServerGoalHandle<Fibonacci>;
using std::placeholders::_1;
using namespace std::chrono_literals;

class FibonacciServerNode : public rclcpp::Node
class FibonacciServerNode : public nav2::LifecycleNode
{
public:
FibonacciServerNode()
: rclcpp::Node("fibonacci_server_node")
: nav2::LifecycleNode("fibonacci_server_node")
{
}

Expand All @@ -48,27 +48,27 @@ class FibonacciServerNode : public rclcpp::Node

deactivate_subs_ = create_subscription<std_msgs::msg::Empty>(
"deactivate_server",
1,
[this](std_msgs::msg::Empty::UniquePtr /*msg*/) {
[this](std_msgs::msg::Empty::SharedPtr /*msg*/) {
RCLCPP_INFO(this->get_logger(), "Deactivating");
action_server_->deactivate();
});
},
rclcpp::QoS(1));

activate_subs_ = create_subscription<std_msgs::msg::Empty>(
"activate_server",
1,
[this](std_msgs::msg::Empty::UniquePtr /*msg*/) {
[this](std_msgs::msg::Empty::SharedPtr /*msg*/) {
RCLCPP_INFO(this->get_logger(), "Activating");
action_server_->activate();
});
},
rclcpp::QoS(1));

omit_preempt_subs_ = create_subscription<std_msgs::msg::Empty>(
"omit_preemption",
1,
[this](std_msgs::msg::Empty::UniquePtr /*msg*/) {
[this](std_msgs::msg::Empty::SharedPtr /*msg*/) {
RCLCPP_INFO(this->get_logger(), "Ignoring preemptions");
do_premptions_ = false;
});
},
rclcpp::QoS(1));
}

void on_term()
Expand Down
76 changes: 76 additions & 0 deletions nav2_ros_common/test/test_lifecycle_sub.cpp
Copy link
Copy Markdown
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

File Purpose: New test executable to validate lifecycle-aware subscription .

What Tests are cover:
Creates a test for lifecycle subscriptions which shows:

  1. Messages are not received while the node is unconfigured or inactive
  2. Messages are processed only after activation
  3. Subscriptions correctly stop receiving messages once the node is deactivated

Test exercise on on_configure , on_activate and on_deactivate callbacks to confirm behavior

Original file line number Diff line number Diff line change
@@ -0,0 +1,76 @@
// Copyright (c) 2025 Open Navigation LLC
//
// Licensed under the Apache License, Version 2.0 (the "License");
// you may not use this file except in compliance with the License.
// You may obtain a copy of the License at
//
// http://www.apache.org/licenses/LICENSE-2.0
//
// Unless required by applicable law or agreed to in writing, software
// distributed under the License is distributed on an "AS IS" BASIS,
// See the License for the specific language governing permissions and
// limitations under the License.

#include <rclcpp/rclcpp.hpp>
#include <std_msgs/msg/string.hpp>
#include "nav2_ros_common/lifecycle_node.hpp"

class TestNode : public nav2::LifecycleNode
{
public:
TestNode()
: nav2::LifecycleNode("test_lifecycle_sub")
{
RCLCPP_INFO(get_logger(), "Test node created");
}

void initialize()
{
RCLCPP_INFO(get_logger(), "Creating lifecycle subscription");

sub_ = create_subscription<std_msgs::msg::String>(
"test_topic",
[this](const std_msgs::msg::String::SharedPtr msg) {
RCLCPP_INFO(get_logger(), "Received: %s", msg->data.c_str());
});

RCLCPP_INFO(get_logger(), "Subscription created");
}

protected:
nav2::CallbackReturn on_configure(const rclcpp_lifecycle::State &) override
{
RCLCPP_INFO(get_logger(), "Configuring");
return nav2::CallbackReturn::SUCCESS;
}

nav2::CallbackReturn on_activate(const rclcpp_lifecycle::State &) override
{
RCLCPP_INFO(get_logger(), "Activating - subscription will now process messages");
return nav2::CallbackReturn::SUCCESS;
}

nav2::CallbackReturn on_deactivate(const rclcpp_lifecycle::State &) override
{
RCLCPP_INFO(get_logger(), "Deactivating - subscription will ignore messages");
return nav2::CallbackReturn::SUCCESS;
}

private:
nav2::Subscription<std_msgs::msg::String>::SharedPtr sub_;
};

int main(int argc, char ** argv)
{
rclcpp::init(argc, argv);

// Create node using shared_ptr
auto node = std::make_shared<TestNode>();

// Initialize when shared_ptr is created
node->initialize();

rclcpp::spin(node->get_node_base_interface());
rclcpp::shutdown();
return 0;
}
2 changes: 1 addition & 1 deletion nav2_route/test/test_operations.cpp
Copy link
Copy Markdown
Author

@Prathmesh2931 Prathmesh2931 Jan 12, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

File Purpose: Updated route operations test callback signature to use SharedPtr for lifecycle subscription compatibility.

Change Summary:

  1. Updated SpeedLimit subscription callback from raw message to SharedPtr pattern
  2. Modified assignment to dereference SharedPtr (*msg)
  3. Preserved both the flag setting and message capture logic

Original file line number Diff line number Diff line change
Expand Up @@ -136,7 +136,7 @@ TEST(OperationsManagerTest, test_processing_speed_on_status)
nav2_msgs::msg::SpeedLimit my_msg;
auto sub = node->create_subscription<nav2_msgs::msg::SpeedLimit>(
"speed_limit",
[&, this](nav2_msgs::msg::SpeedLimit msg) {got_msg = true; my_msg = msg;});
[&, this](nav2_msgs::msg::SpeedLimit::SharedPtr msg) {got_msg = true; my_msg = *msg;});

Node node2;
DirectionalEdge enter;
Expand Down
2 changes: 1 addition & 1 deletion nav2_route/test/test_path_converter.cpp
Copy link
Copy Markdown
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

File Purpose: Updated PathConverter test callback signature to use SharedPtr for lifecycle subscription compatibility.

Change Summary:

  1. Updated Path subscription callback from raw message to SharedPtr pattern
  2. Modified assignment to dereference SharedPtr (*msg)

Original file line number Diff line number Diff line change
Expand Up @@ -39,7 +39,7 @@ TEST(PathConverterTest, test_path_converter_api)

nav_msgs::msg::Path path_msg;
auto sub = node->create_subscription<nav_msgs::msg::Path>(
"plan", [&, this](nav_msgs::msg::Path msg) {path_msg = msg;});
"plan", [&, this](const nav_msgs::msg::Path::SharedPtr msg) {path_msg = *msg;});

PathConverter converter;
converter.configure(node);
Expand Down
4 changes: 2 additions & 2 deletions nav2_util/test/test_twist_publisher.cpp
Copy link
Copy Markdown
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

File Purpose: Updated TwistPublisher test callback signatures to use SharedPtr for lifecycle subscription compatibility.

Change Summary:

  1. Updated create_subscription callbacks from raw message to SharedPtr pattern
  2. Modified assignment to dereference SharedPtr (*msg)

Original file line number Diff line number Diff line change
Expand Up @@ -46,7 +46,7 @@ TEST(TwistPublisher, Unstamped)
geometry_msgs::msg::Twist sub_msg {};
auto my_sub = sub_node->create_subscription<geometry_msgs::msg::Twist>(
"cmd_vel",
[&](const geometry_msgs::msg::Twist msg) {sub_msg = msg;});
[&](const geometry_msgs::msg::Twist::SharedPtr msg) {sub_msg = *msg;});
rclcpp::executors::SingleThreadedExecutor executor;
executor.add_node(sub_node->get_node_base_interface());
vel_publisher->publish(std::move(pub_msg));
Expand Down Expand Up @@ -86,7 +86,7 @@ TEST(TwistPublisher, Stamped)
geometry_msgs::msg::TwistStamped sub_msg {};
auto my_sub = sub_node->create_subscription<geometry_msgs::msg::TwistStamped>(
"cmd_vel",
[&](const geometry_msgs::msg::TwistStamped msg) {sub_msg = msg;});
[&](const geometry_msgs::msg::TwistStamped::SharedPtr msg) {sub_msg = *msg;});
rclcpp::executors::SingleThreadedExecutor executor;
executor.add_node(sub_node->get_node_base_interface());
vel_publisher->publish(std::move(pub_msg));
Expand Down
Loading
Loading