-
Notifications
You must be signed in to change notification settings - Fork 6
Conversation
JanStaschulat
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Looks good to me, just update the unit tests.
rclc/test/rclc/test_init.cpp
Outdated
| rc = rclc_support_fini(&support); | ||
| EXPECT_EQ(RCL_RET_OK, rc); | ||
| // test invalid arguments | ||
| rc = rclc_support_init(nullptr, 0, nullptr, &allocator); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@pablogs9 use also rclc_support_init_with_options function
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Done!
rclc/test/rclc/test_init.cpp
Outdated
| rc = rclc_support_init(nullptr, 0, nullptr, &allocator); | ||
| EXPECT_EQ(RCL_RET_INVALID_ARGUMENT, rc); | ||
| rcutils_reset_error(); | ||
| rc = rclc_support_init(&support, 0, nullptr, nullptr); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@pablogs9 use also rclc_support_init_with_options function as well.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Done!
julionce
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I review it in #33.
|
Changes applied |
This PR adds a convenience function for init with options:
rclc_support_init_with_options.This init function is required when default options from
rcl_get_zero_initialized_init_optionsandrcl_init_options_initare not enough. For example when configuring underlying RMW in micro-ROS.Please let me know your opinions about this feature.