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

Add how-to page for callback groups #2652

Merged
merged 15 commits into from May 27, 2022

Conversation

tanelikor
Copy link
Contributor

Based on the discussion in ROS Discourse,
converted the post into documentation from and added it as a how-to page.

Also added couple short rclpy-side notes on the Callback Groups section of About Executors page, but only on the callback group section for now.

@ros-discourse
Copy link

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

https://discourse.ros.org/t/how-to-use-callback-groups-in-ros2/25255/8

Copy link
Contributor

@ralph-lange ralph-lange left a comment

Choose a reason for hiding this comment

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

Very nice write-up. Thank you very much! I propose minor changes only.

Note: I first considered extending the “Avoiding deadlocks” section by a discussion of the tradeoff between conservative configurations (one or few callback groups with risk of deadlocks and poor utilization of computing resources) vs. optimistic configurations (multiple callback groups or reentrant callback groups with risk of data races), but there is much less to write about the data race risk and it’s already touched in the previous section. Therefore, let’s keep the last section focused on deadlocks and synchronous calls as it is.

source/How-To-Guides/Using-callback-groups.rst Outdated Show resolved Hide resolved
source/How-To-Guides/Using-callback-groups.rst Outdated Show resolved Hide resolved
source/How-To-Guides/Using-callback-groups.rst Outdated Show resolved Hide resolved
source/How-To-Guides/Using-callback-groups.rst Outdated Show resolved Hide resolved
source/How-To-Guides/Using-callback-groups.rst Outdated Show resolved Hide resolved
source/How-To-Guides/Using-callback-groups.rst Outdated Show resolved Hide resolved
source/How-To-Guides/Using-callback-groups.rst Outdated Show resolved Hide resolved
source/How-To-Guides/Using-callback-groups.rst Outdated Show resolved Hide resolved
source/How-To-Guides/Using-callback-groups.rst Outdated Show resolved Hide resolved
source/How-To-Guides/Using-callback-groups.rst Outdated Show resolved Hide resolved
tanelikor and others added 9 commits May 25, 2022 10:32
Co-authored-by: Ralph Lange <ralph-lange@users.noreply.github.com>
Co-authored-by: Ralph Lange <ralph-lange@users.noreply.github.com>
Co-authored-by: Ralph Lange <ralph-lange@users.noreply.github.com>
Co-authored-by: Ralph Lange <ralph-lange@users.noreply.github.com>
Co-authored-by: Ralph Lange <ralph-lange@users.noreply.github.com>
Co-authored-by: Ralph Lange <ralph-lange@users.noreply.github.com>
Co-authored-by: Ralph Lange <ralph-lange@users.noreply.github.com>
Co-authored-by: Ralph Lange <ralph-lange@users.noreply.github.com>
Co-authored-by: Ralph Lange <ralph-lange@users.noreply.github.com>
Copy link
Contributor

@ralph-lange ralph-lange left a comment

Choose a reason for hiding this comment

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

LGTM

source/How-To-Guides/Using-callback-groups.rst Outdated Show resolved Hide resolved
@tanelikor
Copy link
Contributor Author

Thanks for the review and suggestions. I committed the suggested changes.

Copy link
Collaborator

@fujitatomoya fujitatomoya left a comment

Choose a reason for hiding this comment

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

I believe this explains really well. lgtm with minor nitpicks.

source/How-To-Guides/Using-callback-groups.rst Outdated Show resolved Hide resolved
source/How-To-Guides/Using-callback-groups.rst Outdated Show resolved Hide resolved
tanelikor and others added 3 commits May 26, 2022 11:27
Co-authored-by: Tomoya Fujita <Tomoya.Fujita@sony.com>
Signed-off-by: Chris Lalancette <clalancette@openrobotics.org>
Copy link
Contributor

@clalancette clalancette left a comment

Choose a reason for hiding this comment

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

In short, this is fantastic. Thanks so much for doing this.

I pushed one small cleanup commit, since we don't need a redirect for a new page. Other than that, this looks great so I'll approve and once CI comes back clean, I'll go ahead and merge it in.

@clalancette clalancette added the backport-all backport at reviewers discretion; from rolling to all versions label May 27, 2022
@clalancette clalancette merged commit e68f2c5 into ros2:rolling May 27, 2022
mergify bot pushed a commit that referenced this pull request May 27, 2022
* callback groups how-to page

Co-authored-by: Ralph Lange <ralph-lange@users.noreply.github.com>
Co-authored-by: Tomoya Fujita <Tomoya.Fujita@sony.com>
Co-authored-by: tanelikor <taneli.korhonen@karelics.fi>
Co-authored-by: Chris Lalancette <clalancette@openrobotics.org>
(cherry picked from commit e68f2c5)
mergify bot pushed a commit that referenced this pull request May 27, 2022
* callback groups how-to page

Co-authored-by: Ralph Lange <ralph-lange@users.noreply.github.com>
Co-authored-by: Tomoya Fujita <Tomoya.Fujita@sony.com>
Co-authored-by: tanelikor <taneli.korhonen@karelics.fi>
Co-authored-by: Chris Lalancette <clalancette@openrobotics.org>
(cherry picked from commit e68f2c5)
mergify bot pushed a commit that referenced this pull request May 27, 2022
* callback groups how-to page

Co-authored-by: Ralph Lange <ralph-lange@users.noreply.github.com>
Co-authored-by: Tomoya Fujita <Tomoya.Fujita@sony.com>
Co-authored-by: tanelikor <taneli.korhonen@karelics.fi>
Co-authored-by: Chris Lalancette <clalancette@openrobotics.org>
(cherry picked from commit e68f2c5)
clalancette pushed a commit that referenced this pull request May 27, 2022
* callback groups how-to page

Co-authored-by: Ralph Lange <ralph-lange@users.noreply.github.com>
Co-authored-by: Tomoya Fujita <Tomoya.Fujita@sony.com>
Co-authored-by: tanelikor <taneli.korhonen@karelics.fi>
Co-authored-by: Chris Lalancette <clalancette@openrobotics.org>
(cherry picked from commit e68f2c5)

Co-authored-by: Taneli Korhonen <81684354+tanelikor@users.noreply.github.com>
clalancette pushed a commit that referenced this pull request May 27, 2022
* callback groups how-to page

Co-authored-by: Ralph Lange <ralph-lange@users.noreply.github.com>
Co-authored-by: Tomoya Fujita <Tomoya.Fujita@sony.com>
Co-authored-by: tanelikor <taneli.korhonen@karelics.fi>
Co-authored-by: Chris Lalancette <clalancette@openrobotics.org>
(cherry picked from commit e68f2c5)

Co-authored-by: Taneli Korhonen <81684354+tanelikor@users.noreply.github.com>
clalancette pushed a commit that referenced this pull request May 27, 2022
* callback groups how-to page

Co-authored-by: Ralph Lange <ralph-lange@users.noreply.github.com>
Co-authored-by: Tomoya Fujita <Tomoya.Fujita@sony.com>
Co-authored-by: tanelikor <taneli.korhonen@karelics.fi>
Co-authored-by: Chris Lalancette <clalancette@openrobotics.org>
(cherry picked from commit e68f2c5)

Co-authored-by: Taneli Korhonen <81684354+tanelikor@users.noreply.github.com>
@tanelikor tanelikor deleted the callback_groups_how-to branch June 1, 2022 08:38
@tonynajjar
Copy link
Contributor

tonynajjar commented May 10, 2023

Thanks a lot for this tutorial. In addition to this I attempted to create a "decision table" internally for my company and I wonder if you would find this useful to somehow insert in the tutorial:

image

Additionally, I really like the last section "Examples" and think we could add more scenarios:

image
If you agree that these additions are useful, I could create a PR. Let me know.

P.S: this is the result of my personal experimentation so there might be mistakes, happy to get corrections!

@clalancette
Copy link
Contributor

Please feel free to open a new PR; we can iterate there.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
backport-all backport at reviewers discretion; from rolling to all versions
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

6 participants