ros2 / rclcpp

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

spin until timeout #1821

Open briansoe66 opened 3 years ago

briansoe66 commented 3 years ago

Feature request

I propose a spin until timeout function:

void spin_until_timeout(rclcpp::node_interfaces::NodeBaseInterface::SharedPtr node_ptr, 
                        std::chrono::duration< TimeRepT, TimeT > timeout)

This is useful for writing unit tests. Currently, I use spin_until_future_complete(), with a placeholder future, based on code in benchmark_service.cpp

constexpr char empty_service_name[] = "empty_service";
auto client = node->create_client<test_msgs::srv::Empty>(empty_service_name);
auto shared_request = std::make_shared<test_msgs::srv::Empty::Request>();
auto future = client->async_send_request(shared_request);
spin_until_future_complete(node, future, std::chrono::seconds(1));
hliberacki commented 2 years ago

I can take care of that. If it's not taken

fujitatomoya commented 1 year ago

@hliberacki it's been a while, are you still working on this? I think the following PRs need to be rebased and verified once again before merge.

hliberacki commented 1 year ago

@fujitatomoya Yes I'm, sorry I did not have enough capacity lately to do it. AFAIK what is pending was a review of my change in Rclpy repo. If I recall correctly @wjwwood found some issues there, yet those were not reviewed after my fixes.

Nevertheless I will rebase my other MR and see what has to be changed in current codebase. And when Rclpy will be correct, we can proceed to summarize this those changes all together.

Once again sorry for the delays on that

fujitatomoya commented 1 year ago

repo file for these PRs included, https://gist.githubusercontent.com/fujitatomoya/9656dc65ca96a02690370b4695bd65c8/raw/c6a694c597cb7b1ce43960ba59ba939c4a35544c/ros2.repos

fujitatomoya commented 1 year ago

@hliberacki Just FYI.

other PRs can be rebased on your repository, so please rebase all related PRs and test.

you could use this repo file: https://gist.githubusercontent.com/fujitatomoya/9656dc65ca96a02690370b4695bd65c8/raw/c6a694c597cb7b1ce43960ba59ba939c4a35544c/ros2.repos

fujitatomoya commented 1 year ago

@hliberacki friendly ping.

https://github.com/ros2/system_tests/pull/505 has been rebased by @audrow

hliberacki commented 1 year ago

@fujitatomoya sorry for the delay, I will handle it during the weekend - had plenty of work for the last 2 days and I did not manage to make it here.

fujitatomoya commented 1 year ago

@hliberacki that is totally fine, we appreciate your contribution!

fujitatomoya commented 1 year ago

Just posting note, not to miss any part of this API change. https://github.com/ros2/ros2_documentation/pull/2798 has been closed cz it was reverted, but i still do think this doc update would be nice for user.

CC: @SteveMacenski

SteveMacenski commented 1 year ago

That PR was not meant to be closed - I did a clean reset of my fork because it had gotten hilariously out of date. Let me open a new one

fujitatomoya commented 1 year ago

@SteveMacenski thanks 👍 no pressure or harry, we 1st need to rebase all of https://github.com/ros2/rclcpp/issues/1821#issuecomment-1400715537. @hliberacki friendly ping.

hliberacki commented 1 year ago

@fujitatomoya starting to work on that, again sorry for the delays! I was super packed - time-wise, lately.

timassman commented 1 year ago

For others that need a workaround for a "spin until timeout function" until this feature is available, the shortest way I found:

rclcpp::spin_until_future_complete(node, std::promise<bool>().get_future(), std::chrono::seconds(1));