rclcpp icon indicating copy to clipboard operation
rclcpp copied to clipboard

WIP: Allow to pass existing `TimeSource` via `NodeOptions`

Open roncapat opened this issue 1 year ago • 3 comments

Addresses #2659 (task 1).

This allows to put an existing TimeSource in NodeOptions so that a Node in the same process space can avoid re-creating a TimeSource.

For example, the TimeSource may be extracted via rclcpp::Node::get_node_time_source_interface() from an existing node, and passed via NodeOptions to all the others that share the process space.

This is, for example, how I tested it locally in an application of mine.

Main benefit: a single subscriber can be shared when using use_sim_time: better scaling for large rclcpp-composed simulations.

roncapat avatar Nov 13 '24 11:11 roncapat

@roncapat is this still in draft / WIP status?

fujitatomoya avatar Nov 21 '24 23:11 fujitatomoya

@fujitatomoya works nicely for me locally, but I'd like at least a review to understand what are your opinion on this / if I need to change something.

roncapat avatar Nov 22 '24 06:11 roncapat

Thank you for the useful comments. I will surely try to satisfy every request, starting with LifecycleNode support for this option.

Two quick replies:

  • if the TimeSource is shared with use_clock_thread_ disabled, this main Node must be spinning to update the clock for everybody else, otherwise other nodes sharing TimeSource cannot get the update for the simulation clock information. user application must understand this condition.

Do you mean by documentation or via a console warning?

I think if the TimeSource is shared, we cannot reset this in the subordinate Nodes during destruction. only main Node can do the reset once the TimeSource is no longer required to keep?

I think not, the shared_ptr of each node gets just "unlinked" from the TimeSource object, i.e. reference counting descreases, but the object itself gets destroyed at the last reset() only. std::shared_ptr::reset() on cppreference.com:

Releases the ownership of the managed object, if any. After the call, *this manages no object. 
Equivalent to shared_ptr().swap(*this);

If *this already owns an object and it is the last shared_ptr owning it,
the object is destroyed through the owned deleter. 

roncapat avatar Dec 03 '24 16:12 roncapat

@fujitatomoya I'm back on this, sorry for the long wait. I pushed a few fixes and updated the branch.

The remaining tasks are:

  • [ ] documentation
  • [ ] tests

I need some guidance, and I would start from the documentation.

  • the TimeSource is shared with use_clock_thread_ disabled, this main Node must be spinning to update the clock for everybody else, otherwise other nodes sharing TimeSource cannot get the update for the simulation clock information. user application must understand this condition.

See my comment in the code.

  • The subordinate Nodes will not have /use_sim_time parameter neither clock subscription QoS parameters, but only main Node. i think this is also important for user to know, i would suggest doc section in the code and ros2_documentation.

Where to put in code? And suggested place/section in ros2_documentation?

roncapat avatar Jun 30 '25 16:06 roncapat