From 7da1faff84178fd7d407a4b08fcf7ada4c392bbc Mon Sep 17 00:00:00 2001 From: Jacob Perron Date: Tue, 20 Oct 2020 00:06:29 +0000 Subject: [PATCH 1/4] Remove redundant code generation The request and response messages are already generated as part of the srv template. Signed-off-by: Jacob Perron --- rosidl_generator_java/resource/action.cpp.em | 17 ----------------- 1 file changed, 17 deletions(-) diff --git a/rosidl_generator_java/resource/action.cpp.em b/rosidl_generator_java/resource/action.cpp.em index e6291779..b093b313 100644 --- a/rosidl_generator_java/resource/action.cpp.em +++ b/rosidl_generator_java/resource/action.cpp.em @@ -58,25 +58,8 @@ expand_template( data, output_file) -# Generate SendGoal message type -data.update({'msg': action.send_goal_service.request_message}) -output_file = os.path.join(output_dir, *namespaces[1:], '{0}.ep.{1}.cpp'.format(send_goal_type_name, typesupport_impl)) -expand_template( - 'msg.cpp.em', - data, - output_file) - -# Generate GetResult message type -data.update({'msg': action.get_result_service.request_message}) -output_file = os.path.join(output_dir, *namespaces[1:], '{0}.ep.{1}.cpp'.format(send_goal_type_name, typesupport_impl)) -expand_template( - 'msg.cpp.em', - data, - output_file) - data = { 'package_name': package_name, - 'interface_path': interface_path, 'output_dir': output_dir, 'template_basepath': template_basepath, 'typesupport_impl': typesupport_impl, From 50d2789764d7c55054805fb3187a3b5dc91fb1fa Mon Sep 17 00:00:00 2001 From: Jacob Perron Date: Tue, 20 Oct 2020 00:09:04 +0000 Subject: [PATCH 2/4] Strip action service suffixes from C include prefix The generated C headers for actions are included in a single header named after the action. Signed-off-by: Jacob Perron --- rosidl_generator_java/resource/msg.cpp.em | 10 ++++++++++ rosidl_generator_java/resource/srv.cpp.em | 15 ++++++++++++++- 2 files changed, 24 insertions(+), 1 deletion(-) diff --git a/rosidl_generator_java/resource/msg.cpp.em b/rosidl_generator_java/resource/msg.cpp.em index 9c868e60..fc7e764c 100644 --- a/rosidl_generator_java/resource/msg.cpp.em +++ b/rosidl_generator_java/resource/msg.cpp.em @@ -62,6 +62,7 @@ for member in message.structure.members: namespaced_types.add(get_jni_type(type_)) include_prefix = idl_structure_type_to_c_include_prefix(type_) # TODO(jacobperron): Remove this logic after https://github.com/ros2/rosidl/pull/432 (Foxy) + # and https://github.com/ros2/rosidl/pull/538 # Strip off any service or action suffix # There are several types that actions and services are composed of, but they are included # a common header that is based on the action or service name @@ -76,10 +77,15 @@ for member in message.structure.members: include_prefix = include_prefix[:-8] elif include_prefix.endswith('__feedback'): include_prefix = include_prefix[:-10] + elif include_prefix.endswith('__send_goal'): + include_prefix = include_prefix[:-11] + elif include_prefix.endswith('__get_result'): + include_prefix = include_prefix[:-12] member_includes.add(include_prefix + '.h') }@ @{ # TODO(jacobperron): Remove this logic after https://github.com/ros2/rosidl/pull/432 (Foxy) +# and https://github.com/ros2/rosidl/pull/538 message_c_include_prefix = idl_structure_type_to_c_include_prefix(message.structure.namespaced_type) # Strip off any service or action suffix if message_c_include_prefix.endswith('__request'): @@ -92,6 +98,10 @@ elif message_c_include_prefix.endswith('__result'): message_c_include_prefix = message_c_include_prefix[:-8] elif message_c_include_prefix.endswith('__feedback'): message_c_include_prefix = message_c_include_prefix[:-10] +elif message_c_include_prefix.endswith('__send_goal'): + message_c_include_prefix = message_c_include_prefix[:-11] +elif message_c_include_prefix.endswith('__get_result'): + message_c_include_prefix = message_c_include_prefix[:-12] }@ #include diff --git a/rosidl_generator_java/resource/srv.cpp.em b/rosidl_generator_java/resource/srv.cpp.em index c7bd40a6..7bf6209b 100644 --- a/rosidl_generator_java/resource/srv.cpp.em +++ b/rosidl_generator_java/resource/srv.cpp.em @@ -40,7 +40,20 @@ expand_template( #include "rosidl_generator_c/service_type_support_struct.h" -#include "@(idl_structure_type_to_c_include_prefix(service.namespaced_type)).h" +@{ +include_prefix = idl_structure_type_to_c_include_prefix(service.namespaced_type) +# TODO(jacobperron): Remove this logic after https://github.com/ros2/rosidl/pull/538 +# Strip off any service suffix +# There are a couple service types that actions are composed of, but they are included +# a common header that is based on the action name +# ie. there are not separate headers for each type +if include_prefix.endswith('__send_goal'): + include_prefix = include_prefix[:-11] +elif include_prefix.endswith('__get_result'): + include_prefix = include_prefix[:-12] +}@ + +#include "@(include_prefix).h" // Ensure that a jlong is big enough to store raw pointers static_assert(sizeof(jlong) >= sizeof(std::intptr_t), "jlong must be able to store pointers"); From e1a70cf145f8cbbafbe738b50e56b9fa2f72ef5e Mon Sep 17 00:00:00 2001 From: Jacob Perron Date: Tue, 20 Oct 2020 00:12:50 +0000 Subject: [PATCH 3/4] Generate Java code for SendGoal and GetResult service definitions Though not strictly necessary, it is nice to have definitions for these action-specific services for the purpose of writing unit tests. Signed-off-by: Jacob Perron --- rosidl_generator_java/resource/action.java.em | 18 ++++++++++++++++++ 1 file changed, 18 insertions(+) diff --git a/rosidl_generator_java/resource/action.java.em b/rosidl_generator_java/resource/action.java.em index 070ddd61..5e7ebbe9 100644 --- a/rosidl_generator_java/resource/action.java.em +++ b/rosidl_generator_java/resource/action.java.em @@ -13,6 +13,8 @@ type_name = action.namespaced_type.name goal_type_name = action.goal.structure.namespaced_type.name result_type_name = action.result.structure.namespaced_type.name feedback_type_name = action.feedback.structure.namespaced_type.name +send_goal_type_name = action.send_goal_service.namespaced_type.name +get_result_type_name = action.get_result_service.namespaced_type.name data = { 'package_name': package_name, @@ -44,6 +46,22 @@ expand_template( output_file, template_basepath=template_basepath) +data.update({'service': action.send_goal_service}) +output_file = os.path.join(output_dir, *namespaces[1:], send_goal_type_name + '.java') +expand_template( + 'srv.java.em', + data, + output_file, + template_basepath=template_basepath) + +data.update({'service': action.get_result_service}) +output_file = os.path.join(output_dir, *namespaces[1:], get_result_type_name + '.java') +expand_template( + 'srv.java.em', + data, + output_file, + template_basepath=template_basepath) + action_imports = [ 'org.ros2.rcljava.common.JNIUtils', 'org.ros2.rcljava.interfaces.ActionDefinition', From 86b4240b17c76e162ad5f8c452b19bd5a9e36a01 Mon Sep 17 00:00:00 2001 From: Jacob Perron Date: Tue, 20 Oct 2020 00:13:58 +0000 Subject: [PATCH 4/4] Compile generated service definitions for actions Previously, though we were generated service definitions for actions we were not compiling them. Signed-off-by: Jacob Perron --- .../rosidl_generator_java_generate_interfaces.cmake | 12 ++++++++++++ 1 file changed, 12 insertions(+) diff --git a/rosidl_generator_java/cmake/rosidl_generator_java_generate_interfaces.cmake b/rosidl_generator_java/cmake/rosidl_generator_java_generate_interfaces.cmake index 6ca9c9ff..31dc1a59 100644 --- a/rosidl_generator_java/cmake/rosidl_generator_java_generate_interfaces.cmake +++ b/rosidl_generator_java/cmake/rosidl_generator_java_generate_interfaces.cmake @@ -81,11 +81,23 @@ foreach(_abs_idl_file ${rosidl_generate_interfaces_ABS_IDL_FILES}) "${_output_path}/${_parent_folder}/${_idl_name}_Goal.java" "${_output_path}/${_parent_folder}/${_idl_name}_Result.java" "${_output_path}/${_parent_folder}/${_idl_name}_Feedback.java" + "${_output_path}/${_parent_folder}/${_idl_name}_SendGoal.java" + "${_output_path}/${_parent_folder}/${_idl_name}_SendGoal_Request.java" + "${_output_path}/${_parent_folder}/${_idl_name}_SendGoal_Response.java" + "${_output_path}/${_parent_folder}/${_idl_name}_GetResult.java" + "${_output_path}/${_parent_folder}/${_idl_name}_GetResult_Request.java" + "${_output_path}/${_parent_folder}/${_idl_name}_GetResult_Response.java" ) foreach(_typesupport_impl ${_typesupport_impls}) list(APPEND _generated_extension_${_typesupport_impl}_files "${_output_path}/${_parent_folder}/${_idl_name}_Goal.ep.${_typesupport_impl}.cpp") list(APPEND _generated_extension_${_typesupport_impl}_files "${_output_path}/${_parent_folder}/${_idl_name}_Result.ep.${_typesupport_impl}.cpp") list(APPEND _generated_extension_${_typesupport_impl}_files "${_output_path}/${_parent_folder}/${_idl_name}_Feedback.ep.${_typesupport_impl}.cpp") + list(APPEND _generated_extension_${_typesupport_impl}_files "${_output_path}/${_parent_folder}/${_idl_name}_SendGoal.ep.${_typesupport_impl}.cpp") + list(APPEND _generated_extension_${_typesupport_impl}_files "${_output_path}/${_parent_folder}/${_idl_name}_SendGoal_Request.ep.${_typesupport_impl}.cpp") + list(APPEND _generated_extension_${_typesupport_impl}_files "${_output_path}/${_parent_folder}/${_idl_name}_SendGoal_Response.ep.${_typesupport_impl}.cpp") + list(APPEND _generated_extension_${_typesupport_impl}_files "${_output_path}/${_parent_folder}/${_idl_name}_GetResult.ep.${_typesupport_impl}.cpp") + list(APPEND _generated_extension_${_typesupport_impl}_files "${_output_path}/${_parent_folder}/${_idl_name}_GetResult_Request.ep.${_typesupport_impl}.cpp") + list(APPEND _generated_extension_${_typesupport_impl}_files "${_output_path}/${_parent_folder}/${_idl_name}_GetResult_Response.ep.${_typesupport_impl}.cpp") endforeach() endif()