Skip to content

Add const to ServiceBase::get_service_name - #3253

Merged
ahcorde merged 1 commit into
ros2:rollingfrom
pum1k:fix/const-get-service-name
Sep 4, 2026
Merged

ahcorde merged 1 commit into
ros2:rollingfrom
pum1k:fix/const-get-service-name

Conversation

@pum1k

@pum1k pum1k commented Sep 2, 2026

Copy link
Copy Markdown
Contributor

Description

ServiceBase::get_service_name was not marked const, but I see no reason for it not to be.
(ClientBase::get_service_name is already const.)

Is this user-facing behavior change?

Yes, it allows calling the function in more contexts.

Did you use Generative AI?

No

Signed-off-by: pum1k <55055380+pum1k@users.noreply.github.com>

@fujitatomoya fujitatomoya left a comment

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

lgtm with green CI.

Important

this is ABI breaking change, not backporting.

@fujitatomoya

fujitatomoya commented Sep 3, 2026 •

Copy link
Copy Markdown
Collaborator

Pulls: #3253
Gist: https://gist.githubusercontent.com/fujitatomoya/24729d6252082c9a1422a3f625d21681/raw/942d05f0d7150ef77f7ac332372046beb718e2c8/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/20330

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

@ahcorde
ahcorde merged commit 26d08f2 into ros2:rolling Sep 4, 2026
2 checks passed
@pum1k
pum1k deleted the fix/const-get-service-name branch September 4, 2026 08:57
@pum1k

pum1k commented Sep 4, 2026

Copy link
Copy Markdown
Contributor Author

Would it be safe to create a backport that adds the const version as an overload or is that also forbidden?

const char * get_service_name();
const char * get_service_name() const;

@fujitatomoya

Copy link
Copy Markdown
Collaborator

we can do that, but i do not think that is necessary.

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