Update is_idle function to also consider async tasks#901
Open
MegaIng wants to merge 1 commit intoros2:rollingfrom
Open
Update is_idle function to also consider async tasks#901MegaIng wants to merge 1 commit intoros2:rollingfrom
MegaIng wants to merge 1 commit intoros2:rollingfrom
Conversation
MegaIng
force-pushed
the
rolling
branch
2 times, most recently
from
15fdd15 to
ca47ab4
Compare
…ain loop. Fixes ros2#747 Signed-off-by: Cornelius Krupp <cornelius@krupp.hamburg>
|
Any update or feedback regarding this PR? @wjwwood |
|
@ahcorde is there a change we get this moving? Nearly everybody I know who uses ROS2 experienced this issue at some point. |
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
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
3 participants
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.
Description
This PR changes the
LaunchService._is_idlefunction to also consider running background tasks. This is to mitigate the error path described in this comment.The core point is that there might be background tasks running that will generate events in the future, which isn't being taken into consideration, therefore the process exits despite there still being stuff to do.
Fixes #747
Is this user-facing behavior change?
Hopefully and probably not. All old tests still work, but people might be relying on interactions that I can't predict with my limited experience with this software.
Additional Information
Another person I worked with had some issues of a test breaking in a flaky way - but it's not clear if it was caused by this change, the exact changes have changed since then and I couldn't reproduce it. Hopefully that doesn't actually relate to these changes.