Skip to content

Conversation

@mergify
Copy link
Contributor

@mergify mergify bot commented Sep 14, 2025

Part of #1399 and #1469.
To allow an executor to reschedule a task when a future stops blocking, it is crucial that the future is attached to the same executor.
Currently, users are able to initialize a Future instance without supplying the executor parameter.
This behavior is necessary to allow calling client.call_async() before attaching the node to an executor (the future is attached to the running executor when processing the service response).

In asyncio, loop.create_future() was added to discourage users from initializing the Future class directly.
I suggest we follow the same path.


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

* feature: add create_future and test

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

* Use create_future in all executor tests

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

---------

Signed-off-by: Nadav Elkabets <[email protected]>
(cherry picked from commit bcdd663)

# Conflicts:
#	rclpy/src/rclpy/events_executor/events_executor.cpp
#	rclpy/src/rclpy/events_executor/events_executor.hpp
#	rclpy/test/test_executor.py
@mergify mergify bot added the conflicts label Sep 14, 2025
@mergify
Copy link
Contributor Author

mergify bot commented Sep 14, 2025

Cherry-pick of bcdd663 has failed:

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

You are currently cherry-picking commit bcdd663.
  (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/rclpy/executors.py

Unmerged paths:
  (use "git add <file>..." to mark resolution)
	both modified:   rclpy/src/rclpy/events_executor/events_executor.cpp
	both modified:   rclpy/src/rclpy/events_executor/events_executor.hpp
	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]>
@fujitatomoya
Copy link
Collaborator

@nadavelkabets can you review this? i did fix several conflicts.

@fujitatomoya
Copy link
Collaborator

Pulls: #1500
Gist: https://gist.githubusercontent.com/fujitatomoya/cd36c4c5f66a91d06ba8f7f6031f5077/raw/eb309f25e7920229a1794054e9234523cf447b6e/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/16962

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

@fujitatomoya fujitatomoya merged commit ec40fc6 into jazzy Sep 16, 2025
3 checks passed
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.

3 participants