From 8f8a75b41790c03a7d4807df1d5e94e08dcec48d Mon Sep 17 00:00:00 2001 From: Jacob Perron Date: Wed, 29 Jan 2020 16:53:10 -0800 Subject: [PATCH 1/7] Fix service and action interface generation Before, we were not generating code for the messages and services that make up service and action interfaces. Due to issues with duplicate definitions caused by instantiating the msg.cpp.em template multiple times, I've opted to generate separate files for each service, action, and the interfaces that they are made of. This is similar to what we are doing with the generated Java code. I've added a test confirming that generated service code can be used. Adding a test for actions is difficult at the moment due to a circular dependency with action_msgs. Signed-off-by: Jacob Perron --- rosidl_generator_java/CMakeLists.txt | 1 + ...l_generator_java_generate_interfaces.cmake | 9 ++ rosidl_generator_java/resource/action.cpp.em | 95 +++++++++++- rosidl_generator_java/resource/idl.cpp.em | 141 ++++++++---------- rosidl_generator_java/resource/msg.cpp.em | 92 ++++++++++-- rosidl_generator_java/resource/srv.cpp.em | 51 ++++++- .../rosidl_generator_java/__init__.py | 1 + .../org/ros2/generator/InterfacesTest.java | 69 +++++++++ 8 files changed, 352 insertions(+), 107 deletions(-) diff --git a/rosidl_generator_java/CMakeLists.txt b/rosidl_generator_java/CMakeLists.txt index 4b1c2ea5..1e6c8539 100644 --- a/rosidl_generator_java/CMakeLists.txt +++ b/rosidl_generator_java/CMakeLists.txt @@ -67,6 +67,7 @@ if(BUILD_TESTING) set(_deps_library_dirs "") list_append_unique(_deps_library_dirs ${CMAKE_CURRENT_BINARY_DIR}) list_append_unique(_deps_library_dirs ${CMAKE_CURRENT_BINARY_DIR}/rosidl_generator_java/rosidl_generator_java/msg/) + list_append_unique(_deps_library_dirs ${CMAKE_CURRENT_BINARY_DIR}/rosidl_generator_java/rosidl_generator_java/srv/) foreach(testsuite ${${PROJECT_NAME}_testsuites}) ament_add_junit_tests("${PROJECT_NAME}_tests_${testsuite}" 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 e0b0ff17..6ca9c9ff 100644 --- a/rosidl_generator_java/cmake/rosidl_generator_java_generate_interfaces.cmake +++ b/rosidl_generator_java/cmake/rosidl_generator_java_generate_interfaces.cmake @@ -70,6 +70,10 @@ foreach(_abs_idl_file ${rosidl_generate_interfaces_ABS_IDL_FILES}) "${_output_path}/${_parent_folder}/${_idl_name}_Request.java" "${_output_path}/${_parent_folder}/${_idl_name}_Response.java" ) + foreach(_typesupport_impl ${_typesupport_impls}) + list(APPEND _generated_extension_${_typesupport_impl}_files "${_output_path}/${_parent_folder}/${_idl_name}_Request.ep.${_typesupport_impl}.cpp") + list(APPEND _generated_extension_${_typesupport_impl}_files "${_output_path}/${_parent_folder}/${_idl_name}_Response.ep.${_typesupport_impl}.cpp") + endforeach() endif() # Actions generate extra files if(_parent_folder STREQUAL "action") @@ -78,6 +82,11 @@ foreach(_abs_idl_file ${rosidl_generate_interfaces_ABS_IDL_FILES}) "${_output_path}/${_parent_folder}/${_idl_name}_Result.java" "${_output_path}/${_parent_folder}/${_idl_name}_Feedback.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") + endforeach() endif() foreach(_typesupport_impl ${_typesupport_impls}) diff --git a/rosidl_generator_java/resource/action.cpp.em b/rosidl_generator_java/resource/action.cpp.em index 17e456bf..4b02caeb 100644 --- a/rosidl_generator_java/resource/action.cpp.em +++ b/rosidl_generator_java/resource/action.cpp.em @@ -1,23 +1,102 @@ @# Included from rosidl_generator_java/resource/idl.cpp.em @{ +import os + +from rosidl_cmake import expand_template from rosidl_generator_c import idl_structure_type_to_c_include_prefix -action_includes = [ +namespaces = action.namespaced_type.namespaces +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 +feedback_message_type_name = action.feedback_message.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, + 'output_dir': output_dir, + 'template_basepath': template_basepath, +} + +# Generate Goal message type +data.update({'message': action.goal}) +output_file = os.path.join( + output_dir, *namespaces[1:], '{0}.ep.{1}.cpp'.format(goal_type_name, typesupport_impl)) +expand_template( + 'msg.cpp.em', + data, + output_file) + +# Generate Result message type +data.update({'message': action.result}) +output_file = os.path.join( + output_dir, *namespaces[1:], '{0}.ep.{1}.cpp'.format(result_type_name, typesupport_impl)) +expand_template( + 'msg.cpp.em', + data, + output_file) + +# Generate Feedback message type +data.update({'message': action.feedback}) +output_file = os.path.join( + output_dir, *namespaces[1:], '{0}.ep.{1}.cpp'.format(feedback_type_name, typesupport_impl)) +expand_template( + 'msg.cpp.em', + data, + output_file) + +# Generate FeedbackMessage message type +data.update({'message': action.feedback_message}) +output_file = os.path.join( + output_dir, + *namespaces[1:], + '{0}.ep.{1}.cpp'.format(feedback_message_type_name, typesupport_impl)) +expand_template( + 'msg.cpp.em', + data, + output_file) + +# Generate SendGoal service type +data.update({'service': action.send_goal_service}) +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 SendGoal service type +data.update({'service': action.get_result_service}) +output_file = os.path.join( + output_dir, *namespaces[1:], '{0}.ep.{1}.cpp'.format(get_result_type_name, typesupport_impl)) +expand_template( + 'msg.cpp.em', + data, + output_file) + +jni_includes = [ + 'jni.h', +] + +rosidl_includes = [ 'rosidl_generator_c/action_type_support_struct.h', ] }@ -@[for include in action_includes]@ -@[ if include in include_directives]@ -// already included above -// @ -@[ else]@ -@{include_directives.add(include)}@ -@[ end if]@ +@[for include in jni_includes]@ +#include <@(include)> +@[end for]@ + +@[for include in rosidl_includes]@ #include "@(include)" @[end for]@ #include "@(idl_structure_type_to_c_include_prefix(action.namespaced_type)).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"); + #ifdef __cplusplus extern "C" { #endif diff --git a/rosidl_generator_java/resource/idl.cpp.em b/rosidl_generator_java/resource/idl.cpp.em index 48098dd9..a5d303d5 100644 --- a/rosidl_generator_java/resource/idl.cpp.em +++ b/rosidl_generator_java/resource/idl.cpp.em @@ -9,102 +9,93 @@ @# - package_name (string) @# - interface_path (Path relative to the directory named after the package) @# - content (IdlContent, list of elements, e.g. Messages or Services) +@# - output_dir (Path) +@# - template_basepath (Path) +@# - typesupport_impl (string, the typesupport identifier of the generated code) @####################################################################### @{ -include_directives = set() +import os -jni_includes = [ - 'jni.h', -] -include_directives.update(jni_includes) -std_includes = [ - 'cassert', - 'cstdint', - 'string', -] -include_directives.update(std_includes) -rosidl_includes = [ - 'rosidl_generator_c/message_type_support_struct.h', -] -include_directives.update(rosidl_includes) -rcljava_includes = [ - 'rcljava_common/exceptions.h', - 'rcljava_common/signatures.h', -] -include_directives.update(rcljava_includes) -}@ -@[for include in jni_includes]@ -#include <@(include)> -@[end for]@ - -@[for include in std_includes]@ -#include <@(include)> -@[end for]@ - -@[for include in rosidl_includes]@ -#include "@(include)" -@[end for]@ - -@[for include in rcljava_includes]@ -#include "@(include)" -@[end for]@ - -// 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 rosidl_cmake import expand_template +from rosidl_parser.definition import Action +from rosidl_parser.definition import Message +from rosidl_parser.definition import Service -using rcljava_common::exceptions::rcljava_throw_exception; +# jni_package_name = package_name.replace('_', '_1') -@{ -jni_package_name = package_name.replace('_', '_1') }@ @ @####################################################################### @# Handle messages @####################################################################### @{ -from rosidl_parser.definition import Message -}@ -@[for message in content.get_elements_of_type(Message)]@ -@{ -TEMPLATE( - 'msg.cpp.em', - package_name=package_name, - jni_package_name=jni_package_name, - message=message, - include_directives=include_directives) +data = { + 'package_name': package_name, + # 'jni_package_name': jni_package_name, + 'output_dir': output_dir, + 'template_basepath': template_basepath, +} + +for message in content.get_elements_of_type(Message): + data.update({'message': message}) + type_name = message.structure.namespaced_type.name + namespaces = message.structure.namespaced_type.namespaces + output_file = os.path.join( + output_dir, *namespaces[1:], '{0}.ep.{1}.cpp'.format(type_name, typesupport_impl)) + expand_template( + 'msg.cpp.em', + data, + output_file, + template_basepath=template_basepath) }@ -@[end for]@ @ @####################################################################### @# Handle services @####################################################################### @{ -from rosidl_parser.definition import Service -}@ -@[for service in content.get_elements_of_type(Service)]@ -@{ -TEMPLATE( - 'srv.cpp.em', - package_name=package_name, - jni_package_name=jni_package_name, - service=service, - include_directives=include_directives) +data = { + 'package_name': package_name, + 'interface_path': interface_path, + 'output_dir': output_dir, + 'template_basepath': template_basepath, + 'typesupport_impl': typesupport_impl, +} + +for service in content.get_elements_of_type(Service): + data.update({'service': service}) + type_name = service.namespaced_type.name + namespaces = service.namespaced_type.namespaces + output_file = os.path.join( + output_dir, *namespaces[1:], '{0}.ep.{1}.cpp'.format(type_name, typesupport_impl)) + expand_template( + 'srv.cpp.em', + data, + output_file, + template_basepath=template_basepath) + }@ -@[end for]@ @ @####################################################################### @# Handle actions @####################################################################### @{ -from rosidl_parser.definition import Action -}@ -@[for action in content.get_elements_of_type(Action)]@ -@{ -TEMPLATE( - 'action.cpp.em', - package_name=package_name, - jni_package_name=jni_package_name, - action=action, - include_directives=include_directives) +data = { + 'package_name': package_name, + 'interface_path': interface_path, + 'output_dir': output_dir, + 'template_basepath': template_basepath, + 'typesupport_impl': typesupport_impl, +} + +for action in content.get_elements_of_type(Action): + data.update({'action': action}) + type_name = action.namespaced_type.name + namespaces = action.namespaced_type.namespaces + output_file = os.path.join( + output_dir, *namespaces[1:], '{0}.ep.{1}.cpp'.format(type_name, typesupport_impl)) + expand_template( + 'action.cpp.em', + data, + output_file, + template_basepath=template_basepath) }@ -@[end for]@ diff --git a/rosidl_generator_java/resource/msg.cpp.em b/rosidl_generator_java/resource/msg.cpp.em index 6d1f7877..0ffb853e 100644 --- a/rosidl_generator_java/resource/msg.cpp.em +++ b/rosidl_generator_java/resource/msg.cpp.em @@ -19,6 +19,22 @@ from rosidl_parser.definition import Array from rosidl_parser.definition import BasicType from rosidl_parser.definition import NamespacedType +jni_includes = [ + 'jni.h', +] +std_includes = [ + 'cassert', + 'cstdint', + 'string', +] +rosidl_includes = [ + 'rosidl_generator_c/message_type_support_struct.h', +] +rcljava_includes = [ + 'rcljava_common/exceptions.h', + 'rcljava_common/signatures.h', +] + msg_normalized_type = '__'.join(message.structure.namespaced_type.namespaced_name()) msg_jni_type = '/'.join(message.structure.namespaced_type.namespaced_name()) @@ -35,7 +51,7 @@ array_list_jni_type = "java/util/ArrayList" cache = defaultdict(lambda: False) cache[msg_normalized_type] = msg_jni_type namespaced_types = set() -includes = set() +member_includes = set() for member in message.structure.members: type_ = member.type if isinstance(type_, AbstractNestedType): @@ -43,36 +59,80 @@ for member in message.structure.members: cache[array_list_normalized_type] = array_list_jni_type type_ = type_.value_type if isinstance(type_, BasicType): - includes.add('rosidl_generator_c/primitives_sequence.h') - includes.add('rosidl_generator_c/primitives_sequence_functions.h') + member_includes.add('rosidl_generator_c/primitives_sequence.h') + member_includes.add('rosidl_generator_c/primitives_sequence_functions.h') # We do not cache strings because java.lang.String behaves differently if not isinstance(type_, AbstractGenericString): cache[get_normalized_type(type_)] = get_jni_type(type_) if isinstance(type_, AbstractString): - includes.add('rosidl_generator_c/string.h') - includes.add('rosidl_generator_c/string_functions.h') + member_includes.add('rosidl_generator_c/string.h') + member_includes.add('rosidl_generator_c/string_functions.h') if isinstance(type_, AbstractWString): - includes.add('rosidl_generator_c/u16string.h') - includes.add('rosidl_generator_c/u16string_functions.h') + member_includes.add('rosidl_generator_c/u16string.h') + member_includes.add('rosidl_generator_c/u16string_functions.h') if isinstance(type_, NamespacedType): namespaced_types.add(get_jni_type(type_)) - includes.add(idl_structure_type_to_c_include_prefix(type_) + '.h') + include_prefix = idl_structure_type_to_c_include_prefix(type_) + # TODO(jacobperron): Remove this logic after https://github.com/ros2/rosidl/pull/432 (Foxy) + # Strip off any service or action suffix + if include_prefix.endswith('__request'): + include_prefix = include_prefix[:-9] + elif include_prefix.endswith('__response'): + include_prefix = include_prefix[:-10] + if include_prefix.endswith('__goal'): + include_prefix = include_prefix[:-6] + elif include_prefix.endswith('__result'): + include_prefix = include_prefix[:-8] + elif include_prefix.endswith('__feedback'): + include_prefix = include_prefix[:-10] + member_includes.add(include_prefix + '.h') }@ -@[for include in includes]@ -@[ if include in include_directives]@ -// already included above -// @ -@[ else]@ -@{include_directives.add(include)}@ -@[ end if]@ +@{ +# TODO(jacobperron): Remove this logic after https://github.com/ros2/rosidl/pull/432 (Foxy) +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'): + message_c_include_prefix = message_c_include_prefix[:-9] +elif message_c_include_prefix.endswith('__response'): + message_c_include_prefix = message_c_include_prefix[:-10] +if message_c_include_prefix.endswith('__goal'): + message_c_include_prefix = message_c_include_prefix[:-6] +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] +}@ + +@[for include in jni_includes]@ +#include <@(include)> +@[end for]@ + +@[for include in std_includes]@ +#include <@(include)> +@[end for]@ + +@[for include in rosidl_includes]@ #include "@(include)" @[end for]@ -#include "@(idl_structure_type_to_c_include_prefix(message.structure.namespaced_type)).h" +@[for include in rcljava_includes]@ +#include "@(include)" +@[end for]@ + +@[for include in member_includes]@ +#include "@(include)" +@[end for]@ + +#include "@(message_c_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"); + +using rcljava_common::exceptions::rcljava_throw_exception; #ifdef __cplusplus extern "C" { diff --git a/rosidl_generator_java/resource/srv.cpp.em b/rosidl_generator_java/resource/srv.cpp.em index 5c2d2d0b..a67e76a4 100644 --- a/rosidl_generator_java/resource/srv.cpp.em +++ b/rosidl_generator_java/resource/srv.cpp.em @@ -1,23 +1,58 @@ @# Included from rosidl_generator_java/resource/idl.cpp.em @{ +import os +from rosidl_cmake import expand_template from rosidl_generator_c import idl_structure_type_to_c_include_prefix -service_includes = [ +namespaces = service.namespaced_type.namespaces +type_name = service.namespaced_type.name +request_type_name = service.request_message.structure.namespaced_type.name +response_type_name = service.response_message.structure.namespaced_type.name + +data = { + 'package_name': package_name, + 'output_dir': output_dir, + 'template_basepath': template_basepath, +} + +# Generate request message +data.update({'message': service.request_message}) +output_file = os.path.join( + output_dir, *namespaces[1:], '{0}.ep.{1}.cpp'.format(request_type_name, typesupport_impl)) +expand_template( + 'msg.cpp.em', + data, + output_file) + +# Generate response message +data.update({'message': service.response_message}) +output_file = os.path.join( + output_dir, *namespaces[1:], '{0}.ep.{1}.cpp'.format(response_type_name, typesupport_impl)) +expand_template( + 'msg.cpp.em', + data, + output_file) + +jni_includes = [ + 'jni.h', +] +rosidl_includes = [ 'rosidl_generator_c/service_type_support_struct.h', ] }@ -@[for include in service_includes]@ -@[ if include in include_directives]@ -// already included above -// @ -@[ else]@ -@{include_directives.add(include)}@ -@[ end if]@ +@[for include in jni_includes]@ +#include <@(include)> +@[end for]@ + +@[for include in rosidl_includes]@ #include "@(include)" @[end for]@ #include "@(idl_structure_type_to_c_include_prefix(service.namespaced_type)).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"); + #ifdef __cplusplus extern "C" { #endif diff --git a/rosidl_generator_java/rosidl_generator_java/__init__.py b/rosidl_generator_java/rosidl_generator_java/__init__.py index 7941ab69..b87d9714 100644 --- a/rosidl_generator_java/rosidl_generator_java/__init__.py +++ b/rosidl_generator_java/rosidl_generator_java/__init__.py @@ -47,6 +47,7 @@ def generate_java(generator_arguments_file, typesupport_impls): mapping = { 'idl.cpp.em': '%s.ep.{0}.cpp'.format(impl), } + additional_context.update(typesupport_impl=impl) generate_files( generator_arguments_file, mapping, diff --git a/rosidl_generator_java/src/test/java/org/ros2/generator/InterfacesTest.java b/rosidl_generator_java/src/test/java/org/ros2/generator/InterfacesTest.java index dfea4a97..00638e09 100644 --- a/rosidl_generator_java/src/test/java/org/ros2/generator/InterfacesTest.java +++ b/rosidl_generator_java/src/test/java/org/ros2/generator/InterfacesTest.java @@ -532,4 +532,73 @@ public final void testUnboundedSequences() { assertEquals(42, unbounded_seq.getAlignmentCheck()); } + + @Test + public final void testBasicTypesService() { + rosidl_generator_java.srv.BasicTypes_Request basicTypesRequest = + new rosidl_generator_java.srv.BasicTypes_Request(); + rosidl_generator_java.srv.BasicTypes_Response basicTypesResponse = + new rosidl_generator_java.srv.BasicTypes_Response(); + // Set request fields + boolean expectedBool1 = true; + basicTypesRequest.setBoolValue(expectedBool1); + byte expectedByte1 = 123; + basicTypesRequest.setByteValue(expectedByte1); + byte expectedChar1 = 'a'; + basicTypesRequest.setCharValue(expectedChar1); + float expectedFloat1 = 12.34f; + basicTypesRequest.setFloat32Value(expectedFloat1); + double expectedDouble1 = 12.34; + basicTypesRequest.setFloat64Value(expectedDouble1); + byte expectedInt81 = 123; + basicTypesRequest.setInt8Value(expectedInt81); + short expectedInt161 = 1230; + basicTypesRequest.setInt16Value(expectedInt161); + int expectedInt321 = 123000; + basicTypesRequest.setInt32Value(expectedInt321); + long expectedInt641 = 42949672960L; + basicTypesRequest.setInt64Value(expectedInt641); + + // Set response fields + boolean expectedBool2 = false; + basicTypesResponse.setBoolValue(expectedBool2); + byte expectedByte2 = -42; + basicTypesResponse.setByteValue(expectedByte2); + byte expectedChar2 = ' '; + basicTypesResponse.setCharValue(expectedChar2); + float expectedFloat2 = -43.21f; + basicTypesResponse.setFloat32Value(expectedFloat2); + double expectedDouble2 = -43.21; + basicTypesResponse.setFloat64Value(expectedDouble2); + byte expectedInt82 = -42; + basicTypesResponse.setInt8Value(expectedInt82); + short expectedInt162 = -420; + basicTypesResponse.setInt16Value(expectedInt162); + int expectedInt322 = -42000; + basicTypesResponse.setInt32Value(expectedInt322); + long expectedInt642 = -4200000L; + basicTypesResponse.setInt64Value(expectedInt642); + + // Get request fields + assertEquals(expectedBool1, basicTypesRequest.getBoolValue()); + assertEquals(expectedByte1, basicTypesRequest.getByteValue()); + assertEquals(expectedChar1, basicTypesRequest.getCharValue()); + assertEquals(expectedFloat1, basicTypesRequest.getFloat32Value(), 0.01f); + assertEquals(expectedDouble1, basicTypesRequest.getFloat64Value(), 0.01); + assertEquals(expectedInt81, basicTypesRequest.getInt8Value()); + assertEquals(expectedInt161, basicTypesRequest.getInt16Value()); + assertEquals(expectedInt321, basicTypesRequest.getInt32Value()); + assertEquals(expectedInt641, basicTypesRequest.getInt64Value()); + + // Get response fields + assertEquals(expectedBool2, basicTypesResponse.getBoolValue()); + assertEquals(expectedByte2, basicTypesResponse.getByteValue()); + assertEquals(expectedChar2, basicTypesResponse.getCharValue()); + assertEquals(expectedFloat2, basicTypesResponse.getFloat32Value(), 0.01f); + assertEquals(expectedDouble2, basicTypesResponse.getFloat64Value(), 0.01); + assertEquals(expectedInt82, basicTypesResponse.getInt8Value()); + assertEquals(expectedInt162, basicTypesResponse.getInt16Value()); + assertEquals(expectedInt322, basicTypesResponse.getInt32Value()); + assertEquals(expectedInt642, basicTypesResponse.getInt64Value()); + } } From 6196b041828a262de095eea538a64cad43dce4cd Mon Sep 17 00:00:00 2001 From: Jacob Perron Date: Thu, 6 Feb 2020 15:58:01 -0800 Subject: [PATCH 2/7] Add missing header include Signed-off-by: Jacob Perron --- rosidl_generator_java/resource/action.cpp.em | 8 +++++++- rosidl_generator_java/resource/srv.cpp.em | 7 +++++++ 2 files changed, 14 insertions(+), 1 deletion(-) diff --git a/rosidl_generator_java/resource/action.cpp.em b/rosidl_generator_java/resource/action.cpp.em index 4b02caeb..dc9ba943 100644 --- a/rosidl_generator_java/resource/action.cpp.em +++ b/rosidl_generator_java/resource/action.cpp.em @@ -79,7 +79,9 @@ expand_template( jni_includes = [ 'jni.h', ] - +std_includes = [ + 'cstdint', +] rosidl_includes = [ 'rosidl_generator_c/action_type_support_struct.h', ] @@ -88,6 +90,10 @@ rosidl_includes = [ #include <@(include)> @[end for]@ +@[for include in std_includes]@ +#include <@(include)> +@[end for]@ + @[for include in rosidl_includes]@ #include "@(include)" @[end for]@ diff --git a/rosidl_generator_java/resource/srv.cpp.em b/rosidl_generator_java/resource/srv.cpp.em index a67e76a4..8ad9fcc7 100644 --- a/rosidl_generator_java/resource/srv.cpp.em +++ b/rosidl_generator_java/resource/srv.cpp.em @@ -36,6 +36,9 @@ expand_template( jni_includes = [ 'jni.h', ] +std_includes = [ + 'cstdint', +] rosidl_includes = [ 'rosidl_generator_c/service_type_support_struct.h', ] @@ -44,6 +47,10 @@ rosidl_includes = [ #include <@(include)> @[end for]@ +@[for include in std_includes]@ +#include <@(include)> +@[end for]@ + @[for include in rosidl_includes]@ #include "@(include)" @[end for]@ From 00260bfbc1c830c3528b9e77370508a42201e5fc Mon Sep 17 00:00:00 2001 From: Jacob Perron Date: Thu, 6 Feb 2020 16:00:40 -0800 Subject: [PATCH 3/7] Rename top level generated cpp file This avoids name clashing with other generated files. Similar to what we do with generated Java files. Signed-off-by: Jacob Perron --- rosidl_generator_java/rosidl_generator_java/__init__.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/rosidl_generator_java/rosidl_generator_java/__init__.py b/rosidl_generator_java/rosidl_generator_java/__init__.py index b87d9714..a025b2d8 100644 --- a/rosidl_generator_java/rosidl_generator_java/__init__.py +++ b/rosidl_generator_java/rosidl_generator_java/__init__.py @@ -45,7 +45,7 @@ def generate_java(generator_arguments_file, typesupport_impls): for impl in typesupport_impls: mapping = { - 'idl.cpp.em': '%s.ep.{0}.cpp'.format(impl), + 'idl.cpp.em': '_%s.cpp', } additional_context.update(typesupport_impl=impl) generate_files( From efaf468bfd4bbceba067815444f1b472afab87d7 Mon Sep 17 00:00:00 2001 From: Jacob Perron Date: Thu, 6 Feb 2020 16:02:02 -0800 Subject: [PATCH 4/7] Fix JNI name mangling function so it works for service and action subtypes For example, 'example_interfaces/srv/AddTwoInts_Request' should be mangled to 'example_1interfaces_srv_AddTwoInts_1Request'. Signed-off-by: Jacob Perron --- rosidl_generator_java/rosidl_generator_java/__init__.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/rosidl_generator_java/rosidl_generator_java/__init__.py b/rosidl_generator_java/rosidl_generator_java/__init__.py index a025b2d8..32a8e366 100644 --- a/rosidl_generator_java/rosidl_generator_java/__init__.py +++ b/rosidl_generator_java/rosidl_generator_java/__init__.py @@ -204,4 +204,4 @@ def get_jni_signature(type_): def get_jni_mangled_name(fully_qualified_name): # JNI name mangling: # https://docs.oracle.com/javase/8/docs/technotes/guides/jni/spec/design.html#resolving_native_method_names - return fully_qualified_name[0].replace('_', '_1') + '_' + '_'.join(fully_qualified_name[1:]) + return '_'.join(list(map(lambda name: name.replace('_', '_1'), fully_qualified_name))) From 29d1eace3b64b2e8b09109fb9ced7aafe0dc0da9 Mon Sep 17 00:00:00 2001 From: Jacob Perron Date: Wed, 4 Mar 2020 09:50:46 -0800 Subject: [PATCH 5/7] Remove vestigal references to jni_package_name Signed-off-by: Jacob Perron --- rosidl_generator_java/resource/idl.cpp.em | 3 --- 1 file changed, 3 deletions(-) diff --git a/rosidl_generator_java/resource/idl.cpp.em b/rosidl_generator_java/resource/idl.cpp.em index a5d303d5..c909006a 100644 --- a/rosidl_generator_java/resource/idl.cpp.em +++ b/rosidl_generator_java/resource/idl.cpp.em @@ -21,8 +21,6 @@ from rosidl_parser.definition import Action from rosidl_parser.definition import Message from rosidl_parser.definition import Service -# jni_package_name = package_name.replace('_', '_1') - }@ @ @####################################################################### @@ -31,7 +29,6 @@ from rosidl_parser.definition import Service @{ data = { 'package_name': package_name, - # 'jni_package_name': jni_package_name, 'output_dir': output_dir, 'template_basepath': template_basepath, } From 97d0ead336db948eef2a7d8584289fa0dd4bfbbc Mon Sep 17 00:00:00 2001 From: Jacob Perron Date: Wed, 4 Mar 2020 10:01:29 -0800 Subject: [PATCH 6/7] Add comment about action and service headers Signed-off-by: Jacob Perron --- rosidl_generator_java/resource/msg.cpp.em | 3 +++ 1 file changed, 3 insertions(+) diff --git a/rosidl_generator_java/resource/msg.cpp.em b/rosidl_generator_java/resource/msg.cpp.em index 0ffb853e..1ec71b4f 100644 --- a/rosidl_generator_java/resource/msg.cpp.em +++ b/rosidl_generator_java/resource/msg.cpp.em @@ -79,6 +79,9 @@ for member in message.structure.members: include_prefix = idl_structure_type_to_c_include_prefix(type_) # TODO(jacobperron): Remove this logic after https://github.com/ros2/rosidl/pull/432 (Foxy) # 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 + # ie. there are not separate headers for each type if include_prefix.endswith('__request'): include_prefix = include_prefix[:-9] elif include_prefix.endswith('__response'): From dc5d64da86749d35093c6b771e2ca7dd9038f2fb Mon Sep 17 00:00:00 2001 From: Jacob Perron Date: Wed, 4 Mar 2020 10:06:10 -0800 Subject: [PATCH 7/7] Simplify include logic Signed-off-by: Jacob Perron --- rosidl_generator_java/resource/action.cpp.em | 23 +++---------- rosidl_generator_java/resource/msg.cpp.em | 35 ++++---------------- rosidl_generator_java/resource/srv.cpp.em | 23 +++---------- 3 files changed, 15 insertions(+), 66 deletions(-) diff --git a/rosidl_generator_java/resource/action.cpp.em b/rosidl_generator_java/resource/action.cpp.em index dc9ba943..1fcdd7fb 100644 --- a/rosidl_generator_java/resource/action.cpp.em +++ b/rosidl_generator_java/resource/action.cpp.em @@ -75,28 +75,13 @@ expand_template( 'msg.cpp.em', data, output_file) - -jni_includes = [ - 'jni.h', -] -std_includes = [ - 'cstdint', -] -rosidl_includes = [ - 'rosidl_generator_c/action_type_support_struct.h', -] }@ -@[for include in jni_includes]@ -#include <@(include)> -@[end for]@ -@[for include in std_includes]@ -#include <@(include)> -@[end for]@ +#include + +#include -@[for include in rosidl_includes]@ -#include "@(include)" -@[end for]@ +#include "rosidl_generator_c/action_type_support_struct.h" #include "@(idl_structure_type_to_c_include_prefix(action.namespaced_type)).h" diff --git a/rosidl_generator_java/resource/msg.cpp.em b/rosidl_generator_java/resource/msg.cpp.em index 1ec71b4f..4907c5d0 100644 --- a/rosidl_generator_java/resource/msg.cpp.em +++ b/rosidl_generator_java/resource/msg.cpp.em @@ -19,22 +19,6 @@ from rosidl_parser.definition import Array from rosidl_parser.definition import BasicType from rosidl_parser.definition import NamespacedType -jni_includes = [ - 'jni.h', -] -std_includes = [ - 'cassert', - 'cstdint', - 'string', -] -rosidl_includes = [ - 'rosidl_generator_c/message_type_support_struct.h', -] -rcljava_includes = [ - 'rcljava_common/exceptions.h', - 'rcljava_common/signatures.h', -] - msg_normalized_type = '__'.join(message.structure.namespaced_type.namespaced_name()) msg_jni_type = '/'.join(message.structure.namespaced_type.namespaced_name()) @@ -110,21 +94,16 @@ elif message_c_include_prefix.endswith('__feedback'): message_c_include_prefix = message_c_include_prefix[:-10] }@ -@[for include in jni_includes]@ -#include <@(include)> -@[end for]@ +#include -@[for include in std_includes]@ -#include <@(include)> -@[end for]@ +#include +#include +#include -@[for include in rosidl_includes]@ -#include "@(include)" -@[end for]@ +#include "rosidl_generator_c/message_type_support_struct.h" -@[for include in rcljava_includes]@ -#include "@(include)" -@[end for]@ +#include "rcljava_common/exceptions.h" +#include "rcljava_common/signatures.h" @[for include in member_includes]@ #include "@(include)" diff --git a/rosidl_generator_java/resource/srv.cpp.em b/rosidl_generator_java/resource/srv.cpp.em index 8ad9fcc7..c7bd40a6 100644 --- a/rosidl_generator_java/resource/srv.cpp.em +++ b/rosidl_generator_java/resource/srv.cpp.em @@ -32,28 +32,13 @@ expand_template( 'msg.cpp.em', data, output_file) - -jni_includes = [ - 'jni.h', -] -std_includes = [ - 'cstdint', -] -rosidl_includes = [ - 'rosidl_generator_c/service_type_support_struct.h', -] }@ -@[for include in jni_includes]@ -#include <@(include)> -@[end for]@ -@[for include in std_includes]@ -#include <@(include)> -@[end for]@ +#include + +#include -@[for include in rosidl_includes]@ -#include "@(include)" -@[end for]@ +#include "rosidl_generator_c/service_type_support_struct.h" #include "@(idl_structure_type_to_c_include_prefix(service.namespaced_type)).h"