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

Fix hangout in rosbag2 player and recorder when pressing CTRL+C #1081

Merged

Conversation

MichaelOrlov
Copy link
Contributor

Please note that it's almost impossible to cover this fix in unit tests since gtest using redirection of the standard input to the file and KeyboardHandler has detection for this special case and automatically disable keyboard handling from standard input on construction.

Disable SIGINT handler in KeyboardHandler instantiated from rosbag2
player and recorder. Since we have already overriding SIGINT on upper
python level or inside rclcpp::init() and correctly finishing
application with proper calling of all destructors upon receiving SIGINT

Signed-off-by: Michael Orlov <michael.orlov@apex.ai>
@MichaelOrlov MichaelOrlov marked this pull request as ready for review September 8, 2022 02:53
@MichaelOrlov MichaelOrlov requested a review from a team as a code owner September 8, 2022 02:53
@MichaelOrlov MichaelOrlov requested review from emersonknapp and jhdcs and removed request for a team September 8, 2022 02:53
@MichaelOrlov
Copy link
Contributor Author

Gist: https://gist.githubusercontent.com/MichaelOrlov/8b1f44ec30da7cbf685d25a8290e2459/raw/571667e93bb076cef5922df3e096645f31e6cb84/ros2.repos
BUILD args: --packages-above-and-dependencies rosbag2_transport rosbag2_tests
TEST args: --packages-above rosbag2_transport rosbag2_tests
ROS Distro: rolling
Job: ci_launcher
ci_launcher ran: https://ci.ros2.org/job/ci_launcher/10777

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

@MichaelOrlov
Copy link
Contributor Author

Warning on Windows build is known issue existing on baseline and will be handled in #1084

@MichaelOrlov MichaelOrlov merged commit f629e45 into rolling Sep 10, 2022
@delete-merged-branch delete-merged-branch bot deleted the morlov/disable_sigint_in_keyboard_handler_from_rosbag2 branch September 10, 2022 02:33
@MichaelOrlov
Copy link
Contributor Author

https://github.com/Mergifyio backport humble

@mergify
Copy link

mergify bot commented Sep 10, 2022

backport humble

✅ Backports have been created

mergify bot pushed a commit that referenced this pull request Sep 10, 2022
)

Disable SIGINT handler in KeyboardHandler instantiated from rosbag2
player and recorder. Since we have already overriding SIGINT on upper
python level or inside rclcpp::init() and correctly finishing
application with proper calling of all destructors upon receiving SIGINT

Signed-off-by: Michael Orlov <michael.orlov@apex.ai>

Signed-off-by: Michael Orlov <michael.orlov@apex.ai>
(cherry picked from commit f629e45)
@ros-discourse
Copy link

This pull request has been mentioned on ROS Discourse. There might be relevant details there:

https://discourse.ros.org/t/ros-2-tsc-meeting-minutes-2022-9-15/27394/1

MichaelOrlov added a commit that referenced this pull request Sep 23, 2022
) (#1085)

Disable SIGINT handler in KeyboardHandler instantiated from rosbag2
player and recorder. Since we have already overriding SIGINT on upper
python level or inside rclcpp::init() and correctly finishing
application with proper calling of all destructors upon receiving SIGINT

Signed-off-by: Michael Orlov <michael.orlov@apex.ai>

Signed-off-by: Michael Orlov <michael.orlov@apex.ai>
(cherry picked from commit f629e45)

Co-authored-by: Michael Orlov <michael.orlov@apex.ai>
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.

ros2 bag record and play does occasionally not properly closing when pressing Ctrl+C
3 participants