Skip to content

Commit

Permalink
Make sure to initialize the rmw_message_sequence after init. (#175)
Browse files Browse the repository at this point in the history
* Make sure to initialize the rmw_message_sequence after init.

The documentation for "rmw_take_sequence" says:

"Given `message_sequence` must be a valid message sequence, initialized
by rmw_message_sequence_init() and populated with ROS messages whose
type matches the message type support registered with the `subscription`
on creation."

As it stands, the take_sequence test fulfills the first part of that
(it was initialized by rmw_message_sequence_init()), but doesn't fulfill
the second part (populated by ROS messages).  In certain situations,
this could lead to an aborted test later on.  Make sure to do the
initialization to a known type before calling rmw_take_sequence.

Signed-off-by: Chris Lalancette <[email protected]>
  • Loading branch information
clalancette authored Feb 8, 2021
1 parent 55b893d commit ef0fcda
Showing 1 changed file with 11 additions and 2 deletions.
13 changes: 11 additions & 2 deletions test_rmw_implementation/test/test_subscription.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -24,6 +24,7 @@
#include "rmw/error_handling.h"

#include "test_msgs/msg/basic_types.h"
#include "test_msgs/msg/strings.h"
#include "./config.hpp"
#include "./testing_macros.hpp"

Expand Down Expand Up @@ -530,6 +531,12 @@ TEST_F(CLASSNAME(TestSubscriptionUse, RMW_IMPLEMENTATION), take_sequence) {
rmw_message_sequence_t sequence = rmw_get_zero_initialized_message_sequence();
rmw_ret_t ret = rmw_message_sequence_init(&sequence, count, &allocator);
EXPECT_EQ(RMW_RET_OK, ret) << rmw_get_error_string().str;

auto seq = test_msgs__msg__Strings__Sequence__create(count);
for (size_t ii = 0; ii < count; ++ii) {
sequence.data[ii] = &seq->data[ii];
}

rmw_message_info_sequence_t info_sequence = rmw_get_zero_initialized_message_info_sequence();
ret = rmw_message_info_sequence_init(&info_sequence, count, &allocator);
EXPECT_EQ(RMW_RET_OK, ret) << rmw_get_error_string().str;
Expand All @@ -539,11 +546,13 @@ TEST_F(CLASSNAME(TestSubscriptionUse, RMW_IMPLEMENTATION), take_sequence) {
EXPECT_EQ(RMW_RET_OK, ret) << rmw_get_error_string().str;
EXPECT_EQ(taken, 0u);

ret = rmw_message_sequence_fini(&sequence);
ret = rmw_message_info_sequence_fini(&info_sequence);
EXPECT_EQ(RMW_RET_OK, ret) << rmw_get_error_string().str;

ret = rmw_message_info_sequence_fini(&info_sequence);
ret = rmw_message_sequence_fini(&sequence);
EXPECT_EQ(RMW_RET_OK, ret) << rmw_get_error_string().str;

test_msgs__msg__Strings__Sequence__destroy(seq);
}

TEST_F(CLASSNAME(TestSubscriptionUse, RMW_IMPLEMENTATION), take_sequence_with_bad_args) {
Expand Down

0 comments on commit ef0fcda

Please sign in to comment.