Skip to content

Commit

Permalink
Add rcl public comments
Browse files Browse the repository at this point in the history 
Details:
* Address comments from PR
* Add actual_qos to rcl_publisher_t
* Add comments to rcl public functions

Signed-off-by: Ross Desmond <44277324+ross-desmond@users.noreply.github.com>
  • Loading branch information
@ross-desmond
ross-desmond committed Apr 10, 2019
1 parent 6694751 commit 0a9b5fb
Show file tree
Hide file tree
Showing 4 changed files with 90 additions and 41 deletions.
42 changes: 42 additions & 0 deletions rcl/include/rcl/event.h
View file
Original file line number Diff line number Diff line change
Expand Up @@ -63,6 +63,17 @@ RCL_WARN_UNUSED
rcl_event_t
rcl_get_zero_initialized_event(void);

/// Initialize an rcl_event_t with a publisher.
/**
* Fill the rcl_event_t with the publisher and desired event_type.
*
* \param[in,out] event pointer to fill
* \param[in] publisher to get events from
* \param[in] event_type to listen for
* \return `RCL_RET_OK` if the rcl_event_t is filled, or
* \return `RCL_RET_INVALID_ARGUMENT` if any arguments are invalid, or
* \return `RCL_RET_ERROR` if an unspecified error occurs.
*/
RCL_PUBLIC
RCL_WARN_UNUSED
rcl_ret_t
Expand All @@ -71,6 +82,17 @@ rcl_publisher_event_init(
const rcl_publisher_t * publisher,
const rcl_publisher_event_type_t event_type);

/// Initialize an rcl_event_t with a subscription.
/**
* Fill the rcl_event_t with the subscription and desired event_type.
*
* \param[in,out] event pointer to fill
* \param[in] subscription to get events from
* \param[in] event_type to listen for
* \return `RCL_RET_OK` if the rcl_event_t is filled, or
* \return `RCL_RET_INVALID_ARGUMENT` if any arguments are invalid, or
* \return `RCL_RET_ERROR` if an unspecified error occurs.
*/
RCL_PUBLIC
RCL_WARN_UNUSED
rcl_ret_t
Expand All @@ -79,13 +101,33 @@ rcl_subscription_event_init(
const rcl_subscription_t * subscription,
const rcl_subscription_event_type_t event_type);

// Take event using the event handle.
/**
* Take an event from the event handle.
*
* \param[in] event_handle event object to take from
* \param[in, out] event_info event info object to write taken data into
* \param[in, out] taken boolean flag indicating if an event was taken or not
* \return `RCL_RET_OK` if successful, or
* \return `RCL_RET_BAD_ALLOC` if memory allocation failed, or
* \return `RCL_RET_ERROR` if an unexpected error occurs.
*/
RCL_PUBLIC
RCL_WARN_UNUSED
rcl_ret_t
rcl_take_event(
const rcl_event_t * event,
void * event_info);

// Finalize an event.
/**
* Finalize an event.
*
* \param[in] event to finalize
* \return `RCL_RET_OK` if successful, or
* \return `RCL_RET_EVENT_INVALID` if event is null, or
* \return `RCL_RET_ERROR` if an unexpected error occurs.
*/
RCL_PUBLIC
RCL_WARN_UNUSED
rcl_ret_t
Expand Down
14 changes: 6 additions & 8 deletions rcl/src/rcl/event.c
View file
Original file line number Diff line number Diff line change
Expand Up @@ -33,14 +33,12 @@ extern "C"
#include "./publisher_impl.h"
#include "./subscription_impl.h"


typedef struct rcl_event_impl_t
{
rmw_event_t rmw_handle;
rcl_allocator_t allocator;
} rcl_event_impl_t;


rcl_event_t
rcl_get_zero_initialized_event()
{
Expand All @@ -55,7 +53,6 @@ rcl_publisher_event_init(
const rcl_publisher_event_type_t event_type)
{
rcl_ret_t ret = RCL_RET_OK;

// Check publisher and allocator first, so allocator can be used with errors.
RCL_CHECK_ARGUMENT_FOR_NULL(publisher, RCL_RET_INVALID_ARGUMENT);
rcl_allocator_t * allocator = &publisher->impl->options.allocator;
Expand All @@ -79,9 +76,9 @@ rcl_publisher_event_init(
rmw_event_type = RMW_EVENT_LIVELINESS_LOST;
break;
default:
RCL_SET_ERROR_MSG("Event type for publisher not supported");
return RCL_RET_INVALID_ARGUMENT;
}

return rmw_publisher_event_init(
&event->impl->rmw_handle,
publisher->impl->rmw_handle,
Expand All @@ -95,7 +92,6 @@ rcl_subscription_event_init(
const rcl_subscription_event_type_t event_type)
{
rcl_ret_t ret = RCL_RET_OK;

// Check subscription and allocator first, so allocator can be used with errors.
RCL_CHECK_ARGUMENT_FOR_NULL(subscription, RCL_RET_INVALID_ARGUMENT);
rcl_allocator_t * allocator = &subscription->impl->options.allocator;
Expand All @@ -119,9 +115,9 @@ rcl_subscription_event_init(
rmw_event_type = RMW_EVENT_LIVELINESS_CHANGED;
break;
default:
RCL_SET_ERROR_MSG("Event type for subscription not supported");
return RCL_RET_INVALID_ARGUMENT;
}

return rmw_subscription_event_init(
&event->impl->rmw_handle,
subscription->impl->rmw_handle,
Expand All @@ -141,11 +137,13 @@ rcl_take_event(
RCL_SET_ERROR_MSG(rmw_get_error_string().str);
return rcl_convert_rmw_ret_to_rcl_ret(ret);
}
RCUTILS_LOG_DEBUG_NAMED(
ROS_PACKAGE_NAME, "Event take request succeeded: %s", taken ? "true" : "false");
if (!taken) {
RCUTILS_LOG_DEBUG_NAMED(
ROS_PACKAGE_NAME, "take_event request complete, unable to take event");
return RCL_RET_EVENT_TAKE_FAILED;
}
RCUTILS_LOG_DEBUG_NAMED(
ROS_PACKAGE_NAME, "take_event request success");
return rcl_convert_rmw_ret_to_rcl_ret(ret);
}

Expand Down
2 changes: 1 addition & 1 deletion rcl/src/rcl/publisher_impl.h
View file
Original file line number Diff line number Diff line change
Expand Up @@ -19,10 +19,10 @@

#include "rcl/publisher.h"


typedef struct rcl_publisher_impl_t
{
rcl_publisher_options_t options;
rmw_qos_profile_t actual_qos;
rcl_context_t * context;
rmw_publisher_t * rmw_handle;
} rcl_publisher_impl_t;
Expand Down
73 changes: 41 additions & 32 deletions rcl/test/rcl/test_events.cpp
View file
Original file line number Diff line number Diff line change
Expand Up @@ -30,7 +30,6 @@
using std::chrono::milliseconds;
using std::chrono::seconds;

constexpr seconds DISCOVERY_WAIT_TIME_IN_S(1);
constexpr seconds LIVELINESS_LEASE_DURATION_IN_S(1);
constexpr seconds DEADLINE_PERIOD_IN_S(1);

Expand Down Expand Up @@ -74,7 +73,7 @@ class CLASSNAME (TestEventFixture, RMW_IMPLEMENTATION) : public ::testing::Test
ts = ROSIDL_GET_MSG_TYPE_SUPPORT(test_msgs, msg, Primitives);
}

rcl_ret_t SetupPublisher(
rcl_ret_t setupPublisher(
const rmw_time_t & deadline,
const rmw_time_t & lifespan,
const rmw_time_t & liveliness_lease_duration,
Expand All @@ -91,7 +90,7 @@ class CLASSNAME (TestEventFixture, RMW_IMPLEMENTATION) : public ::testing::Test
&publisher_options);
}

rcl_ret_t SetupSubscriber(
rcl_ret_t setupSubscriber(
const rmw_time_t & deadline,
const rmw_time_t & lifespan,
const rmw_time_t & liveliness_lease_duration,
Expand All @@ -109,7 +108,7 @@ class CLASSNAME (TestEventFixture, RMW_IMPLEMENTATION) : public ::testing::Test
&subscription_options);
}

void SetupPublisherAndSubscriber(
void setupPublisherAndSubscriber(
const rcl_publisher_event_type_t & pub_event_type,
const rcl_subscription_event_type_t & sub_event_type)
{
Expand All @@ -121,7 +120,7 @@ class CLASSNAME (TestEventFixture, RMW_IMPLEMENTATION) : public ::testing::Test
rmw_qos_liveliness_policy_t liveliness_policy = RMW_QOS_POLICY_LIVELINESS_AUTOMATIC;

// init publisher
ret = SetupPublisher(deadline, lifespan, lease_duration, liveliness_policy);
ret = setupPublisher(deadline, lifespan, lease_duration, liveliness_policy);
ASSERT_EQ(RCL_RET_OK, ret) << rcl_get_error_string().str;

// init publisher events
Expand All @@ -130,16 +129,32 @@ class CLASSNAME (TestEventFixture, RMW_IMPLEMENTATION) : public ::testing::Test
ASSERT_EQ(RCL_RET_OK, ret) << rcl_get_error_string().str;

// init subscription
ret = SetupSubscriber(deadline, lifespan, lease_duration, liveliness_policy);
ret = setupSubscriber(deadline, lifespan, lease_duration, liveliness_policy);
ASSERT_EQ(RCL_RET_OK, ret) << rcl_get_error_string().str;

// init subscription event
subscription_event = rcl_get_zero_initialized_event();
ret = rcl_subscription_event_init(&subscription_event, &subscription, sub_event_type);
ASSERT_EQ(RCL_RET_OK, ret) << rcl_get_error_string().str;

// wait for discovery
// total wait time of 10 seconds, if never ready
size_t max_iterations = 100;
milliseconds wait_period(10);
size_t iteration = 0;
do {
iteration++;
size_t count = 0;
rcl_ret_t ret = rcl_subscription_get_publisher_count(&subscription, &count);
ASSERT_EQ(RCL_RET_OK, ret);
if (count > 0) {
break;
}
std::this_thread::sleep_for(wait_period);
} while (iteration < max_iterations);
}

void TearDownPubSub()
void tearDownPubSub()
{
rcl_ret_t ret;

Expand Down Expand Up @@ -241,6 +256,7 @@ wait_for_msgs_and_events(
for (size_t i = 0; i < wait_set.size_of_subscriptions; ++i) {
if (wait_set.subscriptions[i] && wait_set.subscriptions[i] == subscription) {
msg_ready = true;
break;
}
}
for (size_t i = 0; i < wait_set.size_of_events; ++i) {
Expand All @@ -264,18 +280,18 @@ TEST_F(CLASSNAME(TestEventFixture, RMW_IMPLEMENTATION), test_unsupported_lifespa
rmw_time_t lease_duration {1, 0};
rmw_qos_liveliness_policy_t liveliness_policy = RMW_QOS_POLICY_LIVELINESS_AUTOMATIC;
EXPECT_EQ(RMW_RET_ERROR,
SetupSubscriber(deadline, lifespan, lease_duration,
setupSubscriber(deadline, lifespan, lease_duration,
liveliness_policy)) << "Initialized subscriber lifespan when unsupported";
EXPECT_EQ(RMW_RET_ERROR,
SetupPublisher(deadline, lifespan, lease_duration,
setupPublisher(deadline, lifespan, lease_duration,
liveliness_policy)) << "Initialized publisher lifespan when unsupported";

lifespan = {0, 1};
EXPECT_EQ(RMW_RET_ERROR,
SetupSubscriber(deadline, lifespan, lease_duration,
setupSubscriber(deadline, lifespan, lease_duration,
liveliness_policy)) << "Initialized subscriber lifespan when unsupported";
EXPECT_EQ(RMW_RET_ERROR,
SetupPublisher(deadline, lifespan, lease_duration,
setupPublisher(deadline, lifespan, lease_duration,
liveliness_policy)) << "Initialized publisher lifespan when unsupported";
}
}
Expand All @@ -287,45 +303,45 @@ TEST_F(CLASSNAME(TestEventFixture, RMW_IMPLEMENTATION), test_unsupported_livelin
rmw_time_t lease_duration {0, 0};
rmw_qos_liveliness_policy_t liveliness_policy = RMW_QOS_POLICY_LIVELINESS_MANUAL_BY_NODE;
EXPECT_EQ(RMW_RET_ERROR,
SetupSubscriber(deadline, lifespan, lease_duration,
setupSubscriber(deadline, lifespan, lease_duration,
liveliness_policy)) <<
"Initialized subscriber RMW_QOS_POLICY_LIVELINESS_MANUAL_BY_NODE when unsupported";
EXPECT_EQ(RMW_RET_ERROR,
SetupPublisher(deadline, lifespan, lease_duration,
setupPublisher(deadline, lifespan, lease_duration,
liveliness_policy)) <<
"Initialized publisher RMW_QOS_POLICY_LIVELINESS_MANUAL_BY_NODE when unsupported";

liveliness_policy = RMW_QOS_POLICY_LIVELINESS_MANUAL_BY_TOPIC;
EXPECT_EQ(RMW_RET_ERROR,
SetupSubscriber(deadline, lifespan, lease_duration,
setupSubscriber(deadline, lifespan, lease_duration,
liveliness_policy)) <<
"Initialized subscriber RMW_QOS_POLICY_LIVELINESS_MANUAL_BY_TOPIC when unsupported";
EXPECT_EQ(RMW_RET_ERROR,
SetupPublisher(deadline, lifespan, lease_duration,
setupPublisher(deadline, lifespan, lease_duration,
liveliness_policy)) <<
"Initialized publisher RMW_QOS_POLICY_LIVELINESS_MANUAL_BY_TOPIC when unsupported";
}
}

TEST_F(CLASSNAME(TestEventFixture, RMW_IMPLEMENTATION), test_unsupported_unsupported_deadline) {
if (is_unsupported) {
rmw_time_t deadline;
rmw_time_t deadline {0, 0};
rmw_time_t lifespan {0, 0};
rmw_time_t lease_duration {0, 0};
rmw_qos_liveliness_policy_t liveliness_policy = RMW_QOS_POLICY_LIVELINESS_AUTOMATIC;
EXPECT_EQ(RMW_RET_ERROR,
SetupSubscriber(deadline, lifespan, lease_duration,
setupSubscriber(deadline, lifespan, lease_duration,
liveliness_policy)) << "Initialized subscriber deadline when unsupported";
EXPECT_EQ(RMW_RET_ERROR,
SetupPublisher(deadline, lifespan, lease_duration,
setupPublisher(deadline, lifespan, lease_duration,
liveliness_policy)) << "Initialized publisher deadline when unsupported";

deadline = {0, 1};
EXPECT_EQ(RMW_RET_ERROR,
SetupSubscriber(deadline, lifespan, lease_duration,
setupSubscriber(deadline, lifespan, lease_duration,
liveliness_policy)) << "Initialized subscriber deadline when unsupported";
EXPECT_EQ(RMW_RET_ERROR,
SetupPublisher(deadline, lifespan, lease_duration,
setupPublisher(deadline, lifespan, lease_duration,
liveliness_policy)) << "Initialized publisher deadline when unsupported";
}
}
Expand All @@ -339,14 +355,7 @@ TEST_F(CLASSNAME(TestEventFixture, RMW_IMPLEMENTATION), test_pubsub_liveliness_k
return;
}

SetupPublisherAndSubscriber(RCL_PUBLISHER_LIVELINESS_LOST, RCL_SUBSCRIPTION_LIVELINESS_CHANGED);

// wait for discovery
// TODO(wjwwood): add logic to wait for the connection to be established
// probably using the count_subscriptions busy wait mechanism
// until then we will sleep for a short period of time
std::this_thread::sleep_for(DISCOVERY_WAIT_TIME_IN_S);

setupPublisherAndSubscriber(RCL_PUBLISHER_LIVELINESS_LOST, RCL_SUBSCRIPTION_LIVELINESS_CHANGED);
rcl_ret_t ret;

// publish message to topic
Expand Down Expand Up @@ -418,7 +427,7 @@ TEST_F(CLASSNAME(TestEventFixture, RMW_IMPLEMENTATION), test_pubsub_deadline_mis
return;
}

SetupPublisherAndSubscriber(RCL_PUBLISHER_OFFERED_DEADLINE_MISSED,
setupPublisherAndSubscriber(RCL_PUBLISHER_OFFERED_DEADLINE_MISSED,
RCL_SUBSCRIPTION_REQUESTED_DEADLINE_MISSED);

// wait for discovery. also adds delay to publishing of message
Expand Down Expand Up @@ -479,7 +488,7 @@ TEST_F(CLASSNAME(TestEventFixture, RMW_IMPLEMENTATION), test_pubsub_deadline_mis
}

// clean up
TearDownPubSub();
tearDownPubSub();
}

/*
Expand All @@ -491,7 +500,7 @@ TEST_F(CLASSNAME(TestEventFixture, RMW_IMPLEMENTATION), test_pubsub_no_deadline_
return;
}

SetupPublisherAndSubscriber(RCL_PUBLISHER_OFFERED_DEADLINE_MISSED,
setupPublisherAndSubscriber(RCL_PUBLISHER_OFFERED_DEADLINE_MISSED,
RCL_SUBSCRIPTION_REQUESTED_DEADLINE_MISSED);

// wait for discovery. also adds delay to publishing of message
Expand Down Expand Up @@ -552,5 +561,5 @@ TEST_F(CLASSNAME(TestEventFixture, RMW_IMPLEMENTATION), test_pubsub_no_deadline_
}

// clean up
TearDownPubSub();
tearDownPubSub();
}

0 comments on commit 0a9b5fb

Please sign in to comment.