Autostart parameter services

[sloretz]

This automatically starts parameter services when a node is constructed. This is one step closer to a future where the services are created by rcl. If the user doesn't have to start the services, then they won't notice when rcl is starting them instead of rclcpp.

I refactored create_service() to get rid of ParameterServices requiring a pointer to Node so that it could be started by NodeParameters.

[sloretz]

CI

• Linux
• Linux-aarch64
• macOS
• Windows

[mjcarroll]

Looks like a failing build on Windows:

01:27:19 C:\J\workspace\ci_windows\ws\src\ros2\rclcpp\rclcpp\src\rclcpp\node_interfaces\node_parameters.cpp(31): error C2653: 'node_interfaces': is not a class or namespace name [C:\J\workspace\ci_windows\ws\build\rclcpp\rclcpp.vcxproj]
01:27:19 C:\J\workspace\ci_windows\ws\src\ros2\rclcpp\rclcpp\src\rclcpp\node_interfaces\node_parameters.cpp(32): error C2653: 'node_interfaces': is not a class or namespace name [C:\J\workspace\ci_windows\ws\build\rclcpp\rclcpp.vcxproj]
01:27:19 C:\J\workspace\ci_windows\ws\src\ros2\rclcpp\rclcpp\src\rclcpp\node_interfaces\node_parameters.cpp(33): error C2653: 'node_interfaces': is not a class or namespace name [C:\J\workspace\ci_windows\ws\build\rclcpp\rclcpp.vcxproj]
01:27:19 C:\J\workspace\ci_windows\ws\src\ros2\rclcpp\rclcpp\src\rclcpp\node_interfaces\node_parameters.cpp(35): error C2511: 'rclcpp::node_interfaces::NodeParameters::NodeParameters(const rclcpp::node_interfaces::NodeParameters::SharedPtr,const rclcpp::node_interfaces::NodeParameters::SharedPtr,const rclcpp::node_interfaces::NodeParameters::SharedPtr,bool)': overloaded member function not found in 'rclcpp::node_interfaces::NodeParameters'node_timers.cpp [C:\J\workspace\ci_windows\ws\build\rclcpp\rclcpp.vcxproj]
01:27:19   
01:27:19   C:\J\workspace\ci_windows\ws\src\ros2\rclcpp\rclcpp\include\rclcpp/node_interfaces/node_parameters.hpp(44): note: see declaration of 'rclcpp::node_interfaces::NodeParameters'
01:27:19 C:\J\workspace\ci_windows\ws\src\ros2\rclcpp\rclcpp\src\rclcpp\node_interfaces\node_parameters.cpp(35): error C2671: 'rclcpp::node_interfaces::NodeParameters::{ctor}': static member functions do not have 'this' pointers [C:\J\workspace\ci_windows\ws\build\rclcpp\rclcpp.vcxproj]
01:27:19 C:\J\workspace\ci_windows\ws\src\ros2\rclcpp\rclcpp\src\rclcpp\node_interfaces\node_parameters.cpp(36): error C2550: 'rclcpp::node_interfaces::NodeParameters::{ctor}': constructor initializer lists are only allowed on constructor definitions [C:\J\workspace\ci_windows\ws\build\rclcpp\rclcpp.vcxproj]
01:27:19 C:\J\workspace\ci_windows\ws\src\ros2\rclcpp\rclcpp\src\rclcpp\node_interfaces\node_parameters.cpp(43): error C2664: 'std::shared_ptr<rclcpp::Publisher<rcl_interfaces::msg::ParameterEvent,std::allocator<void>>> rclcpp::create_publisher<MessageT,AllocatorT,PublisherT>(rclcpp::node_interfaces::NodeTopicsInterface *,const std::string &,const rmw_qos_profile_t &,bool,std::shared_ptr<AllocatorT>)': cannot convert argument 1 from 'rclcpp::node_interfaces::NodeParameters *' to 'rclcpp::node_interfaces::NodeTopicsInterface *' [C:\J\workspace\ci_windows\ws\build\rclcpp\rclcpp.vcxproj]

[sloretz]

Partial run of windows CI to check if issues are fixed

[mikaelarguedas]

Should we provide an option for users to not start the parameter server automatically ?

Or do we skip it for no until you get a chance to use the API provided in ros2/rcl#194?

[sloretz]

Not starting parameter services also means opting out of features like use_sim_time too (edit: oops, actually use_sim_time could still be used once we have initialization from a yaml file). I vote to skip it for now.

[sloretz]

• Linux
• Linux-aarch64
• macOS
• Windows

[wjwwood]

@sloretz I actually think it's pretty important to be able to avoid creating the ROS part of the parameters service. That system is still pretty heavy weight at the moment. In fact @serge-nikulin already mentioned they'd like this option to disable any ROS interfaces related to parameters.

[wjwwood]

I would be fine with it being in a follow up pr, however. I just think it's worth doing soon.

[wjwwood]

lgtm, just had one comment/question.

[wjwwood]

node_name is already a string? Or is it because it's const?

[sloretz]

Neither, I didn't pay close enough attention when replacing node->get_name() with node_name 07c0b03

<string> defines an operator+ taking a const string and a c string http://en.cppreference.com/w/cpp/string/basic_string/operator%2B

[mikaelarguedas]

code change looks good to me:

2 things that should be done before considering merging:

• ticket the remaining work of making this optional
• modify the turtlebot2 packages that will break with this change (https://ci.ros2.org/job/ci_turtlebot-demo_linux/123)

[sloretz]

@wjwwood added bool start_parameter_services to node constructor in c95925d

CI

• Linux
• Linux-aarch64
• macOS
• Windows

[wjwwood]

Lgtm, thanks for making the change.

[sloretz]

@mikaelarguedas turtlebot 2 demo

[sloretz]

CI looks good and prs #478, ros2/demos#236, ros2/system_tests#270 are approved.

Turtlebot demo PRs passed CI too, but those prs (ros2/turtlebot2_demo#83, ros2/depthimage_to_laserscan#7, ros2/navigation#27, ros2/teleop_twist_joy#7) aren't approved. I'll merge the prs in ros2.repos now and wait for approvals on the turtlebot2 demo repos separately.

[dirk-thomas]

I am not sure of this patch is related but in the last nightly this test failure showed up and this PR is the only merge changed in this area recently.

[sloretz]

Seems like this PR might be the culprit. --retest-until-fail 1000 passed on my machine using the commit prior. Using master with --retest-until-fail 100 finds a failure most of the time. Investigating.

[sloretz]

@dirk-thomas created PR #483