feat: Support hidden services that are autogenerated by actions #804
Conversation
|
It seems like the rolling CI is failing due to unreleased nav2_msgs. Since they are only used for testing and the package I just knew had more "complex" names, I can happily replace those with any other package or just add a rosbridge_test_interfaces for this sole purpose. |
| @@ -37,6 +37,7 @@ | |||
| <test_depend>example_interfaces</test_depend> | |||
| <test_depend>geometry_msgs</test_depend> | |||
| <test_depend>nav_msgs</test_depend> | |||
| <test_depend>nav2_msgs</test_depend> | |||
There was a problem hiding this comment.
You can add some action definitions to rosbridge_test_msgs. That way we avoid an extra test dependency.
There was a problem hiding this comment.
sure thing. I can't tell how I missed the package.
| if len(splits) == 4 and splits[1] == "action": | ||
| # Special case for hidden services that are autogenerated for actions | ||
| return (splits[0], splits[3]) |
There was a problem hiding this comment.
I think this is the only required change to allow loading hidden service definitions. If I do that, I can successfully load a action service from the client.
echo '{ "op": "advertise_service", "type": "test_msgs/action/_nested_message/NestedMessage_SendGoal", "service": "foo" }' | websocat ws://localhost:9090 --no-close
Note however, that the client needs to specify the explicit path of the service (test_msgs/action/NestedMessage_SendGoal would not work), but I think this is acceptable for now?
We could probably combine the logic and write it simpler like so:
if len(splits) >= 3:
return (splits[0], splits[-1])
There was a problem hiding this comment.
I think for advertise service this is easier since the client (roslibjs in our case) needs to specify a type and the AdvertiseService Capability takes the type into consideration.
For calling a service however the type is automatically determined by the rclpy implementation and a given type argument would get ignored (https://github.com/RobotWebTools/rosbridge_suite/blob/ros2/rosbridge_library/src/rosbridge_library/internal/services.py#L109) . So it should either be optional to specify a type for service calls or a way to work around the rcl issue. (Best case would be that it gets fixed in rclpy or upstream, I couldn't pinpoint the issue yet)
Pros for making it optional for the call_service operation as well:
- the implementation won't implicitly fix up/modify the type after determination
- decreased complexity in guarding this one special case
- decreased complexity to automatically determine the "_snake_cased" service type
One big con:
- the type argument is not part of the Protocol for call_service: https://github.com/RobotWebTools/rosbridge_suite/blob/ros2/ROSBRIDGE_PROTOCOL.md#336-call-service
Also (but this shouldn't affect any implementation) a lot of the current test cases cover things like "////invalid_name" where with
if len(splits >=2): # taking your suggestion one step further
return (splits[0], splits[-1])
all of those won't raise InvalidTypeStringException anymore and another way to determine an invalid service name is needed
There was a problem hiding this comment.
Good point, make sense.
Best would be indeed if we could fix the underlying rclpy/rcl issue.
I'm not very familiar with ROS2 actions, but I quickly tried running the example action server but ros2 service --include-hidden-services list -t does not list any action related services 🤔
There was a problem hiding this comment.
It doesn't? Thats strange. For me running ros2 run action_tutorials_cpp fibonacci_action_server in one terminal and in another one ros2 service --include-hidden-services list -t yields
/fibonacci/_action/cancel_goal [action_msgs/srv/CancelGoal]
/fibonacci/_action/get_result [action_tutorials_interfaces/action/Fibonacci_GetResult]
/fibonacci/_action/send_goal [action_tutorials_interfaces/action/Fibonacci_SendGoal]
/fibonacci_action_server/describe_parameters [rcl_interfaces/srv/DescribeParameters]
/fibonacci_action_server/get_parameter_types [rcl_interfaces/srv/GetParameterTypes]
/fibonacci_action_server/get_parameters [rcl_interfaces/srv/GetParameters]
/fibonacci_action_server/list_parameters [rcl_interfaces/srv/ListParameters]
/fibonacci_action_server/set_parameters [rcl_interfaces/srv/SetParameters]
/fibonacci_action_server/set_parameters_atomically [rcl_interfaces/srv/SetParametersAtomically]
(excerpt from ros2 doctor --report:
PLATFORM INFORMATION
system : Linux
platform info : Linux-5.4.0-126-generic-x86_64-with-glibc2.29
release : 5.4.0-126-generic
processor : x86_64
RMW MIDDLEWARE
middleware name : rmw_fastrtps_cpp
ROS 2 INFORMATION
distribution name : foxy
distribution type : ros2
distribution status : active
release platforms : {'ubuntu': ['focal']}
There was a problem hiding this comment.
here it is already "obvious" that even the ros2 cli command returns the same non-existing service type (without the "_Fibonacci" that is necessary.
Unfortunately I think (am kind of afraid of) that fixing the issue in rclpy/rcl might be really hard (waiting for releases, building everything as source overlay until then, ..). Nonetheless for us we found a working solution (using my fork) so it isn't that urgent. Not sure about the feasibility of that for everybody else though 😄
There was a problem hiding this comment.
I'm in a humble docker container and it seems that the action services are not listed anymore with --include-hidden-services
ros2 service --include-hidden-services list -t
root@1052752a55e6:/opt/ros2_ws# ros2 service --include-hidden-services list -t /fibonacci_action_server/describe_parameters [rcl_interfaces/srv/DescribeParameters] /fibonacci_action_server/get_parameter_types [rcl_interfaces/srv/GetParameterTypes] /fibonacci_action_server/get_parameters [rcl_interfaces/srv/GetParameters] /fibonacci_action_server/list_parameters [rcl_interfaces/srv/ListParameters] /fibonacci_action_server/set_parameters [rcl_interfaces/srv/SetParameters] /fibonacci_action_server/set_parameters_atomically [rcl_interfaces/srv/SetParametersAtomically]
ros2 doctor --report
PLATFORM INFORMATION system : Linux platform info : Linux-5.15.0-48-generic-x86_64-with-glibc2.35 release : 5.15.0-48-generic processor : x86_64QOS COMPATIBILITY LIST
topic [type] : /parameter_events [rcl_interfaces/msg/ParameterEvent]
publisher node : fibonacci_action_server
subscriber node : fibonacci_action_server
compatibility status : OK
topic [type] : /parameter_events [rcl_interfaces/msg/ParameterEvent]
publisher node : _ros2cli_daemon_0_f0e1bafb876f436eb7416ed601db5de3
subscriber node : fibonacci_action_server
compatibility status : OKRMW MIDDLEWARE
middleware name : rmw_fastrtps_cppROS 2 INFORMATION
distribution name : humble
distribution type : ros2
distribution status : active
release platforms : {'debian': ['bullseye'], 'rhel': ['8'], 'ubuntu': ['jammy']}TOPIC LIST
topic : /fibonacci/_action/feedback
publisher count : 1
subscriber count : 0
topic : /fibonacci/_action/status
publisher count : 1
subscriber count : 0
Unfortunately I think (am kind of afraid of) that fixing the issue in rclpy/rcl might be really hard (waiting for releases, building everything as source overlay until then, ..). Nonetheless for us we found a working solution (using my fork) so it isn't that urgent. Not sure about the feasibility of that for everybody else though smile
Would be great if you could open an issue at rclpy and report the issue there. The PR seems a little hacky, but I'm OK with merging it if another reviewer approves.
There was a problem hiding this comment.
It might be an issue/design inconsistency/faulty expectation with how rosidl_python creates the python implementations for the action types. I will check with rclcpp and link the resulting issue in this PR as well. Especially if the workaround is necessary for humble as well (might just another reason to upgrade)
|
any updates on this? |
|
This PR has been marked as stale because there has been no activity in the past 6 months. Please add a comment to keep it open. |
Public API Changes
None
Description
In ROS 2 the underlying communication channels for actions are services for canceling goals, sending goals and getting the result. Those autogenerated services don't match the usual conventions of using
srvassubnamebut for an actionActionNamesubnametakes the formaction._action_name.To allow roslibjs or other clients to send goals to action servers or getting results, this PR adds this "hidden" subname component for all services that are intended to be actions. This is guarded by a) the initial subname actually being
actionand b) the service type having one of the suffixes {_GetResult,_SendGoal}.This should fix #697
Side Notes
Unfortunately I'm not sure where the issue is rooted. We noticed that
ros2 service --include-hidden-services list -tfor actions already outputs the "wrong" service type for the two affected suffixes, so it seems to be rooted deep in rclpy if not rcl itself. Even with proper autodetection bynode_handle.get_service_names_and_types()those changes (in parts) would be necessary.The dependency on nav2_msgs was added because they provide an action with more than one word (compared to "Fibonacci" from the existing example_interfaces