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

Update spin_until_complete #919

Open
wants to merge 3 commits into
base: rolling
Choose a base branch
from

Conversation

hliberacki
Copy link

Due to changes in rclcpp replace spin_until_future_complete with spin_until_complete, add spin.

  • Deprecate spin_until_future_complete
  • Add spin_until_complete
  • Add spin_for method
  • Udpdate unit tests

Signed-off-by: Hubert Liberacki [email protected]

@hliberacki
Copy link
Author

Due to changes in rclcpp I've pushed a PR updating pythong side as well.
PR in rclcpp - ros2/rclcpp#1874

@hliberacki hliberacki force-pushed the hliberacki/spin_until branch 2 times, most recently from 5c75743 to 09fc152 Compare March 30, 2022 13:58
rclpy/rclpy/executors.py Outdated Show resolved Hide resolved
rclpy/rclpy/executors.py Outdated Show resolved Hide resolved
rclpy/test/test_executor.py Outdated Show resolved Hide resolved
rclpy/test/test_executor.py Outdated Show resolved Hide resolved
rclpy/test/test_executor.py Outdated Show resolved Hide resolved
@hliberacki
Copy link
Author

ros2/rclcpp#1874 (comment) Passing CI with all related PRs linked and build together.

Copy link
Member

@wjwwood wjwwood left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This has some necessary changes. I think it can be decoupled from the ros2/rclcpp#1874 pr.

rclpy/rclpy/__init__.py Outdated Show resolved Hide resolved
rclpy/rclpy/executors.py Outdated Show resolved Hide resolved
rclpy/rclpy/executors.py Outdated Show resolved Hide resolved
rclpy/rclpy/__init__.py Show resolved Hide resolved
…_for method

* Deprecate spin_until_future_complete
* Add spin_until_complete
* Add spin_for method
* Udpdate unit tests

Signed-off-by: Hubert Liberacki <[email protected]>
* Deprecate old methods
* fix comments

Signed-off-by: Hubert Liberacki <[email protected]>
@hliberacki hliberacki requested a review from wjwwood July 4, 2022 08:54
@ihasdapie
Copy link
Member

Now that #959 is merged this PR should update rclpy/test/test_parameter_client.py to use the new spin_until_complete methods instead of spin_until_future_complete

@hliberacki
Copy link
Author

Now that #959 is merged this PR should update rclpy/test/test_parameter_client.py to use the new spin_until_complete methods instead of spin_until_future_complete

I was out of town, I will fix it shortly.

@wjwwood did you have a chance to take a look into new changes addressing your comments?

@christophebedard
Copy link
Member

I've opened #1268 to replace this PR.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants