apply actual QoS from rmw to the IPC publisher. by fujitatomoya · Pull Request #2707 · ros2/rclcpp

fujitatomoya · 2024-12-13T08:10:34Z

closes #2705

note: backport required to jazzy and humble.

rclcpp/include/rclcpp/publisher.hpp

rclcpp/test/rclcpp/test_publisher.cpp

fujitatomoya · 2024-12-13T08:20:41Z

@alsora i would like to request the review.

clalancette

This is absolutely the right change, thanks for finding it @fujitatomoya . I left one thought inline.

clalancette · 2024-12-13T14:54:21Z

rclcpp/include/rclcpp/publisher.hpp

    (void)topic;
+    (void)qos;
    (void)options;


Sigh. I'm wondering whether we should deprecate this API signature, and just reduce this to:

virtual void post_init_setup(rclcpp::node_interfaces::NodeBaseInterface * node_base);

As far as I can tell, there are no downstream users of this API, and just continually setting this to (void) seems useless to me. Thoughts?

i am okay with that.

maybe we can refactor the following, but i would like to do that with another PR.

introduce intra_process_init non-virtual function under Publisher and Subscriber classes, and implement whatever needs to be setup for intra-process pub/sub.

deprecate post_init_setup for Publisher and Subscriber classes, Subscriber one is empty already. Or we can keep it for fully virtual function? so that user can override this to setup post process initialization?

fujitatomoya · 2024-12-13T18:01:42Z

Pulls: #2707
Gist: https://gist.githubusercontent.com/fujitatomoya/1864914d39a602bf8236970ff6d5d595/raw/1e4e0757031d664d082ecea21c6e706f36eee0b0/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/14955

Linux
Linux-aarch64
Linux-rhel
Windows

fujitatomoya · 2024-12-14T04:41:32Z

Pulls: #2707
Gist: https://gist.githubusercontent.com/fujitatomoya/77dd98edbbde2b1cf7e7afdf880e66d3/raw/1e4e0757031d664d082ecea21c6e706f36eee0b0/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/14961

Linux
Linux-aarch64
Linux-rhel
Windows

Signed-off-by: Tomoya Fujita <Tomoya.Fujita@sony.com>

fujitatomoya · 2024-12-14T23:29:39Z

Pulls: #2707
Gist: https://gist.githubusercontent.com/fujitatomoya/0f28f78e9f92d6bde8551ba4f679d785/raw/1e4e0757031d664d082ecea21c6e706f36eee0b0/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/14964

Linux
Linux-aarch64
Linux-rhel
Windows

fujitatomoya · 2024-12-16T05:46:26Z

@Mergifyio backport humble jazzy

mergify · 2024-12-16T05:46:50Z

backport humble jazzy

✅ Backports have been created

Details

#2711 apply actual QoS from rmw to the IPC publisher. (backport #2707) has been created for branch humble but encountered conflicts
#2712 apply actual QoS from rmw to the IPC publisher. (backport #2707) has been created for branch jazzy

* apply actual QoS from rmw to the IPC publisher. Signed-off-by: Tomoya Fujita <Tomoya.Fujita@sony.com> * address uncrustify warning. Signed-off-by: Tomoya Fujita <Tomoya.Fujita@sony.com> --------- Signed-off-by: Tomoya Fujita <Tomoya.Fujita@sony.com> (cherry picked from commit 016cfea) # Conflicts: # rclcpp/include/rclcpp/publisher.hpp

* apply actual QoS from rmw to the IPC publisher. Signed-off-by: Tomoya Fujita <Tomoya.Fujita@sony.com> * address uncrustify warning. Signed-off-by: Tomoya Fujita <Tomoya.Fujita@sony.com> --------- Signed-off-by: Tomoya Fujita <Tomoya.Fujita@sony.com> (cherry picked from commit 016cfea)

* apply actual QoS from rmw to the IPC publisher. Signed-off-by: Tomoya Fujita <Tomoya.Fujita@sony.com> * address uncrustify warning. Signed-off-by: Tomoya Fujita <Tomoya.Fujita@sony.com> --------- Signed-off-by: Tomoya Fujita <Tomoya.Fujita@sony.com> (cherry picked from commit 016cfea) Co-authored-by: Tomoya Fujita <Tomoya.Fujita@sony.com>

* apply actual QoS from rmw to the IPC publisher. (#2707) * apply actual QoS from rmw to the IPC publisher. Signed-off-by: Tomoya Fujita <Tomoya.Fujita@sony.com> * address uncrustify warning. Signed-off-by: Tomoya Fujita <Tomoya.Fujita@sony.com> --------- Signed-off-by: Tomoya Fujita <Tomoya.Fujita@sony.com> (cherry picked from commit 016cfea) # Conflicts: # rclcpp/include/rclcpp/publisher.hpp * resolve conflicts for backport humble. Signed-off-by: Tomoya Fujita <Tomoya.Fujita@sony.com> * address uncrustify failure. Signed-off-by: Tomoya Fujita <Tomoya.Fujita@sony.com> --------- Signed-off-by: Tomoya Fujita <Tomoya.Fujita@sony.com> Co-authored-by: Tomoya Fujita <Tomoya.Fujita@sony.com>

fujitatomoya mentioned this pull request Dec 13, 2024

SystemDefaultsQoS - force to have depth > 0 for Publishers #2705

Closed

fujitatomoya commented Dec 13, 2024

View reviewed changes

rclcpp/include/rclcpp/publisher.hpp Show resolved Hide resolved

rclcpp/test/rclcpp/test_publisher.cpp Show resolved Hide resolved

fujitatomoya requested a review from alsora December 13, 2024 08:20

fujitatomoya self-assigned this Dec 13, 2024

fujitatomoya added the bug Something isn't working label Dec 13, 2024

mjcarroll approved these changes Dec 13, 2024

View reviewed changes

clalancette reviewed Dec 13, 2024

View reviewed changes

apply actual QoS from rmw to the IPC publisher.

71a386d

Signed-off-by: Tomoya Fujita <Tomoya.Fujita@sony.com>

fujitatomoya force-pushed the fujitatomoya/ipc-should-apply-actual-qos branch from 94ba4a4 to d3d745d Compare December 14, 2024 23:26

address uncrustify warning.

9c4cdd0

Signed-off-by: Tomoya Fujita <Tomoya.Fujita@sony.com>

fujitatomoya force-pushed the fujitatomoya/ipc-should-apply-actual-qos branch from d3d745d to 9c4cdd0 Compare December 14, 2024 23:28

fujitatomoya merged commit 016cfea into rolling Dec 16, 2024

mergify bot mentioned this pull request Dec 16, 2024

apply actual QoS from rmw to the IPC publisher. (backport #2707) #2711

Merged

mergify bot mentioned this pull request Dec 16, 2024

apply actual QoS from rmw to the IPC publisher. (backport #2707) #2712

Merged

clalancette deleted the fujitatomoya/ipc-should-apply-actual-qos branch December 16, 2024 12:37

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

apply actual QoS from rmw to the IPC publisher.#2707

apply actual QoS from rmw to the IPC publisher.#2707
fujitatomoya merged 2 commits intorollingfrom
fujitatomoya/ipc-should-apply-actual-qos

fujitatomoya commented Dec 13, 2024 •

edited

Loading

Uh oh!

Uh oh!

Uh oh!

fujitatomoya commented Dec 13, 2024

Uh oh!

clalancette left a comment

Uh oh!

clalancette Dec 13, 2024

Uh oh!

fujitatomoya Dec 13, 2024

Uh oh!

fujitatomoya commented Dec 13, 2024

Uh oh!

fujitatomoya commented Dec 14, 2024 •

edited

Loading

Uh oh!

fujitatomoya commented Dec 14, 2024 •

edited

Loading

Uh oh!

fujitatomoya commented Dec 16, 2024

Uh oh!

mergify bot commented Dec 16, 2024 •

edited

Loading

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

3 participants

Conversation

fujitatomoya commented Dec 13, 2024 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

Uh oh!

Uh oh!

fujitatomoya commented Dec 13, 2024

Uh oh!

clalancette left a comment

Choose a reason for hiding this comment

Uh oh!

clalancette Dec 13, 2024

Choose a reason for hiding this comment

Uh oh!

fujitatomoya Dec 13, 2024

Choose a reason for hiding this comment

Uh oh!

fujitatomoya commented Dec 13, 2024

Uh oh!

fujitatomoya commented Dec 14, 2024 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

fujitatomoya commented Dec 14, 2024 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

fujitatomoya commented Dec 16, 2024

Uh oh!

mergify bot commented Dec 16, 2024 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

✅ Backports have been created

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

3 participants

fujitatomoya commented Dec 13, 2024 •

edited

Loading

fujitatomoya commented Dec 14, 2024 •

edited

Loading

fujitatomoya commented Dec 14, 2024 •

edited

Loading

mergify bot commented Dec 16, 2024 •

edited

Loading