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

Add service_msgs package #143

Merged
merged 12 commits into from
Feb 2, 2023
Merged

Add service_msgs package #143

merged 12 commits into from
Feb 2, 2023

Conversation

ihasdapie
Copy link
Member

This PR is a prototype for the service introspection REP (ros-infrastructure/rep#360) adding a service_msgs package implementing the proposed embedded message approach (ros-infrastructure/rep#360 (comment)).

For instructions on how to build and run this see the meta-ticket ros2/ros2#1285.

Copy link
Member

@ivanpauno ivanpauno left a comment

Choose a reason for hiding this comment

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

LGTM!

do you already have a PR for generating the new messages for each service?

service_msgs/CHANGELOG.rst Outdated Show resolved Hide resolved
service_msgs/CMakeLists.txt Outdated Show resolved Hide resolved
service_msgs/QUALITY_DECLARATION.md Outdated Show resolved Hide resolved
service_msgs/QUALITY_DECLARATION.md Outdated Show resolved Hide resolved
service_msgs/QUALITY_DECLARATION.md Outdated Show resolved Hide resolved
service_msgs/QUALITY_DECLARATION.md Outdated Show resolved Hide resolved
service_msgs/README.md Outdated Show resolved Hide resolved
service_msgs/README.md Outdated Show resolved Hide resolved
service_msgs/README.md Outdated Show resolved Hide resolved
@ihasdapie ihasdapie force-pushed the service_introspection branch from ba74ac1 to 8c871b5 Compare August 18, 2022 22:42
@jacobperron jacobperron self-assigned this Sep 1, 2022
@jacobperron
Copy link
Member

rebased on rolling

@jacobperron jacobperron marked this pull request as ready for review September 29, 2022 21:39
Copy link
Member

@ivanpauno ivanpauno left a comment

Choose a reason for hiding this comment

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

LGTM, we should double check if using unique_identifier_msgs/UUID is fine or not before merging

service_msgs/msg/ServiceEventInfo.msg Outdated Show resolved Hide resolved
@ivanpauno ivanpauno force-pushed the service_introspection branch from eae4004 to d0c3096 Compare November 11, 2022 17:50
@ivanpauno
Copy link
Member

Rebased and added d0c3096, if not action_msgs doesn't build

@ivanpauno ivanpauno requested a review from wjwwood November 16, 2022 14:09
ihasdapie and others added 12 commits January 12, 2023 14:55
Signed-off-by: Brian Chen <[email protected]>
Signed-off-by: Jacob Perron <[email protected]>
* Add useful links to info about services and REP 2012
* Minor refactoring

Signed-off-by: Jacob Perron <[email protected]>
* Minor wording changes
* Remove/replace dead links

Signed-off-by: Jacob Perron <[email protected]>
* Add stricter compile options
* Add comment describing why we use 'core' generators instead of 'default'

Signed-off-by: Jacob Perron <[email protected]>
Technically, it is not a dependency. Only a dependency on actual action interfaces being generated.
This dependency should probably be exported by rosidl_defaults

Signed-off-by: Jacob Perron <[email protected]>
Signed-off-by: Ivan Santiago Paunovic <[email protected]>
That is, this field should be a GID, not a UUID (even though
they happen to be the same length).

Signed-off-by: Chris Lalancette <[email protected]>
@clalancette clalancette force-pushed the service_introspection branch from d0c3096 to b8d7674 Compare January 25, 2023 18:27
@clalancette clalancette requested a review from gbiggs as a code owner January 25, 2023 18:27
@clalancette
Copy link
Contributor

CI for this is in ros2/ros2#1285 (comment)

@clalancette clalancette merged commit 466e085 into rolling Feb 2, 2023
@delete-merged-branch delete-merged-branch bot deleted the service_introspection branch February 2, 2023 14:34
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.

7 participants