niklas/rcl
1
0
Fork 0
Code Issues Pull requests Projects Releases Packages Wiki Activity Actions

Add allocator parameter to rcl_X_is_valid functions (#180)

Browse source 
* Add allocator parameter to rcl_client_is_valid.

* Add allocator parameter to rcl_publisher_is_valid.

* Add allocator parameter to rcl_subscription_is_valid.

* Add allocator parameter to rcl_service_is_valid.

* Satisfy linter, and make error messages more consistent.

* Add calls to allocator check.

* Fixed macro calls.

* Change parameter name for clarity.

* Updated header parameter names.

* Resolve merge conflict after rebase.

* Address comments by @mikaelarguedas

* Linting

* add missing checks and homogenize error message

* non const allocator

* 'error msg allocator is invalid' -> 'allocator is invalid'

* missed one const

* cast to remove const without warnings
This commit is contained in:
Hunter Allen 2017-12-04 17:35:29 -05:00 • committed by Mikael Arguedas
parent 8ce42c6768
commit 77e6979d9f
14 changed files with 92 additions and 75 deletions
Download patch file Download diff file Expand all files Collapse all files

2
rcl/include/rcl/client.h
View file

@ -394,7 +394,7 @@ rcl_client_get_rmw_handle(const rcl_client_t * client);
*/
RCL_PUBLIC
bool
rcl_client_is_valid(const rcl_client_t * client);
rcl_client_is_valid(const rcl_client_t * client, rcl_allocator_t * error_msg_allocator);
#if __cplusplus
}

2
rcl/include/rcl/node.h
View file

@ -243,7 +243,7 @@ rcl_node_get_default_options(void);
*/
RCL_PUBLIC
bool
rcl_node_is_valid(const rcl_node_t * node, const rcl_allocator_t * allocator);
rcl_node_is_valid(const rcl_node_t * node, rcl_allocator_t * error_msg_allocator);
/// Return the name of the node.
/**

4
rcl/include/rcl/publisher.h
View file

@ -358,7 +358,9 @@ rcl_publisher_get_rmw_handle(const rcl_publisher_t * publisher);
*/
RCL_PUBLIC
bool
rcl_publisher_is_valid(const rcl_publisher_t * publisher);
rcl_publisher_is_valid(
const rcl_publisher_t * publisher,
rcl_allocator_t * error_msg_allocator);
#if __cplusplus
}

2
rcl/include/rcl/service.h
View file

@ -407,7 +407,7 @@ rcl_service_get_rmw_handle(const rcl_service_t * service);
*/
RCL_PUBLIC
bool
rcl_service_is_valid(const rcl_service_t * service);
rcl_service_is_valid(const rcl_service_t * service, rcl_allocator_t * error_msg_allocator);
#if __cplusplus
}

4
rcl/include/rcl/subscription.h
View file

@ -362,7 +362,9 @@ rcl_subscription_get_rmw_handle(const rcl_subscription_t * subscription);
*/
RCL_PUBLIC
bool
rcl_subscription_is_valid(const rcl_subscription_t * subscription);
rcl_subscription_is_valid(
const rcl_subscription_t * subscription,
rcl_allocator_t * error_msg_allocator);
#if __cplusplus
}

26
rcl/src/rcl/client.c
View file

@ -58,7 +58,7 @@ rcl_client_init(
// check the options and allocator first, so the allocator can be passed to errors
RCL_CHECK_ARGUMENT_FOR_NULL(options, RCL_RET_INVALID_ARGUMENT, rcl_get_default_allocator());
const rcl_allocator_t * allocator = &options->allocator;
rcl_allocator_t * allocator = (rcl_allocator_t *)&options->allocator;
RCL_CHECK_ALLOCATOR_WITH_MSG(allocator, "invalid allocator", return RCL_RET_INVALID_ARGUMENT);
RCL_CHECK_ARGUMENT_FOR_NULL(client, RCL_RET_INVALID_ARGUMENT, *allocator);
RCL_CHECK_ARGUMENT_FOR_NULL(node, RCL_RET_INVALID_ARGUMENT, *allocator);
@ -205,7 +205,7 @@ rcl_client_get_default_options()
const char *
rcl_client_get_service_name(const rcl_client_t * client)
{
if (!rcl_client_is_valid(client)) {
if (!rcl_client_is_valid(client, NULL)) {
return NULL; // error already set
}
return client->impl->rmw_handle->service_name;
@ -218,7 +218,7 @@ rcl_client_get_service_name(const rcl_client_t * client)
const rcl_client_options_t *
rcl_client_get_options(const rcl_client_t * client)
{
if (!rcl_client_is_valid(client)) {
if (!rcl_client_is_valid(client, NULL)) {
return NULL; // error already set
}
return _client_get_options(client);
@ -227,7 +227,7 @@ rcl_client_get_options(const rcl_client_t * client)
rmw_client_t *
rcl_client_get_rmw_handle(const rcl_client_t * client)
{
if (!rcl_client_is_valid(client)) {
if (!rcl_client_is_valid(client, NULL)) {
return NULL; // error already set
}
return client->impl->rmw_handle;
@ -239,7 +239,7 @@ rcl_ret_t
rcl_send_request(const rcl_client_t * client, const void * ros_request, int64_t * sequence_number)
{
RCUTILS_LOG_DEBUG_NAMED(ROS_PACKAGE_NAME, "Client sending service request")
if (!rcl_client_is_valid(client)) {
if (!rcl_client_is_valid(client, NULL)) {
return RCL_RET_CLIENT_INVALID;
}
RCL_CHECK_ARGUMENT_FOR_NULL(ros_request, RCL_RET_INVALID_ARGUMENT, rcl_get_default_allocator());
@ -265,7 +265,7 @@ rcl_take_response(
void * ros_response)
{
RCUTILS_LOG_DEBUG_NAMED(ROS_PACKAGE_NAME, "Client taking service response")
if (!rcl_client_is_valid(client)) {
if (!rcl_client_is_valid(client, NULL)) {
return RCL_RET_CLIENT_INVALID;
}
@ -288,16 +288,20 @@ rcl_take_response(
return RCL_RET_OK;
}
bool rcl_client_is_valid(const rcl_client_t * client)
bool rcl_client_is_valid(const rcl_client_t * client, rcl_allocator_t * error_msg_allocator)
{
const rcl_client_options_t * options;
RCL_CHECK_ARGUMENT_FOR_NULL(
client, false, rcl_get_default_allocator());
rcl_allocator_t alloc =
error_msg_allocator ? *error_msg_allocator : rcl_get_default_allocator();
RCL_CHECK_ALLOCATOR_WITH_MSG(&alloc, "allocator is invalid", return false);
RCL_CHECK_ARGUMENT_FOR_NULL(client, false, alloc);
RCL_CHECK_FOR_NULL_WITH_MSG(
client->impl, "client's rmw implementation is invalid", return false, alloc);
options = _client_get_options(client);
RCL_CHECK_FOR_NULL_WITH_MSG(
options, "client's options pointer is invalid", return false, rcl_get_default_allocator());
options, "client's options pointer is invalid", return false, alloc);
RCL_CHECK_FOR_NULL_WITH_MSG(
client->impl, "client's rmw implementation is invalid", return false, options->allocator);
client->impl->rmw_handle, "client's rmw handle is invalid", return false, alloc);
return true;
}
#if __cplusplus

8
rcl/src/rcl/node.c
View file

@ -370,10 +370,10 @@ rcl_node_fini(rcl_node_t * node)
}
bool
rcl_node_is_valid(const rcl_node_t * node, const rcl_allocator_t * allocator)
rcl_node_is_valid(const rcl_node_t * node, rcl_allocator_t * error_msg_allocator)
{
rcl_allocator_t alloc = allocator ? *allocator : rcl_get_default_allocator();
RCL_CHECK_ALLOCATOR_WITH_MSG(&alloc, "invalid allocator", return false);
rcl_allocator_t alloc = error_msg_allocator ? *error_msg_allocator : rcl_get_default_allocator();
RCL_CHECK_ALLOCATOR_WITH_MSG(&alloc, "allocator is invalid", return false);
RCL_CHECK_ARGUMENT_FOR_NULL(node, false, alloc);
RCL_CHECK_FOR_NULL_WITH_MSG(
node->impl, "rcl node implementation is invalid", return false, alloc);
@ -382,6 +382,8 @@ rcl_node_is_valid(const rcl_node_t * node, const rcl_allocator_t * allocator)
"rcl node is invalid, rcl instance id does not match", alloc);
return false;
}
RCL_CHECK_FOR_NULL_WITH_MSG(
node->impl->rmw_node_handle, "rcl node's rmw handle is invalid", return false, alloc);
return true;
}

27
rcl/src/rcl/publisher.c
View file

@ -55,7 +55,7 @@ rcl_publisher_init(
// Check options and allocator first, so allocator can be used with errors.
RCL_CHECK_ARGUMENT_FOR_NULL(options, RCL_RET_INVALID_ARGUMENT, rcl_get_default_allocator());
const rcl_allocator_t * allocator = &options->allocator;
rcl_allocator_t * allocator = (rcl_allocator_t *)&options->allocator;
RCL_CHECK_ALLOCATOR_WITH_MSG(allocator, "invalid allocator", return RCL_RET_INVALID_ARGUMENT);
RCL_CHECK_ARGUMENT_FOR_NULL(publisher, RCL_RET_INVALID_ARGUMENT, *allocator);
@ -203,7 +203,7 @@ rcl_ret_t
rcl_publish(const rcl_publisher_t * publisher, const void * ros_message)
{
RCUTILS_LOG_DEBUG_NAMED(ROS_PACKAGE_NAME, "Publisher publishing message")
if (!rcl_publisher_is_valid(publisher)) {
if (!rcl_publisher_is_valid(publisher, NULL)) {
return RCL_RET_PUBLISHER_INVALID;
}
if (rmw_publish(publisher->impl->rmw_handle, ros_message) != RMW_RET_OK) {
@ -216,7 +216,7 @@ rcl_publish(const rcl_publisher_t * publisher, const void * ros_message)
const char *
rcl_publisher_get_topic_name(const rcl_publisher_t * publisher)
{
if (!rcl_publisher_is_valid(publisher)) {
if (!rcl_publisher_is_valid(publisher, NULL)) {
return NULL;
}
return publisher->impl->rmw_handle->topic_name;
@ -229,7 +229,7 @@ rcl_publisher_get_topic_name(const rcl_publisher_t * publisher)
const rcl_publisher_options_t *
rcl_publisher_get_options(const rcl_publisher_t * publisher)
{
if (!rcl_publisher_is_valid(publisher)) {
if (!rcl_publisher_is_valid(publisher, NULL)) {
return NULL;
}
return _publisher_get_options(publisher);
@ -238,24 +238,29 @@ rcl_publisher_get_options(const rcl_publisher_t * publisher)
rmw_publisher_t *
rcl_publisher_get_rmw_handle(const rcl_publisher_t * publisher)
{
if (!rcl_publisher_is_valid(publisher)) {
if (!rcl_publisher_is_valid(publisher, NULL)) {
return NULL;
}
return publisher->impl->rmw_handle;
}
bool
rcl_publisher_is_valid(const rcl_publisher_t * publisher)
rcl_publisher_is_valid(
const rcl_publisher_t * publisher,
rcl_allocator_t * error_msg_allocator)
{
const rcl_publisher_options_t * options;
RCL_CHECK_ARGUMENT_FOR_NULL(publisher, false, rcl_get_default_allocator());
rcl_allocator_t alloc =
error_msg_allocator ? *error_msg_allocator : rcl_get_default_allocator();
RCL_CHECK_ALLOCATOR_WITH_MSG(&alloc, "allocator is invalid", return false);
RCL_CHECK_ARGUMENT_FOR_NULL(publisher, false, alloc);
RCL_CHECK_FOR_NULL_WITH_MSG(
publisher->impl, "publisher implementation is invalid", return false, alloc);
options = _publisher_get_options(publisher);
RCL_CHECK_FOR_NULL_WITH_MSG(
options, "publisher's options pointer is invalid", return false, rcl_get_default_allocator());
options, "publisher's options pointer is invalid", return false, alloc);
RCL_CHECK_FOR_NULL_WITH_MSG(
publisher->impl, "publisher implementation is invalid", return false, options->allocator);
RCL_CHECK_FOR_NULL_WITH_MSG(
publisher->impl->rmw_handle, "publisher rmw handle invalid", return false, options->allocator);
publisher->impl->rmw_handle, "publisher's rmw handle is invalid", return false, alloc);
return true;
}

31
rcl/src/rcl/service.c
View file

@ -54,7 +54,7 @@ rcl_service_init(
// Check options and allocator first, so the allocator can be used in errors.
RCL_CHECK_ARGUMENT_FOR_NULL(options, RCL_RET_INVALID_ARGUMENT, rcl_get_default_allocator());
const rcl_allocator_t * allocator = &options->allocator;
rcl_allocator_t * allocator = (rcl_allocator_t *)&options->allocator;
RCL_CHECK_ALLOCATOR_WITH_MSG(allocator, "invalid allocator", return RCL_RET_INVALID_ARGUMENT);
RCL_CHECK_ARGUMENT_FOR_NULL(service, RCL_RET_INVALID_ARGUMENT, *allocator);
@ -224,7 +224,7 @@ rcl_service_get_service_name(const rcl_service_t * service)
const rcl_service_options_t *
rcl_service_get_options(const rcl_service_t * service)
{
if (!rcl_service_is_valid(service)) {
if (!rcl_service_is_valid(service, NULL)) {
return NULL;
}
return _service_get_options(service);
@ -233,7 +233,7 @@ rcl_service_get_options(const rcl_service_t * service)
rmw_service_t *
rcl_service_get_rmw_handle(const rcl_service_t * service)
{
if (!rcl_service_is_valid(service)) {
if (!rcl_service_is_valid(service, NULL)) {
return NULL;
}
return service->impl->rmw_handle;
@ -286,23 +286,20 @@ rcl_send_response(
}
bool
rcl_service_is_valid(const rcl_service_t * service)
rcl_service_is_valid(const rcl_service_t * service, rcl_allocator_t * error_msg_allocator)
{
const rcl_service_options_t * options;
RCL_CHECK_ARGUMENT_FOR_NULL(service, false, rcl_get_default_allocator());
rcl_allocator_t alloc =
error_msg_allocator ? *error_msg_allocator : rcl_get_default_allocator();
RCL_CHECK_ALLOCATOR_WITH_MSG(&alloc, "allocator is invalid", return false);
RCL_CHECK_ARGUMENT_FOR_NULL(service, false, alloc);
RCL_CHECK_FOR_NULL_WITH_MSG(
service->impl, "service's implementation is invalid", return false, alloc);
options = _service_get_options(service);
RCL_CHECK_FOR_NULL_WITH_MSG(options,
"service's options pointer is invalid",
return false,
rcl_get_default_allocator());
RCL_CHECK_FOR_NULL_WITH_MSG(service->impl,
"service implementation is invalid",
return false,
options->allocator);
RCL_CHECK_FOR_NULL_WITH_MSG(service->impl->rmw_handle,
"service's rmw handle is invalid",
return false,
options->allocator);
RCL_CHECK_FOR_NULL_WITH_MSG(
options, "service's options pointer is invalid", return false, alloc);
RCL_CHECK_FOR_NULL_WITH_MSG(
service->impl->rmw_handle, "service's rmw handle is invalid", return false, alloc);
return true;
}

37
rcl/src/rcl/subscription.c
View file

@ -53,7 +53,7 @@ rcl_subscription_init(
// Check options and allocator first, so the allocator can be used in errors.
RCL_CHECK_ARGUMENT_FOR_NULL(options, RCL_RET_INVALID_ARGUMENT, rcl_get_default_allocator());
const rcl_allocator_t * allocator = &options->allocator;
rcl_allocator_t * allocator = (rcl_allocator_t *)&options->allocator;
RCL_CHECK_ALLOCATOR_WITH_MSG(allocator, "invalid allocator", return RCL_RET_INVALID_ARGUMENT);
RCL_CHECK_ARGUMENT_FOR_NULL(subscription, RCL_RET_INVALID_ARGUMENT, *allocator);
RCL_CHECK_ARGUMENT_FOR_NULL(node, RCL_RET_INVALID_ARGUMENT, *allocator);
@ -242,7 +242,7 @@ rcl_take(
const char *
rcl_subscription_get_topic_name(const rcl_subscription_t * subscription)
{
if (!rcl_subscription_is_valid(subscription)) {
if (!rcl_subscription_is_valid(subscription, NULL)) {
return NULL;
}
return subscription->impl->rmw_handle->topic_name;
@ -255,7 +255,7 @@ rcl_subscription_get_topic_name(const rcl_subscription_t * subscription)
const rcl_subscription_options_t *
rcl_subscription_get_options(const rcl_subscription_t * subscription)
{
if (!rcl_subscription_is_valid(subscription)) {
if (!rcl_subscription_is_valid(subscription, NULL)) {
return NULL;
}
return _subscription_get_options(subscription);
@ -264,30 +264,35 @@ rcl_subscription_get_options(const rcl_subscription_t * subscription)
rmw_subscription_t *
rcl_subscription_get_rmw_handle(const rcl_subscription_t * subscription)
{
if (!rcl_subscription_is_valid(subscription)) {
if (!rcl_subscription_is_valid(subscription, NULL)) {
return NULL;
}
return subscription->impl->rmw_handle;
}
bool
rcl_subscription_is_valid(const rcl_subscription_t * subscription)
rcl_subscription_is_valid(
const rcl_subscription_t * subscription,
rcl_allocator_t * error_msg_allocator)
{
const rcl_subscription_options_t * options;
rcl_allocator_t alloc =
error_msg_allocator ? *error_msg_allocator : rcl_get_default_allocator();
RCL_CHECK_ALLOCATOR_WITH_MSG(&alloc, "allocator is invalid", return false);
RCL_CHECK_ARGUMENT_FOR_NULL(subscription, false, rcl_get_default_allocator());
RCL_CHECK_FOR_NULL_WITH_MSG(
subscription->impl,
"subscription's implementation is invalid",
return false,
alloc);
options = _subscription_get_options(subscription);
RCL_CHECK_FOR_NULL_WITH_MSG(options,
"subscription's option pointer is invalid",
RCL_CHECK_FOR_NULL_WITH_MSG(
options, "subscription's option pointer is invalid", return false, alloc);
RCL_CHECK_FOR_NULL_WITH_MSG(
subscription->impl->rmw_handle,
"subscription's rmw handle is invalid",
return false,
rcl_get_default_allocator());
RCL_CHECK_FOR_NULL_WITH_MSG(subscription->impl,
"subscription implementation is invalid",
return false,
options->allocator);
RCL_CHECK_FOR_NULL_WITH_MSG(subscription->impl->rmw_handle,
"subscription implementation rmw_handle is invalid",
return false,
options->allocator);
alloc);
return true;
}

6
rcl/test/rcl/test_client.cpp
View file

@ -130,19 +130,19 @@ TEST_F(TestClientFixture, test_client_init_fini) {
rcl_reset_error();
// Check if null publisher is valid
EXPECT_FALSE(rcl_client_is_valid(nullptr));
EXPECT_FALSE(rcl_client_is_valid(nullptr, nullptr));
rcl_reset_error();
// Check if zero initialized client is valid
client = rcl_get_zero_initialized_client();
EXPECT_FALSE(rcl_client_is_valid(&client));
EXPECT_FALSE(rcl_client_is_valid(&client, nullptr));
rcl_reset_error();
// Check that a valid client is valid
client = rcl_get_zero_initialized_client();
ret = rcl_client_init(&client, this->node_ptr, ts, topic_name, &default_client_options);
EXPECT_EQ(RCL_RET_OK, ret) << rcl_get_error_string_safe();
EXPECT_TRUE(rcl_client_is_valid(&client));
EXPECT_TRUE(rcl_client_is_valid(&client, nullptr));
rcl_reset_error();
// Try passing an invalid (uninitialized) node in init.

6
rcl/test/rcl/test_publisher.cpp
View file

@ -145,18 +145,18 @@ TEST_F(CLASSNAME(TestPublisherFixture, RMW_IMPLEMENTATION), test_publisher_init_
rcl_publisher_options_t default_publisher_options = rcl_publisher_get_default_options();
// Check if null publisher is valid
EXPECT_FALSE(rcl_publisher_is_valid(nullptr));
EXPECT_FALSE(rcl_publisher_is_valid(nullptr, nullptr));
// Check if zero initialized node is valid
publisher = rcl_get_zero_initialized_publisher();
EXPECT_FALSE(rcl_publisher_is_valid(&publisher));
EXPECT_FALSE(rcl_publisher_is_valid(&publisher, nullptr));
rcl_reset_error();
// Check that valid publisher is valid
publisher = rcl_get_zero_initialized_publisher();
ret = rcl_publisher_init(&publisher, this->node_ptr, ts, topic_name, &default_publisher_options);
EXPECT_EQ(RCL_RET_OK, ret) << rcl_get_error_string_safe();
EXPECT_TRUE(rcl_publisher_is_valid(&publisher));
EXPECT_TRUE(rcl_publisher_is_valid(&publisher, nullptr));
rcl_reset_error();
// Try passing null for publisher in init.

6
rcl/test/rcl/test_service.cpp
View file

@ -128,19 +128,19 @@ TEST_F(CLASSNAME(TestServiceFixture, RMW_IMPLEMENTATION), test_service_nominal)
ASSERT_EQ(RCL_RET_OK, ret) << rcl_get_error_string_safe();
// Check if null service is valid
EXPECT_FALSE(rcl_service_is_valid(nullptr));
EXPECT_FALSE(rcl_service_is_valid(nullptr, nullptr));
rcl_reset_error();
// Check if zero initialized client is valid
service = rcl_get_zero_initialized_service();
EXPECT_FALSE(rcl_service_is_valid(&service));
EXPECT_FALSE(rcl_service_is_valid(&service, nullptr));
rcl_reset_error();
// Check that a valid service is valid
service = rcl_get_zero_initialized_service();
ret = rcl_service_init(&service, this->node_ptr, ts, topic, &service_options);
EXPECT_EQ(RCL_RET_OK, ret) << rcl_get_error_string_safe();
EXPECT_TRUE(rcl_service_is_valid(&service));
EXPECT_TRUE(rcl_service_is_valid(&service, nullptr));
rcl_reset_error();
// Check that the service name matches what we assigned.

6
rcl/test/rcl/test_subscription.cpp
View file

@ -154,19 +154,19 @@ TEST_F(CLASSNAME(TestSubscriptionFixture, RMW_IMPLEMENTATION), test_subscription
EXPECT_EQ(strcmp(rcl_subscription_get_topic_name(&subscription), expected_topic), 0);
// Test is_valid for subscription with nullptr
EXPECT_FALSE(rcl_subscription_is_valid(nullptr));
EXPECT_FALSE(rcl_subscription_is_valid(nullptr, nullptr));
rcl_reset_error();
// Test is_valid for zero initialized subscription
subscription = rcl_get_zero_initialized_subscription();
EXPECT_FALSE(rcl_subscription_is_valid(&subscription));
EXPECT_FALSE(rcl_subscription_is_valid(&subscription, nullptr));
rcl_reset_error();
// Check that valid subscriber is valid
subscription = rcl_get_zero_initialized_subscription();
ret = rcl_subscription_init(&subscription, this->node_ptr, ts, topic, &subscription_options);
EXPECT_EQ(RCL_RET_OK, ret) << rcl_get_error_string_safe();
EXPECT_TRUE(rcl_subscription_is_valid(&subscription));
EXPECT_TRUE(rcl_subscription_is_valid(&subscription, nullptr));
rcl_reset_error();
// TODO(wjwwood): add logic to wait for the connection to be established