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 introspection typesupport tests for C/C++ services #653

Merged
merged 1 commit into from
Jan 28, 2022

Conversation

hidmic
Copy link
Contributor

@hidmic hidmic commented Jan 26, 2022

Precisely what the title says, adding to #651 , to be rebased and merged after it.

@delete-merged-branch delete-merged-branch bot deleted the branch master January 28, 2022 14:02
@hidmic hidmic force-pushed the hidmic/service_introspection_tests branch from 0eb2b67 to feb5ff9 Compare January 28, 2022 14:07
@hidmic hidmic changed the base branch from hidmic/rosidl_typesupport_introspection_tests to master January 28, 2022 14:07
@hidmic
Copy link
Contributor Author

hidmic commented Jan 28, 2022

CI up to rosidl_typesupport_introspection_tests:

  • Linux Build Status
  • Linux-aarch64 Build Status
  • Windows Build Status

Signed-off-by: Michel Hidalgo <michel@ekumenlabs.com>
@hidmic hidmic force-pushed the hidmic/service_introspection_tests branch from feb5ff9 to 5d3afac Compare January 28, 2022 14:19
Copy link
Contributor

@j-rivero j-rivero left a comment

Choose a reason for hiding this comment

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

One minor question, looks pretty similar to the work done in #651

DEFINE_CXX_API_FOR_C_SERVICE(rosidl_typesupport_introspection_tests, srv, Arrays)
DEFINE_CXX_API_FOR_C_SERVICE(rosidl_typesupport_introspection_tests, srv, BasicTypes)
DEFINE_CXX_API_FOR_C_SERVICE(rosidl_typesupport_introspection_tests, srv, Empty)

Copy link
Contributor

Choose a reason for hiding this comment

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

Out of curiosity, what is the reason not to include the same types in srv than the ones used in msg? Only Arrays, BasicTypes and Empty.

Copy link
Contributor Author

@hidmic hidmic Jan 28, 2022

Choose a reason for hiding this comment

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

@j-rivero only these exist. See https://github.com/ros2/test_interface_files/tree/master/srv.

We're covered by message tests though, you can already see tests are quite similar once you get to service request/response messages.

@hidmic hidmic merged commit 1612b07 into master Jan 28, 2022
@delete-merged-branch delete-merged-branch bot deleted the hidmic/service_introspection_tests branch January 28, 2022 16:10
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