Added Lifecycle Subscription wrapper #5772
Conversation
Signed-off-by: Prathmesh Atkale <[email protected]>
|
@Prathmesh2931, this does not build, did you test this? https://app.circleci.com/pipelines/github/ros-navigation/navigation2/17109/workflows/dd740d4c-6d31-44af-90d1-a5133d5950e9/jobs/50347 |
|
@mini-1235 |
|
Take a look at https://github.com/ros-navigation/nav2_docker/pkgs/container/nav2_docker, you can directly pull the image: |
|
Please fill in the PR template properly :-) |
| const bool allow_parameter_qos_overrides = true, | ||
| const rclcpp::CallbackGroup::SharedPtr callback_group_ptr = nullptr, | ||
| rclcpp::QOSMessageLostCallbackType qos_message_lost_callback = nullptr, | ||
| rclcpp::SubscriptionMatchedCallbackType subscription_matched_callback = nullptr, | ||
| rclcpp::IncompatibleTypeCallbackType incompatible_qos_type_callback = nullptr, | ||
| rclcpp::QOSRequestedIncompatibleQoSCallbackType requested_incompatible_qos_callback = nullptr, | ||
| rclcpp::QOSDeadlineRequestedCallbackType qos_deadline_requested_callback = nullptr, | ||
| rclcpp::QOSLivelinessChangedCallbackType qos_liveliness_changed_callback = nullptr) |
There was a problem hiding this comment.
We need all this, do not change
| */ | ||
| template<typename MessageT> | ||
| using Subscription = rclcpp::Subscription<MessageT>; | ||
| using Subscription = LifecycleSubscription<MessageT>; |
There was a problem hiding this comment.
This file should contain the new subscription, not lifecycle_subscription.hpp. This is now Nav2 subscription
| double bond_heartbeat_period{0.1}; | ||
| rclcpp::TimerBase::SharedPtr autostart_timer_; | ||
|
|
||
| // ADDED: Vector to store all managed entities (publishers, subscriptions, etc.) |
There was a problem hiding this comment.
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.
There was a problem hiding this comment.
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 .
There was a problem hiding this comment.
Its ok - just for the future its important to do your own QC before involving other developers :-)
|
Any update? |
|
Yes , I am doing some lifecycle subscription testing |
Add nav2::Subscription class that wraps rclcpp subscriptions with lifecycle state management, blocking message processing when inactive. Signed-off-by: Prathmesh Atkale <[email protected]>
|
I am running into a build issue here - the existing test files are trying to use the old subscription type but I changed it to a lifecycle wrapper class. |
|
And also the ordering of parameter is different of rclcpp::Node::create_subscription and nav2::LifecycleNode::create_subscription . |
3 participants
Added new LifecycleSubscription wrapper class.
Integrated lifecycle-state checking before executing user callbacks.
Added new create_subscription() helper that automatically:
Applies Nav2 QoS & SubscriptionOptions
Wraps user callbacks in lifecycle checks
Registers the subscription inside the lifecycle wrapper
Updated:
to support the new lifecycle subscription interface.
Added the new header:
nav2_ros_common/lifecycle_subscription.hpp