From cca21b79db1824ddfc5d9c886d7885caa462323c Mon Sep 17 00:00:00 2001 From: Michel Hidalgo Date: Mon, 10 Aug 2020 18:37:44 -0300 Subject: [PATCH 1/7] Update serialization/deserialization API documentation. Signed-off-by: Michel Hidalgo --- rmw/include/rmw/rmw.h | 21 +++++++++++++++++---- 1 file changed, 17 insertions(+), 4 deletions(-) diff --git a/rmw/include/rmw/rmw.h b/rmw/include/rmw/rmw.h index d57809d1..a7851799 100644 --- a/rmw/include/rmw/rmw.h +++ b/rmw/include/rmw/rmw.h @@ -556,7 +556,14 @@ rmw_publisher_assert_liveliness(const rmw_publisher_t * publisher); /** * The ROS message is serialized into a byte stream contained within the * rmw_serialized_message_t structure. - * The serialization format depends on the underlying middleware. + * The serialization format depends on the underlying implementation. + * + * \pre Given ROS message must be a valid non-null instance, initialized + * by the caller and matching the provided typesupport. + * \pre Given typesupport must be a valid non-null instance, as provided + * by `rosidl` APIs. + * \pre Given serialized message must be a valid non-null instance, initialized + * by the caller. * * \param[in] ros_message the typed ROS message * \param[in] type_support the typesupport for the ROS message @@ -577,10 +584,16 @@ rmw_serialize( /** * The given rmw_serialized_message_t's internal byte stream buffer is deserialized * into the given ROS message. - * The ROS message must already be allocated and initialized, and must match - * the given typesupport structure. * The serialization format expected in the rmw_serialized_message_t depends on the - * underlying middleware. + * underlying implementation. + * + * \pre Given serialized message must be a valid non-null instance, such + * as that returned by `rmw_serialize()`, matching provided typesupport + * and ROS message. + * \pre Given typesupport must be a valid non-null instance, as provided + * by `rosidl` APIs. + * \pre Given ROS message must be a valid non-null instance, initialized + * by the caller. * * \param[in] serialized_message the serialized message holding the byte stream * \param[in] type_support the typesupport for the typed ros message From b821e77144185d17dac276fdf3bb2b4b094687ea Mon Sep 17 00:00:00 2001 From: Michel Hidalgo Date: Mon, 10 Aug 2020 19:11:35 -0300 Subject: [PATCH 2/7] Add rmw_serialize_message_t API documentation. Signed-off-by: Michel Hidalgo --- rmw/include/rmw/serialized_message.h | 40 ++++++++++++++++++++++++++++ 1 file changed, 40 insertions(+) diff --git a/rmw/include/rmw/serialized_message.h b/rmw/include/rmw/serialized_message.h index 08ed3ac9..79e1d960 100644 --- a/rmw/include/rmw/serialized_message.h +++ b/rmw/include/rmw/serialized_message.h @@ -29,9 +29,49 @@ extern "C" * more complex and is therefore aliased. */ typedef rcutils_uint8_array_t rmw_serialized_message_t; + +/// Return a zero initialized serialized message struct. #define rmw_get_zero_initialized_serialized_message rcutils_get_zero_initialized_uint8_array + +/// Initialize a serialized message, zero initializing its contents. +/** + * \param[inout] serialized_message a pointer to the serialized message to initialize + * \param[in] message_capacity the size of the memory to allocate + * \param[in] allocator the allocator to use for the memory allocation + * \return `RMW_RET_OK` if successful, or + * \return `RMW_RET_INVALID_ARGUMENT` if any arguments are invalid, or + * \return 'RMW_RET_BAD_ALLOC` if no memory could be allocated correctly + * \return `RMW_RET_ERROR` if an unexpected error occurs + */ #define rmw_serialized_message_init rcutils_uint8_array_init + +/// Finalize a serialized message. +/** + * Passing an uninitialized instance to this function leads to undefined + * behavior. + * + * \param[in] serialized_message pointer to the serialized message to be cleaned up + * \return `RMW_RET_OK` if successful, or + * \return `RMW_RET_INVALID_ARGUMENT` if the serialized_message argument is invalid + * \return `RMW_RET_ERROR` if an unexpected error occurs + */ #define rmw_serialized_message_fini rcutils_uint8_array_fini + +/// Resize the internal buffer of the serialized message. +/** + * The internal buffer of the serialized message can be resized dynamically if needed. + * If the new size is smaller than the current capacity, then the memory is + * truncated. + * Be aware, that this might deallocate the memory and therefore invalidates any + * pointers to this storage. + * + * \param[inout] serialized_message pointer to the serialized message to be resized + * \param[in] new_size the new size of the internal buffer + * \return `RMW_RET_OK` if successful, or + * \return `RMW_RET_INVALID_ARGUMENT` if new_size is set to zero + * \return `RMW_RET_BAD_ALLOC` if memory allocation failed, or + * \return `RMW_RET_ERROR` if an unexpected error occurs + */ #define rmw_serialized_message_resize rcutils_uint8_array_resize #if __cplusplus From 6e652f2e067c1130fc9dd73c830248b49af872d8 Mon Sep 17 00:00:00 2001 From: Michel Hidalgo Date: Tue, 11 Aug 2020 12:34:43 -0300 Subject: [PATCH 3/7] Address peer review comments. Signed-off-by: Michel Hidalgo --- rmw/include/rmw/serialized_message.h | 34 ++++++++++++++++++---------- 1 file changed, 22 insertions(+), 12 deletions(-) diff --git a/rmw/include/rmw/serialized_message.h b/rmw/include/rmw/serialized_message.h index 79e1d960..e8882ab2 100644 --- a/rmw/include/rmw/serialized_message.h +++ b/rmw/include/rmw/serialized_message.h @@ -35,44 +35,54 @@ typedef rcutils_uint8_array_t rmw_serialized_message_t; /// Initialize a serialized message, zero initializing its contents. /** + * \pre Given serialized message must have been zero initialized. + * * \param[inout] serialized_message a pointer to the serialized message to initialize * \param[in] message_capacity the size of the memory to allocate * \param[in] allocator the allocator to use for the memory allocation * \return `RMW_RET_OK` if successful, or * \return `RMW_RET_INVALID_ARGUMENT` if any arguments are invalid, or - * \return 'RMW_RET_BAD_ALLOC` if no memory could be allocated correctly + * \return 'RMW_RET_BAD_ALLOC` if no memory could be allocated correctly, or * \return `RMW_RET_ERROR` if an unexpected error occurs */ -#define rmw_serialized_message_init rcutils_uint8_array_init +#define rmw_serialized_message_init(serialized_message, message_capacity, allocator) \ + rcutils_uint8_array_init(serialized_message, message_capacity, allocator) /// Finalize a serialized message. /** - * Passing an uninitialized instance to this function leads to undefined - * behavior. + * \pre Given serialized message must have been initialized with `rmw_serialized_message_init()`. + * + * \remarks If serialized message is zero initialized, then `RMW_RET_INVALID_ARGUMENT` is returned. * * \param[in] serialized_message pointer to the serialized message to be cleaned up * \return `RMW_RET_OK` if successful, or - * \return `RMW_RET_INVALID_ARGUMENT` if the serialized_message argument is invalid + * \return `RMW_RET_INVALID_ARGUMENT` if serialized_message is invalid, or * \return `RMW_RET_ERROR` if an unexpected error occurs */ -#define rmw_serialized_message_fini rcutils_uint8_array_fini +#define rmw_serialized_message_fini(serialized_message) \ + rcutils_uint8_array_fini(serialized_message) /// Resize the internal buffer of the serialized message. /** - * The internal buffer of the serialized message can be resized dynamically if needed. + * The internal buffer of the serialized message can be resized dynamically + * if needed. * If the new size is smaller than the current capacity, then the memory is * truncated. - * Be aware, that this might deallocate the memory and therefore invalidates any - * pointers to this storage. * - * \param[inout] serialized_message pointer to the serialized message to be resized + * \remarks Be aware that this might deallocate the memory and therefore + * invalidate any pointers to the internal buffer. + * + * \param[inout] serialized_message pointer to the serialized message + * to be resized * \param[in] new_size the new size of the internal buffer * \return `RMW_RET_OK` if successful, or - * \return `RMW_RET_INVALID_ARGUMENT` if new_size is set to zero + * \return `RMW_RET_INVALID_ARGUMENT` if serialized_message is invalid + * or new_size is set to zero, or * \return `RMW_RET_BAD_ALLOC` if memory allocation failed, or * \return `RMW_RET_ERROR` if an unexpected error occurs */ -#define rmw_serialized_message_resize rcutils_uint8_array_resize +#define rmw_serialized_message_resize(serialized_message, new_size) \ + rcutils_uint8_array_resize(serialized_message, new_size) #if __cplusplus } From 2625c33f83b1c29d0a29aa5378e2ab0b9b0a656b Mon Sep 17 00:00:00 2001 From: Michel Hidalgo Date: Tue, 11 Aug 2020 12:35:15 -0300 Subject: [PATCH 4/7] Document rmw_(de)serialize() API attributes. Signed-off-by: Michel Hidalgo --- rmw/include/rmw/rmw.h | 22 ++++++++++++++++++++++ 1 file changed, 22 insertions(+) diff --git a/rmw/include/rmw/rmw.h b/rmw/include/rmw/rmw.h index a7851799..ee835a30 100644 --- a/rmw/include/rmw/rmw.h +++ b/rmw/include/rmw/rmw.h @@ -565,6 +565,18 @@ rmw_publisher_assert_liveliness(const rmw_publisher_t * publisher); * \pre Given serialized message must be a valid non-null instance, initialized * by the caller. * + *
+ * Attribute | Adherence + * ------------------ | ------------- + * Allocates Memory | Maybe [1] + * Thread-Safe | Yes [2] + * Uses Atomics | Yes + * Lock-Free | Yes + * [1] if the given serialized message does not have enough capacity to hold + * the ROS message serialization + * [2] as long as no two concurrent calls make use of the same `serialized_message` + * object and a given `serialized_message` allocator is thread-safe as well + * * \param[in] ros_message the typed ROS message * \param[in] type_support the typesupport for the ROS message * \param[out] serialized_message the destination for the serialize ROS message @@ -595,6 +607,16 @@ rmw_serialize( * \pre Given ROS message must be a valid non-null instance, initialized * by the caller. * + *
+ * Attribute | Adherence + * ------------------ | ------------- + * Allocates Memory | Maybe [1] + * Thread-Safe | Yes [2] + * Uses Atomics | Yes + * Lock-Free | Yes + * [1] if the given ROS message contains unbounded fields + * [2] as long as no two concurrent calls make use of the same `ros_message` object + * * \param[in] serialized_message the serialized message holding the byte stream * \param[in] type_support the typesupport for the typed ros message * \param[out] ros_message destination for the deserialized ROS message From b43d14edf66b4a2a406b753075f9b1f9452d7e83 Mon Sep 17 00:00:00 2001 From: Michel Hidalgo Date: Tue, 11 Aug 2020 12:37:39 -0300 Subject: [PATCH 5/7] Relax rmw_(de)serialize() API attributes. Signed-off-by: Michel Hidalgo --- rmw/include/rmw/rmw.h | 10 ++++++---- 1 file changed, 6 insertions(+), 4 deletions(-) diff --git a/rmw/include/rmw/rmw.h b/rmw/include/rmw/rmw.h index ee835a30..a800c681 100644 --- a/rmw/include/rmw/rmw.h +++ b/rmw/include/rmw/rmw.h @@ -570,12 +570,13 @@ rmw_publisher_assert_liveliness(const rmw_publisher_t * publisher); * ------------------ | ------------- * Allocates Memory | Maybe [1] * Thread-Safe | Yes [2] - * Uses Atomics | Yes - * Lock-Free | Yes + * Uses Atomics | Maybe [3] + * Lock-Free | Maybe [3] * [1] if the given serialized message does not have enough capacity to hold * the ROS message serialization * [2] as long as no two concurrent calls make use of the same `serialized_message` * object and a given `serialized_message` allocator is thread-safe as well + * [3] rmw implementation defined, check the implementation documentation * * \param[in] ros_message the typed ROS message * \param[in] type_support the typesupport for the ROS message @@ -612,10 +613,11 @@ rmw_serialize( * ------------------ | ------------- * Allocates Memory | Maybe [1] * Thread-Safe | Yes [2] - * Uses Atomics | Yes - * Lock-Free | Yes + * Uses Atomics | Maybe [3] + * Lock-Free | Maybe [3] * [1] if the given ROS message contains unbounded fields * [2] as long as no two concurrent calls make use of the same `ros_message` object + * [3] rmw implementation defined, check the implementation documentation * * \param[in] serialized_message the serialized message holding the byte stream * \param[in] type_support the typesupport for the typed ros message From 106d36ce16371b86e894a4f4c0684b3680d11c8e Mon Sep 17 00:00:00 2001 From: Michel Hidalgo Date: Tue, 11 Aug 2020 14:29:10 -0300 Subject: [PATCH 6/7] Augment rmw_serialized_message_t documentation. Signed-off-by: Michel Hidalgo --- rmw/include/rmw/serialized_message.h | 22 ++++++++++++++++++---- 1 file changed, 18 insertions(+), 4 deletions(-) diff --git a/rmw/include/rmw/serialized_message.h b/rmw/include/rmw/serialized_message.h index e8882ab2..4f2d34c3 100644 --- a/rmw/include/rmw/serialized_message.h +++ b/rmw/include/rmw/serialized_message.h @@ -22,9 +22,18 @@ extern "C" #include "rcutils/types/uint8_array.h" -// aliases for rcutils_uint8_array_t -/* - * For now this is a simple aliasing from a serialized message to a uint8 array. +/** + * \struct rmw_serialized_message_t + * + * \brief Serialized message as a string of bytes. + * + * It includes (but it is not limited to) the following members: + * \i \c buffer the reference to internal storage, as a pointer + * \i \c buffer_length the size of stored contents, as an unsigned integer + * \i \c buffer_capacity the capacity of internal storage, as an unsigned integer + */ + +/* For now this is a simple aliasing from a serialized message to a uint8 array. * However, in future developments this serialized message can become something * more complex and is therefore aliased. */ @@ -69,7 +78,12 @@ typedef rcutils_uint8_array_t rmw_serialized_message_t; * If the new size is smaller than the current capacity, then the memory is * truncated. * - * \remarks Be aware that this might deallocate the memory and therefore + * \pre Given serialized message must have been initialized with `rmw_serialized_message_init()`. + * + * \remarks If serialized message is zero initialized, then `RMW_RET_INVALID_ARGUMENT` + * is returned. + * + * \warning Be aware that this might deallocate the memory and therefore * invalidate any pointers to the internal buffer. * * \param[inout] serialized_message pointer to the serialized message From 839edab9ae7cb76548b137f9ea1872e124961e88 Mon Sep 17 00:00:00 2001 From: Michel Hidalgo Date: Wed, 12 Aug 2020 16:10:27 -0300 Subject: [PATCH 7/7] Relax thread-safety specs. Signed-off-by: Michel Hidalgo --- rmw/include/rmw/rmw.h | 19 ++++++++----------- 1 file changed, 8 insertions(+), 11 deletions(-) diff --git a/rmw/include/rmw/rmw.h b/rmw/include/rmw/rmw.h index a800c681..f9d19cbd 100644 --- a/rmw/include/rmw/rmw.h +++ b/rmw/include/rmw/rmw.h @@ -569,14 +569,12 @@ rmw_publisher_assert_liveliness(const rmw_publisher_t * publisher); * Attribute | Adherence * ------------------ | ------------- * Allocates Memory | Maybe [1] - * Thread-Safe | Yes [2] - * Uses Atomics | Maybe [3] - * Lock-Free | Maybe [3] + * Thread-Safe | No + * Uses Atomics | Maybe [2] + * Lock-Free | Maybe [2] * [1] if the given serialized message does not have enough capacity to hold * the ROS message serialization - * [2] as long as no two concurrent calls make use of the same `serialized_message` - * object and a given `serialized_message` allocator is thread-safe as well - * [3] rmw implementation defined, check the implementation documentation + * [2] rmw implementation defined, check the implementation documentation * * \param[in] ros_message the typed ROS message * \param[in] type_support the typesupport for the ROS message @@ -612,12 +610,11 @@ rmw_serialize( * Attribute | Adherence * ------------------ | ------------- * Allocates Memory | Maybe [1] - * Thread-Safe | Yes [2] - * Uses Atomics | Maybe [3] - * Lock-Free | Maybe [3] + * Thread-Safe | No + * Uses Atomics | Maybe [2] + * Lock-Free | Maybe [2] * [1] if the given ROS message contains unbounded fields - * [2] as long as no two concurrent calls make use of the same `ros_message` object - * [3] rmw implementation defined, check the implementation documentation + * [2] rmw implementation defined, check the implementation documentation * * \param[in] serialized_message the serialized message holding the byte stream * \param[in] type_support the typesupport for the typed ros message