change generators to IDL-based pipeline#15
Conversation
ae4e5a5 to
efb8f9b
Compare
efb8f9b to
8a90203
Compare
hidmic
left a comment
There was a problem hiding this comment.
Is this PR lacking the checks to support actions that both FastRTPS and OpenSplice will have? Why?
The generator will be refactored to use the IDL-based pipeline so no need to add this temporarily. |
8a90203 to
dab5a6b
Compare
|
Wouldn't a C++ template approach reduce the need for vendor specific code, have one generation from msg to idl that is generic and provide a set of traits to the template instantiation to work with a specific vendor? The amount of duplicated code between DDS vendors looks huge to me |
@jwillemsen While certainly possible it is out-of-scope for the IDL work in this PR. The amount of changes to switch the pipeline to use IDL is already extremely large. But after that has landed we would happily take pull requests to consolidate these parts of the code base. |
* Switch C++ type support generation pipeline. * Adds outer C++ IDL templates. * Adapts C++ msg & srv header templates. * Adapts C++ msg & srv source templates. * Switch C type support generation pipeline. * Adds outer C IDL templates. * Adapts C msg & srv header templates. * Adapts C msg source template. * Adapts C msg & srv source templates. * Adds missing Python imports in templates. * Converts IDL files to RTI Connext IDL files. * Fixing typesupport generation issues. * Uses rosidl_dds again for IDL2IDL conversion. * Applies many fixes to C & C++ typesupport. * Fixes for C++ srv templates. * Many fixes to C & C++ type support. * Yet more fixes to C & C++ type support. * Fixes copyright dates. * Adds back attribute to generated code constants. * Adds missing type support for action messages.
c125802 to
9f271ba
Compare
hidmic
left a comment
There was a problem hiding this comment.
LGTM but for a few minor comments and pending green CI
| @[ elif field.type.is_primitive_type()]@ | ||
| dds_message.@(field.name)_[static_cast<DDS_Long>(i)] = | ||
| ros_message.@(field.name)[i]; | ||
| @[ if isinstance(member.type.basetype, BaseString)]@ |
There was a problem hiding this comment.
@dirk-thomas just to double check, DDS_String_* functions operate with wchar_t-based std::base_string instances, correct? Otherwise, consider checking for String and asserting on WString.
| dds_message.@(member.name)_[static_cast<DDS_Long>(i)]@(' == DDS_BOOLEAN_TRUE' if member.type.basetype.type == 'boolean' else ''); | ||
| @[ elif isinstance(member.type.basetype, BaseString)]@ | ||
| ros_message.@(member.name)[i] = | ||
| dds_message.@(member.name)_[static_cast<DDS_Long>(i)]; |
There was a problem hiding this comment.
@dirk-thomas same comment about wchar_t strings present or not.
Connected to ros2/rosidl#334.