Executor bug fixes and WaitSet refactor #140
Closed
Conversation
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
sloretz
force-pushed
the
rclpy_executor_refactor
branch
from
a0729e0 to
1bcf742
Compare
sloretz
added
in review
sloretz
changed the title
Moved wait set code to its own class for code reuse Added timeout_sec_to_nsec() wait_for_service() implemented with timers Added unit tests for timeout_sec_to_nsec() Added test for WaitSet class Use negative timeouts to mean block forever Double quotes to single quotes Added wait_for_service() tests and fixed bugs it caught Eliminate blind exception warning Reduce flakiness of test by increasing time to 0.1s Comment says negative timeouts block forever Use :returns: Move add_subscriptions() arugments -> arguments Daemon as keyword arg Remove unnecessary namespace argument Use S_TO_NS in test More tests using S_TO_NS Use monotonic clock for testing timer time Increased test timeout by 30 seconds CheckExact -> IsValid Fixed wait_set not clearing ready_pointers Remove unnecessary namespace keyword arg Non-blocking wait remove expression that always evaluates to True Raise ValueError on invalid capsule Simplified timeout_sec_to_nsec Added 'WaitSet.destroy()' and made executor use it GraphListener periodically checks if rclpy is shutdown Misc fixes after pycapsule names Wait set class always clears entities before waiting Remove underscore on import Reformat timeout line Use () when raising exceptions Removed _ on imports Executor optimizations ~5% less overhead in wait_for_ready_callbacks() Fixed executor yielding entities to wrong node Also refactored some code to a sub-generator Use list() only where necessary Docstring in imperitive mood Executors reuse iterator moved some wait_set code into C Avoid another list comprehension Replaced WaitSet with C code in executor Remove test code Use lists instead of set Use locally defined function instead of member function Shorten code using macro Move everything to new wait_set code protect against ImportError.path being None (#134) Free memory when things don't go as planned (#138)
Python library _rclpy could not be imported due to rclpy_sigint.dll not being on PATH. This uses 'APPEND_LIBRARY_DIRS' argument to ament_add_nose_test to add that directory to the path.
sloretz
force-pushed
the
rclpy_executor_refactor
branch
from
1bcf742 to
a3e3ece
Compare
Merged
sloretz
added
in progress
This was referenced Nov 28, 2017
This was referenced Jan 4, 2018
Member
|
@sloretz Will this PR be replaced by favor of https://github.com/ros2/rclpy/pull/173/files when we switch to pybind11? |
Contributor
Author
|
@mikaelarguedas There is a little bit of code in here that might make it to a future PR, but most of the code here has already made it or will be replaced by a switch to pybind11. The code worth saving is in a fork, so I'll close this and delete the branch. |
YuanYuYuan
pushed a commit
to YuanYuYuan/rclpy
that referenced
this pull request
Nov 12, 2025
Trying to figure out which tests are flaky and fix, adding debug info…
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
This PR is split from #127. It has performance improvements to the executor that get the 1kHz timer test to pass consistently. If you've already reviewed #127 the new changes are:
WaitSetclass as a cpython extensionChanges that were already in #127
wait_for_callbacksrefactored readability + performancewait_for_callbackscould yield the wrong nodetimeout_sec_to_nsecSingleThreadedExecutorandMultiThreadedExecutorreuse the iterator fromwait_for_callbacksover multiplespin_oncecallsCI