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

Generate service header including request and response message #97

Merged
merged 1 commit into from
Mar 24, 2016

Conversation

jacquelinekay
Copy link
Contributor

Connects to ros2/rcl#27

Bit of a bikeshed question, should srv.h be a pass-through header that only includes files, and should there additionally be a srv__type_support header with the ROSIDL_GET_TYPESUPPORT_FUNCTION definition?

@jacquelinekay jacquelinekay added the in progress Actively being worked on (Kanban column) label Feb 24, 2016
@jacquelinekay jacquelinekay self-assigned this Feb 24, 2016
@jacquelinekay jacquelinekay mentioned this pull request Feb 24, 2016
5 tasks
@jacquelinekay
Copy link
Contributor Author

need to make some modifications so that services work with introspection C.

@jacquelinekay
Copy link
Contributor Author

huh, still not working, I think I need to change generator target dependencies

@jacquelinekay jacquelinekay force-pushed the c_services branch 2 times, most recently from daacc06 to 9bdd75f Compare March 4, 2016 20:47
@jacquelinekay
Copy link
Contributor Author

Should work now

@jacquelinekay jacquelinekay force-pushed the c_services branch 2 times, most recently from 624995c to 7998697 Compare March 9, 2016 23:36
"${_output_path}/srv/rosidl_generator_c__visibility_control.h")
configure_file(
"${rosidl_generator_c_TEMPLATE_DIR}/rosidl_generator_c__visibility_control.h.in"
"${_srv_visibility_control_file}"
Copy link
Member

Choose a reason for hiding this comment

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

Services don't need a separate visibility control file.

@jacquelinekay
Copy link
Contributor Author

@dirk-thomas This is ready for re-review.

This is a recent CI job (the lint errors were fixed in a subsequent commit): http://ci.ros2.org/job/ci_windows/1171/

@@ -107,14 +109,15 @@ add_custom_command(
)

# generate header to switch between export and import for a specific package
set(_visibility_control_file
set(_msg_visibility_control_file
Copy link
Member

Choose a reason for hiding this comment

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

Please avoid the rename.

@jacquelinekay
Copy link
Contributor Author

I've addressed all new comments so far.

@dirk-thomas
Copy link
Member

LGTM.

Does any code use this? Aka does it make sense to trigger CI?

@jacquelinekay
Copy link
Contributor Author

There is code in the pending rcl pull request with the same branch name that uses this.

@jacquelinekay jacquelinekay removed the in progress Actively being worked on (Kanban column) label Mar 24, 2016
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