[20310] Feature: topic keys with non breaking ABI #2
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
MiguelCompany
requested changes
Feb 14, 2024
rosidl_typesupport_fastrtps_c/include/rosidl_typesupport_fastrtps_c/identifier.h
Show resolved
Hide resolved
rosidl_typesupport_fastrtps_cpp/include/rosidl_typesupport_fastrtps_cpp/identifier.hpp
Show resolved
Hide resolved
rosidl_typesupport_fastrtps_cpp/resource/msg__type_support.cpp.em
Outdated
Show resolved
Hide resolved
rosidl_typesupport_fastrtps_cpp/resource/msg__type_support.cpp.em
Outdated
Show resolved
Hide resolved
MiguelCompany
requested changes
Feb 27, 2024
Comment on lines
32
to
35
| /// Callback function for message serialization | ||
| /** | ||
| * \param[in] untyped_ros_message Type erased pointer to message instance. | ||
| * \param [in,out] Serialized FastCDR data object. |
There was a problem hiding this comment.
Suggested change
| /// Callback function for message serialization | |
| /** | |
| * \param[in] untyped_ros_message Type erased pointer to message instance. | |
| * \param [in,out] Serialized FastCDR data object. | |
| /// Callback function for key serialization | |
| /** | |
| * \param [in] untyped_ros_message Type erased pointer to message instance. | |
| * \param [in,out] Serialized FastCDR data object. |
Comment on lines
42
to
45
| /// Callback function for message deserialization | ||
| /** | ||
| * \param [in] Serialized FastCDR data object. | ||
| * \param[out] untyped_ros_message Type erased pointer to message instance. |
There was a problem hiding this comment.
Suggested change
| /// Callback function for message deserialization | |
| /** | |
| * \param [in] Serialized FastCDR data object. | |
| * \param[out] untyped_ros_message Type erased pointer to message instance. | |
| /// Callback function for key deserialization | |
| /** | |
| * \param [in] Serialized FastCDR data object. | |
| * \param [out] untyped_ros_message Type erased pointer to message instance. |
|
|
||
| /// Callback function to get size of the key data | ||
| /** | ||
| * \return The size of the serialized message in bytes. |
There was a problem hiding this comment.
Add corresponding \param descriptions here
Comment on lines
52
to
53
|
|
||
| struct message_type_support_key_callbacks_t; |
There was a problem hiding this comment.
I don't think this is necessary
Suggested change
| struct message_type_support_key_callbacks_t; |
| @@ -90,6 +104,89 @@ namespace @(ns) | |||
| @{forward_declared_types.add(message.structure.namespaced_type.namespaced_name())}@ | |||
| namespace typesupport_fastrtps_cpp | |||
| { | |||
| @{ | |||
| def generate_member_for_cdr_serialize(member, suffix): | |||
| from rosidl_generator_cpp import msg_type_only_to_cpp | |||
There was a problem hiding this comment.
Add documentation of this python method
Comment on lines
616
to
694
| static | ||
| size_t | ||
| _@(message.structure.namespaced_type.name)__get_serialized_size_key( | ||
| const void * untyped_ros_message, | ||
| size_t initial_alignment) | ||
| { | ||
| auto typed_message = | ||
| static_cast<const @('::'.join([package_name] + list(interface_path.parents[0].parts) + [message.structure.namespaced_type.name])) *>( | ||
| untyped_ros_message); | ||
|
|
||
| return static_cast<uint32_t>(get_serialized_size_key(*typed_message, initial_alignment)); | ||
| } |
There was a problem hiding this comment.
Suggested change
| static | |
| size_t | |
| _@(message.structure.namespaced_type.name)__get_serialized_size_key( | |
| const void * untyped_ros_message, | |
| size_t initial_alignment) | |
| { | |
| auto typed_message = | |
| static_cast<const @('::'.join([package_name] + list(interface_path.parents[0].parts) + [message.structure.namespaced_type.name])) *>( | |
| untyped_ros_message); | |
| return static_cast<uint32_t>(get_serialized_size_key(*typed_message, initial_alignment)); | |
| } | |
| static | |
| size_t | |
| _@(message.structure.namespaced_type.name)__get_serialized_size_key( | |
| const void * untyped_ros_message) | |
| { | |
| auto typed_message = | |
| static_cast<const @('::'.join([package_name] + list(interface_path.parents[0].parts) + [message.structure.namespaced_type.name])) *>( | |
| untyped_ros_message); | |
| return get_serialized_size_key(*typed_message, 0); | |
| } |
Comment on lines
629
to
639
| static size_t _@(message.structure.namespaced_type.name)__max_serialized_size_key( | ||
| size_t current_alignment, | ||
| bool & is_unbounded) | ||
| { | ||
| bool full_bounded = true; | ||
| bool is_plain = true; | ||
|
|
||
| size_t ret_val = max_serialized_size_key_@(message.structure.namespaced_type.name)( | ||
| full_bounded, | ||
| is_plain, | ||
| current_alignment); |
There was a problem hiding this comment.
Suggested change
| static size_t _@(message.structure.namespaced_type.name)__max_serialized_size_key( | |
| size_t current_alignment, | |
| bool & is_unbounded) | |
| { | |
| bool full_bounded = true; | |
| bool is_plain = true; | |
| size_t ret_val = max_serialized_size_key_@(message.structure.namespaced_type.name)( | |
| full_bounded, | |
| is_plain, | |
| current_alignment); | |
| static size_t _@(message.structure.namespaced_type.name)__max_serialized_size_key( | |
| bool & is_unbounded) | |
| { | |
| bool full_bounded = true; | |
| bool is_plain = true; | |
| size_t ret_val = max_serialized_size_key_@(message.structure.namespaced_type.name)( | |
| full_bounded, | |
| is_plain, | |
| 0); |
There was a problem hiding this comment.
I also changed it in the c interfaces
| @@ -162,6 +177,134 @@ const rosidl_message_type_support_t * | |||
|
|
|||
| using _@(message.structure.namespaced_type.name)__ros_msg_type = @('__'.join(message.structure.namespaced_type.namespaced_name())); | |||
|
|
|||
| @{ | |||
| def generate_member_for_cdr_serialize(member, suffix): | |||
| from rosidl_generator_cpp import msg_type_only_to_cpp | |||
There was a problem hiding this comment.
Add documentation for this python method
| if suffix == '': | ||
| strlist.append(' if (!callbacks->cdr_serialize(') | ||
| else: | ||
| strlist.append(' if (!callbacks->%s_callbacks->cdr_serialize%s(' % ((''.join(c for c in suffix if c not in '(){}<>_*')), suffix)) |
There was a problem hiding this comment.
Beware! callbacks->key_callbacks might be nullptr!
| return current_alignment - initial_alignment; | ||
| } | ||
|
|
||
| @[ if message.structure.has_any_member_with_annotation('key') ]@ |
There was a problem hiding this comment.
I think we should follow the same approach we are using for the cpp type support, i.e. always declaring and exporting the type-aware implementations, and then generate the void * implementations when the type has a key annotation.
This would imply changing the way we call nested types and making it not rely on the callbacks structure
There was a problem hiding this comment.
Okay, lets give it a try. Then, not applying the former comment
…ethod Signed-off-by: Mario Dominguez <[email protected]>
Signed-off-by: Mario Dominguez <[email protected]>
Signed-off-by: Mario Dominguez <[email protected]>
…s for supporting keys Signed-off-by: Mario Dominguez <[email protected]>
…ods for supporting keys Signed-off-by: Mario Dominguez <[email protected]>
Signed-off-by: Mario Dominguez <[email protected]>
… structure Signed-off-by: Mario Dominguez <[email protected]>
Signed-off-by: Mario Dominguez <[email protected]>
…erialized Signed-off-by: Mario Dominguez <[email protected]>
…called from outside in headers Signed-off-by: Mario Dominguez <[email protected]>
…rations Signed-off-by: Mario Dominguez <[email protected]>
…ate_members_for_cdr_serialize Signed-off-by: Mario Dominguez <[email protected]>
…ate_members_for_get_serialized_size Signed-off-by: Mario Dominguez <[email protected]>
…ate_members_for_max_serialized_size Signed-off-by: Mario Dominguez <[email protected]>
…llback structure Signed-off-by: Mario Dominguez <[email protected]>
…alize_key and cdr_deserialize_key methods that are not going to be used nor exported if the type do not have keys Signed-off-by: Mario Dominguez <[email protected]>
…e called from outside in headers Signed-off-by: Mario Dominguez <[email protected]>
…larations Signed-off-by: Mario Dominguez <[email protected]>
…erate_members_for_cdr_serialize. Also, generate proxy methods only when the type has a key Signed-off-by: Mario Dominguez <[email protected]>
…erate_members_for_get_serialized_size adn proxy Signed-off-by: Mario Dominguez <[email protected]>
…erate_members_for_max_serialized_size and proxy Signed-off-by: Mario Dominguez <[email protected]>
…ey method and proxy Signed-off-by: Mario Dominguez <[email protected]>
…callback structure Signed-off-by: Mario Dominguez <[email protected]>
Signed-off-by: Mario Dominguez <[email protected]>
Signed-off-by: Mario Dominguez <[email protected]>
Mario-DL
force-pushed
the
feature/no-abi-break-keys
branch
from
9a332c7
to
491fea2
Compare
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
This PR brings the changes in the
rosidl_typesupport_fastrtpsgenerator packages to support topic keys feature without breaking ABI.Note: This branch starts from the previous work from #1