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

apojomovsky · 2018-10-26T18:13:29Z

Closes #13 .
This PR allows the generation of messages and services that are defined in the module action.
It depends on ros2/rosidl#308 , ros2/rosidl_dds#44 and ros2/rosidl_typesupport#33 to compile successfully.
It was tested with the definitions found in this rcl_interfaces' branch .

connects to ros2/rosidl#301

hidmic

@apojomovsky I left a few comments !

hidmic · 2018-10-26T18:24:08Z

rosidl_generator_py/resource/_msg_pkg_typesupport_entry_point.c.em

@@ -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__@(type_name)__@(subfolder)(PyObject * pymodule)


@apojomovsky consider reversing the order of interpolated variables, as in _register_msg_type__@(subfolder)_@(type_name)(PyObject * pymodule).

Oh, you're right! Thanks for the suggestion, done!

hidmic · 2018-10-26T18:24:20Z

rosidl_generator_py/resource/_msg_pkg_typesupport_entry_point.c.em


 int8_t
-_register_srv_type__@(type_name)(PyObject * pymodule)
+_register_srv_type__@(type_name)__@(subfolder)(PyObject * pymodule)


@apojomovsky same as above.

done, thanks!

hidmic · 2018-10-26T18:26:03Z

rosidl_generator_py/resource/_msg_support.c.em

 @[    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)__@(subfolder)__@(lowercase_field_type)__convert_to_py(void * raw_ros_message);


@apojomovsky I don't think this will be true in general e.g. a _Request message that is part of a service definition below srv/ could have a field that's a message described in a message definition below msg/.

You're right, I just restored this line. Thanks!

apojomovsky · 2018-10-26T19:45:50Z

Thanks for the review @hidmic , I just finished addressing your comments. Could you PTAL?

sloretz · 2018-10-30T00:33:31Z

rosidl_generator_py/cmake/rosidl_generator_py_generate_interfaces.cmake

@@ -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(.*)")


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

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~~

Fixed in 98d630c

sloretz

Tested with

Repos file

repositories:
  ros2/rcl_interfaces:
    type: git
    url: https://github.com/ros2/rcl_interfaces.git
    version: sloretz/msg_srv_in_action
  ros2/rosidl:
    type: git
    url: https://github.com/ros2/rosidl.git
    version: hidmic/prepare_for_action_generation
  ros2/rosidl_dds:
    type: git
    url: https://github.com/ros2/rosidl_dds.git
    version: hidmic/prepare_for_action_generation
  ros2/rosidl_python:
    type: git
    url: https://github.com/ros2/rosidl_python.git
    version: apojomovsky/allow_generating_msg_and_srv_in_action
  ros2/rosidl_typesupport:
    type: git
    url: https://github.com/ros2/rosidl_typesupport.git
    version: hidmic/prepare_for_action_generation
  ros2/rosidl_typesupport_connext:
    type: git
    url: https://github.com/ros2/rosidl_typesupport_connext.git
    version: hidmic/subfolder_aware_srv_generation
  ros2/rosidl_typesupport_fastrtps:
    type: git
    url: https://github.com/ros2/rosidl_typesupport_fastrtps.git
    version: apojomovsky/subfolder_aware_srv_generation
  ros2/rosidl_typesupport_opensplice:
    type: git
    url: https://github.com/ros2/rosidl_typesupport_opensplice.git
    version: sservulo/prepare_for_action_generation
  ros2/system_tests:
    type: git
    url: https://github.com/ros2/system_tests.git
    version: hidmic/namespace-messages-with-subfolder

Publisher

from time import sleep

import rclpy

from test_msgs.action import Something

# We do not recommend this style as ROS 2 provides timers for this purpose,
# and it is recommended that all nodes call a variation of spin.
# This example is only included for completeness because it is similar to examples in ROS 1.
# For periodic publication please see the other examples using timers.


def main(args=None):
    rclpy.init(args=args)

    node = rclpy.create_node('minimal_publisher')

    publisher = node.create_publisher(Something, 'topic')

    msg = Something()

    i = 0
    while rclpy.ok():
        msg.string_value = 'Hello World: %d' % i
        i += 1
        node.get_logger().info('Publishing: "%s"' % msg.string_value)
        publisher.publish(msg)
        sleep(0.5)  # seconds

    # Destroy the node explicitly
    # (optional - otherwise it will be done automatically
    # when the garbage collector destroys the node object)
    node.destroy_node()
    rclpy.shutdown()


if __name__ == '__main__':
    main()

Subscriber

import rclpy

from test_msgs.action import Something

g_node = None


def chatter_callback(msg):
    global g_node
    g_node.get_logger().info(
        'I heard: "%s"' % msg.string_value)


def main(args=None):
    global g_node
    rclpy.init(args=args)

    g_node = rclpy.create_node('minimal_subscriber')

    subscription = g_node.create_subscription(Something, 'topic', chatter_callback)
    subscription  # prevent unused variable warning

    while rclpy.ok():
        rclpy.spin_once(g_node)

    # Destroy the node explicitly
    # (optional - otherwise it will be done automatically
    # when the garbage collector destroys the node object)
    g_node.destroy_node()
    rclpy.shutdown()


if __name__ == '__main__':
    main()

Service Client

from test_msgs.action import Primitives

import rclpy


def main(args=None):
    rclpy.init(args=args)
    node = rclpy.create_node('minimal_client')
    cli = node.create_client(Primitives, 'add_two_ints')

    req = Primitives.Request()
    req.int8_value = 41
    req.int16_value = 1
    while not cli.wait_for_service(timeout_sec=1.0):
        node.get_logger().info('service not available, waiting again...')

    future = cli.call_async(req)
    rclpy.spin_until_future_complete(node, future)

    if future.result() is not None:
        node.get_logger().info(
            'Result of add_two_ints: for %d + %d = %d' %
            (req.int8_value, req.int16_value, future.result().int32_value))
    else:
        node.get_logger().info('Service call failed %r' % (future.exception(),))

    node.destroy_node()
    rclpy.shutdown()


if __name__ == '__main__':
    main()

Service server

from test_msgs.action import Primitives

import rclpy

g_node = None


def add_two_ints_callback(request, response):
    global g_node
    response.int32_value = request.int8_value + request.int16_value
    g_node.get_logger().info(
        'Incoming request\na: %d b: %d' % (request.int8_value, request.int16_value))

    return response


def main(args=None):
    global g_node
    rclpy.init(args=args)

    g_node = rclpy.create_node('minimal_service')

    srv = g_node.create_service(Primitives, 'add_two_ints', add_two_ints_callback)
    while rclpy.ok():
        rclpy.spin_once(g_node)

    # Destroy the service attached to the node explicitly
    # (optional - otherwise it will be done automatically
    # when the garbage collector destroys the node object)
    g_node.destroy_service(srv)
    rclpy.shutdown()


if __name__ == '__main__':
    main()

hidmic · 2018-10-30T18:29:00Z

@sloretz cbf6a2c fixes an issue with Python bindings for messages with the same name under different subfolders.

apojomovsky · 2018-10-30T19:47:14Z

Ok, given that we have green light I'm proceeding to merge this and its related PRs

Added support to msg/srv generation from within an action directory

dd88cb5

apojomovsky requested review from jacobperron, sloretz and hidmic October 26, 2018 18:13

tfoote added the in progress Actively being worked on (Kanban column) label Oct 26, 2018

hidmic requested changes Oct 26, 2018

View reviewed changes

Addresses peer review comments

66deb8d

apojomovsky added 3 commits October 26, 2018 17:53

Fixed style issue with python file

0fe280e

Fixed template c function naming

a9fa75b

Fixed cmake filest to process files under action directory

55a5c9c

sloretz reviewed Oct 30, 2018

View reviewed changes

Generate .c file for any .msg regardless of folder

98d630c

sloretz approved these changes Oct 30, 2018

View reviewed changes

Fixed function names in empy templates

cbf6a2c

apojomovsky merged commit 0df76f5 into master Oct 30, 2018

tfoote removed the in progress Actively being worked on (Kanban column) label Oct 30, 2018

apojomovsky deleted the apojomovsky/allow_generating_msg_and_srv_in_action branch October 30, 2018 19:48

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

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

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

apojomovsky commented Oct 26, 2018 •

edited by sloretz

Loading

hidmic left a comment

hidmic Oct 26, 2018

apojomovsky Oct 26, 2018

hidmic Oct 26, 2018

apojomovsky Oct 26, 2018

hidmic Oct 26, 2018 •

edited

Loading

apojomovsky Oct 26, 2018

apojomovsky commented Oct 26, 2018

sloretz Oct 30, 2018

sloretz Oct 30, 2018 •

edited

Loading

sloretz Oct 30, 2018

sloretz left a comment

hidmic commented Oct 30, 2018

apojomovsky commented Oct 30, 2018

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

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

Conversation

apojomovsky commented Oct 26, 2018 • edited by sloretz Loading

hidmic left a comment

Choose a reason for hiding this comment

hidmic Oct 26, 2018

Choose a reason for hiding this comment

apojomovsky Oct 26, 2018

Choose a reason for hiding this comment

hidmic Oct 26, 2018

Choose a reason for hiding this comment

apojomovsky Oct 26, 2018

Choose a reason for hiding this comment

hidmic Oct 26, 2018 • edited Loading

Choose a reason for hiding this comment

apojomovsky Oct 26, 2018

Choose a reason for hiding this comment

apojomovsky commented Oct 26, 2018

sloretz Oct 30, 2018

Choose a reason for hiding this comment

sloretz Oct 30, 2018 • edited Loading

Choose a reason for hiding this comment

sloretz Oct 30, 2018

Choose a reason for hiding this comment

sloretz left a comment

Choose a reason for hiding this comment

hidmic commented Oct 30, 2018

apojomovsky commented Oct 30, 2018

apojomovsky commented Oct 26, 2018 •

edited by sloretz

Loading

hidmic Oct 26, 2018 •

edited

Loading

sloretz Oct 30, 2018 •

edited

Loading