Skip to content

Conversation

@fabianhirmann
Copy link
Contributor

Description

This small PR adds a const version of get_node_x_interface() which returns a const shared pointer.

Is this user-facing behavior change?

No

Did you use Generative AI?

No

Additional Information

This addition is required when the NodeInterfaces class is a member of a class and one then would like to use the NodeInterfaces class member in a const member function of that class to access at least const member functions of the separate node interfaces. Without this addition this is not possible at all and NodeInterfaces class members can only be accessed in non-const member functions.

It would also be great if this can be backported to older versions as well (especially in my case to Humble which gets just now the unified node interfaces backported with #3002).

…turns a const shared pointer which can be used in e.g. const member functions to access at least the const member functions of the node interfaces

Signed-off-by: Fabian Hirmann <[email protected]>
@fabianhirmann
Copy link
Contributor Author

Hi @ahcorde and @alsora,

Since you two were/are currently looking at my other PR #3002, I was hoping that you maybe could also look into that one too. If not, please let me know.

After #3002 is merged, it would be great if you could also trigger backporting this change to Humble using mergifyio (and probably to all other ROS versions which are having the NodeInterfaces class). If this is not possible, also let me know and I just manually backport it and create a PR.

Many thanks,
Fabian

@ahcorde
Copy link
Contributor

ahcorde commented Dec 11, 2025

Pulls: #3006
Gist: https://gist.githubusercontent.com/ahcorde/41627880c8bf08ed26b66e2f689519db/raw/d6189a15622704c13df24410a1a8e81b9beadeb9/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/17755

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

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.

@fabianhirmann this fails to build, you can see the build log here https://ci.ros2.org/job/ci_linux/26520/console.
guess that there are no such methods with signature generated by this macro?

@fabianhirmann
Copy link
Contributor Author

Thank you for your comments @ahcorde and @alsora! I found the issue at the build and locally it successfully built on rolling now. The issue was that in #2075 the method for directly creating NodeInterfaces from std::shared_ptr<NodeT> node has been removed and I only tested the changes on top of my other PR #3002. I'm sorry that I missed that.

The question for me now is if I should also backport #2075 to Humble after #3002 is completed (or as part of #3002) such that the constructors of NodeInterfaces are the same in Humble and all later releases or just ignore that change since I believe my newer commit b41fe0c fixes the original issue of #2075 (which was writing about build issues in RHEL, and Linux-rhel is now successfully building at #3002)?

@fabianhirmann fabianhirmann force-pushed the feature/unified-node-interfaces-add-const-get_node_x_interface branch from fe5b48e to 45f9c67 Compare December 12, 2025 09:48
@ahcorde
Copy link
Contributor

ahcorde commented Dec 12, 2025

Pulls: #3006
Gist: https://gist.githubusercontent.com/ahcorde/70ad6f43b18704b2306359d0e35b0e54/raw/d6189a15622704c13df24410a1a8e81b9beadeb9/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/17767

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

@fabianhirmann
Copy link
Contributor Author

@ahcorde Thank you for triggering another build. I saw now that it is failing but from the logs it looks like there is another error at the build which is unrelated to my changes and looks like a general CI issue. Therefore it would be great if you could have a look on it (or just trigger another build).

@ahcorde
Copy link
Contributor

ahcorde commented Dec 15, 2025

Pulls: #3006
Gist: https://gist.githubusercontent.com/ahcorde/c43f46797c5325eeeb85397746a6441a/raw/d6189a15622704c13df24410a1a8e81b9beadeb9/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/17792

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

@ahcorde ahcorde merged commit 825b4e4 into ros2:rolling Dec 16, 2025
2 of 3 checks passed
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.

4 participants