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.

Sign up for GitHub

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

Jump to bottom

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 •
edited
Loading

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
@hliberacki
Add support for spin_until_timeout (ros2#1821)
3a75f1a 
* 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
@hliberacki
Add support for spin_until_timeout (ros2#1821)
f69da46 
* 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
@hliberacki
Add support for spin_until_timeout (ros2#1821)
5a6122a 
* Created spin_until_timeout() method
* Created spin_node_until_timeout() method
* Extended unit tests

Signed-off-by: Hubert Liberacki <[email protected]>
@fujitatomoya fujitatomoya mentioned this issue Jan 22, 2022
Merged
hliberacki added a commit to hliberacki/rclcpp that referenced this issue Mar 25, 2022
@hliberacki
DRAFT: Add support for spin_for(timeout) and refactor spin_until_feat…
fd895b5 
…ure_complete (ros2#1821)

Signed-off-by: Hubert Liberacki [email protected]
hliberacki added a commit to hliberacki/rclcpp that referenced this issue Mar 28, 2022
@hliberacki
Introduce executors new spin_for method, replace spin_until_future_co…
5b0294f 
…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
@hliberacki
Introduce executors new spin_for method, replace spin_until_future_co…
2f1b739 
…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
@hliberacki
Introduce executors new spin_for method, replace spin_until_future_co…
06df077 
…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
@hliberacki
Introduce executors new spin_for method, replace spin_until_future_co…
b030e63 
…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
@hliberacki
Introduce executors new spin_for method, replace spin_until_future_co…
02043c3 
…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
@hliberacki
Introduce executors new spin_for method, replace spin_until_future_co…
91e4739 
…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
@hliberacki
Introduce executors new spin_for method, replace spin_until_future_co…
2564dd6 
…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
@hliberacki
Introduce executors new spin_for method, replace spin_until_future_co…
351a4c5 
…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
@hliberacki
Introduce executors new spin_for method, replace spin_until_future_co…
3c8e89d 
…mplete with spin_until_complete. (#1821) (#1874)
wjwwood added a commit that referenced this issue Jun 24, 2022
@wjwwood
Revert "Introduce executors new spin_for method, replace spin_until_f…
b48d8df 
…uture_complete with spin_until_complete. (#1821) (#1874)"

This reverts commit 3c8e89d.
wjwwood added a commit that referenced this issue Jun 24, 2022
@wjwwood
Revert "Introduce executors new spin_for method, replace spin_until_f…
f43a919 
…uture_complete with spin_until_complete. (#1821) (#1874)" (#1956)

This reverts commit 3c8e89d.
wjwwood added a commit that referenced this issue Jun 24, 2022
@wjwwood
Revert "Revert "Introduce executors new spin_for method, replace spin…
0f90756 
…_until_future_complete with spin_until_complete. (#1821) (#1874)" (#1956)"

This reverts commit f43a919.
@fujitatomoya
Copy link
Collaborator

fujitatomoya commented Jan 23, 2023 •
edited
Loading

@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
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

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

@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
Open
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Assignees
No one assigned
Labels
help wanted Extra attention is needed
Projects
None yet
Milestone
No milestone
Development

Successfully merging a pull request may close this issue.

Add support for spin_until_complete (take 2) ros2/rclcpp
6 participants
@clalancette @hliberacki @briansoe66 @timassman @SteveMacenski @fujitatomoya