Fix up rmw_cyclonedds timestamp testing. (#1156)

We are about to fix it so that rmw_cyclonedds has receive_timestamp support, so we also need to enable that support here in rcl. We actually rewrite the logic a bit because now the only combination that doesn't work is rmw_connextdds on Windows. Signed-off-by: Chris Lalancette <[email protected]>
ros2 · May 9, 2024 · 6d53d24 · 6d53d24
1 parent f190677
commit 6d53d24
Show file tree

Hide file tree

Showing 3 changed files with 5 additions and 27 deletions.
diff --git a/rcl/test/CMakeLists.txt b/rcl/test/CMakeLists.txt
@@ -295,24 +295,14 @@ function(test_target)
   ament_add_test_label(test_subscription${target_suffix} mimick)
   # TODO(asorbini) Enable message timestamp tests for rmw_connextdds on Windows
   # once clock incompatibilities are resolved.
-  if(rmw_implementation STREQUAL "rmw_fastrtps_cpp" OR
-    rmw_implementation STREQUAL "rmw_fastrtps_dynamic_cpp" OR
-    (rmw_implementation STREQUAL "rmw_connextdds" AND NOT WIN32))
+  if(rmw_implementation STREQUAL "rmw_connextdds" AND WIN32)
+    message(STATUS "Disabling message timestamp test for ${rmw_implementation}")
+  else()
     message(STATUS "Enabling message timestamp test for ${rmw_implementation}")
     target_compile_definitions(test_subscription${target_suffix}
-      PUBLIC "RMW_TIMESTAMPS_SUPPORTED=1" "RMW_RECEIVED_TIMESTAMP_SUPPORTED=1")
+      PRIVATE "RMW_TIMESTAMPS_SUPPORTED=1")
     target_compile_definitions(test_service${target_suffix}
-      PUBLIC "RMW_TIMESTAMPS_SUPPORTED=1" "RMW_RECEIVED_TIMESTAMP_SUPPORTED=1")
-  else()
-    if(rmw_implementation STREQUAL "rmw_cyclonedds_cpp")
-      message(STATUS "Enabling only source timestamp test for ${rmw_implementation}")
-      target_compile_definitions(test_subscription${target_suffix}
-        PUBLIC "RMW_TIMESTAMPS_SUPPORTED=1")
-      target_compile_definitions(test_service${target_suffix}
-        PUBLIC "RMW_TIMESTAMPS_SUPPORTED=1")
-    else()
-      message(STATUS "Disabling message timestamp test for ${rmw_implementation}")
-    endif()
+      PRIVATE "RMW_TIMESTAMPS_SUPPORTED=1")
   endif()
 
   ament_add_gtest_test(test_events

diff --git a/rcl/test/rcl/test_service.cpp b/rcl/test/rcl/test_service.cpp
@@ -183,12 +183,8 @@ TEST_F(TestServiceFixture, test_service_nominal) {
     EXPECT_EQ(2UL, service_request.uint32_value);
 #ifdef RMW_TIMESTAMPS_SUPPORTED
     EXPECT_GE(header.source_timestamp, start_timestamp);
-#ifdef RMW_RECEIVED_TIMESTAMP_SUPPORTED
     EXPECT_GE(header.received_timestamp, start_timestamp);
     EXPECT_GE(header.received_timestamp, header.source_timestamp);
-#else
-    EXPECT_EQ(0u, header.received_timestamp);
-#endif
 #else
     EXPECT_EQ(0u, header.source_timestamp);
     EXPECT_EQ(0u, header.received_timestamp);
@@ -214,12 +210,8 @@ TEST_F(TestServiceFixture, test_service_nominal) {
   EXPECT_EQ(header.request_id.sequence_number, 1);
 #ifdef RMW_TIMESTAMPS_SUPPORTED
   EXPECT_GE(header.source_timestamp, start_timestamp);
-#ifdef RMW_RECEIVED_TIMESTAMP_SUPPORTED
   EXPECT_GE(header.received_timestamp, start_timestamp);
   EXPECT_GE(header.received_timestamp, header.source_timestamp);
-#else
-  EXPECT_EQ(0u, header.received_timestamp);
-#endif
 #else
   EXPECT_EQ(0u, header.source_timestamp);
   EXPECT_EQ(0u, header.received_timestamp);

diff --git a/rcl/test/rcl/test_subscription.cpp b/rcl/test/rcl/test_subscription.cpp
@@ -318,13 +318,9 @@ TEST_F(TestSubscriptionFixture, test_subscription_nominal) {
     EXPECT_NE(0u, message_info.source_timestamp);
     EXPECT_TRUE(pre_publish_time <= message_info.source_timestamp) <<
       pre_publish_time << " > " << message_info.source_timestamp;
-  #ifdef RMW_RECEIVED_TIMESTAMP_SUPPORTED
     EXPECT_NE(0u, message_info.received_timestamp);
     EXPECT_TRUE(pre_publish_time <= message_info.received_timestamp);
     EXPECT_TRUE(message_info.source_timestamp <= message_info.received_timestamp);
-  #else
-    EXPECT_EQ(0u, message_info.received_timestamp);
-  #endif
   #else
     EXPECT_EQ(0u, message_info.source_timestamp);
     EXPECT_EQ(0u, message_info.received_timestamp);