Skip to content

Add qos parameter for wait_for_message function #2903

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

Merged

Conversation

ghanta1996
Copy link
Contributor

Description

A small change to include the QoS settings as one of the parameters for rclcpp::wait_for_message. This is useful in cases where we want just to get the last published message on the topic, rather than always waiting for a new message.

Fixes # (issue)

Is this user-facing behavior change?

Did you use Generative AI?

No

Additional Information

@ghanta1996 ghanta1996 force-pushed the feat/qos-profile-for-wait_for_message branch from 9e72fbc to 68ed3dd Compare July 12, 2025 02:33
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.

Can you also add the test for QoS optional argument?

@ghanta1996 ghanta1996 force-pushed the feat/qos-profile-for-wait_for_message branch 2 times, most recently from acb459f to 3b88d28 Compare July 13, 2025 07:53
@fujitatomoya
Copy link
Collaborator

@ghanta1996 there is another thing i should have mentioned before, can your retarget this PR against rolling? that is our development branch and if that fix can keep the API/ABI compatibility, we can consider the backport to the downstream distros. sorry i should have mentioned this 1st.

@ghanta1996 ghanta1996 changed the base branch from jazzy to rolling July 13, 2025 23:15
@ghanta1996 ghanta1996 force-pushed the feat/qos-profile-for-wait_for_message branch from 3b88d28 to 22b6109 Compare July 13, 2025 23:21
@ghanta1996
Copy link
Contributor Author

@fujitatomoya No worries. I have rebased my branch to rolling and updated the PR accordingly. Thanks

Signed-off-by: Sriharsha Ghanta <[email protected]>
Signed-off-by: Sriharsha Ghanta <[email protected]>
Signed-off-by: Sriharsha Ghanta <[email protected]>
Signed-off-by: Sriharsha Ghanta <[email protected]>

Signed-off-by: Sriharsha Ghanta <[email protected]>
Signed-off-by: Sriharsha Ghanta <[email protected]>
Signed-off-by: Sriharsha Ghanta <[email protected]>
Add test case for using QoS

Signed-off-by: Sriharsha Ghanta <[email protected]>
Signed-off-by: Sriharsha Ghanta <[email protected]>
Signed-off-by: Sriharsha Ghanta <[email protected]>
Signed-off-by: Sriharsha Ghanta <[email protected]>
Signed-off-by: Sriharsha Ghanta <[email protected]>
Signed-off-by: Sriharsha Ghanta <[email protected]>
Signed-off-by: Sriharsha Ghanta <[email protected]>
Signed-off-by: Sriharsha Ghanta <[email protected]>
Signed-off-by: Sriharsha Ghanta <[email protected]>
@ghanta1996 ghanta1996 force-pushed the feat/qos-profile-for-wait_for_message branch 2 times, most recently from a31cd3c to 87bc402 Compare July 14, 2025 02:00
Signed-off-by: Sriharsha Ghanta <[email protected]>
@ghanta1996 ghanta1996 force-pushed the feat/qos-profile-for-wait_for_message branch from 87bc402 to 117d00f Compare July 14, 2025 02:16
@ghanta1996 ghanta1996 requested a review from fujitatomoya July 14, 2025 02:35
Copy link
Contributor

@ahcorde ahcorde left a comment

Choose a reason for hiding this comment

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

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.

just a minor nitpick, and lgtm with @ahcorde 's comment resolved. thanks 👍

@@ -78,11 +79,13 @@ bool wait_for_message(
/// Wait for the next incoming message.
/**
* Wait for the next incoming message to arrive on a specified topic before the specified timeout.
*
* \param[out] out is the message to be filled when a new message is arriving.
* Specify the QoS settings for the subscription to control how messages are received.
Copy link
Collaborator

Choose a reason for hiding this comment

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

i would remove this comment, it is pretty much the duplication of qos parameter explanation in the main body.

Signed-off-by: Sriharsha Ghanta <[email protected]>
@ghanta1996 ghanta1996 requested a review from ahcorde July 14, 2025 16:06
Signed-off-by: Alejandro Hernandez Cordero <[email protected]>
@ahcorde
Copy link
Contributor

ahcorde commented Jul 14, 2025

Pulls: #2903
Gist: https://gist.githubusercontent.com/ahcorde/ed15e9d8cb0ee699f0da4c2e779588d4/raw/1e21fb39d53a36fc56f61494a103fab3e9d7b2bd/ros2.repos
BUILD args: --packages-above-and-dependencies rclcpp
TEST args: --packages-above rclcpp
ROS Distro: rolling
Job: ci_launcher
ci_launcher ran: https://ci.ros2.org/job/ci_launcher/16493

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

@ahcorde ahcorde merged commit 2fcef70 into ros2:rolling Jul 16, 2025
2 of 3 checks passed
@fujitatomoya
Copy link
Collaborator

The added parameter in the function signature is given a default value.
This means existing code that calls wait_for_message without specifying the qos parameter will continue to work exactly as before.
The function’s usage, binary compatibility (ABI), and source compatibility (API) are preserved.

I am gonna try to backport this fix to downstream distros.

@fujitatomoya
Copy link
Collaborator

@Mergifyio backport kilted jazzy humble

Copy link
Contributor

mergify bot commented Jul 16, 2025

backport kilted jazzy humble

✅ Backports have been created

mergify bot pushed a commit that referenced this pull request Jul 16, 2025
Signed-off-by: Sriharsha Ghanta <[email protected]>
Signed-off-by: Alejandro Hernandez Cordero <[email protected]>
Co-authored-by: Alejandro Hernandez Cordero <[email protected]>
(cherry picked from commit 2fcef70)
mergify bot pushed a commit that referenced this pull request Jul 16, 2025
Signed-off-by: Sriharsha Ghanta <[email protected]>
Signed-off-by: Alejandro Hernandez Cordero <[email protected]>
Co-authored-by: Alejandro Hernandez Cordero <[email protected]>
(cherry picked from commit 2fcef70)
mergify bot pushed a commit that referenced this pull request Jul 16, 2025
Signed-off-by: Sriharsha Ghanta <[email protected]>
Signed-off-by: Alejandro Hernandez Cordero <[email protected]>
Co-authored-by: Alejandro Hernandez Cordero <[email protected]>
(cherry picked from commit 2fcef70)
fujitatomoya pushed a commit that referenced this pull request Jul 16, 2025
(cherry picked from commit 2fcef70)

Signed-off-by: Sriharsha Ghanta <[email protected]>
Signed-off-by: Alejandro Hernandez Cordero <[email protected]>
Co-authored-by: Sriharsha Ghanta <[email protected]>
Co-authored-by: Alejandro Hernandez Cordero <[email protected]>
fujitatomoya pushed a commit that referenced this pull request Jul 16, 2025
(cherry picked from commit 2fcef70)

Signed-off-by: Sriharsha Ghanta <[email protected]>
Signed-off-by: Alejandro Hernandez Cordero <[email protected]>
Co-authored-by: Sriharsha Ghanta <[email protected]>
Co-authored-by: Alejandro Hernandez Cordero <[email protected]>
fujitatomoya pushed a commit that referenced this pull request Jul 17, 2025
(cherry picked from commit 2fcef70)

Signed-off-by: Sriharsha Ghanta <[email protected]>
Signed-off-by: Alejandro Hernandez Cordero <[email protected]>
Co-authored-by: Sriharsha Ghanta <[email protected]>
Co-authored-by: Alejandro Hernandez Cordero <[email protected]>
@ghanta1996 ghanta1996 deleted the feat/qos-profile-for-wait_for_message branch July 18, 2025 06:37
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.

3 participants