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

Generate msg/srv from within an 'action' directory/module #15

Merged
merged 7 commits into from
Oct 30, 2018
Merged

Generate msg/srv from within an 'action' directory/module #15

Show file tree
Hide file tree
Changes from 5 commits
Commits
Show all changes
7 commits
Select commit Hold shift + click to select a range
dd88cb5
Added support to msg/srv generation from within an action directory
Oct 25, 2018
66deb8d
Addresses peer review comments
Oct 26, 2018
0fe280e
Fixed style issue with python file
Oct 26, 2018
a9fa75b
Fixed template c function naming
Oct 29, 2018
55a5c9c
Fixed cmake filest to process files under action directory
Oct 29, 2018
98d630c
Generate .c file for any .msg regardless of folder
sloretz Oct 30, 2018
cbf6a2c
Fixed function names in empy templates
Oct 30, 2018
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
4 changes: 3 additions & 1 deletion rosidl_generator_py/cmake/custom_command.cmake
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -13,7 +13,7 @@
# limitations under the License.

add_custom_command(
OUTPUT ${_generated_extension_files} ${_generated_msg_py_files} ${_generated_msg_c_files} ${_generated_srv_py_files} ${_generated_srv_c_files}
OUTPUT ${_generated_extension_files} ${_generated_msg_py_files} ${_generated_msg_c_files} ${_generated_srv_py_files} ${_generated_srv_c_files} ${_generated_action_py_files} ${_generated_action_c_files}
COMMAND ${PYTHON_EXECUTABLE} ${rosidl_generator_py_BIN}
--generator-arguments-file "${generator_arguments_file}"
--typesupport-impls "${_typesupport_impls}"
Expand All @@ -33,5 +33,7 @@ else()
${_generated_msg_c_files}
${_generated_srv_py_files}
${_generated_srv_c_files}
${_generated_action_py_files}
${_generated_action_c_files}
)
endif()
39 changes: 36 additions & 3 deletions rosidl_generator_py/cmake/rosidl_generator_py_generate_interfaces.cmake
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -38,6 +38,8 @@ set(_generated_msg_py_files "")
set(_generated_msg_c_files "")
set(_generated_srv_py_files "")
set(_generated_srv_c_files "")
set(_generated_action_py_files "")
set(_generated_action_c_files "")

foreach(_typesupport_impl ${_typesupport_impls})
set(_generated_extension_${_typesupport_impl}_files "")
Expand Down Expand Up @@ -65,6 +67,15 @@ foreach(_idl_file ${rosidl_generate_interfaces_IDL_FILES})
list(APPEND _generated_srv_py_files
"${_output_path}/${_parent_folder}/_${_module_name}.py"
)
elseif(_parent_folder STREQUAL "action")
if("_${_module_name}_s.c" MATCHES "(.*)__response(.*)" OR "_${_module_name}_s.c" MATCHES "(.*)__request(.*)")
Copy link
Contributor

Choose a reason for hiding this comment

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

This works for services, but not messages. After adding action/Something.msg in test_msgs I see this traceback

undefined symbol: test_msgs__action__something__convert_from_py

342: (test_publisher) pid 7968: ['/usr/bin/python3', '/home/developer/workspaces/ros2-dev0/src/ros2/system_tests/test_communication/test/publisher_py.py', 'BoundedArrayPrimitives', '/test_time_23_08_40'] (stderr > stdout, all > console)
342: (test_subscriber) pid 7969: ['/home/developer/workspaces/ros2-dev0/build/test_communication/test_subscriber_cpp', 'BoundedArrayPrimitives', '/test_time_23_08_40'] (stderr > stdout, all > console)
342: [test_publisher] exception in talker:
342: [test_publisher] Traceback (most recent call last):
342: [test_publisher]   File "/home/developer/workspaces/ros2-dev0/install/rosidl_generator_py/lib/python3.6/site-packages/rosidl_generator_py/import_type_support_impl.py", line 42, in import_type_support
342: [test_publisher]     return importlib.import_module(module_name, package=pkg_name)
342: [test_publisher]   File "/usr/lib/python3.6/importlib/__init__.py", line 126, in import_module
342: [test_publisher]     return _bootstrap._gcd_import(name[level:], package, level)
342: [test_publisher]   File "<frozen importlib._bootstrap>", line 994, in _gcd_import
342: [test_publisher]   File "<frozen importlib._bootstrap>", line 971, in _find_and_load
342: [test_publisher]   File "<frozen importlib._bootstrap>", line 955, in _find_and_load_unlocked
342: [test_publisher]   File "<frozen importlib._bootstrap>", line 658, in _load_unlocked
342: [test_publisher]   File "<frozen importlib._bootstrap>", line 571, in module_from_spec
342: [test_publisher]   File "<frozen importlib._bootstrap_external>", line 922, in create_module
342: [test_publisher]   File "<frozen importlib._bootstrap>", line 219, in _call_with_frames_removed
342: [test_publisher] ImportError: /home/developer/workspaces/ros2-dev0/install/test_msgs/lib/python3.6/site-packages/test_msgs/test_msgs_s__rosidl_typesupport_c.cpython-36m-x86_64-linux-gnu.so: undefined symbol: test_msgs__action__something__convert_from_py

Copy link
Contributor

@sloretz sloretz Oct 30, 2018 •
edited
Loading

Choose a reason for hiding this comment

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

Changing this condition gets a little further,

    get_filename_component(_ext "${_idl_file}" EXT)
    # C files generated for <msg>.msg, <service>_Request.msg and <service>_Response.msg but not <service>.srv
    if(_ext STREQUAL ".msg")
      list(APPEND _generated_action_c_files
        "${_output_path}/${_parent_folder}/_${_module_name}_s.c"
      )
    endif()
    list(APPEND _generated_action_py_files
      "${_output_path}/${_parent_folder}/_${_module_name}.py"
    )

Edit: NVM, the test communication problem is not caused by this PR
Though test_communication still fails with

18: [test_publisher] exception in talker:
18: [test_publisher] Traceback (most recent call last):
18: [test_publisher]   File "/home/developer/workspaces/ros2-dev0/src/ros2/system_tests/test_communication/test/publisher_py.py", line 63, in <module>
18: [test_publisher]     number_of_cycles=args.number_of_cycles)
18: [test_publisher]   File "/home/developer/workspaces/ros2-dev0/src/ros2/system_tests/test_communication/test/publisher_py.py", line 27, in talker
18: [test_publisher]     msg_mod = getattr(module, message_name)
18: [test_publisher] AttributeError: module 'test_msgs.msg' has no attribute 'Something'
18: [test_subscriber] exception in subscriber:
18: [test_subscriber] Traceback (most recent call last):
18: [test_subscriber]   File "/home/developer/workspaces/ros2-dev0/src/ros2/system_tests/test_communication/test/subscriber_py.py", line 81, in <module>
18: [test_subscriber]     listener(message_name=args.message_name, namespace=args.namespace)
18: [test_subscriber]   File "/home/developer/workspaces/ros2-dev0/src/ros2/system_tests/test_communication/test/subscriber_py.py", line 47, in listener
18: [test_subscriber]     msg_mod = getattr(module, message_name)
18: [test_subscriber] AttributeError: module 'test_msgs.msg' has no attribute 'Something'

but I'm not yet sure if the problem is here or there

Copy link
Contributor

Choose a reason for hiding this comment

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

Fixed in 98d630c

list(APPEND _generated_action_c_files
"${_output_path}/${_parent_folder}/_${_module_name}_s.c"
)
endif()
list(APPEND _generated_action_py_files
"${_output_path}/${_parent_folder}/_${_module_name}.py"
)
else()
message(FATAL_ERROR "Interface file with unknown parent folder: ${_idl_file}")
endif()
Expand All @@ -89,7 +100,15 @@ if(NOT _generated_srv_py_files STREQUAL "")
)
endif()

if(NOT _generated_msg_c_files STREQUAL "" OR NOT _generated_srv_c_files STREQUAL "")
if(NOT _generated_action_py_files STREQUAL "")
list(GET _generated_action_py_files 0 _action_file)
get_filename_component(_parent_folder "${_action_file}" DIRECTORY)
list(APPEND _generated_action_py_files
"${_parent_folder}/__init__.py"
)
endif()

if(NOT _generated_msg_c_files STREQUAL "" OR NOT _generated_srv_c_files STREQUAL "" OR NOT _generated_action_c_files STREQUAL "")
foreach(_typesupport_impl ${_typesupport_impls})
list(APPEND _generated_extension_${_typesupport_impl}_files "${_output_path}/_${PROJECT_NAME}_s.ep.${_typesupport_impl}.c")
list(APPEND _generated_extension_files "${_generated_extension_${_typesupport_impl}_files}")
Expand Down Expand Up @@ -145,6 +164,12 @@ if(NOT _generated_srv_py_files STREQUAL "")
get_filename_component(_srv_package_dir2 "${_srv_package_dir1}" NAME)
endif()

if(NOT _generated_action_py_files STREQUAL "")
list(GET _generated_action_py_files 0 _action_file)
get_filename_component(_action_package_dir1 "${_action_file}" DIRECTORY)
get_filename_component(_action_package_dir2 "${_action_package_dir1}" NAME)
endif()

if(NOT rosidl_generate_interfaces_SKIP_INSTALL)
ament_python_install_module("${_output_path}/__init__.py"
DESTINATION_SUFFIX "${PROJECT_NAME}"
Expand All @@ -167,6 +192,11 @@ if(NOT rosidl_generate_interfaces_SKIP_INSTALL)
DESTINATION "${PYTHON_INSTALL_DIR}/${PROJECT_NAME}/${_srv_package_dir2}"
)
endif()
if(NOT _action_package_dir2 STREQUAL "")
install(FILES ${_generated_action_py_files}
DESTINATION "${PYTHON_INSTALL_DIR}/${PROJECT_NAME}/${_action_package_dir2}"
)
endif()
endif()

set(_target_suffix "__py")
Expand All @@ -179,7 +209,7 @@ file(WRITE "${_subdir}/CMakeLists.txt" "${_custom_command}")
add_subdirectory("${_subdir}" ${rosidl_generate_interfaces_TARGET}${_target_suffix})
set_property(
SOURCE
${_generated_extension_files} ${_generated_msg_py_files} ${_generated_msg_c_files} ${_generated_srv_py_files} ${_generated_srv_c_files}
${_generated_extension_files} ${_generated_msg_py_files} ${_generated_msg_c_files} ${_generated_srv_py_files} ${_generated_srv_c_files} ${_generated_action_py_files} ${_generated_action_c_files}
PROPERTY GENERATED 1)

macro(set_properties _build_type)
Expand All @@ -203,6 +233,7 @@ set(_target_name_lib "${rosidl_generate_interfaces_TARGET}__python")
add_library(${_target_name_lib} SHARED
${_generated_msg_c_files}
${_generated_srv_c_files}
${_generated_action_c_files}
)
add_dependencies(
${_target_name_lib}
Expand Down Expand Up @@ -331,7 +362,9 @@ if(BUILD_TESTING AND rosidl_generate_interfaces_ADD_LINTER_TESTS)
NOT _generated_extension_files STREQUAL "" OR
NOT _generated_msg_c_files STREQUAL "" OR
NOT _generated_srv_py_files STREQUAL "" OR
NOT _generated_srv_c_files STREQUAL ""
NOT _generated_srv_c_files STREQUAL "" OR
NOT _generated_action_c_files STREQUAL "" OR
NOT _generated_action_py_files STREQUAL ""
)
find_package(ament_cmake_cppcheck REQUIRED)
ament_cppcheck(
Expand Down
24 changes: 12 additions & 12 deletions rosidl_generator_py/resource/_msg_pkg_typesupport_entry_point.c.em
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -26,7 +26,7 @@ static_includes = set([
for spec, subfolder in message_specs:
if subfolder == 'msg':
static_includes.add('#include <rosidl_generator_c/message_type_support_struct.h>')
elif subfolder == 'srv':
elif subfolder == 'srv' or subfolder == 'action':
static_includes.add('#include <rosidl_generator_c/service_type_support_struct.h>')
}@
@[for value in sorted(static_includes)]@
Expand Down Expand Up @@ -61,21 +61,21 @@ module_name = convert_camel_case_to_lower_case_underscore(type_name)
msg_typename = '%s__%s__%s' % (pkg_name, subfolder, type_name)
}@

static void * @(pkg_name)__@(module_name)__create_ros_message(void)
static void * @(pkg_name)__@(subfolder)__@(module_name)__create_ros_message(void)
{
return @(msg_typename)__create();
}

static void @(pkg_name)__@(module_name)__destroy_ros_message(void * raw_ros_message)
static void @(pkg_name)__@(subfolder)__@(module_name)__destroy_ros_message(void * raw_ros_message)
{
@(msg_typename) * ros_message = (@(msg_typename) *)raw_ros_message;
@(msg_typename)__destroy(ros_message);
}

ROSIDL_GENERATOR_C_IMPORT
bool @(pkg_name)__@(module_name)__convert_from_py(PyObject * _pymsg, void * ros_message);
bool @(pkg_name)__@(subfolder)__@(module_name)__convert_from_py(PyObject * _pymsg, void * ros_message);
ROSIDL_GENERATOR_C_IMPORT
PyObject * @(pkg_name)__@(module_name)__convert_to_py(void * raw_ros_message);
PyObject * @(pkg_name)__@(subfolder)__@(module_name)__convert_to_py(void * raw_ros_message);
@[end for]@

static PyMethodDef @(package_name)__methods[] = {
Expand Down Expand Up @@ -105,15 +105,15 @@ const rosidl_message_type_support_t *
ROSIDL_GET_MSG_TYPE_SUPPORT(@(pkg_name), @(subfolder), @(spec.msg_name));

int8_t
_register_msg_type__@(type_name)(PyObject * pymodule)
_register_msg_type__@(subfolder)__@(type_name)(PyObject * pymodule)
{
int8_t err;
@[ for function_name in function_names]@

PyObject * pyobject_@(function_name) = NULL;
pyobject_@(function_name) = PyCapsule_New(
@[ if function_name != 'type_support']@
(void *)&@(pkg_name)__@(type_name)__@(function_name),
(void *)&@(pkg_name)__@(subfolder)__@(type_name)__@(function_name),
@[ else]@
(void *)ROSIDL_GET_MSG_TYPE_SUPPORT(@(pkg_name), @(subfolder), @(spec.msg_name)),
@[ end if]@
Expand Down Expand Up @@ -144,15 +144,15 @@ function_name = 'type_support'

ROSIDL_GENERATOR_C_IMPORT
const rosidl_service_type_support_t *
ROSIDL_TYPESUPPORT_INTERFACE__SERVICE_SYMBOL_NAME(rosidl_typesupport_c, @(spec.pkg_name), @(spec.srv_name))();
ROSIDL_TYPESUPPORT_INTERFACE__SERVICE_SYMBOL_NAME(rosidl_typesupport_c, @(spec.pkg_name), @(subfolder), @(spec.srv_name))();

int8_t
_register_srv_type__@(type_name)(PyObject * pymodule)
_register_srv_type__@(subfolder)__@(type_name)(PyObject * pymodule)
{
int8_t err;
PyObject * pyobject_@(function_name) = NULL;
pyobject_@(function_name) = PyCapsule_New(
(void *)ROSIDL_TYPESUPPORT_INTERFACE__SERVICE_SYMBOL_NAME(rosidl_typesupport_c, @(spec.pkg_name), @(spec.srv_name))(),
(void *)ROSIDL_TYPESUPPORT_INTERFACE__SERVICE_SYMBOL_NAME(rosidl_typesupport_c, @(spec.pkg_name), @(subfolder), @(spec.srv_name))(),
NULL, NULL);
if (!pyobject_@(function_name)) {
// previously added objects will be removed when the module is destroyed
Expand Down Expand Up @@ -185,7 +185,7 @@ PyInit_@(package_name)_s__@(typesupport_impl)(void)
@{
type_name = convert_camel_case_to_lower_case_underscore(spec.base_type.type)
}@
err = _register_msg_type__@(type_name)(pymodule);
err = _register_msg_type__@(subfolder)__@(type_name)(pymodule);
if (err) {
Py_XDECREF(pymodule);
return NULL;
Expand All @@ -195,7 +195,7 @@ type_name = convert_camel_case_to_lower_case_underscore(spec.base_type.type)
@{
type_name = convert_camel_case_to_lower_case_underscore(spec.srv_name)
}@
err = _register_srv_type__@(type_name)(pymodule);
err = _register_srv_type__@(subfolder)__@(type_name)(pymodule);
if (err) {
Py_XDECREF(pymodule);
return NULL;
Expand Down
16 changes: 8 additions & 8 deletions rosidl_generator_py/resource/_msg_support.c.em
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -61,16 +61,16 @@ msg_typename = '%s__%s__%s' % (spec.base_type.pkg_name, subfolder, spec.base_typ
@[ if spec.base_type.pkg_name != field.type.pkg_name]@
ROSIDL_GENERATOR_C_IMPORT
@[ end if]@
bool @(field.type.pkg_name)__@(lowercase_field_type)__convert_from_py(PyObject * _pymsg, void * _ros_message);
bool @(field.type.pkg_name)__msg__@(lowercase_field_type)__convert_from_py(PyObject * _pymsg, void * _ros_message);
@[ if spec.base_type.pkg_name != field.type.pkg_name]@
ROSIDL_GENERATOR_C_IMPORT
@[ end if]@
PyObject * @(field.type.pkg_name)__@(lowercase_field_type)__convert_to_py(void * raw_ros_message);
PyObject * @(field.type.pkg_name)__msg__@(lowercase_field_type)__convert_to_py(void * raw_ros_message);
@[ end if]@
@[end for]@

ROSIDL_GENERATOR_C_EXPORT
bool @(spec.base_type.pkg_name)__@(module_name)__convert_from_py(PyObject * _pymsg, void * _ros_message)
bool @(spec.base_type.pkg_name)__@(subfolder)__@(module_name)__convert_from_py(PyObject * _pymsg, void * _ros_message)
{
@{
full_classname = '%s.%s._%s.%s' % (spec.base_type.pkg_name, subfolder, module_name, spec.base_type.type)
Expand Down Expand Up @@ -146,15 +146,15 @@ lowercase_field_type = convert_camel_case_to_lower_case_underscore(field.type.ty
@(nested_type) * dest = ros_message->@(field.name);
@[ end if]@
for (Py_ssize_t i = 0; i < size; ++i) {
if (!@(field.type.pkg_name)__@(lowercase_field_type)__convert_from_py(PySequence_Fast_GET_ITEM(seq_field, i), &dest[i])) {
if (!@(field.type.pkg_name)__msg__@(lowercase_field_type)__convert_from_py(PySequence_Fast_GET_ITEM(seq_field, i), &dest[i])) {
Py_DECREF(seq_field);
Py_DECREF(field);
return false;
}
}
Py_DECREF(seq_field);
@[ else]@
if (!@(field.type.pkg_name)__@(lowercase_field_type)__convert_from_py(field, &ros_message->@(field.name))) {
if (!@(field.type.pkg_name)__msg__@(lowercase_field_type)__convert_from_py(field, &ros_message->@(field.name))) {
Py_DECREF(field);
return false;
}
Expand Down Expand Up @@ -302,7 +302,7 @@ lowercase_field_type = convert_camel_case_to_lower_case_underscore(field.type.ty
}

ROSIDL_GENERATOR_C_EXPORT
PyObject * @(spec.base_type.pkg_name)__@(module_name)__convert_to_py(void * raw_ros_message)
PyObject * @(spec.base_type.pkg_name)__@(subfolder)__@(module_name)__convert_to_py(void * raw_ros_message)
{
/* NOTE(esteve): Call constructor of @(spec.base_type.type) */
PyObject * _pymessage = NULL;
Expand Down Expand Up @@ -347,7 +347,7 @@ lowercase_field_type = convert_camel_case_to_lower_case_underscore(field.type.ty
@[ else]@
item = &(ros_message->@(field.name)[i]);
@[ end if]@
PyObject * pyitem = @(field.type.pkg_name)__@(lowercase_field_type)__convert_to_py(item);
PyObject * pyitem = @(field.type.pkg_name)__msg__@(lowercase_field_type)__convert_to_py(item);
if (!pyitem) {
Py_DECREF(field);
return NULL;
Expand All @@ -358,7 +358,7 @@ lowercase_field_type = convert_camel_case_to_lower_case_underscore(field.type.ty
}
assert(PySequence_Check(field));
@[ else]@
field = @(field.type.pkg_name)__@(lowercase_field_type)__convert_to_py(&ros_message->@(field.name));
field = @(field.type.pkg_name)__msg__@(lowercase_field_type)__convert_to_py(&ros_message->@(field.name));
if (!field) {
return NULL;
}
Expand Down
6 changes: 3 additions & 3 deletions rosidl_generator_py/resource/_srv.py.em
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -35,15 +35,15 @@ class Metaclass(type):
@{
srv_name = '_' + convert_camel_case_to_lower_case_underscore(spec.srv_name)
for field_name in [srv_name + '__request', srv_name + '__response']:
print('%sfrom %s.srv import %s' % (' ' * 4 * 3, package_name, field_name))
print('%sfrom %s.%s import %s' % (' ' * 4 * 3, package_name, subfolder, field_name))
print('%sif %s.Metaclass._TYPE_SUPPORT is None:' % (' ' * 4 * 3, field_name))
print('%s%s.Metaclass.__import_type_support__()' % (' ' * 4 * 4, field_name))
}@


class @(spec.srv_name)(metaclass=Metaclass):
from @(package_name).srv._@convert_camel_case_to_lower_case_underscore(spec.srv_name)__request import @(spec.srv_name)_Request as Request
from @(package_name).srv._@convert_camel_case_to_lower_case_underscore(spec.srv_name)__response import @(spec.srv_name)_Response as Response
from @(package_name).@(subfolder)._@convert_camel_case_to_lower_case_underscore(spec.srv_name)__request import @(spec.srv_name)_Request as Request
from @(package_name).@(subfolder)._@convert_camel_case_to_lower_case_underscore(spec.srv_name)__response import @(spec.srv_name)_Response as Response

def __init__(self):
raise NotImplementedError('Service classes can not be instanciated')
3 changes: 2 additions & 1 deletion rosidl_generator_py/rosidl_generator_py/generate_py_impl.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -98,7 +98,8 @@ def generate_py(generator_arguments_file, typesupport_impls):
for subfolder in modules.keys():
import_list = {}
for module_name, type_ in modules[subfolder]:
if subfolder == 'srv' and (type_.endswith('Request') or type_.endswith('Response')):
if (subfolder == 'srv' or subfolder == 'action') and \
(type_.endswith('Request') or type_.endswith('Response')):
continue
import_list['%s # noqa\n' % type_] = 'from %s.%s._%s import %s\n' % \
(args['package_name'], subfolder, module_name, type_)
Expand Down