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

Asynchronously wait for load node service response #174

Merged
merged 2 commits into from
Aug 28, 2020
Merged

Conversation

jacobperron
Copy link
Member

Fixes #171

By asychronously waiting for the service response, we can monitor if launch is shutting down and abandon the request so we don't block the shutdown process.

Fixes #171

By asychronously waiting for the service response, we can monitor if launch is shutting down and abandon the request so we don't block the shutdown process.

Signed-off-by: Jacob Perron <jacob@openrobotics.org>
@jacobperron jacobperron added the bug Something isn't working label Aug 18, 2020
@jacobperron jacobperron self-assigned this Aug 18, 2020
It was accidentally removed.

Signed-off-by: Jacob Perron <jacob@openrobotics.org>
@jacobperron jacobperron added this to Proposed in Foxy Patch Release 3 via automation Aug 19, 2020
@jacobperron jacobperron moved this from Proposed to In progress in Foxy Patch Release 3 Aug 19, 2020
Copy link
Member

@ivanpauno ivanpauno left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I'm not sure if the service async call can interact with the launch asyncio loop in a nicer way (rclpy futures are an awaitable https://github.com/ros2/rclpy/blob/16a72fa711a18376c6d5e97eb82d8af6b113a67a/rclpy/rclpy/task.py#L54-L58).

This is solving the reported issue, so LGTM.

@ivanpauno ivanpauno requested a review from hidmic August 26, 2020 20:25
@jacobperron
Copy link
Member Author

Maybe there's a better way, but I just copied the logic from the LifecycleNode action. If we come up with something better, we can refactor both.

@jacobperron
Copy link
Member Author

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

@jacobperron jacobperron merged commit 066d8fe into master Aug 28, 2020
@delete-merged-branch delete-merged-branch bot deleted the jacob/fix_171 branch August 28, 2020 00:15
@jacobperron jacobperron moved this from In progress to Needs backport in Foxy Patch Release 3 Oct 2, 2020
@jacobperron jacobperron removed this from Needs backport in Foxy Patch Release 3 Oct 9, 2020
@jacobperron jacobperron added this to Proposed in Foxy Patch Release 4 via automation Oct 9, 2020
@jacobperron jacobperron moved this from Proposed to Needs backport in Foxy Patch Release 4 Oct 9, 2020
@nuclearsandwich nuclearsandwich moved this from Released to Proposed in Dashing Patch Release 8 Nov 17, 2020
jacobperron added a commit that referenced this pull request Nov 19, 2020
* Asynchronously wait for load node service response

Fixes #171

By asychronously waiting for the service response, we can monitor if launch is shutting down and abandon the request so we don't block the shutdown process.

Signed-off-by: Jacob Perron <jacob@openrobotics.org>

* Add back debug log

It was accidentally removed.

Signed-off-by: Jacob Perron <jacob@openrobotics.org>
jacobperron added a commit that referenced this pull request Nov 19, 2020
* Asynchronously wait for load node service response

Fixes #171

By asychronously waiting for the service response, we can monitor if launch is shutting down and abandon the request so we don't block the shutdown process.

Signed-off-by: Jacob Perron <jacob@openrobotics.org>

* Add back debug log

It was accidentally removed.

Signed-off-by: Jacob Perron <jacob@openrobotics.org>
jacobperron added a commit that referenced this pull request Nov 19, 2020
* Asynchronously wait for load node service response

Fixes #171

By asychronously waiting for the service response, we can monitor if launch is shutting down and abandon the request so we don't block the shutdown process.

Signed-off-by: Jacob Perron <jacob@openrobotics.org>

* Add back debug log

It was accidentally removed.

Signed-off-by: Jacob Perron <jacob@openrobotics.org>
jacobperron added a commit that referenced this pull request Nov 19, 2020
* Asynchronously wait for load node service response

Fixes #171

By asychronously waiting for the service response, we can monitor if launch is shutting down and abandon the request so we don't block the shutdown process.

Signed-off-by: Jacob Perron <jacob@openrobotics.org>

* Add back debug log

It was accidentally removed.

Signed-off-by: Jacob Perron <jacob@openrobotics.org>
jacobperron added a commit that referenced this pull request Nov 19, 2020
* Asynchronously wait for load node service response

Fixes #171

By asychronously waiting for the service response, we can monitor if launch is shutting down and abandon the request so we don't block the shutdown process.

Signed-off-by: Jacob Perron <jacob@openrobotics.org>

* Add back debug log

It was accidentally removed.

Signed-off-by: Jacob Perron <jacob@openrobotics.org>
jacobperron added a commit that referenced this pull request Nov 19, 2020
* Asynchronously wait for load node service response

Fixes #171

By asychronously waiting for the service response, we can monitor if launch is shutting down and abandon the request so we don't block the shutdown process.

Signed-off-by: Jacob Perron <jacob@openrobotics.org>

* Add back debug log

It was accidentally removed.

Signed-off-by: Jacob Perron <jacob@openrobotics.org>
jacobperron added a commit that referenced this pull request Nov 20, 2020
* Asynchronously wait for load node service response

Fixes #171

By asychronously waiting for the service response, we can monitor if launch is shutting down and abandon the request so we don't block the shutdown process.

Signed-off-by: Jacob Perron <jacob@openrobotics.org>

* Add back debug log

It was accidentally removed.

Signed-off-by: Jacob Perron <jacob@openrobotics.org>
@jacobperron jacobperron removed this from Proposed in Dashing Patch Release 8 Nov 21, 2020
@jacobperron jacobperron added this to Proposed in Dashing Patch Release 9 via automation Nov 21, 2020
@jacobperron jacobperron removed this from Needs backport in Foxy Patch Release 4 Dec 11, 2020
@jacobperron jacobperron added this to Proposed in Foxy Patch Release 5 via automation Dec 11, 2020
@jacobperron jacobperron moved this from Proposed to Needs backport in Foxy Patch Release 5 Dec 11, 2020
@adamdbrw
Copy link

Is there a plan to backport this to Foxy? It is a real issue for some users. Is there anything I can do to help? @jacobperron @clalancette

@jacobperron
Copy link
Member Author

@adamdbrw Thanks for the ping. I think I was planning to backport this (along with other changes) to Foxy, but it got a bit gnarly with Git conflicts. I'm happy to review a backport PR if you'd like to open one. Keep in mind that we should stay API compatible.

@adamdbrw
Copy link

adamdbrw commented Apr 29, 2021

@jacobperron for me this patch applied on Foxy without conflicts. Perhaps after several backports the problem is solved already?

@jacobperron
Copy link
Member Author

@adamdbrw That's quite possible 🙂

hayato-m126 pushed a commit to tier4/launch_ros that referenced this pull request May 19, 2021
* Asynchronously wait for load node service response

Fixes ros2#171

By asychronously waiting for the service response, we can monitor if launch is shutting down and abandon the request so we don't block the shutdown process.

Signed-off-by: Jacob Perron <jacob@openrobotics.org>

* Add back debug log

It was accidentally removed.

Signed-off-by: Jacob Perron <jacob@openrobotics.org>
jacobperron added a commit that referenced this pull request May 25, 2021
* Asynchronously wait for load node service response

Fixes #171

By asychronously waiting for the service response, we can monitor if launch is shutting down and abandon the request so we don't block the shutdown process.

Signed-off-by: Jacob Perron <jacob@openrobotics.org>

* Add back debug log

It was accidentally removed.

Signed-off-by: Jacob Perron <jacob@openrobotics.org>
@jacobperron jacobperron removed this from Needs Backport in Dashing Patch Release 9 May 25, 2021
@jacobperron jacobperron removed this from Needs backport in Foxy Patch Release 5 May 25, 2021
@jacobperron
Copy link
Member Author

Foxy backport: #240

jacobperron added a commit that referenced this pull request May 27, 2021
* Asynchronously wait for load node service response

Fixes #171

By asychronously waiting for the service response, we can monitor if launch is shutting down and abandon the request so we don't block the shutdown process.

Signed-off-by: Jacob Perron <jacob@openrobotics.org>

* Add back debug log

It was accidentally removed.

Signed-off-by: Jacob Perron <jacob@openrobotics.org>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working
Projects
No open projects
Development

Successfully merging this pull request may close these issues.

LoadComposableNodes action can cause launch process to hang forever
3 participants