Expose remapped service name for service clients

[Bjoernolav]

Feature request

Expose remapped service name for service clients

I'm writing a node connecting to two services. For the sake of generality, I'm using a name in the private namespace of the node, and intend to use remapping to connect to the actual service. Furthermore, I'm using wait_for_service to make sure that the service is available before continuing.

In the case where the service is unavailable, I would like to inform the user that the service was unavailable, and also including the remapped name in the log statement. It seems like it is not possible to get the remapped name for service clients yet?

Example:

import rclpy
from rclpy.node import Node
from std_srvs.srv import Trigger

class TestNode(Node):
    def __init__(self, **kwargs) -> None:
        if 'node_name' not in kwargs:
            kwargs['node_name'] = 'test_node'
        super().__init__(**kwargs)
        self.client = self.create_client(Trigger, '~/service_name')
        if not self.client.wait_for_service(1.0):
            msg = f'No connection to service: {self.client.srv_name}'
            self.get_logger().fatal(msg)
            raise RuntimeError(msg)
        ....

Running this using ros2 run pkg_name exec_name --ros_args -r '~/service_name:=/test', it seems like self.client.srv_name returns the original service name, and not the remapped one.

rclpy.publisher.Publisher implements the topic_name property which seems to resolve the name. Is it possible to do the same for service clients?

[fujitatomoya]

How about using this one?

https://github.com/ros2/rclpy/blob/5c5393d96cbcebea768916aa38dd12298695da94/rclpy/rclpy/node.py#L1228-L1240

[Bjoernolav]

Oops, missed that one - thanks!

But is there a reason why the Client implementation does not expose a service_name property in the same way as the Publisher class exposes a topic_name property? I guess it would be nice for them to expose similar APIs if possible.

[amalnanavati]

Adding onto the comment about similar APIs, whereas create_publisher and create_subscription call resolve_topic_name by default, create_client and create_service do not call resolve_service_name by default. I'd say this is unexpected functionality. Imho, when someone calls the default functions to create topics/services from a node, I think it's reasonable to expect that both topic names and service names will be remapped.

[fujitatomoya]

@amalnanavati good eye. thanks for pointing that out :+1:

@Bjoernolav https://github.com/ros2/rclpy/pull/1156 should fix the problem, it would be appreciated if you can try.

[amalnanavati]

Hmm, my setup is not particularly amenable to testing this, because I neither build rclpy from source nor use ROS Rolling (I use Humble).

I did look through the code though, and I don't think it resolves the issue I raised. node.py still directly passed on the name given to create_service, without first calling resolve_service_name. So the parameter that users pass doesn't get remapped.

[Barry-Xu-2018]

I did look through the code though, and I don't think it resolves the issue I raised. node.py still directly passed on the name given to create_service, without first calling resolve_service_name. So the parameter that users pass doesn't get remapped.

I explain the change in #1156. This patch introduces a new property service_name in service & client class to get remapped service name. srv_name keep service name input by users. So you can use service_name instead of srv_name.

[amalnanavati]

EDIT:

I ran the below test and it worked. I think the "bug" that I noticed above must have been caused by something else. I'll look more into my code and create another issue if necessary. Sorry for the bother!

PRE-EDIT:

@Barry-Xu-2018 I understand that you created a property to let users of the service/client class more easily access the remapped name. But as far as I can tell, it doesn’t change the name that a service/client created by node.create_service and node.create_client actually listens on.

Here is a test to illustrate what I am saying. Create the minimal rclpy service as documented here. Then run:

• ros2 run py_srvcli service —ros-args —remap add_two_ints:=remapped_add_two_ints
• ros2 service list

Based on my understanding of your code (and the issue I have been seeing on my side), I believe the service you see in the list will still be add_two_ints, although it should be remapped_add_two_ints. The same issue also exists for clients. Imo this should get addressed by adding a resolve_service_name call in node.create_service and node.create_client, like is done with node.create_subscription and node.create_publisher.

So while I recognize the value of your patch, and it does address the original issue raised in this thread, I don’t think it addresses the issue I raised. I’d be happy to open a separate Issue, so that this can be closed with #1156 .

[Bjoernolav]

@Bjoernolav #1156 should fix the problem, it would be appreciated if you can try.

Verified that the client.service_name returns the remapped name as expected using #1156. Thanks for solving this!

[fujitatomoya]

@Bjoernolav thanks for checking!

[fujitatomoya]

@amalnanavati

But as far as I can tell, it doesn’t change the name that a service/client created by node.create_service and node.create_client actually listens on.

Can you elaborate what this listens on is supposed to mean?

Here is a test to illustrate what I am saying. Create the minimal rclpy service as documented here. Then run: Based on my understanding of your code (and the issue I have been seeing on my side), I believe the service you see in the list will still be add_two_ints, although it should be remapped_add_two_ints.

service name is remapped into remapped_add_two_ints. this is expected behavior. those names are collected via ROS 2 graph, so it is irrelevant to the PR.

root@tomoyafujita:~/ros2_ws/colcon_ws# ros2 run demo_nodes_cpp add_two_ints_server --ros-args --remap add_two_ints:=remapped_add_two_ints

root@tomoyafujita:~/ros2_ws/colcon_ws# ros2 service list
/add_two_ints_server/describe_parameters
/add_two_ints_server/get_parameter_types
/add_two_ints_server/get_parameters
/add_two_ints_server/get_type_description
/add_two_ints_server/list_parameters
/add_two_ints_server/set_parameters
/add_two_ints_server/set_parameters_atomically
/remapped_add_two_ints

w/o remapping,

root@tomoyafujita:~/ros2_ws/colcon_ws# ros2 run demo_nodes_cpp add_two_ints_serve

root@tomoyafujita:~/ros2_ws/colcon_ws# ros2 service list
/add_two_ints
/add_two_ints_server/describe_parameters
/add_two_ints_server/get_parameter_types
/add_two_ints_server/get_parameters
/add_two_ints_server/get_type_description
/add_two_ints_server/list_parameters
/add_two_ints_server/set_parameters
/add_two_ints_server/set_parameters_atomically

please feel free to open the issue, we can also discuss on your possible problem there.

[amalnanavati]

@fujitatomoya I agree, I also ran the above tests and it worked as expected.

The "issue" I was facing was not a bug in rclpy, it was merely mistakes on my part, caused by a combination of:

1. I may not have rebuilt after adding a remap to the launchfile 🤦🏾‍♂️
2. I was logging the srv_name attribute of a Service which led me to think the name was not getting remapped. That is addressed by #1156 -- as long as I log service_name instead it should be fine :)

Once again, sorry for the bother!

[fujitatomoya]

@amalnanavati no worries, it is always nice to check. thanks for the reply!

[fujitatomoya]

https://github.com/ros2/rclpy/pull/1156 is merged, i will go ahead to close this.