Skip to content

Commit 5305522

Browse files
jacobperronjacobperron
committed
Fix service and action interface generation (#76)
* 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 <[email protected]> * Add missing header include Signed-off-by: Jacob Perron <[email protected]> * 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 <[email protected]> * 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 <[email protected]> * Remove vestigal references to jni_package_name Signed-off-by: Jacob Perron <[email protected]> * Add comment about action and service headers Signed-off-by: Jacob Perron <[email protected]> * Simplify include logic Signed-off-by: Jacob Perron <[email protected]>
1 parent dcc6a54 commit 5305522

File tree

8 files changed

+325
-118
lines changed

8 files changed

+325
-118
lines changed

‎rosidl_generator_java/CMakeLists.txt‎

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -67,6 +67,7 @@ if(BUILD_TESTING)
6767
set(_deps_library_dirs "")
6868
list_append_unique(_deps_library_dirs ${CMAKE_CURRENT_BINARY_DIR})
6969
list_append_unique(_deps_library_dirs ${CMAKE_CURRENT_BINARY_DIR}/rosidl_generator_java/rosidl_generator_java/msg/)
70+
list_append_unique(_deps_library_dirs ${CMAKE_CURRENT_BINARY_DIR}/rosidl_generator_java/rosidl_generator_java/srv/)
7071

7172
foreach(testsuite ${${PROJECT_NAME}_testsuites})
7273
ament_add_junit_tests("${PROJECT_NAME}_tests_${testsuite}"

‎rosidl_generator_java/cmake/rosidl_generator_java_generate_interfaces.cmake‎

Lines changed: 9 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -70,6 +70,10 @@ foreach(_abs_idl_file ${rosidl_generate_interfaces_ABS_IDL_FILES})
7070
"${_output_path}/${_parent_folder}/${_idl_name}_Request.java"
7171
"${_output_path}/${_parent_folder}/${_idl_name}_Response.java"
7272
)
73+
foreach(_typesupport_impl ${_typesupport_impls})
74+
list(APPEND _generated_extension_${_typesupport_impl}_files "${_output_path}/${_parent_folder}/${_idl_name}_Request.ep.${_typesupport_impl}.cpp")
75+
list(APPEND _generated_extension_${_typesupport_impl}_files "${_output_path}/${_parent_folder}/${_idl_name}_Response.ep.${_typesupport_impl}.cpp")
76+
endforeach()
7377
endif()
7478
# Actions generate extra files
7579
if(_parent_folder STREQUAL "action")
@@ -78,6 +82,11 @@ foreach(_abs_idl_file ${rosidl_generate_interfaces_ABS_IDL_FILES})
7882
"${_output_path}/${_parent_folder}/${_idl_name}_Result.java"
7983
"${_output_path}/${_parent_folder}/${_idl_name}_Feedback.java"
8084
)
85+
foreach(_typesupport_impl ${_typesupport_impls})
86+
list(APPEND _generated_extension_${_typesupport_impl}_files "${_output_path}/${_parent_folder}/${_idl_name}_Goal.ep.${_typesupport_impl}.cpp")
87+
list(APPEND _generated_extension_${_typesupport_impl}_files "${_output_path}/${_parent_folder}/${_idl_name}_Result.ep.${_typesupport_impl}.cpp")
88+
list(APPEND _generated_extension_${_typesupport_impl}_files "${_output_path}/${_parent_folder}/${_idl_name}_Feedback.ep.${_typesupport_impl}.cpp")
89+
endforeach()
8190
endif()
8291

8392
foreach(_typesupport_impl ${_typesupport_impls})

‎rosidl_generator_java/resource/action.cpp.em‎

Lines changed: 82 additions & 12 deletions
Original file line numberDiff line numberDiff line change
@@ -1,23 +1,93 @@
11
@# Included from rosidl_generator_java/resource/idl.cpp.em
22
@{
3+
import os
4+
5+
from rosidl_cmake import expand_template
36
from rosidl_generator_c import idl_structure_type_to_c_include_prefix
47

5-
action_includes = [
6-
'rosidl_generator_c/action_type_support_struct.h',
7-
]
8+
namespaces = action.namespaced_type.namespaces
9+
type_name = action.namespaced_type.name
10+
goal_type_name = action.goal.structure.namespaced_type.name
11+
result_type_name = action.result.structure.namespaced_type.name
12+
feedback_type_name = action.feedback.structure.namespaced_type.name
13+
feedback_message_type_name = action.feedback_message.structure.namespaced_type.name
14+
send_goal_type_name = action.send_goal_service.namespaced_type.name
15+
get_result_type_name = action.get_result_service.namespaced_type.name
16+
17+
data = {
18+
'package_name': package_name,
19+
'output_dir': output_dir,
20+
'template_basepath': template_basepath,
21+
}
22+
23+
# Generate Goal message type
24+
data.update({'message': action.goal})
25+
output_file = os.path.join(
26+
output_dir, *namespaces[1:], '{0}.ep.{1}.cpp'.format(goal_type_name, typesupport_impl))
27+
expand_template(
28+
'msg.cpp.em',
29+
data,
30+
output_file)
31+
32+
# Generate Result message type
33+
data.update({'message': action.result})
34+
output_file = os.path.join(
35+
output_dir, *namespaces[1:], '{0}.ep.{1}.cpp'.format(result_type_name, typesupport_impl))
36+
expand_template(
37+
'msg.cpp.em',
38+
data,
39+
output_file)
40+
41+
# Generate Feedback message type
42+
data.update({'message': action.feedback})
43+
output_file = os.path.join(
44+
output_dir, *namespaces[1:], '{0}.ep.{1}.cpp'.format(feedback_type_name, typesupport_impl))
45+
expand_template(
46+
'msg.cpp.em',
47+
data,
48+
output_file)
49+
50+
# Generate FeedbackMessage message type
51+
data.update({'message': action.feedback_message})
52+
output_file = os.path.join(
53+
output_dir,
54+
*namespaces[1:],
55+
'{0}.ep.{1}.cpp'.format(feedback_message_type_name, typesupport_impl))
56+
expand_template(
57+
'msg.cpp.em',
58+
data,
59+
output_file)
60+
61+
# Generate SendGoal service type
62+
data.update({'service': action.send_goal_service})
63+
output_file = os.path.join(
64+
output_dir, *namespaces[1:], '{0}.ep.{1}.cpp'.format(send_goal_type_name, typesupport_impl))
65+
expand_template(
66+
'msg.cpp.em',
67+
data,
68+
output_file)
69+
70+
# Generate SendGoal service type
71+
data.update({'service': action.get_result_service})
72+
output_file = os.path.join(
73+
output_dir, *namespaces[1:], '{0}.ep.{1}.cpp'.format(get_result_type_name, typesupport_impl))
74+
expand_template(
75+
'msg.cpp.em',
76+
data,
77+
output_file)
878
}@
9-
@[for include in action_includes]@
10-
@[ if include in include_directives]@
11-
// already included above
12-
// @
13-
@[ else]@
14-
@{include_directives.add(include)}@
15-
@[ end if]@
16-
#include "@(include)"
17-
@[end for]@
79+
80+
#include <jni.h>
81+
82+
#include <cstdint>
83+
84+
#include "rosidl_generator_c/action_type_support_struct.h"
1885

1986
#include "@(idl_structure_type_to_c_include_prefix(action.namespaced_type)).h"
2087

88+
// Ensure that a jlong is big enough to store raw pointers
89+
static_assert(sizeof(jlong) >= sizeof(std::intptr_t), "jlong must be able to store pointers");
90+
2191
#ifdef __cplusplus
2292
extern "C" {
2393
#endif

‎rosidl_generator_java/resource/idl.cpp.em‎

Lines changed: 64 additions & 76 deletions
Original file line numberDiff line numberDiff line change
@@ -9,102 +9,90 @@
99
@# - package_name (string)
1010
@# - interface_path (Path relative to the directory named after the package)
1111
@# - content (IdlContent, list of elements, e.g. Messages or Services)
12+
@# - output_dir (Path)
13+
@# - template_basepath (Path)
14+
@# - typesupport_impl (string, the typesupport identifier of the generated code)
1215
@#######################################################################
1316
@{
14-
include_directives = set()
17+
import os
1518

16-
jni_includes = [
17-
'jni.h',
18-
]
19-
include_directives.update(jni_includes)
20-
std_includes = [
21-
'cassert',
22-
'cstdint',
23-
'string',
24-
]
25-
include_directives.update(std_includes)
26-
rosidl_includes = [
27-
'rosidl_generator_c/message_type_support_struct.h',
28-
]
29-
include_directives.update(rosidl_includes)
30-
rcljava_includes = [
31-
'rcljava_common/exceptions.h',
32-
'rcljava_common/signatures.h',
33-
]
34-
include_directives.update(rcljava_includes)
35-
}@
36-
@[for include in jni_includes]@
37-
#include <@(include)>
38-
@[end for]@
39-
40-
@[for include in std_includes]@
41-
#include <@(include)>
42-
@[end for]@
43-
44-
@[for include in rosidl_includes]@
45-
#include "@(include)"
46-
@[end for]@
47-
48-
@[for include in rcljava_includes]@
49-
#include "@(include)"
50-
@[end for]@
51-
52-
// Ensure that a jlong is big enough to store raw pointers
53-
static_assert(sizeof(jlong) >= sizeof(std::intptr_t), "jlong must be able to store pointers");
54-
55-
using rcljava_common::exceptions::rcljava_throw_exception;
19+
from rosidl_cmake import expand_template
20+
from rosidl_parser.definition import Action
21+
from rosidl_parser.definition import Message
22+
from rosidl_parser.definition import Service
5623

57-
@{
58-
jni_package_name = package_name.replace('_', '_1')
5924
}@
6025
@
6126
@#######################################################################
6227
@# Handle messages
6328
@#######################################################################
6429
@{
65-
from rosidl_parser.definition import Message
66-
}@
67-
@[for message in content.get_elements_of_type(Message)]@
68-
@{
69-
TEMPLATE(
70-
'msg.cpp.em',
71-
package_name=package_name,
72-
jni_package_name=jni_package_name,
73-
message=message,
74-
include_directives=include_directives)
30+
data = {
31+
'package_name': package_name,
32+
'output_dir': output_dir,
33+
'template_basepath': template_basepath,
34+
}
35+
36+
for message in content.get_elements_of_type(Message):
37+
data.update({'message': message})
38+
type_name = message.structure.namespaced_type.name
39+
namespaces = message.structure.namespaced_type.namespaces
40+
output_file = os.path.join(
41+
output_dir, *namespaces[1:], '{0}.ep.{1}.cpp'.format(type_name, typesupport_impl))
42+
expand_template(
43+
'msg.cpp.em',
44+
data,
45+
output_file,
46+
template_basepath=template_basepath)
7547
}@
76-
@[end for]@
7748
@
7849
@#######################################################################
7950
@# Handle services
8051
@#######################################################################
8152
@{
82-
from rosidl_parser.definition import Service
83-
}@
84-
@[for service in content.get_elements_of_type(Service)]@
85-
@{
86-
TEMPLATE(
87-
'srv.cpp.em',
88-
package_name=package_name,
89-
jni_package_name=jni_package_name,
90-
service=service,
91-
include_directives=include_directives)
53+
data = {
54+
'package_name': package_name,
55+
'interface_path': interface_path,
56+
'output_dir': output_dir,
57+
'template_basepath': template_basepath,
58+
'typesupport_impl': typesupport_impl,
59+
}
60+
61+
for service in content.get_elements_of_type(Service):
62+
data.update({'service': service})
63+
type_name = service.namespaced_type.name
64+
namespaces = service.namespaced_type.namespaces
65+
output_file = os.path.join(
66+
output_dir, *namespaces[1:], '{0}.ep.{1}.cpp'.format(type_name, typesupport_impl))
67+
expand_template(
68+
'srv.cpp.em',
69+
data,
70+
output_file,
71+
template_basepath=template_basepath)
72+
9273
}@
93-
@[end for]@
9474
@
9575
@#######################################################################
9676
@# Handle actions
9777
@#######################################################################
9878
@{
99-
from rosidl_parser.definition import Action
100-
}@
101-
@[for action in content.get_elements_of_type(Action)]@
102-
@{
103-
TEMPLATE(
104-
'action.cpp.em',
105-
package_name=package_name,
106-
jni_package_name=jni_package_name,
107-
action=action,
108-
include_directives=include_directives)
79+
data = {
80+
'package_name': package_name,
81+
'interface_path': interface_path,
82+
'output_dir': output_dir,
83+
'template_basepath': template_basepath,
84+
'typesupport_impl': typesupport_impl,
85+
}
86+
87+
for action in content.get_elements_of_type(Action):
88+
data.update({'action': action})
89+
type_name = action.namespaced_type.name
90+
namespaces = action.namespaced_type.namespaces
91+
output_file = os.path.join(
92+
output_dir, *namespaces[1:], '{0}.ep.{1}.cpp'.format(type_name, typesupport_impl))
93+
expand_template(
94+
'action.cpp.em',
95+
data,
96+
output_file,
97+
template_basepath=template_basepath)
10998
}@
110-
@[end for]@

0 commit comments

Comments
 (0)