Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Update GraphCache for services & clients along with remaining graph introspection methods #90

Merged
merged 15 commits into from
Jan 17, 2024

Conversation

Yadunund
Copy link
Member

@Yadunund Yadunund commented Dec 28, 2023

This PR builds on top of #86 but with changes from #87. Reviewing might be easier if we merge #87 in and rebase #86.

Changes introduced:

  • Liveliness tokens are put/deleted for service and client entities and GraphCache is updated with this information.
  • Implement all remaining graph introspection functions.
  • Fixes liveliness tokens when node or topic names have / in the middle. Eg /add_two_ints_server/get_type_description. Previously such names would get appended to the liveliness token without any substitution which could cause the parsing and reconstruction of liveliness::Entity to have incorrect values for types and qos. Now a SLASH_REPLACEMENT char is introduced and such names and mangled and demangled to correctly transmit and reconstruct graph information.

To Test:

# terminal 1
  ros2 run demo_nodes_cpp add_two_ints_server
# terminal 2
ros2 service list

# result
/add_two_ints_server/set_parameters
/add_two_ints_server/set_parameters_atomically
/add_two_ints
/add_two_ints_server/get_type_description
/add_two_ints_server/get_parameter_types
/add_two_ints_server/list_parameters
/add_two_ints_server/get_parameters
/add_two_ints_server/describe_parameters
ros2 node info /add_two_ints

# result 
/add_two_ints_server
  Subscribers:
    /parameter_events: rcl_interfaces/msg/ParameterEvent
  Publishers:
    /rosout: rcl_interfaces/msg/Log
    /parameter_events: rcl_interfaces/msg/ParameterEvent
  Service Servers:
    /add_two_ints_server/get_type_description: type_description_interfaces/srv/GetTypeDescription
    /add_two_ints: example_interfaces/srv/AddTwoInts
    /add_two_ints_server/get_parameter_types: rcl_interfaces/srv/GetParameterTypes
    /add_two_ints_server/list_parameters: rcl_interfaces/srv/ListParameters
    /add_two_ints_server/get_parameters: rcl_interfaces/srv/GetParameters
    /add_two_ints_server/describe_parameters: rcl_interfaces/srv/DescribeParameters
    /add_two_ints_server/set_parameters_atomically: rcl_interfaces/srv/SetParametersAtomically
    /add_two_ints_server/set_parameters: rcl_interfaces/srv/SetParameters
  Service Clients:

  Action Servers:

  Action Clients:

Known issue (currently being investigated)

Starting 3 or more nodes causes the liveliness get query at node initialization to hang indefinitely.

@clalancette clalancette self-assigned this Jan 4, 2024
@delete-merged-branch delete-merged-branch bot deleted the branch rolling January 12, 2024 03:32
@Yadunund Yadunund changed the base branch from francocipollone/service_support_via_queryable to rolling January 12, 2024 03:33
Signed-off-by: Yadunund <yadunund@openrobotics.org>
Signed-off-by: Yadunund <yadunund@openrobotics.org>
Signed-off-by: Yadunund <yadunund@openrobotics.org>
Signed-off-by: Yadunund <yadunund@openrobotics.org>
Signed-off-by: Yadunund <yadunund@openrobotics.org>
Signed-off-by: Yadunund <yadunund@openrobotics.org>
Signed-off-by: Yadunund <yadunund@openrobotics.org>
Signed-off-by: Yadunund <yadunund@openrobotics.org>
Signed-off-by: Yadunund <yadunund@openrobotics.org>
Signed-off-by: Yadunund <yadunund@openrobotics.org>
Signed-off-by: Yadunund <yadunund@openrobotics.org>
Signed-off-by: Yadunund <yadunund@openrobotics.org>
Copy link
Collaborator

@clalancette clalancette left a comment

Choose a reason for hiding this comment

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

I've left a few things to think about, but overall I think this is pretty good. I have a whole bunch of fixes to go on top of this, so hopefully we can get this cleaned up and merged soon.

rmw_zenoh_cpp/src/detail/graph_cache.cpp Show resolved Hide resolved
rmw_zenoh_cpp/src/detail/graph_cache.cpp Outdated Show resolved Hide resolved
rmw_zenoh_cpp/src/detail/graph_cache.cpp Outdated Show resolved Hide resolved
rmw_zenoh_cpp/src/detail/liveliness_utils.hpp Outdated Show resolved Hide resolved
rmw_zenoh_cpp/src/detail/liveliness_utils.hpp Outdated Show resolved Hide resolved
@clalancette clalancette mentioned this pull request Jan 16, 2024
4 tasks
Yadunund and others added 3 commits January 17, 2024 10:18
Co-authored-by: Chris Lalancette <clalancette@gmail.com>
Signed-off-by: Yadu <yadunund@gmail.com>
Co-authored-by: Chris Lalancette <clalancette@gmail.com>
Signed-off-by: Yadu <yadunund@gmail.com>
Signed-off-by: Yadunund <yadunund@openrobotics.org>
@clalancette clalancette merged commit 9823c65 into rolling Jan 17, 2024
5 checks passed
@delete-merged-branch delete-merged-branch bot deleted the yadu/service_introspection branch January 17, 2024 15:56
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.

None yet

2 participants