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.

Sign up for GitHub

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

Jump to bottom

Update rosidl_generator_java for Dashing compatibility #66

Merged
merged 14 commits into from
Jan 10, 2020
Merged

Update rosidl_generator_java for Dashing compatibility #66

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
14 commits
Select commit Hold shift + click to select a range
2de9069
Update rosidl_generator_java for new IDL pipeline
jacobperron Sep 25, 2019
430c70d
Fix name JNI name mangling
jacobperron Sep 25, 2019
58f81f0
Remove java compile flags
jacobperron Sep 25, 2019
d2c4b6e
Refactor to support services
jacobperron Sep 25, 2019
0926d5e
Wide string support
jacobperron Sep 25, 2019
77a762e
Avoid duplicate includes
jacobperron Sep 26, 2019
b9d74d9
Use test_interface_files
jacobperron Sep 26, 2019
7e9bb7f
Support for actions
jacobperron Sep 26, 2019
c0cc2fc
Add suffix for long literals
jacobperron Sep 26, 2019
edab2cc
Handle unsigned literals
jacobperron Sep 26, 2019
8a7e102
Fix escape string function
jacobperron Sep 26, 2019
bbb4c3a
Use cast to workaround integer literals
jacobperron Sep 26, 2019
49b303c
Minor refactor
jacobperron Oct 1, 2019
b59c5b2
Remove TODO
jacobperron Dec 4, 2019
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
1 change: 1 addition & 0 deletions rcljava_common/CMakeLists.txt
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -36,6 +36,7 @@ set(${PROJECT_NAME}_java_sources
"src/main/java/org/ros2/rcljava/common/JNIUtils.java"
"src/main/java/org/ros2/rcljava/exceptions/RCLException.java"
"src/main/java/org/ros2/rcljava/exceptions/RCLReturn.java"
"src/main/java/org/ros2/rcljava/interfaces/ActionDefinition.java"
"src/main/java/org/ros2/rcljava/interfaces/Disposable.java"
"src/main/java/org/ros2/rcljava/interfaces/MessageDefinition.java"
"src/main/java/org/ros2/rcljava/interfaces/ServiceDefinition.java"
Expand Down
18 changes: 18 additions & 0 deletions rcljava_common/src/main/java/org/ros2/rcljava/interfaces/ActionDefinition.java
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,18 @@
/* Copyright 2019 Open Source Robotics Foundation, Inc.
Copy link
Member

Choose a reason for hiding this comment

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

Would it be ok with OSRF if the copyright notice is the same as the rest of the files?

Copy link

Choose a reason for hiding this comment

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

For this specific file it should no problem to use the same copyright notice since the actual code snippet is minimal / trivial. In other case where the contributions is significant I think the copyright notice should reflect the entity providing it.

Copy link

Choose a reason for hiding this comment

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

In rviz we would also have multiple copyright claims when there were large changes, e.g.:

https://github.com/ros2/rviz/blob/5595a69398df4d93aea711d408772ea34c53c230/rviz_common/include/rviz_common/factory/factory.hpp#L2-L4

*
* Licensed under the Apache License, Version 2.0 (the "License");
* you may not use this file except in compliance with the License.
* You may obtain a copy of the License at
*
* http://www.apache.org/licenses/LICENSE-2.0
*
* Unless required by applicable law or agreed to in writing, software
* distributed under the License is distributed on an "AS IS" BASIS,
* WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
* See the License for the specific language governing permissions and
* limitations under the License.
*/

package org.ros2.rcljava.interfaces;

public interface ActionDefinition {}
10 changes: 5 additions & 5 deletions rosidl_generator_java/CMakeLists.txt
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -24,19 +24,17 @@ endif()
include(UseJava)
include(JavaExtra)

set(CMAKE_JAVA_COMPILE_FLAGS "-source" "1.6" "-target" "1.6")

ament_index_register_resource("rosidl_generator_packages")

if(BUILD_TESTING)
find_package(rosidl_cmake REQUIRED)
find_package(rosidl_generator_c REQUIRED)

find_package(ament_lint_auto REQUIRED)
find_package(test_interface_files REQUIRED)

find_package(ament_lint_auto REQUIRED)
ament_lint_auto_find_test_dependencies()

# TODO(jacobperron): Use test_interface_files instead and update tests
jacobperron marked this conversation as resolved.
Show resolved Hide resolved
set(message_files
"msg/Bool.msg"
"msg/Byte.msg"
Expand Down Expand Up @@ -70,7 +68,9 @@ if(BUILD_TESTING)
"${CMAKE_CURRENT_SOURCE_DIR}/resource"
)

rosidl_generate_interfaces(${PROJECT_NAME} ${message_files}
rosidl_generate_interfaces(${PROJECT_NAME}
${message_files}
${test_interface_files_SRV_FILES}
SKIP_INSTALL
)

Expand Down
6 changes: 1 addition & 5 deletions rosidl_generator_java/bin/rosidl_generator_java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -27,17 +27,13 @@ def main(argv=sys.argv[1:]):
'--generator-arguments-file',
required=True,
help='The location of the file containing the generator arguments')
parser.add_argument(
'--typesupport-impl',
required=True,
help='The typesupport implementation targeted by the C interface')
parser.add_argument(
'--typesupport-impls',
required=True,
help='All the available typesupport implementations')
args = parser.parse_args(argv)

return generate_java(args.generator_arguments_file, args.typesupport_impl, args.typesupport_impls)
return generate_java(args.generator_arguments_file, args.typesupport_impls.split(';'))


if __name__ == '__main__':
Expand Down
20 changes: 6 additions & 14 deletions rosidl_generator_java/cmake/custom_command.cmake
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -11,19 +11,14 @@
# WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
# See the License for the specific language governing permissions and
# limitations under the License.


add_custom_command(
OUTPUT
${_generated_msg_java_files}
${_generated_msg_cpp_files}
${_generated_srv_java_files}
${_generated_srv_cpp_files}
${_generated_extension_files}
${_generated_java_files}
COMMAND ${PYTHON_EXECUTABLE} ${rosidl_generator_java_BIN}
--generator-arguments-file "${generator_arguments_file}"
--typesupport-impl "${_typesupport_impl}"
--typesupport-impls "${_typesupport_impls}"
DEPENDS ${target_dependencies}
DEPENDS ${target_dependencies} ${rosidl_generate_interfaces_TARGET}
COMMENT "Generating Java code for ROS interfaces"
VERBATIM
)
Expand All @@ -34,17 +29,14 @@ else()
add_custom_target(
${rosidl_generate_interfaces_TARGET}${_target_suffix}
DEPENDS
${_generated_msg_java_files}
${_generated_msg_cpp_files}
${_generated_srv_java_files}
${_generated_srv_cpp_files}
${_generated_extension_files}
${_generated_java_files}
)
endif()

add_jar("${PROJECT_NAME}_messages_jar"
SOURCES
${_generated_msg_java_files}
${_generated_srv_java_files}
${_generated_java_files}
OUTPUT_NAME
${PROJECT_NAME}_messages
INCLUDE_JARS
Expand Down
4 changes: 2 additions & 2 deletions rosidl_generator_java/cmake/register_java.cmake
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -13,9 +13,9 @@
# limitations under the License.

macro(rosidl_generator_java_extras BIN GENERATOR_FILES TEMPLATE_DIR)
find_package(ament_cmake_core QUIET REQUIRED)
find_package(ament_cmake_core REQUIRED)
ament_register_extension(
"rosidl_generate_interfaces"
"rosidl_generate_idl_interfaces"
"rosidl_generator_java"
"rosidl_generator_java_generate_interfaces.cmake")

Expand Down
125 changes: 59 additions & 66 deletions rosidl_generator_java/cmake/rosidl_generator_java_generate_interfaces.cmake
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -39,8 +39,6 @@ if(NOT WIN32)
endif()
endif()

set(CMAKE_JAVA_COMPILE_FLAGS "-source" "1.6" "-target" "1.6")

# Get a list of typesupport implementations from valid rmw implementations.
rosidl_generator_java_get_typesupports(_typesupport_impls)

Expand All @@ -51,48 +49,53 @@ endif()

set(_output_path
"${CMAKE_CURRENT_BINARY_DIR}/rosidl_generator_java/${PROJECT_NAME}")
set(_generated_cpp_files "")
set(_generated_msg_java_files "")
set(_generated_msg_cpp_files "")
set(_generated_srv_java_files "")
set(_generated_srv_cpp_files "")
set(_generated_extension_files "")
set(_generated_java_files "")

foreach(_idl_file ${rosidl_generate_interfaces_IDL_FILES})
get_filename_component(_parent_folder "${_idl_file}" DIRECTORY)
get_filename_component(_parent_folder "${_parent_folder}" NAME)
get_filename_component(_module_name "${_idl_file}" NAME_WE)
foreach(_typesupport_impl ${_typesupport_impls})
set(_generated_extension_${_typesupport_impl}_files "")
endforeach()

if(_parent_folder STREQUAL "msg")
list(APPEND _generated_msg_java_files
"${_output_path}/${_parent_folder}/${_module_name}.java"
foreach(_abs_idl_file ${rosidl_generate_interfaces_ABS_IDL_FILES})
get_filename_component(_parent_folder "${_abs_idl_file}" DIRECTORY)
get_filename_component(_parent_folder "${_parent_folder}" NAME)
get_filename_component(_idl_name "${_abs_idl_file}" NAME_WE)
list(APPEND _generated_java_files
"${_output_path}/${_parent_folder}/${_idl_name}.java"
)
# TODO(jacobperron): Is there a more robust way of detecting services and actions
# Services generate extra files
if(_parent_folder STREQUAL "srv")
list(APPEND _generated_java_files
"${_output_path}/${_parent_folder}/${_idl_name}_Request.java"
"${_output_path}/${_parent_folder}/${_idl_name}_Response.java"
)

foreach(_typesupport_impl ${_typesupport_impls})
list_append_unique(_generated_msg_cpp_files
"${_output_path}/${_parent_folder}/${_module_name}.ep.${_typesupport_impl}.cpp"
)
list(APPEND _type_support_by_generated_msg_cpp_files "${_typesupport_impl}")
endforeach()
elseif(_parent_folder STREQUAL "srv")
list(APPEND _generated_srv_java_files
"${_output_path}/${_parent_folder}/${_module_name}.java"
endif()
# Actions generate extra files
if(_parent_folder STREQUAL "action")
list(APPEND _generated_java_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"
)

foreach(_typesupport_impl ${_typesupport_impls})
list_append_unique(_generated_srv_cpp_files
"${_output_path}/${_parent_folder}/${_module_name}.ep.${_typesupport_impl}.cpp"
)
list(APPEND _type_support_by_generated_srv_cpp_files "${_typesupport_impl}")
endforeach()
else()
message(FATAL_ERROR "Interface file with unknown parent folder: ${_idl_file}")
endif()

foreach(_typesupport_impl ${_typesupport_impls})
list(APPEND _generated_extension_${_typesupport_impl}_files "${_output_path}/${_parent_folder}/${_idl_name}.ep.${_typesupport_impl}.cpp")
endforeach()
endforeach()

foreach(_typesupport_impl ${_typesupport_impls})
foreach(_generated_file ${_generated_extension_${_typesupport_impl}_files})
list(APPEND _generated_extension_files "${_generated_file}")
list(APPEND _typesupport_by_generated_cpp_files "${_typesupport_impl}")
endforeach()
endforeach()

set(_dependency_files "")
set(_dependencies "")
foreach(_pkg_name ${rosidl_generate_interfaces_DEPENDENCY_PACKAGE_NAMES})
foreach(_idl_file ${${_pkg_name}_INTERFACE_FILES})
foreach(_idl_file ${${_pkg_name}_IDL_FILES})
set(_abs_idl_file "${${_pkg_name}_DIR}/../${_idl_file}")
normalize_path(_abs_idl_file "${_abs_idl_file}")
list(APPEND _dependency_files "${_abs_idl_file}")
Expand All @@ -103,11 +106,15 @@ endforeach()
set(target_dependencies
"${rosidl_generator_java_BIN}"
${rosidl_generator_java_GENERATOR_FILES}
"${rosidl_generator_java_TEMPLATE_DIR}/action.cpp.em"
"${rosidl_generator_java_TEMPLATE_DIR}/idl.cpp.em"
"${rosidl_generator_java_TEMPLATE_DIR}/msg.cpp.em"
"${rosidl_generator_java_TEMPLATE_DIR}/srv.cpp.em"
"${rosidl_generator_java_TEMPLATE_DIR}/action.java.em"
"${rosidl_generator_java_TEMPLATE_DIR}/idl.java.em"
"${rosidl_generator_java_TEMPLATE_DIR}/msg.java.em"
"${rosidl_generator_java_TEMPLATE_DIR}/srv.java.em"
${rosidl_generate_interfaces_IDL_FILES}
${rosidl_generate_interfaces_ABS_IDL_FILES}
${_dependency_files})
foreach(dep ${target_dependencies})
if(NOT EXISTS "${dep}")
Expand All @@ -119,7 +126,7 @@ set(generator_arguments_file "${CMAKE_BINARY_DIR}/rosidl_generator_java__argumen
rosidl_write_generator_arguments(
"${generator_arguments_file}"
PACKAGE_NAME "${PROJECT_NAME}"
ROS_INTERFACE_FILES "${rosidl_generate_interfaces_IDL_FILES}"
IDL_TUPLES "${rosidl_generate_interfaces_IDL_TUPLES}"
ROS_INTERFACE_DEPENDENCIES "${_dependencies}"
OUTPUT_DIR "${_output_path}"
TEMPLATE_DIR "${rosidl_generator_java_TEMPLATE_DIR}"
Expand All @@ -128,7 +135,6 @@ rosidl_write_generator_arguments(

file(MAKE_DIRECTORY "${_output_path}")

set(_generated_extension_files "")
set(_extension_dependencies "")
set(_target_suffix "__java")

Expand Down Expand Up @@ -156,10 +162,8 @@ add_subdirectory("${_subdir}" ${rosidl_generate_interfaces_TARGET}${_target_suff

set_property(
SOURCE
${_generated_msg_java_files}
${_generated_msg_cpp_files}
${_generated_srv_java_files}
${_generated_srv_cpp_files}
${_generated_extension_files}
${_generated_java_files}
PROPERTY GENERATED 1)

macro(set_properties _build_type)
Expand All @@ -172,35 +176,29 @@ macro(set_properties _build_type)
OUTPUT_NAME "${_library_path}")
endmacro()

set(_type_support_by_generated_cpp_files ${_type_support_by_generated_msg_cpp_files} ${_type_support_by_generated_srv_cpp_files})
set(_generated_cpp_files ${_generated_msg_cpp_files} ${_generated_srv_cpp_files})

set(_javaext_suffix "__javaext")
foreach(_generated_cpp_file ${_generated_cpp_files})
get_filename_component(_full_folder "${_generated_cpp_file}" DIRECTORY)
get_filename_component(_package_folder "${_full_folder}" DIRECTORY)
get_filename_component(_package_name "${_package_folder}" NAME)
get_filename_component(_parent_folder "${_full_folder}" NAME)
get_filename_component(_base_msg_name "${_generated_cpp_file}" NAME_WE)
list(FIND _generated_cpp_files ${_generated_cpp_file} _file_index)
list(GET _type_support_by_generated_cpp_files ${_file_index} _typesupport_impl)
foreach(_generated_cpp_file ${_generated_extension_files})
list(FIND _generated_extension_files ${_generated_cpp_file} _file_index)
list(GET _typesupport_by_generated_cpp_files ${_file_index} _typesupport_impl)
find_package(${_typesupport_impl} REQUIRED)
set(_generated_msg_cpp_common_file "${_full_folder}/${_base_msg_name}.cpp")
get_filename_component(_parent_folder "${_generated_cpp_file}" DIRECTORY)
get_filename_component(_parent_folder "${_parent_folder}" NAME)
get_filename_component(_parent_folder "${_parent_folder}" NAME)
get_filename_component(_base_name "${_generated_cpp_file}" NAME_WE)
string(REGEX REPLACE "^rosidl_typesupport_" "" _short_typesupport_impl ${_typesupport_impl})
set(_library_name
"${_parent_folder}${_base_msg_name}${_short_typesupport_impl}"
)
set(_library_name "${_parent_folder}${_base_name}${_short_typesupport_impl}")
set(_library_path
"${_package_name}_${_parent_folder}_${_base_msg_name}__jni__${_typesupport_impl}"
"${rosidl_generate_interfaces_TARGET}_${_parent_folder}_${_base_name}__jni__${_typesupport_impl}"
)
string_camel_case_to_lower_case_underscore(${_library_path} _library_path)

add_library(${_library_name} SHARED
${_generated_cpp_file}
)
add_dependencies(
${_library_name}
${rosidl_generate_interfaces_TARGET}${_target_suffix}
${rosidl_generate_interfaces_TARGET}__rosidl_typesupport_c
${_target_dependencies}
)
set(_extension_compile_flags "")
if(CMAKE_COMPILER_IS_GNUCXX OR CMAKE_CXX_COMPILER_ID MATCHES "Clang")
Expand Down Expand Up @@ -248,9 +246,6 @@ foreach(_generated_cpp_file ${_generated_cpp_files})
)
target_link_libraries(${_library_name} ${${_pkg_name}_JNI_LIBRARIES})
endforeach()
add_dependencies(${_library_name}
${rosidl_generate_interfaces_TARGET}__${_typesupport_impl}
)
if(NOT rosidl_generate_interfaces_SKIP_INSTALL)
install(TARGETS ${_library_name}
ARCHIVE DESTINATION lib/jni
Expand All @@ -277,18 +272,16 @@ add_custom_command(

if(NOT rosidl_generate_interfaces_SKIP_INSTALL)
set(_install_jar_dir "${CMAKE_CURRENT_BINARY_DIR}/${PROJECT_NAME}")
if(NOT _generated_msg_java_files STREQUAL "" OR NOT _generated_srv_java_files STREQUAL "")
if(NOT _generated_java_files STREQUAL "")
install_jar("${PROJECT_NAME}_messages_jar" "share/${PROJECT_NAME}/java")
ament_export_jars("share/${PROJECT_NAME}/java/${PROJECT_NAME}_messages.jar")
endif()
endif()

if(BUILD_TESTING AND rosidl_generate_interfaces_ADD_LINTER_TESTS)
if(
NOT _generated_msg_java_files STREQUAL "" OR
NOT _generated_msg_cpp_files STREQUAL "" OR
NOT _generated_srv_java_files STREQUAL "" OR
NOT _generated_srv_cpp_files STREQUAL ""
NOT _generated_java_files STREQUAL "" OR
NOT _generated_extension_files STREQUAL ""
)
find_package(ament_cmake_cppcheck REQUIRED)
ament_cppcheck(
Expand Down
5 changes: 4 additions & 1 deletion rosidl_generator_java/package.xml
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -35,15 +35,18 @@
<test_depend>rmw_implementation_cmake</test_depend>
<test_depend>rosidl_generator_c</test_depend>

<test_depend>rosidl_parser</test_depend>
<test_depend>rosidl_cmake</test_depend>
<test_depend>rosidl_parser</test_depend>
<!-- Duplicated from rosidl_default_generator in order to avoid a circular dependency. -->
Copy link
Contributor Author

Choose a reason for hiding this comment

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

I think most (if not all) of the rosidl dependencies below can be removed. I'll give it a try.

Copy link
Member

Choose a reason for hiding this comment

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

@jacobperron were you able to remove the rest of the dependencies? Does it still work?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

It didn't seem to work. Needs more investigation, but leaving these dependencies here for now is okay.

<test_depend>rosidl_typesupport_c</test_depend>
<test_depend>rosidl_typesupport_connext_c</test_depend>
<test_depend>rosidl_typesupport_fastrtps_c</test_depend>
<test_depend>rosidl_typesupport_introspection_c</test_depend>
<test_depend>rosidl_typesupport_opensplice_c</test_depend>
<test_depend>test_interface_files</test_depend>

<member_of_group>rosidl_generator_packages</member_of_group>
<member_of_group>rosidl_runtime_packages</member_of_group>

<export>
<build_type>ament_cmake</build_type>
Expand Down
Loading