Skip to content

Commit

Permalink
Add unit tests to cover message's send and received timestamps during…
Browse files Browse the repository at this point in the history
… recording (#1641) (#1673)

* Workaround for zeros in a message's received timestamp with CycloneDDS

- CycloneDDS doesn't support 'received_timestamps' and settling up
zeros in message_info for that field.
- Fallback to the timestamp from node clock in case of the CycloneDDS.

Signed-off-by: Michael Orlov <[email protected]>

* Fix typo in rmw_has_received_timestamp_support

Signed-off-by: Michael Orlov <[email protected]>

* Add one more check in tests that "recv_timestamp >= send_timestamp"

- Also fixed typo in 'rwm_has_send_timestamp_support' in test

Signed-off-by: Michael Orlov <[email protected]>

* Revert workaround for `rmw_cyclonedds` in recorder.cpp

- The fix will be done on the `rmw_cyclonedds` layer in the scope of the
ros2/rmw_cyclonedds#491

Signed-off-by: Michael Orlov <[email protected]>

---------

Signed-off-by: Michael Orlov <[email protected]>
(cherry picked from commit b7d9b5b)

Co-authored-by: Michael Orlov <[email protected]>
  • Loading branch information
mergify[bot] and MichaelOrlov authored May 25, 2024
1 parent 3840224 commit ff81091
Show file tree
Hide file tree
Showing 2 changed files with 27 additions and 4 deletions.
25 changes: 24 additions & 1 deletion rosbag2_transport/test/rosbag2_transport/test_record.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -70,8 +70,10 @@ TEST_F(RecordIntegrationTestFixture, published_messages_from_multiple_topics_are
return mock_writer.get_messages().size() >= expected_messages;
});
auto recorded_messages = mock_writer.get_messages();
EXPECT_TRUE(ret) << "failed to capture expected messages in time";
EXPECT_TRUE(ret) << "failed to capture expected messages in time" <<
"recorded messages = " << recorded_messages.size();
EXPECT_THAT(recorded_messages, SizeIs(expected_messages));
stop_spinning();

auto recorded_topics = mock_writer.get_topics();
ASSERT_THAT(recorded_topics, SizeIs(2));
Expand All @@ -87,6 +89,27 @@ TEST_F(RecordIntegrationTestFixture, published_messages_from_multiple_topics_are
EXPECT_THAT(string_messages[0]->string_value, Eq(string_message->string_value));
EXPECT_THAT(array_messages[0]->bool_values, Eq(array_message->bool_values));
EXPECT_THAT(array_messages[0]->float32_values, Eq(array_message->float32_values));

// Check for send and received timestamps
bool rmw_has_send_timestamp_support = true;
#ifdef _WIN32
if (std::string(rmw_get_implementation_identifier()).find("rmw_connextdds") !=
std::string::npos)
{
rmw_has_send_timestamp_support = false;
}
#endif
for (const auto & message : recorded_messages) {
EXPECT_NE(message->recv_timestamp, 0) << "topic : " << message->topic_name;
if (rmw_has_send_timestamp_support) {
// Check that the send_timestamp is not the same as the clock message
EXPECT_NE(message->send_timestamp, 0);
EXPECT_THAT(message->recv_timestamp, Ge(message->send_timestamp));
} else {
// if rwm has not sent timestamp support, send_timestamp must be zero
EXPECT_EQ(message->send_timestamp, 0);
}
}
}

TEST_F(RecordIntegrationTestFixture, can_record_again_after_stop)
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -137,17 +137,17 @@ TEST_F(RecordIntegrationTestFixture, record_all_with_sim_time)
// check that the timestamp is same as the clock message
EXPECT_THAT(string_messages[0]->recv_timestamp, time_value);

bool rwm_has_timestamp_support = true;
bool rmw_has_timestamp_support = true;

#ifdef _WIN32
if (std::string(rmw_get_implementation_identifier()).find("rmw_connextdds") !=
std::string::npos)
{
rwm_has_timestamp_support = false;
rmw_has_timestamp_support = false;
}
#endif

if (rwm_has_timestamp_support) {
if (rmw_has_timestamp_support) {
// Check that the send_timestamp is not the same as the clock message
EXPECT_NE(string_messages[0]->send_timestamp, time_value);
EXPECT_NE(string_messages[0]->send_timestamp, 0);
Expand Down

0 comments on commit ff81091

Please sign in to comment.