-
Notifications
You must be signed in to change notification settings - Fork 18
Use the events executor for most suitable python nodes #693
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
Conversation
…eature/new_bitbots_tf_buffer_api
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Pull Request Overview
This PR updates multiple Python nodes to use the experimental EventsExecutor instead of the MultiThreadedExecutor. The key changes include replacing the executor instantiation and spin logic across various nodes, and in some cases adding a try/except block for graceful shutdown handling.
Reviewed Changes
Copilot reviewed 13 out of 13 changed files in this pull request and generated no comments.
Show a summary per file
| File | Description |
|---|---|
| bitbots_robot_filter/filter.py | Replaces MultiThreadedExecutor with EventsExecutor. |
| bitbots_ball_filter/ball_filter.py | Updates executor usage to EventsExecutor. |
| bitbots_vision/vision.py | Adopts EventsExecutor with try/except for spin. |
| bitbots_team_communication/bitbots_team_communication/bitbots_team_communication.py | Switches to EventsExecutor and adds keyboard interrupt logging. |
| bitbots_simulation/bitbots_webots_sim/scripts/start_webots_ros_supervisor.py | Changes executor usage in threaded spin for supervision. |
| bitbots_simulation/bitbots_webots_sim/scripts/start_single.py | Replaces the executor call by using EventsExecutor in a thread. |
| bitbots_simulation/bitbots_webots_sim/scripts/start_simulator.py | Updates executor and threading for simulation startup. |
| bitbots_simulation/bitbots_robocup_api/bitbots_robocup_api/command_proxy.py | Changes executor creation and integrates a try/except block. |
| bitbots_navigation/bitbots_path_planning/bitbots_path_planning/path_planning.py | Updates executor logic with keyboard interrupt handling. |
| bitbots_navigation/bitbots_localization_handler/bitbots_localization_handler/localization_handler.py | Replaces MultiThreadedExecutor with EventsExecutor (with a naming note). |
| bitbots_motion/bitbots_hcm/bitbots_hcm/humanoid_control_module.py | Uses EventsExecutor in a dedicated spin thread. |
| bitbots_misc/bitbots_tts/bitbots_tts/tts.py | Updates executor to use EventsExecutor. |
| bitbots_misc/bitbots_teleop/bitbots_teleop/joy_node.py | Switches to EventsExecutor and adds logging on shutdown. |
Comments suppressed due to low confidence (2)
bitbots_navigation/bitbots_localization_handler/bitbots_localization_handler/localization_handler.py:70
- [nitpick] Consider renaming the variable 'multi_executor' to 'executor' for clarity since the type of executor has changed, improving code readability and consistency with other files.
multi_executor = EventsExecutor()
bitbots_simulation/bitbots_webots_sim/scripts/start_webots_ros_supervisor.py:41
- The explicit call to rclpy.shutdown() has been removed; please confirm that the new EventsExecutor-based shutdown process properly terminates all nodes to avoid potential resource leaks.
# rclpy.shutdown()
jaagut
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
On KeyboardInterrupt, sometimes you log something, sometimes not. The log-messages also vary a lot. Please unify this.
bitbots_world_model/bitbots_ball_filter/bitbots_ball_filter/ball_filter.py
Outdated
Show resolved
Hide resolved
…ll_filter.py Co-authored-by: Jan Gutsche <[email protected]>
Summary
Use the python jazzy events executor for all suitable nodes
Profiling
After:

Before:

Related issues
Closes #682
Checklist
colcon build