-
Notifications
You must be signed in to change notification settings - Fork 163
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
Service Introspection #990
Conversation
Signed-off-by: Brian Chen <[email protected]>
Signed-off-by: Brian Chen <[email protected]>
…shed Signed-off-by: Brian Chen <[email protected]>
Signed-off-by: Brian Chen <[email protected]>
Signed-off-by: Brian Chen <[email protected]>
Signed-off-by: Brian Chen <[email protected]>
I just took a quick look, and it looks like there is already an (build) indirect dependency between
|
Signed-off-by: Brian Chen <[email protected]>
Signed-off-by: Brian Chen <[email protected]>
…ervice introspection Signed-off-by: Brian Chen <[email protected]>
Signed-off-by: Brian Chen <[email protected]>
Signed-off-by: Brian Chen <[email protected]>
Signed-off-by: Brian Chen <[email protected]>
Signed-off-by: Brian Chen <[email protected]>
…tion configure Signed-off-by: Brian Chen <[email protected]>
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
partial review
Signed-off-by: Brian Chen <[email protected]>
Signed-off-by: Brian Chen <[email protected]>
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The approach looks good to me. Besides the string manipulation related to service typesupport, the code looks straight-forward 👍 I also agree with Ivan's comments.
I realize you haven't focused on error handling yet, but when you do you should look out for the following (which might help uncover bugs downstream):
- Always check the return values of functions (this isn't happening in many places)
- Make sure to do any necessary memory deallocation before returning from functions (pay attention to places where you are returning early due to an error).
- Check arguments to functions before using them (e.g. if they are null or invalid).
Closing in lieu of #997 to keep all service introspection work under a single branch name |
@ihasdapie I think you meant to close this PR (replaced with #997), so I'm closing it now. |
@ivanpauno Yup, thanks! I had forgotten. |
This PR is a prototype for the service introspection REP (ros-infrastructure/rep#360) implementing the proposed serialized service event approach.
Few items of note
rcl
onrosidl_typesupport_introspection_c
androsidl_typesupport_c
rcl/rcl/src/rcl/introspection.c
Lines 57 to 119 in 9c0b42e
client_id
is not populated on client send request events due to thewriter_guid
not being accessible from theclient_send_request
scope. PRs will be made to thermw
implementations to make this accessible.For instructions on how to build and run this see the meta-ticket ros2/ros2#1285
Resolves ros2/ros2#1285