fix api differences on executors in ROS2 humble #42
Conversation
This patch fixes some changes on deprecated API rclcpp::executor by the workspace rclcpp::Executor and the options passed to the executor.
|
We should either add preprocessor conditionals to use different symbol names depending on the ROS version, or start ot maintain separate branches. If this is the only change required due to API updates, I would prefer the first option. At least we should support all versions that are not end-of-life yet. |
There was a problem hiding this comment.
This change is required for Humble, but would not work for any older version. I suggest to add a preprocessor conditional for shutdown_on_signal at least, to still support all versions since Foxy.
rclcpp::Executor::Executor::SharedPtr seems to be wrong?
Also related to newer ROS distros support: The Travis config is also outdated (Dashing, Eloquent, Foxy) and could be removed or replaced. The service is no longer active for this repository.
| @@ -51,14 +51,14 @@ struct Node : public RTT::Service | |||
| virtual ~Node(); | |||
|
|
|||
| rclcpp::Node::SharedPtr node() {return node_;} | |||
| rclcpp::executor::Executor::SharedPtr executor() {return executor_;} | |||
| rclcpp::Executor::Executor::SharedPtr executor() {return executor_;} | |||
There was a problem hiding this comment.
| rclcpp::Executor::Executor::SharedPtr executor() {return executor_;} | |
| rclcpp::Executor::SharedPtr executor() {return executor_;} |
The namespace executor was deprecated since ros2/rclcpp#1083 and removed in ros2/rclcpp#1622, so all ROS 2 versions since Foxy. So probably fine to drop support of older versions. Same for rclcpp::ExecutorOptions below.
But I don't see how rclcpp::Executor::Executor::SharedPtr should work? rclcpp::Executor::Executor would refer to the constructor of class rclcpp::Executor, no?
|
|
||
| void spin(unsigned int number_of_threads = 1); | ||
| void cancel(); | ||
|
|
||
| protected: | ||
| rclcpp::Node::SharedPtr node_; | ||
| rclcpp::executor::Executor::SharedPtr executor_; | ||
| rclcpp::Executor::Executor::SharedPtr executor_; |
There was a problem hiding this comment.
| rclcpp::Executor::Executor::SharedPtr executor_; | |
| rclcpp::Executor::SharedPtr executor_; |
| @@ -74,7 +74,7 @@ static void loadGlobalROSService() | |||
|
|
|||
| // Call rclcpp::init() | |||
| rclcpp::InitOptions init_options; | |||
| init_options.shutdown_on_sigint = false; | |||
| init_options.shutdown_on_signal = false; | |||
There was a problem hiding this comment.
This flag was renamed in version 13.1.0 of rclcpp (ros2/rclcpp#1771), so only since ROS 2 Humble. Even though Galactic and older have reached their end-of-life already, I suggest to add a preprocessor condition in this case, to still support older ROS versions without having to maintain multiple branches.
Something like
| init_options.shutdown_on_signal = false; | |
| #if RCLCPP_VERSION_GTE(13, 1, 0) | |
| init_options.shutdown_on_signal = false; | |
| #else | |
| init_options.shutdown_on_sigint = false; | |
| #endif |
and
#include "rclcpp/version.h"above should do.
Fix API changes in Humble
Description
This patch fixes some changes on deprecated API
rclcpp::executorby the workspacerclcpp::Executorand the options passed to the executor.