Skip to content
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

Dynamic Subscription (BONUS: Allocators): rcl #1057

Merged
merged 4 commits into from
Apr 11, 2023
Merged

Dynamic Subscription (BONUS: Allocators): rcl #1057

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
c67656f
Refactor dynamic message type support init
methylDragon Apr 9, 2023
3d654b1
Refactor serialization support to use allocators and refs
methylDragon Apr 10, 2023
5a33394
Use rmw_serialization_support_init
methylDragon Apr 10, 2023
2a028b1
Check if allocator is initialized
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
14 changes: 8 additions & 6 deletions rcl/include/rcl/dynamic_message_type_support.h
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -22,26 +22,28 @@ extern "C" {
#include <rosidl_runtime_c/type_description/type_description__struct.h>
#include <rosidl_runtime_c/message_type_support_struct.h>

#include "rcl/allocator.h"
#include "rcl/macros.h"
#include "rcl/types.h"
#include "rcl/visibility_control.h"

/// Initialize a rosidl_message_type_support_t struct with dynamic message type support
/// Does not take ownership of description (copies)
/// Allocates the `ts` arg. The caller takes ownership of the `ts` arg.
RCL_PUBLIC
RCL_WARN_UNUSED
rcl_ret_t
rcl_dynamic_message_type_support_handle_create(
rcl_dynamic_message_type_support_handle_init(
const char * serialization_lib_name,
const rosidl_runtime_c__type_description__TypeDescription * desc,
rosidl_message_type_support_t ** ts); // OUT
rcl_allocator_t * allocator,
rosidl_message_type_support_t * ts); // OUT

/// Finalize a rosidl_message_type_support_t obtained with
/// `rcl_dynamic_message_type_support_handle_create()`
/// Finalize a rosidl_message_type_support_t initialized with
/// `rcl_dynamic_message_type_support_handle_init()`
RCL_PUBLIC
RCL_WARN_UNUSED
rcl_ret_t
rcl_dynamic_message_type_support_handle_destroy(rosidl_message_type_support_t * ts);
rcl_dynamic_message_type_support_handle_fini(rosidl_message_type_support_t * ts);

#ifdef __cplusplus
}
Expand Down
67 changes: 30 additions & 37 deletions rcl/src/rcl/dynamic_message_type_support.c
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -23,23 +23,30 @@ extern "C"

#include "rmw/dynamic_message_type_support.h"

#include "rcl/allocator.h"
#include "rcl/common.h"
#include "rcl/error_handling.h"
#include "rcl/dynamic_message_type_support.h"
#include "rcl/type_hash.h"
#include "rcl/types.h"


/// Create a rosidl_message_type_support_t from a TypeDescription message
/// Initialize a rosidl_message_type_support_t from a TypeDescription message
RCL_PUBLIC
RCL_WARN_UNUSED
rcl_ret_t
rcl_dynamic_message_type_support_handle_create(
rcl_dynamic_message_type_support_handle_init(
const char * serialization_lib_name,
const rosidl_runtime_c__type_description__TypeDescription * description,
rosidl_message_type_support_t ** ts)
rcl_allocator_t * allocator,
rosidl_message_type_support_t * ts)
{
RCUTILS_CHECK_ARGUMENT_FOR_NULL(ts, RCUTILS_RET_INVALID_ARGUMENT);
RCUTILS_CHECK_ARGUMENT_FOR_NULL(allocator, RCUTILS_RET_INVALID_ARGUMENT);
Copy link
Collaborator

@emersonknapp emersonknapp Apr 10, 2023 •
edited
Loading

Choose a reason for hiding this comment

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

You maybe should also check

  if (!rcutils_allocator_is_valid(&allocator)) {

You can have a non-null allocator that isn't initialized yet and can't be used

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I might defer this so we don't invalidate the windows debug CI run :x

Copy link
Contributor

Choose a reason for hiding this comment

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

You are going to have to re-run it anyway, since you changed all of the function signatures for rmw_init_serialization_support

if (!rcutils_allocator_is_valid(allocator)) {
RCUTILS_SET_ERROR_MSG("allocator is invalid");
return RCUTILS_RET_INVALID_ARGUMENT;
}

// TODO(methylDragon): Remove if and when the deferred description path is supported
if (description == NULL) {
Expand All @@ -63,10 +70,10 @@ rcl_dynamic_message_type_support_handle_create(
return RCUTILS_RET_INVALID_ARGUMENT;
}

rosidl_dynamic_typesupport_serialization_support_t * serialization_support = NULL;
rosidl_dynamic_typesupport_serialization_support_t serialization_support;
rcl_ret_t ret = rcl_convert_rmw_ret_to_rcl_ret(
rmw_get_serialization_support(serialization_lib_name, &serialization_support));
if (ret != RCL_RET_OK || serialization_support == NULL) {
rmw_serialization_support_init(serialization_lib_name, allocator, &serialization_support));
if (ret != RCL_RET_OK) {
RCL_SET_ERROR_MSG("failed to get serialization support");
if (ret == RCL_RET_OK) { // It means serialization support was NULL
return RCL_RET_ERROR;
Expand All @@ -75,58 +82,44 @@ rcl_dynamic_message_type_support_handle_create(
}
}

rcutils_allocator_t allocator = rcutils_get_default_allocator();
rosidl_type_hash_t * type_hash = allocator.zero_allocate(
1, sizeof(rosidl_type_hash_t), &allocator.state);
if (!type_hash) {
RCUTILS_SET_ERROR_MSG("Could not allocate type hash");
return RCL_RET_ERROR;
}

rosidl_type_hash_t type_hash;
ret = rcl_calculate_type_hash(
// TODO(methylDragon): Replace this cast with the conversion function when it is ready
// Either a custom function, or from https://github.com/ros2/rcl/pull/1052
(const type_description_interfaces__msg__TypeDescription *) description, type_hash);
if (ret != RCL_RET_OK || type_hash == NULL) {
(const type_description_interfaces__msg__TypeDescription *) description, &type_hash);
if (ret != RCL_RET_OK) {
RCL_SET_ERROR_MSG("failed to get type hash");
allocator.deallocate(type_hash, &allocator.state);
if (ret == RCL_RET_OK) {
return RCL_RET_ERROR;
} else {
return ret;
}
return ret;
}

ret = rcl_convert_rcutils_ret_to_rcl_ret(
rosidl_dynamic_message_type_support_handle_create(
serialization_support,
type_hash, // type_hash
rosidl_dynamic_message_type_support_handle_init(
&serialization_support,
&type_hash, // type_hash
description, // type_description
NULL, // type_description_sources
allocator,
ts
)
);

if (!ts) {
RCL_SET_ERROR_MSG("failed to init rosidl_message_type_support");
allocator.deallocate(type_hash, &allocator.state);
if (ret == RCL_RET_OK) {
return RCL_RET_ERROR;
} else {
return ret;
}
if (ret != RCL_RET_OK) {
rcutils_error_string_t error_string = rcutils_get_error_string();
rcutils_reset_error();
RCL_SET_ERROR_MSG_WITH_FORMAT_STRING(
"failed to init rosidl_message_type_support:\n%s", error_string.str);
return ret;
}

return RCL_RET_OK;
}


RCL_PUBLIC
RCL_WARN_UNUSED
rcl_ret_t
rcl_dynamic_message_type_support_handle_destroy(rosidl_message_type_support_t * ts)
rcl_dynamic_message_type_support_handle_fini(rosidl_message_type_support_t * ts)
{
RCL_CHECK_ARGUMENT_FOR_NULL(ts, RCL_RET_INVALID_ARGUMENT);
return rcl_convert_rcutils_ret_to_rcl_ret(rosidl_dynamic_message_type_support_handle_destroy(ts));
return rcl_convert_rcutils_ret_to_rcl_ret(rosidl_dynamic_message_type_support_handle_fini(ts));
}

#ifdef __cplusplus
Expand Down