Adding support for lifecycle nodes

[JohnTGZ]

I'm writing a lifecycle node that uses the robot_model_loader::RobotModelLoader, planning_pipeline and planning_scene_monitor::PlanningSceneMonitor API. However, the constructor to these clases only take in a std::shared_ptr<rclcpp::Node> type node in their arguments.

# Planning Pipeline Constructor
PlanningPipeline(const moveit::core::RobotModelConstPtr& model, const std::shared_ptr<rclcpp::Node>& node,
                   const std::string& parameter_namespace,
                   const std::string& planning_plugin_param_name = "planning_plugin",
                   const std::string& adapter_plugins_param_name = "request_adapters");

# Planning Scene Monitor Constructor
PlanningSceneMonitor(const rclcpp::Node::SharedPtr& node, const std::string& robot_description,
                       const std::string& name = "");

# Robot Model Loader Constructor
RobotModelLoader(const rclcpp::Node::SharedPtr& node, const std::string& robot_description,
                   bool load_kinematics_solvers = true);

The case for using lifecycle nodes is so that an external orchestrator can control the instantiation and execution of individual nodes (such as the node using parts of the moveit library). There is also the ability to incorporate recovery behaviours for unresponsive nodes or error states.

Some ideas on how it could be implemented would be:

1. Overload the existing constructor to take in both rclcpp::node_interfaces::NodeBaseInterface::SharedPtr and rclcpp::node_interfaces::NodeBaseInterface::SharedPtr node interfaces, which are the components shared by both the regular node and the lifecycle node.

   • Add a deprecation notice for the default constructor taking in a std::shared_ptr<rclcpp::Node> regular node.
   • This has the added benefit of supporting future ROS2 node implementations (aside from lifecycle nodes). However, this could make it more complex to instantiate the moveit library objects.

2. Overload the existing constructor to take in a std::shared_ptr<rclcpp_lifecycle::LifecycleNode> type node

   • However, this seems like an in-elegant solution. Should more ROS2 node implementations be introduced in future distributions, it would require more maintenance in the long run.

An issue of interest might be https://github.com/ros2/geometry2/issues/94, with the corresponding pull fix https://github.com/ros2/geometry2/pull/108

Adding support for lifecycle nodes within the moveit API would definitely extend the utility of lifecycle nodes to MoveIt components, and I would be keen to contribute to such efforts. Discussion and suggestions are most welcome :)

[vatanaksoytezer]

Hello @JohnTGZ thank you very much for bringing up this topic! I liked the first solution and I would be happy to support you in the process of contributing it. I think this topic has been brought up a few times in our weekly standups and was not prioritized, but would be a very meaningful addition to MoveIt. Pinging @tylerjw, @henningkayser, @JafarAbdi and @AndyZe for additional thoughts.

[JohnTGZ]

@vatanaksoytezer That's great to hear! If I were to make a pull request, should it be on the rolling or humble branch? Perhaps I can make some preliminary changes and test it out first

[vatanaksoytezer]

@JohnTGZ thank you so much! PRs should be to the main branch and we will backport to the older distros with the handy mergify bot if requested and does not break API.

[henningkayser]

There have already been some related efforts here https://github.com/ros-planning/moveit2/pull/1329. The node is passed with a wrapper class that provides getter functions for the node interfaces. I would prefer that solution over constructor overloads, especially since future refactorings would be more straight forward. I'll see if we can get this other PR in soon. However, this is clearly an API break which will make it very difficult to backport to other distros.