Skip to content

Dynamic Subscription (BONUS: Allocators): rosidl_dynamic_typesupport #2

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
merged 4 commits into from
Apr 11, 2023
Merged

Dynamic Subscription (BONUS: Allocators): rosidl_dynamic_typesupport #2

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
4 commits
Select commit Hold shift + click to select a range
69d2518
Refactor dynamic message type support impl to use allocators
methylDragon Apr 9, 2023
7d0d91b
Refactor serialization support to use allocators and refs
methylDragon Apr 10, 2023
7693180
Refactor dynamic type support structs to use allocators and refs
methylDragon Apr 10, 2023
efcac65
Check if allocator is valid
methylDragon Apr 10, 2023
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
1 change: 0 additions & 1 deletion CMakeLists.txt
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -36,7 +36,6 @@ add_library(${PROJECT_NAME}

"src/dynamic_message_type_support_struct.c"
"src/identifier.c"
"src/types.c"
)
if(WIN32)
target_compile_definitions(${PROJECT_NAME}
Expand Down
19 changes: 11 additions & 8 deletions README.md
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -10,25 +10,28 @@ Properly implemented, a user should be able to, given a serialized buffer, the s

```cpp
// Init Serialization Support (in this case, using FastRTPS)
rosidl_dynamic_typesupport_serialization_support_t * serialization_support =
rosidl_dynamic_typesupport_serialization_support_create(
rosidl_dynamic_typesupport_fastrtps_create_serialization_support_impl(),
rosidl_dynamic_typesupport_fastrtps_create_serialization_support_interface());
rosidl_dynamic_typesupport_serialization_support_t serialization_support;
rosidl_dynamic_typesupport_serialization_support_init(
rosidl_dynamic_typesupport_fastrtps_init_serialization_support_impl(),
rosidl_dynamic_typesupport_fastrtps_init_serialization_support_interface(),
rcutils_get_default_allocator(),
&serialization_support);

// Configure Dynamic Type Builder
rosidl_dynamic_typesupport_dynamic_type_builder_t * flat_builder = rosidl_dynamic_typesupport_dynamic_type_builder_create(serialization_support, "flat");
rosidl_dynamic_typesupport_dynamic_type_builder_t * flat_builder =
rosidl_dynamic_typesupport_dynamic_type_builder_init(&serialization_support, "flat");
rosidl_dynamic_typesupport_dynamic_type_builder_add_bool_member(flat_builder, 0, "bool_field");
rosidl_dynamic_typesupport_dynamic_type_builder_add_int32_member(flat_builder, 1, "int32_field");
rosidl_dynamic_typesupport_dynamic_type_builder_add_string_member(flat_builder, 2, "string_field");

// Create Dynamic Type
rosidl_dynamic_typesupport_dynamic_type_t * flat_type =
rosidl_dynamic_typesupport_dynamic_type_builder_build(flat_builder);
rosidl_dynamic_typesupport_dynamic_type_builder_destroy(flat_builder);
rosidl_dynamic_typesupport_dynamic_type_builder_fini(flat_builder);

// Create Dynamic Data
rosidl_dynamic_typesupport_dynamic_data_t * flat_data =
rosidl_dynamic_typesupport_dynamic_data_create_from_dynamic_type(flat_type);
rosidl_dynamic_typesupport_dynamic_data_init_from_dynamic_type(flat_type);

// Dynamic Data Setters
rosidl_dynamic_typesupport_dynamic_data_set_bool_value(flat_data, 0, true);
Expand All @@ -46,7 +49,7 @@ rosidl_dynamic_typesupport_dynamic_data_get_string_value(flat_data, 2, &c); //

// Cleanup
free(c);
rosidl_dynamic_typesupport_dynamic_data_destroy(flat_data);
rosidl_dynamic_typesupport_dynamic_data_fini(flat_data);

```

Expand Down
70 changes: 56 additions & 14 deletions include/rosidl_dynamic_typesupport/api/dynamic_data.h
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -22,12 +22,39 @@
extern "C" {
#endif

#include <rcutils/allocator.h>
#include <rcutils/types/rcutils_ret.h>
#include <rcutils/types/uint8_array.h>
#include <rosidl_dynamic_typesupport/api/serialization_support_interface.h>
#include <rosidl_dynamic_typesupport/visibility_control.h>
#include <rosidl_dynamic_typesupport/uchar.h>

#include "rosidl_dynamic_typesupport/api/serialization_support.h"
#include "rosidl_dynamic_typesupport/api/serialization_support_interface.h"
#include "rosidl_dynamic_typesupport/visibility_control.h"
#include "rosidl_dynamic_typesupport/types.h"
#include "rosidl_dynamic_typesupport/uchar.h"

// Dynamic Data Impl
struct rosidl_dynamic_typesupport_dynamic_data_impl_s
{
rcutils_allocator_t allocator;
void * handle;
};

ROSIDL_DYNAMIC_TYPESUPPORT_PUBLIC
rosidl_dynamic_typesupport_dynamic_data_impl_t
rosidl_dynamic_typesupport_get_zero_initialized_dynamic_data_impl(void);

// Dynamic Data
struct rosidl_dynamic_typesupport_dynamic_data_s
{
rcutils_allocator_t allocator;
rosidl_dynamic_typesupport_dynamic_data_impl_t impl;
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Not using hidden-type pimpl?

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I was thinking of doing that, but I thought it would be better to enforce that the impl is a complete type (and then defer it to the handle), so that we can enforce that the impl type has an allocator

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

But isn't the the type defined just above? That is, this implementation is completely in control of that structure, and thus can guarantee that it always has an allocator, no?

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

ah, right.. I think I'm avoiding the use of a pointer so we don't have to keep doing null checks on the impl member as well..

The impl is unusable without the associated serialization support in either case, so in my view composition makes sense

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yeah, actually, I largely agree with you. The only problem here is that if we want to add, remove, or change structure members in the implementation, we won't be able to backport it due to ABI concerns. But if we are OK with that limitation for now, we can leave it as-is.

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The implementation contains a void * handle, so that's a good escape hatch I think?

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yeah, it's good enough for now. It isn't our usual style, but it will do in a pinch.

// !!! Lifetime is NOT managed by this struct
rosidl_dynamic_typesupport_serialization_support_t * serialization_support;
};

ROSIDL_DYNAMIC_TYPESUPPORT_PUBLIC
rosidl_dynamic_typesupport_dynamic_data_t
rosidl_dynamic_typesupport_get_zero_initialized_dynamic_data(void);

// ===============================================================================================
// DYNAMIC DATA
Expand Down Expand Up @@ -90,14 +117,21 @@ rcutils_ret_t
rosidl_dynamic_typesupport_dynamic_data_loan_value(
rosidl_dynamic_typesupport_dynamic_data_t * dynamic_data,
rosidl_dynamic_typesupport_member_id_t id,
rosidl_dynamic_typesupport_dynamic_data_t ** loaned_dynamic_data); // OUT
rcutils_allocator_t * allocator,
rosidl_dynamic_typesupport_dynamic_data_t * loaned_dynamic_data); // OUT

ROSIDL_DYNAMIC_TYPESUPPORT_PUBLIC
rcutils_ret_t
rosidl_dynamic_typesupport_dynamic_data_return_loaned_value(
rosidl_dynamic_typesupport_dynamic_data_t * dynamic_data,
rosidl_dynamic_typesupport_dynamic_data_t * inner_dynamic_data);

ROSIDL_DYNAMIC_TYPESUPPORT_PUBLIC
rcutils_ret_t
rosidl_dynamic_typesupport_dynamic_data_return_and_destroy_loaned_value(
rosidl_dynamic_typesupport_dynamic_data_t * dynamic_data,
rosidl_dynamic_typesupport_dynamic_data_t * inner_dynamic_data);

ROSIDL_DYNAMIC_TYPESUPPORT_PUBLIC
rcutils_ret_t
rosidl_dynamic_typesupport_dynamic_data_get_name(
Expand All @@ -109,21 +143,29 @@ rosidl_dynamic_typesupport_dynamic_data_get_name(
// DYNAMIC DATA CONSTRUCTION =======================================================================
ROSIDL_DYNAMIC_TYPESUPPORT_PUBLIC
rcutils_ret_t
rosidl_dynamic_typesupport_dynamic_data_create_from_dynamic_type_builder(
rosidl_dynamic_typesupport_dynamic_data_init_from_dynamic_type_builder(
rosidl_dynamic_typesupport_dynamic_type_builder_t * dynamic_type_builder,
rosidl_dynamic_typesupport_dynamic_data_t ** dynamic_data); // OUT
rcutils_allocator_t * allocator,
rosidl_dynamic_typesupport_dynamic_data_t * dynamic_data); // OUT

ROSIDL_DYNAMIC_TYPESUPPORT_PUBLIC
rcutils_ret_t
rosidl_dynamic_typesupport_dynamic_data_create_from_dynamic_type(
rosidl_dynamic_typesupport_dynamic_data_init_from_dynamic_type(
rosidl_dynamic_typesupport_dynamic_type_t * dynamic_type,
rosidl_dynamic_typesupport_dynamic_data_t ** dynamic_data); // OUT
rcutils_allocator_t * allocator,
rosidl_dynamic_typesupport_dynamic_data_t * dynamic_data); // OUT

ROSIDL_DYNAMIC_TYPESUPPORT_PUBLIC
rcutils_ret_t
rosidl_dynamic_typesupport_dynamic_data_clone(
const rosidl_dynamic_typesupport_dynamic_data_t * other_dynamic_data,
rosidl_dynamic_typesupport_dynamic_data_t ** dynamic_data); // OUT
rcutils_allocator_t * allocator,
rosidl_dynamic_typesupport_dynamic_data_t * dynamic_data); // OUT

ROSIDL_DYNAMIC_TYPESUPPORT_PUBLIC
rcutils_ret_t
rosidl_dynamic_typesupport_dynamic_data_fini(
rosidl_dynamic_typesupport_dynamic_data_t * dynamic_data);
Comment on lines 158 to +168
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

For other reviewers, the vast majority of changes for this pr can be summed up with this diff, basically adding allocators where we do allocation and avoiding double pointers as out parameters (matching how most of the rcl-like interfaces work, and the rest is mostly mechanical changes related to this shift.


ROSIDL_DYNAMIC_TYPESUPPORT_PUBLIC
rcutils_ret_t
Expand All @@ -136,15 +178,13 @@ ROSIDL_DYNAMIC_TYPESUPPORT_PUBLIC
rcutils_ret_t
rosidl_dynamic_typesupport_dynamic_data_serialize(
rosidl_dynamic_typesupport_dynamic_data_t * dynamic_data,
rcutils_uint8_array_t * buffer, // OUT
bool * serialized); // OUT
rcutils_uint8_array_t * buffer); // OUT

ROSIDL_DYNAMIC_TYPESUPPORT_PUBLIC
rcutils_ret_t
rosidl_dynamic_typesupport_dynamic_data_deserialize(
rosidl_dynamic_typesupport_dynamic_data_t * dynamic_data, // OUT
rcutils_uint8_array_t * buffer,
bool * success); // OUT
rcutils_uint8_array_t * buffer);


// DYNAMIC DATA PRIMITIVE MEMBER GETTERS ===========================================================
Expand Down Expand Up @@ -564,7 +604,9 @@ ROSIDL_DYNAMIC_TYPESUPPORT_PUBLIC
rcutils_ret_t
rosidl_dynamic_typesupport_dynamic_data_get_complex_value(
const rosidl_dynamic_typesupport_dynamic_data_t * dynamic_data,
rosidl_dynamic_typesupport_member_id_t id, rosidl_dynamic_typesupport_dynamic_data_t ** value);
rosidl_dynamic_typesupport_member_id_t id,
rcutils_allocator_t * allocator,
rosidl_dynamic_typesupport_dynamic_data_t * value);

ROSIDL_DYNAMIC_TYPESUPPORT_PUBLIC
rcutils_ret_t
Expand Down
90 changes: 78 additions & 12 deletions include/rosidl_dynamic_typesupport/api/dynamic_type.h
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -22,11 +22,61 @@
extern "C" {
#endif

#include <rcutils/allocator.h>
#include <rcutils/types/rcutils_ret.h>

#include <rosidl_dynamic_typesupport/api/serialization_support_interface.h>
#include <rosidl_dynamic_typesupport/visibility_control.h>
#include "rosidl_dynamic_typesupport/api/serialization_support.h"
#include "rosidl_dynamic_typesupport/api/serialization_support_interface.h"
#include "rosidl_dynamic_typesupport/types.h"
#include "rosidl_dynamic_typesupport/visibility_control.h"

// Dynamic Type Builder Impl
struct rosidl_dynamic_typesupport_dynamic_type_builder_impl_s
{
rcutils_allocator_t allocator;
void * handle;
};

ROSIDL_DYNAMIC_TYPESUPPORT_PUBLIC
rosidl_dynamic_typesupport_dynamic_type_builder_impl_t
rosidl_dynamic_typesupport_get_zero_initialized_dynamic_type_builder_impl(void);

// Dynamic Type Builder
struct rosidl_dynamic_typesupport_dynamic_type_builder_s
{
rcutils_allocator_t allocator;
rosidl_dynamic_typesupport_dynamic_type_builder_impl_t impl;
// !!! Lifetime is NOT managed by this struct
rosidl_dynamic_typesupport_serialization_support_t * serialization_support;
};

ROSIDL_DYNAMIC_TYPESUPPORT_PUBLIC
rosidl_dynamic_typesupport_dynamic_type_builder_t
rosidl_dynamic_typesupport_get_zero_initialized_dynamic_type_builder(void);

// Dynamic Type Impl
struct rosidl_dynamic_typesupport_dynamic_type_impl_s
{
rcutils_allocator_t allocator;
void * handle;
};

ROSIDL_DYNAMIC_TYPESUPPORT_PUBLIC
rosidl_dynamic_typesupport_dynamic_type_impl_t
rosidl_dynamic_typesupport_get_zero_initialized_dynamic_type_impl(void);

// Dynamic Type
struct rosidl_dynamic_typesupport_dynamic_type_s
{
rcutils_allocator_t allocator;
rosidl_dynamic_typesupport_dynamic_type_impl_t impl;
// !!! Lifetime is NOT managed by this struct
rosidl_dynamic_typesupport_serialization_support_t * serialization_support;
};

ROSIDL_DYNAMIC_TYPESUPPORT_PUBLIC
rosidl_dynamic_typesupport_dynamic_type_t
rosidl_dynamic_typesupport_get_zero_initialized_dynamic_type(void);

// =================================================================================================
// DYNAMIC TYPE
Expand All @@ -50,23 +100,31 @@ rosidl_dynamic_typesupport_dynamic_type_get_member_count(
// DYNAMIC TYPE CONSTRUCTION =======================================================================
ROSIDL_DYNAMIC_TYPESUPPORT_PUBLIC
rcutils_ret_t
rosidl_dynamic_typesupport_dynamic_type_builder_create(
rosidl_dynamic_typesupport_dynamic_type_builder_init(
rosidl_dynamic_typesupport_serialization_support_t * serialization_support, const char * name,
size_t name_length,
rosidl_dynamic_typesupport_dynamic_type_builder_t ** dynamic_type_builder); // OUT
rcutils_allocator_t * allocator,
rosidl_dynamic_typesupport_dynamic_type_builder_t * dynamic_type_builder); // OUT

ROSIDL_DYNAMIC_TYPESUPPORT_PUBLIC
rcutils_ret_t
rosidl_dynamic_typesupport_dynamic_type_builder_clone(
const rosidl_dynamic_typesupport_dynamic_type_builder_t * other,
rosidl_dynamic_typesupport_dynamic_type_builder_t ** dynamic_type_builder); // OUT
rcutils_allocator_t * allocator,
rosidl_dynamic_typesupport_dynamic_type_builder_t * dynamic_type_builder); // OUT

ROSIDL_DYNAMIC_TYPESUPPORT_PUBLIC
rcutils_ret_t
rosidl_dynamic_typesupport_dynamic_type_builder_create_from_description(
rosidl_dynamic_typesupport_dynamic_type_builder_init_from_description(
rosidl_dynamic_typesupport_serialization_support_t * serialization_support,
const rosidl_runtime_c__type_description__TypeDescription * description,
rosidl_dynamic_typesupport_dynamic_type_builder_t ** dynamic_type_builder); // OUT
rcutils_allocator_t * allocator,
rosidl_dynamic_typesupport_dynamic_type_builder_t * dynamic_type_builder); // OUT

ROSIDL_DYNAMIC_TYPESUPPORT_PUBLIC
rcutils_ret_t
rosidl_dynamic_typesupport_dynamic_type_builder_fini(
rosidl_dynamic_typesupport_dynamic_type_builder_t * dynamic_type_builder);

ROSIDL_DYNAMIC_TYPESUPPORT_PUBLIC
rcutils_ret_t
Expand All @@ -75,22 +133,30 @@ rosidl_dynamic_typesupport_dynamic_type_builder_destroy(

ROSIDL_DYNAMIC_TYPESUPPORT_PUBLIC
rcutils_ret_t
rosidl_dynamic_typesupport_dynamic_type_create_from_dynamic_type_builder(
rosidl_dynamic_typesupport_dynamic_type_init_from_dynamic_type_builder(
rosidl_dynamic_typesupport_dynamic_type_builder_t * dynamic_type_builder,
rosidl_dynamic_typesupport_dynamic_type_t ** dynamic_type); // OUT
rcutils_allocator_t * allocator,
rosidl_dynamic_typesupport_dynamic_type_t * dynamic_type); // OUT

ROSIDL_DYNAMIC_TYPESUPPORT_PUBLIC
rcutils_ret_t
rosidl_dynamic_typesupport_dynamic_type_create_from_description(
rosidl_dynamic_typesupport_dynamic_type_init_from_description(
rosidl_dynamic_typesupport_serialization_support_t * serialization_support,
const rosidl_runtime_c__type_description__TypeDescription * description,
rosidl_dynamic_typesupport_dynamic_type_t ** dynamic_type); // OUT
rcutils_allocator_t * allocator,
rosidl_dynamic_typesupport_dynamic_type_t * dynamic_type); // OUT

ROSIDL_DYNAMIC_TYPESUPPORT_PUBLIC
rcutils_ret_t
rosidl_dynamic_typesupport_dynamic_type_clone(
const rosidl_dynamic_typesupport_dynamic_type_t * other,
rosidl_dynamic_typesupport_dynamic_type_t ** dynamic_type); // OUT
rcutils_allocator_t * allocator,
rosidl_dynamic_typesupport_dynamic_type_t * dynamic_type); // OUT

ROSIDL_DYNAMIC_TYPESUPPORT_PUBLIC
rcutils_ret_t
rosidl_dynamic_typesupport_dynamic_type_fini(
rosidl_dynamic_typesupport_dynamic_type_t * dynamic_type);

ROSIDL_DYNAMIC_TYPESUPPORT_PUBLIC
rcutils_ret_t
Expand Down
44 changes: 38 additions & 6 deletions include/rosidl_dynamic_typesupport/api/serialization_support.h
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -22,15 +22,47 @@
extern "C" {
#endif

#include <rosidl_dynamic_typesupport/types.h>

#include <rosidl_dynamic_typesupport/api/serialization_support_interface.h>
#include <rosidl_dynamic_typesupport/api/dynamic_data.h>
#include <rosidl_dynamic_typesupport/api/dynamic_type.h>
#include <rosidl_dynamic_typesupport/visibility_control.h>

#include <rcutils/types/rcutils_ret.h>

#include "rosidl_dynamic_typesupport/types.h"

/// Serialization Support Impl
/// For anything necessary or useful for the operation of the serialization lib
/// (e.g. singleton dynamic type and dynamic data factories)
struct rosidl_dynamic_typesupport_serialization_support_impl_s
{
rcutils_allocator_t allocator;
const char * serialization_library_identifier;
void * handle;
};

ROSIDL_DYNAMIC_TYPESUPPORT_PUBLIC
rosidl_dynamic_typesupport_serialization_support_impl_t
rosidl_dynamic_typesupport_get_zero_initialized_serialization_support_impl(void);

/// Serialization Support
/// This is the main structure that encompasses:
/// - impl - The library-specific objects or implementation details
/// - methods - The shared serialization support interface, populated with serialization
/// library-specific function pointers
struct rosidl_dynamic_typesupport_serialization_support_s
{
rcutils_allocator_t allocator;
const char * serialization_library_identifier;

rosidl_dynamic_typesupport_serialization_support_impl_t impl;
// Can't call it `interface` because it's a reserved term in some Windows versions...
rosidl_dynamic_typesupport_serialization_support_interface_t methods;
};

ROSIDL_DYNAMIC_TYPESUPPORT_PUBLIC
rosidl_dynamic_typesupport_serialization_support_t
rosidl_dynamic_typesupport_get_zero_initialized_serialization_support(void);

// CORE ============================================================================================
ROSIDL_DYNAMIC_TYPESUPPORT_PUBLIC
Expand All @@ -40,17 +72,17 @@ rosidl_dynamic_typesupport_serialization_support_get_library_identifier(

ROSIDL_DYNAMIC_TYPESUPPORT_PUBLIC
rcutils_ret_t
rosidl_dynamic_typesupport_serialization_support_create(
rosidl_dynamic_typesupport_serialization_support_init(
rosidl_dynamic_typesupport_serialization_support_impl_t * impl,
rosidl_dynamic_typesupport_serialization_support_interface_t * methods,
rosidl_dynamic_typesupport_serialization_support_t ** serialization_support); // OUT
rcutils_allocator_t * allocator,
rosidl_dynamic_typesupport_serialization_support_t * serialization_support); // OUT

ROSIDL_DYNAMIC_TYPESUPPORT_PUBLIC
rcutils_ret_t
rosidl_dynamic_typesupport_serialization_support_destroy(
rosidl_dynamic_typesupport_serialization_support_fini(
rosidl_dynamic_typesupport_serialization_support_t * serialization_support);


#ifdef __cplusplus
}
#endif
Expand Down
Loading