init and fini function for creating introspection messages #416

Karsten1987 · 2019-10-02T08:19:52Z

Signed-off-by: Karsten Knese karsten@openrobotics.org

With this PR, one can create a ROS message solely by providing the rosidl_typesupport_introspection_<c|cpp>.

depicted below for C, works analog to this in C++

  auto string_typesupports =
    ROSIDL_GET_MSG_TYPE_SUPPORT(std_msgs, msg, String);
  auto string_ts_c = get_message_typesupport_handle(string_typesupports, rosidl_typesupport_introspection_c__identifier);
  auto introspection_ts = static_cast<const rosidl_typesupport_introspection_c__MessageMembers *>(string_ts_c->data);
  void * message_memory = malloc(introspection_ts->size_of_);
  introspection_ts->init_function(message_memory, true);
  std_msgs__msg__String msg = *static_cast<std_msgs__msg__String *>(message_memory);
  rosidl_generator_c__String__assign(&msg.data, "Hello world!");
  introspection_ts->fini_function(message_memory);
  free(message_memory);
  message_memory = nullptr;

Question: I know that in C we can specify whether the data structure should be default initialized or not. Hence the second bool parameter for the init function. However, looking at the initializer functions for the C datatype, I haven't found any way to specify that. How do I do that?

dirk-thomas · 2019-10-02T19:57:10Z

rosidl_typesupport_introspection_cpp/resource/msg__type_support.cpp.em

@@ -49,6 +49,17 @@ namespace @(ns)
 namespace rosidl_typesupport_introspection_cpp
 {

+void init_function__@(message.structure.namespaced_type.name)(void * message_memory, bool default_initialize)


The constructor of C++ message classes uses more than a bool to determine how to initialize the values (see

rosidl/rosidl_generator_cpp/resource/msg__struct.hpp.em

Line 143 in 2e7f775

explicit @(message.structure.namespaced_type.name)_(rosidl_generator_cpp::MessageInitialization _init = rosidl_generator_cpp::MessageInitialization::ALL)

). Why shouldn't this signature offer the same?

Also atm the argument doesn't seem to be used?

That is correct, I've used a bool as a placeholder because I wasn't exactly aware of the right datastructure/enum to use for the initializers. I'll change that accordingly.
After talking with @clalancette, the default initializers are only yet implemented in C++. I'll change the API for the C datatypes accordingly though to accept a enum as described here: https://github.com/ros2/rosidl/blob/master/rosidl_generator_c/include/rosidl_generator_c/message_initialization.h#L20

done in 319d6c7

dirk-thomas · 2019-10-02T19:57:43Z

rosidl_typesupport_introspection_cpp/resource/msg__type_support.cpp.em

@@ -132,7 +143,7 @@ for index, member in enumerate(message.structure.members):
        # size_t string_upper_bound
        print('    0,  // upper bound of string')
        # const rosidl_generator_c::MessageTypeSupportHandle * members_
-        print('    NULL,  // members of sub message')
+        print('    nullptr,  // members of sub message')


This seems unrelated?

Same below.

it is unrelated, but given that it's really just a two line change, I believe it can be changed within the scope of this PR as well.
I don't have any problem reverting this change here though if preferred.

rosidl_typesupport_introspection_c/resource/msg__type_support.c.em

dirk-thomas · 2019-10-02T19:59:05Z

rosidl_typesupport_introspection_c/resource/msg__type_support.c.em

-  @(function_prefix)__@(message.structure.namespaced_type.name)_message_member_array  // message members
+  @(function_prefix)__@(message.structure.namespaced_type.name)_message_member_array,  // message members
+  @(function_prefix)__init_function,  // function to initialize message memory (memory has to be allocated)
+  @(function_prefix)__fini_function  // function to terminate message instance (will not free memory)


Is there a reason why these functions don't use _@(message.structure.namespaced_type.name) as the other symbols?

done in 71404ca

dirk-thomas · 2019-10-02T19:59:36Z

rosidl_typesupport_introspection_cpp/resource/msg__type_support.cpp.em

-  @(message.structure.namespaced_type.name)_message_member_array  // message members
+  @(message.structure.namespaced_type.name)_message_member_array,  // message members
+  init_function__@(message.structure.namespaced_type.name),  // function to initialize message memory (memory has to be allocated)
+  fini_function__@(message.structure.namespaced_type.name)  // function to terminate message instance (will not free memory)


Is there a reason why these functions use the opposite order of message name and hard coded name parts?

done in 71404ca

dirk-thomas · 2019-10-02T20:01:50Z

fixes #404

That tickets mentions message allocation atm. Either it needs to be changed (see also the pending question #404 (comment)) or this PR addresses something different.

Karsten1987 · 2019-10-02T22:25:14Z

first round of CI (using fastrtps dynamic as they make use of typesupport introspection):

Linux
Linux-aarch64
macOS
Windows

Karsten1987 · 2019-10-03T05:19:10Z

new round of CI:

Linux
Linux-aarch64
macOS
Windows

Karsten1987 · 2019-10-03T15:41:07Z

I put that up for another round of reviews.
CI on MS fails because of connectivity errors.

dirk-thomas

One more minor comment inline.

LGTM with passing CI.

dirk-thomas · 2019-10-03T16:13:59Z

rosidl_typesupport_introspection_c/resource/msg__type_support.c.em

+void @(function_prefix)__@(message.structure.namespaced_type.name)_init_function(
+  void * message_memory, enum rosidl_runtime_c_message_initialization _init)
+{
+  // initializers are not yet implemented for typesupport c


Add a TODO here.

Karsten1987 · 2019-10-03T17:15:26Z

re-running windows only:

Karsten1987 · 2019-10-03T19:51:21Z

ms warnings seem unrelated.

Signed-off-by: Karsten Knese <karsten@openrobotics.org>

dirk-thomas reviewed Oct 2, 2019

View reviewed changes

dirk-thomas approved these changes Oct 3, 2019

View reviewed changes

jacobperron mentioned this pull request Oct 3, 2019

Fix Windows compiler warning ros/ros_tutorials#69

Merged

Karsten1987 mentioned this pull request Oct 3, 2019

zero copy api ros2/rosbag2#168

Merged

mjcarroll approved these changes Oct 15, 2019

View reviewed changes

Karsten1987 added 5 commits October 17, 2019 12:44

init and fini function for creating introspection messages

444228e

Signed-off-by: Karsten Knese <karsten@openrobotics.org>

consistent member naming

4db0edf

Signed-off-by: Karsten Knese <karsten@openrobotics.org>

init function takes message initializer enum

89a3248

Signed-off-by: Karsten Knese <karsten@openrobotics.org>

fix linter

de52c54

Signed-off-by: Karsten Knese <karsten@openrobotics.org>

add TODO notice for initializers in C

e285ada

Signed-off-by: Karsten Knese <karsten@openrobotics.org>

Karsten1987 force-pushed the zero_copy_api branch from 8f5beea to e285ada Compare October 18, 2019 20:00

Karsten1987 merged commit e146b6d into master Oct 18, 2019

delete-merged-branch bot deleted the zero_copy_api branch October 18, 2019 21:50

Karsten1987 mentioned this pull request Feb 18, 2020

feature: replace rosbag time stamper with sim time(current time) ros2/rosbag2#300

Closed

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

init and fini function for creating introspection messages #416

init and fini function for creating introspection messages #416

Karsten1987 commented Oct 2, 2019

dirk-thomas Oct 2, 2019

Karsten1987 Oct 2, 2019

Karsten1987 Oct 2, 2019

dirk-thomas Oct 2, 2019

Karsten1987 Oct 2, 2019

dirk-thomas Oct 2, 2019

Karsten1987 Oct 2, 2019

dirk-thomas Oct 2, 2019

Karsten1987 Oct 2, 2019

dirk-thomas commented Oct 2, 2019

Karsten1987 commented Oct 2, 2019 •

edited

Loading

Karsten1987 commented Oct 3, 2019

Karsten1987 commented Oct 3, 2019

dirk-thomas left a comment

dirk-thomas Oct 3, 2019

Karsten1987 commented Oct 3, 2019

Karsten1987 commented Oct 3, 2019

init and fini function for creating introspection messages #416

init and fini function for creating introspection messages #416

Conversation

Karsten1987 commented Oct 2, 2019

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

dirk-thomas commented Oct 2, 2019

Karsten1987 commented Oct 2, 2019 • edited Loading

Karsten1987 commented Oct 3, 2019

Karsten1987 commented Oct 3, 2019

dirk-thomas left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Karsten1987 commented Oct 3, 2019

Karsten1987 commented Oct 3, 2019

Karsten1987 commented Oct 2, 2019 •

edited

Loading