Conversation
This was referenced Aug 10, 2022
Closed
Member
Author
|
b40a655 is a refactor partially addressing the review in #990.
Commits reconciling these changes with rclcpp and rclpy are here: |
Member
Author
|
service_event_publisher qos is now exposed in |
jacobperron
reviewed
Aug 16, 2022
Member
jacobperron
left a comment
jacobperron
left a comment
There was a problem hiding this comment.
Partial review of the client code changes. Some of the comments apply to services as well.
clalancette
force-pushed
the
service_introspection
branch
2 times, most recently
from
484c73e to
07fb3ba
Compare
clalancette
force-pushed
the
service_introspection
branch
2 times, most recently
from
400b531 to
c12351c
Compare
Contributor
|
I'm in the middle of a rewrite here, so actually changing back to a draft for now. |
clalancette
force-pushed
the
service_introspection
branch
from
f79e6d2 to
db4ebe7
Compare
Contributor
|
This has been substantially rewritten now, so I'm going to dismiss all of the review comments, and pull this out of draft. |
This PR adds in the rcl implementation of service introspection. In particular, what it adds in are the implementations of enabling and disabling service introspection, as well as creating the publisher when the introspection is enabled. Signed-off-by: Brian Chen <brian.chen@openrobotics.org> Signed-off-by: Ivan Santiago Paunovic <ivanpauno@ekumenlabs.com> Signed-off-by: Chris Lalancette <clalancette@openrobotics.org>
The idea here is that by default, clients and services are created just like they were before. However, we add an additional API where the user can choose to configure service introspection, either to turn it OFF, send out METADATA, or send out CONTENTS (and METADATA). There are 4 different events that can get sent out if introspection is configured for METADATA or CONTENTS; REQUEST_SENT (from the client), REQUEST_RECEIVED (from the service), RESPONSE_SENT (from the service), or RESPONSE_RECEIVED (from the client). For each of these, a separate message is sent out a topic called <service_name>_service_event , so an outside observer can listen. Signed-off-by: Chris Lalancette <clalancette@gmail.com>
Signed-off-by: Chris Lalancette <clalancette@gmail.com>
Along the way, I noticed quite a few potential bugs, so this ends up mostly being a rewrite of the tests. With this in place, tests pass on all RMWs. Signed-off-by: Chris Lalancette <clalancette@gmail.com>
clalancette
force-pushed
the
service_introspection
branch
from
75ea33e to
fe72a35
Compare
Signed-off-by: Chris Lalancette <clalancette@gmail.com>
Signed-off-by: Chris Lalancette <clalancette@gmail.com>
mjcarroll
approved these changes
Feb 28, 2023
Contributor
|
I should mention that CI for this is in ros2/ros2#1285 (comment) |
danthony06
pushed a commit
to danthony06/rcl
that referenced
this pull request
Jun 14, 2023
* Add in the APIs to enable service introspection. This PR adds in the rcl implementation of service introspection. In particular, what it adds in are the implementations of enabling and disabling service introspection, as well as creating the publisher when the introspection is enabled. The idea here is that by default, clients and services are created just like they were before. However, we add an additional API where the user can choose to configure service introspection, either to turn it OFF, send out METADATA, or send out CONTENTS (and METADATA). There are 4 different events that can get sent out if introspection is configured for METADATA or CONTENTS; REQUEST_SENT (from the client), REQUEST_RECEIVED (from the service), RESPONSE_SENT (from the service), or RESPONSE_RECEIVED (from the client). For each of these, a separate message is sent out a topic called <service_name>_service_event , so an outside observer can listen. Signed-off-by: Brian Chen <brian.chen@openrobotics.org> Signed-off-by: Ivan Santiago Paunovic <ivanpauno@ekumenlabs.com> Signed-off-by: Chris Lalancette <clalancette@openrobotics.org>
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
6 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
This PR is part of a prototype implementation for ros-infrastructure/rep#360 ros2/ros2#1285.
Elements of note
service introspection logic is encapsulated in
introspection.{c.h}(file name subject to change) and hooks into the service init/fini and various send/take request/response functions.runtime configuration done via
rcl_service_introspection_configure_client_eventsetc. functions exposed to client library to be called on parameter event callbacksServices and headers have been broken up into
service_impl.handservice.hin line with what is done for publishers to improve dependency ergonomics with service introspection (and same for clients)Service event message creationg and population is delegated to typesupport libraries
rcl_{service,client}_options_tnow has a clock and enable_service_introspection member.Related issue: Service Introspection ros2#1285
Note: this PR replaces Service Introspection #990. Reviews left on that PR have yet be resolved as of time of creating this PR but will be resolved in this PR.