Skip to content

Commit

Permalink
Add public facing API for validating rcl_wait_set_t (ros2#538)
Browse files Browse the repository at this point in the history
I've exposed the existing function as public API and added documentation and unit tests.

Signed-off-by: Jacob Perron <jacob@openrobotics.org>
  • Loading branch information
jacobperron authored and ralph-lange committed Dec 10, 2019
1 parent 18c1eb3 commit 92b9f79
Show file tree
Hide file tree
Showing 3 changed files with 51 additions and 8 deletions.
23 changes: 23 additions & 0 deletions rcl/include/rcl/wait.h
Original file line number Diff line number Diff line change
Expand Up @@ -459,6 +459,29 @@ RCL_WARN_UNUSED
rcl_ret_t
rcl_wait(rcl_wait_set_t * wait_set, int64_t timeout);

/// Return `true` if the wait set is valid, else `false`.
/**
* A wait set is invalid if:
* - the implementation is `NULL` (rcl_wait_set_init not called or failed)
* - the wait set has been finalized with rcl_wait_set_fini
*
* Also return `false` if the wait set pointer is `NULL`.
*
* <hr>
* Attribute | Adherence
* ------------------ | -------------
* Allocates Memory | No
* Thread-Safe | No
* Uses Atomics | No
* Lock-Free | Yes
*
* \param[in] wait_set the rcl_wait_set_t to be validated
* \return `true` if the wait_set is valid, otherwise `false`.
*/
RCL_PUBLIC
bool
rcl_wait_set_is_valid(const rcl_wait_set_t * wait_set);

#ifdef __cplusplus
}
#endif
Expand Down
16 changes: 8 additions & 8 deletions rcl/src/rcl/wait.c
Original file line number Diff line number Diff line change
Expand Up @@ -79,8 +79,8 @@ rcl_get_zero_initialized_wait_set()
return null_wait_set;
}

static bool
__wait_set_is_valid(const rcl_wait_set_t * wait_set)
bool
rcl_wait_set_is_valid(const rcl_wait_set_t * wait_set)
{
return wait_set && wait_set->impl;
}
Expand Down Expand Up @@ -118,7 +118,7 @@ rcl_wait_set_init(

RCL_CHECK_ALLOCATOR_WITH_MSG(&allocator, "invalid allocator", return RCL_RET_INVALID_ARGUMENT);
RCL_CHECK_ARGUMENT_FOR_NULL(wait_set, RCL_RET_INVALID_ARGUMENT);
if (__wait_set_is_valid(wait_set)) {
if (rcl_wait_set_is_valid(wait_set)) {
RCL_SET_ERROR_MSG("wait_set already initialized, or memory was uninitialized.");
return RCL_RET_ALREADY_INIT;
}
Expand Down Expand Up @@ -173,7 +173,7 @@ rcl_wait_set_init(
}
return RCL_RET_OK;
fail:
if (__wait_set_is_valid(wait_set)) {
if (rcl_wait_set_is_valid(wait_set)) {
rmw_ret_t ret = rmw_destroy_wait_set(wait_set->impl->rmw_wait_set);
if (ret != RMW_RET_OK) {
fail_ret = RCL_RET_WAIT_SET_INVALID;
Expand All @@ -189,7 +189,7 @@ rcl_wait_set_fini(rcl_wait_set_t * wait_set)
rcl_ret_t result = RCL_RET_OK;
RCL_CHECK_ARGUMENT_FOR_NULL(wait_set, RCL_RET_INVALID_ARGUMENT);

if (__wait_set_is_valid(wait_set)) {
if (rcl_wait_set_is_valid(wait_set)) {
rmw_ret_t ret = rmw_destroy_wait_set(wait_set->impl->rmw_wait_set);
if (ret != RMW_RET_OK) {
RCL_SET_ERROR_MSG(rmw_get_error_string().str);
Expand All @@ -204,7 +204,7 @@ rcl_ret_t
rcl_wait_set_get_allocator(const rcl_wait_set_t * wait_set, rcl_allocator_t * allocator)
{
RCL_CHECK_ARGUMENT_FOR_NULL(wait_set, RCL_RET_INVALID_ARGUMENT);
if (!__wait_set_is_valid(wait_set)) {
if (!rcl_wait_set_is_valid(wait_set)) {
RCL_SET_ERROR_MSG("wait set is invalid");
return RCL_RET_WAIT_SET_INVALID;
}
Expand All @@ -215,7 +215,7 @@ rcl_wait_set_get_allocator(const rcl_wait_set_t * wait_set, rcl_allocator_t * al

#define SET_ADD(Type) \
RCL_CHECK_ARGUMENT_FOR_NULL(wait_set, RCL_RET_INVALID_ARGUMENT); \
if (!__wait_set_is_valid(wait_set)) { \
if (!rcl_wait_set_is_valid(wait_set)) { \
RCL_SET_ERROR_MSG("wait set is invalid"); \
return RCL_RET_WAIT_SET_INVALID; \
} \
Expand Down Expand Up @@ -515,7 +515,7 @@ rcl_ret_t
rcl_wait(rcl_wait_set_t * wait_set, int64_t timeout)
{
RCL_CHECK_ARGUMENT_FOR_NULL(wait_set, RCL_RET_INVALID_ARGUMENT);
if (!__wait_set_is_valid(wait_set)) {
if (!rcl_wait_set_is_valid(wait_set)) {
RCL_SET_ERROR_MSG("wait set is invalid");
return RCL_RET_WAIT_SET_INVALID;
}
Expand Down
20 changes: 20 additions & 0 deletions rcl/test/rcl/test_wait.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -65,6 +65,26 @@ class CLASSNAME (WaitSetTestFixture, RMW_IMPLEMENTATION) : public ::testing::Tes
}
};

TEST_F(CLASSNAME(WaitSetTestFixture, RMW_IMPLEMENTATION), wait_set_is_valid) {
// null pointers are invalid
EXPECT_FALSE(rcl_wait_set_is_valid(nullptr));

// uninitialized wait set is invalid
rcl_wait_set_t wait_set = rcl_get_zero_initialized_wait_set();
EXPECT_FALSE(rcl_wait_set_is_valid(&wait_set));

// initialized wait set is valid
rcl_ret_t ret =
rcl_wait_set_init(&wait_set, 1, 1, 1, 1, 1, 0, context_ptr, rcl_get_default_allocator());
EXPECT_EQ(RCL_RET_OK, ret) << rcl_get_error_string().str;
EXPECT_TRUE(rcl_wait_set_is_valid(&wait_set));

// finalized wait set is invalid
ret = rcl_wait_set_fini(&wait_set);
EXPECT_EQ(RCL_RET_OK, ret) << rcl_get_error_string().str;
EXPECT_FALSE(rcl_wait_set_is_valid(&wait_set));
}

TEST_F(CLASSNAME(WaitSetTestFixture, RMW_IMPLEMENTATION), test_resize_to_zero) {
// Initialize a wait set with a subscription and then resize it to zero.
rcl_wait_set_t wait_set = rcl_get_zero_initialized_wait_set();
Expand Down

0 comments on commit 92b9f79

Please sign in to comment.