Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

spin until timeout #1821

Open
briansoe66 opened this issue Nov 10, 2021 · 13 comments · May be fixed by #2475
Open

spin until timeout #1821

briansoe66 opened this issue Nov 10, 2021 · 13 comments · May be fixed by #2475
Labels
help wanted Extra attention is needed

Comments

@briansoe66
Copy link

briansoe66 commented Nov 10, 2021

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));
@clalancette clalancette added the help wanted Extra attention is needed label Dec 2, 2021
@hliberacki
Copy link
Contributor

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

hliberacki added a commit to hliberacki/rclcpp that referenced this issue Jan 21, 2022
* Created spin_until_timeout() method
* Created spin_node_until_timeout() method
* Extended unit tests

Signed-off-by: Hubert Liberacki <[email protected]>
hliberacki added a commit to hliberacki/rclcpp that referenced this issue Jan 21, 2022
* Created spin_until_timeout() method
* Created spin_node_until_timeout() method
* Extended unit tests

Signed-off-by: Hubert Liberacki <[email protected]>
hliberacki added a commit to hliberacki/rclcpp that referenced this issue Jan 21, 2022
* Created spin_until_timeout() method
* Created spin_node_until_timeout() method
* Extended unit tests

Signed-off-by: Hubert Liberacki <[email protected]>
hliberacki added a commit to hliberacki/rclcpp that referenced this issue Mar 25, 2022
hliberacki added a commit to hliberacki/rclcpp that referenced this issue Mar 28, 2022
…mplete

with spin_until_complete. (ros2#1821)

* Introduce spin_for method.
* Introduce spin_until_complete.
* Deprecate spin_until_future_complete.
* Replace usage of deprecated method.
* Update unit-tests.

Signed-off-by: Hubert Liberacki <[email protected]>
hliberacki added a commit to hliberacki/rclcpp that referenced this issue Mar 28, 2022
…mplete

with spin_until_complete. (ros2#1821)

* Introduce spin_for method.
* Introduce spin_until_complete.
* Deprecate spin_until_future_complete.
* Replace usage of deprecated method.
* Update unit-tests.

Signed-off-by: Hubert Liberacki <[email protected]>
hliberacki added a commit to hliberacki/rclcpp that referenced this issue Mar 28, 2022
…mplete with spin_until_complete. (ros2#1821)

 * Introduce spin_for method.
 * Introduce spin_until_complete.
 * Deprecate spin_until_future_complete.
 * Replace usage of deprecated method.
 * Update unit-tests.

Signed-off-by: Hubert Liberacki <[email protected]>
hliberacki added a commit to hliberacki/rclcpp that referenced this issue Mar 29, 2022
…mplete with spin_until_complete. (ros2#1821)

 * Introduce spin_for method.
 * Introduce spin_until_complete.
 * Deprecate spin_until_future_complete.
 * Replace usage of deprecated method.
 * Update unit-tests.

Signed-off-by: Hubert Liberacki <[email protected]>
hliberacki added a commit to hliberacki/rclcpp that referenced this issue Mar 29, 2022
…mplete with spin_until_complete. (ros2#1821)

 * Introduce spin_for method.
 * Introduce spin_until_complete.
 * Deprecate spin_until_future_complete.
 * Replace usage of deprecated method.
 * Update unit-tests.

Signed-off-by: Hubert Liberacki <[email protected]>
hliberacki added a commit to hliberacki/rclcpp that referenced this issue Apr 2, 2022
…mplete with spin_until_complete. (ros2#1821)

 * Introduce spin_for method.
 * Introduce spin_until_complete.
 * Deprecate spin_until_future_complete.
 * Replace usage of deprecated method.
 * Update unit-tests.

Signed-off-by: Hubert Liberacki <[email protected]>
hliberacki added a commit to hliberacki/rclcpp that referenced this issue Jun 3, 2022
…mplete with spin_until_complete. (ros2#1821)

 * Introduce spin_for method.
 * Introduce spin_until_complete.
 * Deprecate spin_until_future_complete.
 * Replace usage of deprecated method.
 * Update unit-tests.

Signed-off-by: Hubert Liberacki <[email protected]>
hliberacki added a commit to hliberacki/rclcpp that referenced this issue Jun 3, 2022
…mplete with spin_until_complete. (ros2#1821)

 * Introduce spin_for method.
 * Introduce spin_until_complete.
 * Deprecate spin_until_future_complete.
 * Replace usage of deprecated method.
 * Update unit-tests.

Signed-off-by: Hubert Liberacki <[email protected]>
wjwwood pushed a commit that referenced this issue Jun 24, 2022
wjwwood added a commit that referenced this issue Jun 24, 2022
…uture_complete with spin_until_complete. (#1821) (#1874)"

This reverts commit 3c8e89d.
wjwwood added a commit that referenced this issue Jun 24, 2022
…uture_complete with spin_until_complete. (#1821) (#1874)" (#1956)

This reverts commit 3c8e89d.
wjwwood added a commit that referenced this issue Jun 24, 2022
…_until_future_complete with spin_until_complete. (#1821) (#1874)" (#1956)"

This reverts commit f43a919.
@hliberacki
Copy link
Contributor

@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
Copy link
Collaborator

@fujitatomoya fujitatomoya reopened this Jan 23, 2023
@fujitatomoya
Copy link
Collaborator

@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
Copy link
Collaborator

@hliberacki friendly ping.

ros2/system_tests#505 has been rebased by @audrow

@hliberacki
Copy link
Contributor

@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
Copy link
Collaborator

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

@fujitatomoya
Copy link
Collaborator

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

CC: @SteveMacenski

@SteveMacenski
Copy link
Collaborator

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
Copy link
Collaborator

@SteveMacenski thanks 👍 no pressure or harry, we 1st need to rebase all of #1821 (comment). @hliberacki friendly ping.

@hliberacki
Copy link
Contributor

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

@timassman
Copy link

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));

@fujitatomoya fujitatomoya linked a pull request Apr 5, 2024 that will close this issue
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
help wanted Extra attention is needed
Projects
None yet
Development

Successfully merging a pull request may close this issue.

6 participants