ros2 / rclcpp

rclcpp (ROS Client Library for C++)
Apache License 2.0
536 stars 417 forks source link

Add support for spin_until_complete (take 2) #2475

Open christophebedard opened 6 months ago

christophebedard commented 6 months ago

Reverts #1956, so it un-reverts #1874 (with some major updates), see https://github.com/ros2/rclcpp/pull/1874#issuecomment-1165811614

Replaces #1957

Closes #1821

This adds spin_until_complete(condition, timeout).

This also adds rclcpp::Executor::spin_for(duration). Original Issue: #1821 (Added by @fujitatomoya)

This goes along with the following PRs:

wjwwood commented 5 months ago

I had a few questions/suggestions/comments, but generally the approach lgtm. I think once you've addressed them to your satisfaction @christophebedard, then it should be good to go.

wjwwood commented 5 months ago

CI:

christophebedard commented 5 months ago

Should we merge this now, or wait until the rclpy PR (https://github.com/ros2/rclpy/pull/1268) has been reviewed and is ready to go too?

wjwwood commented 5 months ago

We should really merge them together, because I don't think it's good if we have rclpy and rclcpp deviate from one another.

wjwwood commented 5 months ago

My CI above did not include your changes for rclpy unfortunately. We'll have to run separate CI for that.

roncapat commented 2 months ago

Any news on this?