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

Check if Task(Future) is canceled. (backport #1377) #1404

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

Conversation

mergify[bot]
Copy link
Contributor

@mergify mergify bot commented Jan 17, 2025

closes #1099


This is an automatic backport of pull request #1377 done by Mergify.

* Check if Task(Future) is canceled.

Signed-off-by: Tomoya Fujita <[email protected]>

* Close cancelled coroutine (#1394)

* Add FutureState

Signed-off-by: Nadav Elkabets <[email protected]>

* Close canceled coroutine

Signed-off-by: Nadav Elkabets <[email protected]>

* Fixed behavior in test

Signed-off-by: Nadav Elkabets <[email protected]>

---------

Signed-off-by: Nadav Elkabets <[email protected]>
Signed-off-by: Tomoya Fujita <[email protected]>

* address flake8 and pep257 failures.

Signed-off-by: Tomoya Fujita <[email protected]>

* Cancelled future is not done (#1397)

* Remove redundant coro.close

Signed-off-by: nadav <[email protected]>

* Only finished future is done

Signed-off-by: nadav <[email protected]>

* Add function _pending and fix checks

Signed-off-by: = <[email protected]>

* Replace check in done from pending to finished

Signed-off-by: = <[email protected]>

* Adapt test to new behavior

Signed-off-by: = <[email protected]>

* Add tests

Signed-off-by: = <[email protected]>

* Make changes within active task mutex

Signed-off-by: = <[email protected]>

---------

Signed-off-by: nadav <[email protected]>
Signed-off-by: = <[email protected]>

* keep the consistent behavior to avoid exception, and adjusted some tests accordingly.

Signed-off-by: Tomoya Fujita <[email protected]>

* revert doc section to raise the exception.

Signed-off-by: Tomoya Fujita <[email protected]>

* remove StrEnum and put logical operator in the beginning of line.

Signed-off-by: Tomoya Fujita <[email protected]>

* add more test to check Task state.

Signed-off-by: Tomoya Fujita <[email protected]>

---------

Signed-off-by: Tomoya Fujita <[email protected]>
Signed-off-by: Nadav Elkabets <[email protected]>
Signed-off-by: nadav <[email protected]>
Signed-off-by: = <[email protected]>
Co-authored-by: Nadav Elkabets <[email protected]>
Co-authored-by: Nadav Elkabets <[email protected]>
(cherry picked from commit 9a144bf)

# Conflicts:
#	rclpy/rclpy/executors.py
#	rclpy/rclpy/task.py
#	rclpy/test/test_executor.py
Copy link
Contributor Author

mergify bot commented Jan 17, 2025

Cherry-pick of 9a144bf has failed:

On branch mergify/bp/jazzy/pr-1377
Your branch is up to date with 'origin/jazzy'.

You are currently cherry-picking commit 9a144bf.
  (fix conflicts and run "git cherry-pick --continue")
  (use "git cherry-pick --skip" to skip this patch)
  (use "git cherry-pick --abort" to cancel the cherry-pick operation)

Changes to be committed:
	modified:   rclpy/test/test_task.py

Unmerged paths:
  (use "git add <file>..." to mark resolution)
	both modified:   rclpy/rclpy/executors.py
	both modified:   rclpy/rclpy/task.py
	both modified:   rclpy/test/test_executor.py

To fix up this pull request, you can check it out locally. See documentation: https://docs.github.com/en/pull-requests/collaborating-with-pull-requests/reviewing-changes-in-pull-requests/checking-out-pull-requests-locally

Signed-off-by: Tomoya Fujita <[email protected]>
Copy link
Collaborator

@fujitatomoya fujitatomoya left a comment

Choose a reason for hiding this comment

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

lgtm with green CI

@fujitatomoya
Copy link
Collaborator

fujitatomoya commented Jan 17, 2025

Pulls: #1404
Gist: https://gist.githubusercontent.com/fujitatomoya/cedf41d8baad175e90ce12760455b07d/raw/d38156296f80ff00b6615723f748c7dd55b0a479/ros2.repos
BUILD args: --packages-above-and-dependencies rclpy
TEST args: --packages-above rclpy
ROS Distro: jazzy
Job: ci_launcher
ci_launcher ran: https://ci.ros2.org/job/ci_launcher/15072

  • Linux Build Status
  • Linux-aarch64 Build Status
  • Linux-rhel Build Status
  • Windows Build Status

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

Successfully merging this pull request may close these issues.

1 participant