From 9de9593d1967586a3765aee18d065ea38b31507f Mon Sep 17 00:00:00 2001 From: Andrea Sorbini Date: Fri, 5 Mar 2021 10:07:32 -0800 Subject: [PATCH 1/7] Avoid implicit integer to double conversion Signed-off-by: Andrea Sorbini --- .../test/rclcpp/executors/test_multi_threaded_executor.cpp | 7 +++++-- 1 file changed, 5 insertions(+), 2 deletions(-) diff --git a/rclcpp/test/rclcpp/executors/test_multi_threaded_executor.cpp b/rclcpp/test/rclcpp/executors/test_multi_threaded_executor.cpp index b6fd2c3fda..cd0046a02c 100644 --- a/rclcpp/test/rclcpp/executors/test_multi_threaded_executor.cpp +++ b/rclcpp/test/rclcpp/executors/test_multi_threaded_executor.cpp @@ -86,10 +86,13 @@ TEST_F(TestMultiThreadedExecutor, timer_over_take) { double diff = static_cast(std::abs((now - last).nanoseconds())) / 1.0e9; last = now; - if (diff < PERIOD - TOLERANCE) { + double diff_exp = + static_cast(PERIOD) - static_cast(TOLERANCE); + if (diff < diff_exp) { executor.cancel(); - ASSERT_GT(diff, PERIOD - TOLERANCE); + ASSERT_GT(diff, diff_exp); } + } }; From 5646285bce53a0b1aaeeb6b29bb398ca77d00211 Mon Sep 17 00:00:00 2001 From: Andrea Sorbini Date: Fri, 5 Mar 2021 12:22:51 -0800 Subject: [PATCH 2/7] Replace rmw_connext_cpp with rmw_connextdds Signed-off-by: Andrea Sorbini --- .../test/rclcpp/strategies/test_allocator_memory_strategy.cpp | 4 +--- rclcpp/test/rclcpp/test_add_callback_groups_to_executor.cpp | 4 ---- rclcpp/test/rclcpp/test_publisher.cpp | 4 ---- rclcpp/test/rclcpp/test_subscription.cpp | 4 ---- rclcpp_action/test/test_server.cpp | 4 ---- rclcpp_lifecycle/test/test_lifecycle_node.cpp | 4 ---- rclcpp_lifecycle/test/test_lifecycle_service_client.cpp | 4 ---- 7 files changed, 1 insertion(+), 27 deletions(-) diff --git a/rclcpp/test/rclcpp/strategies/test_allocator_memory_strategy.cpp b/rclcpp/test/rclcpp/strategies/test_allocator_memory_strategy.cpp index 696b9f319a..b85f4bc6ad 100644 --- a/rclcpp/test/rclcpp/strategies/test_allocator_memory_strategy.cpp +++ b/rclcpp/test/rclcpp/strategies/test_allocator_memory_strategy.cpp @@ -511,9 +511,7 @@ TEST_F(TestAllocatorMemoryStrategy, number_of_entities_with_subscription) { RclWaitSetSizes expected_sizes = {}; expected_sizes.size_of_subscriptions = 1; const std::string implementation_identifier = rmw_get_implementation_identifier(); - if (implementation_identifier == "rmw_connext_cpp" || - implementation_identifier == "rmw_cyclonedds_cpp") - { + if (implementation_identifier == "rmw_cyclonedds_cpp") { // For connext, a subscription will also add an event and waitable expected_sizes.size_of_events += 1; expected_sizes.size_of_waitables += 1; diff --git a/rclcpp/test/rclcpp/test_add_callback_groups_to_executor.cpp b/rclcpp/test/rclcpp/test_add_callback_groups_to_executor.cpp index 5c63fd5962..25bd2aba03 100644 --- a/rclcpp/test/rclcpp/test_add_callback_groups_to_executor.cpp +++ b/rclcpp/test/rclcpp/test_add_callback_groups_to_executor.cpp @@ -30,10 +30,6 @@ #include "rclcpp/executor.hpp" #include "rclcpp/rclcpp.hpp" -// Note: This is a long running test with rmw_connext_cpp, if you change this file, please check -// that this test can complete fully, or adjust the timeout as necessary. -// See https://github.com/ros2/rmw_connext/issues/325 for resolution] - using namespace std::chrono_literals; template diff --git a/rclcpp/test/rclcpp/test_publisher.cpp b/rclcpp/test/rclcpp/test_publisher.cpp index 5492844b23..7d2c9d14c1 100644 --- a/rclcpp/test/rclcpp/test_publisher.cpp +++ b/rclcpp/test/rclcpp/test_publisher.cpp @@ -29,10 +29,6 @@ #include "test_msgs/msg/empty.hpp" -// Note: This is a long running test with rmw_connext_cpp, if you change this file, please check -// that this test can complete fully, or adjust the timeout as necessary. -// See https://github.com/ros2/rmw_connext/issues/325 for resolution - class TestPublisher : public ::testing::Test { public: diff --git a/rclcpp/test/rclcpp/test_subscription.cpp b/rclcpp/test/rclcpp/test_subscription.cpp index 2788b97bea..9b190fea18 100644 --- a/rclcpp/test/rclcpp/test_subscription.cpp +++ b/rclcpp/test/rclcpp/test_subscription.cpp @@ -29,10 +29,6 @@ #include "test_msgs/msg/empty.hpp" -// Note: This is a long running test with rmw_connext_cpp, if you change this file, please check -// that this test can complete fully, or adjust the timeout as necessary. -// See https://github.com/ros2/rmw_connext/issues/325 for resolution - using namespace std::chrono_literals; class TestSubscription : public ::testing::Test diff --git a/rclcpp_action/test/test_server.cpp b/rclcpp_action/test/test_server.cpp index 5fc2b7ee29..7ff9838658 100644 --- a/rclcpp_action/test/test_server.cpp +++ b/rclcpp_action/test/test_server.cpp @@ -29,10 +29,6 @@ #include "rclcpp_action/server.hpp" #include "./mocking_utils/patch.hpp" -// Note: This is a long running test with rmw_connext_cpp, if you change this file, please check -// that this test can complete fully, or adjust the timeout as necessary. -// See https://github.com/ros2/rmw_connext/issues/325 for resolution - using Fibonacci = test_msgs::action::Fibonacci; using CancelResponse = typename Fibonacci::Impl::CancelGoalService::Response; using GoalUUID = rclcpp_action::GoalUUID; diff --git a/rclcpp_lifecycle/test/test_lifecycle_node.cpp b/rclcpp_lifecycle/test/test_lifecycle_node.cpp index 1bd3032999..a52fc5ca7f 100644 --- a/rclcpp_lifecycle/test/test_lifecycle_node.cpp +++ b/rclcpp_lifecycle/test/test_lifecycle_node.cpp @@ -31,10 +31,6 @@ #include "./mocking_utils/patch.hpp" -// Note: This is a long running test with rmw_connext_cpp, if you change this file, please check -// that this test can complete fully, or adjust the timeout as necessary. -// See https://github.com/ros2/rmw_connext/issues/325 for resolution - using lifecycle_msgs::msg::State; using lifecycle_msgs::msg::Transition; diff --git a/rclcpp_lifecycle/test/test_lifecycle_service_client.cpp b/rclcpp_lifecycle/test/test_lifecycle_service_client.cpp index ab0fc4b466..109b1448a3 100644 --- a/rclcpp_lifecycle/test/test_lifecycle_service_client.cpp +++ b/rclcpp_lifecycle/test/test_lifecycle_service_client.cpp @@ -52,10 +52,6 @@ constexpr char const * node_get_transition_graph_topic = "/lc_talker/get_transition_graph"; const lifecycle_msgs::msg::State unknown_state = lifecycle_msgs::msg::State(); -// Note: This is a long running test with rmw_connext_cpp, if you change this file, please check -// that this test can complete fully, or adjust the timeout as necessary. -// See https://github.com/ros2/rmw_connext/issues/325 for resolution - class EmptyLifecycleNode : public rclcpp_lifecycle::LifecycleNode { public: From 31220d262f4f610defd9ee598eb80de512e444d5 Mon Sep 17 00:00:00 2001 From: Andrea Sorbini Date: Fri, 5 Mar 2021 15:48:26 -0800 Subject: [PATCH 3/7] Correct line indentation Signed-off-by: Andrea Sorbini --- rclcpp/test/rclcpp/executors/test_multi_threaded_executor.cpp | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/rclcpp/test/rclcpp/executors/test_multi_threaded_executor.cpp b/rclcpp/test/rclcpp/executors/test_multi_threaded_executor.cpp index cd0046a02c..7f44b67348 100644 --- a/rclcpp/test/rclcpp/executors/test_multi_threaded_executor.cpp +++ b/rclcpp/test/rclcpp/executors/test_multi_threaded_executor.cpp @@ -87,7 +87,7 @@ TEST_F(TestMultiThreadedExecutor, timer_over_take) { last = now; double diff_exp = - static_cast(PERIOD) - static_cast(TOLERANCE); + static_cast(PERIOD) - static_cast(TOLERANCE); if (diff < diff_exp) { executor.cancel(); ASSERT_GT(diff, diff_exp); From f1e3b5cb1c92959adb131aa9036fe38cea49884e Mon Sep 17 00:00:00 2001 From: Andrea Sorbini Date: Fri, 5 Mar 2021 18:14:56 -0800 Subject: [PATCH 4/7] Remove empty line from end of block. Signed-off-by: Andrea Sorbini --- rclcpp/test/rclcpp/executors/test_multi_threaded_executor.cpp | 1 - 1 file changed, 1 deletion(-) diff --git a/rclcpp/test/rclcpp/executors/test_multi_threaded_executor.cpp b/rclcpp/test/rclcpp/executors/test_multi_threaded_executor.cpp index 7f44b67348..5d6d1c4e3f 100644 --- a/rclcpp/test/rclcpp/executors/test_multi_threaded_executor.cpp +++ b/rclcpp/test/rclcpp/executors/test_multi_threaded_executor.cpp @@ -92,7 +92,6 @@ TEST_F(TestMultiThreadedExecutor, timer_over_take) { executor.cancel(); ASSERT_GT(diff, diff_exp); } - } }; From 7277605e19e213c5e39e7aa0d977bc7a65f687aa Mon Sep 17 00:00:00 2001 From: Andrea Sorbini Date: Mon, 8 Mar 2021 11:13:37 -0800 Subject: [PATCH 5/7] Replace stale reference to Connext Signed-off-by: Andrea Sorbini Co-authored-by: Chris Lalancette --- .../test/rclcpp/strategies/test_allocator_memory_strategy.cpp | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/rclcpp/test/rclcpp/strategies/test_allocator_memory_strategy.cpp b/rclcpp/test/rclcpp/strategies/test_allocator_memory_strategy.cpp index b85f4bc6ad..b0951c9218 100644 --- a/rclcpp/test/rclcpp/strategies/test_allocator_memory_strategy.cpp +++ b/rclcpp/test/rclcpp/strategies/test_allocator_memory_strategy.cpp @@ -512,7 +512,7 @@ TEST_F(TestAllocatorMemoryStrategy, number_of_entities_with_subscription) { expected_sizes.size_of_subscriptions = 1; const std::string implementation_identifier = rmw_get_implementation_identifier(); if (implementation_identifier == "rmw_cyclonedds_cpp") { - // For connext, a subscription will also add an event and waitable + // For cyclonedds, a subscription will also add an event and waitable expected_sizes.size_of_events += 1; expected_sizes.size_of_waitables += 1; } From 84c9fcd605a9d4a8a0e4314a85aeeb8c77dec31d Mon Sep 17 00:00:00 2001 From: Andrea Sorbini Date: Mon, 8 Mar 2021 12:15:19 -0800 Subject: [PATCH 6/7] Revert "Avoid implicit integer to double conversion" This reverts commit 9de9593d1967586a3765aee18d065ea38b31507f. Signed-off-by: Andrea Sorbini --- .../test/rclcpp/executors/test_multi_threaded_executor.cpp | 6 ++---- 1 file changed, 2 insertions(+), 4 deletions(-) diff --git a/rclcpp/test/rclcpp/executors/test_multi_threaded_executor.cpp b/rclcpp/test/rclcpp/executors/test_multi_threaded_executor.cpp index 5d6d1c4e3f..b6fd2c3fda 100644 --- a/rclcpp/test/rclcpp/executors/test_multi_threaded_executor.cpp +++ b/rclcpp/test/rclcpp/executors/test_multi_threaded_executor.cpp @@ -86,11 +86,9 @@ TEST_F(TestMultiThreadedExecutor, timer_over_take) { double diff = static_cast(std::abs((now - last).nanoseconds())) / 1.0e9; last = now; - double diff_exp = - static_cast(PERIOD) - static_cast(TOLERANCE); - if (diff < diff_exp) { + if (diff < PERIOD - TOLERANCE) { executor.cancel(); - ASSERT_GT(diff, diff_exp); + ASSERT_GT(diff, PERIOD - TOLERANCE); } } }; From 14838e9ed6971043d52123c61f23c46a08b7a46d Mon Sep 17 00:00:00 2001 From: Andrea Sorbini Date: Mon, 8 Mar 2021 16:04:47 -0800 Subject: [PATCH 7/7] Restore exceptions for ros2/rmw_connext to ease transition to rticommunity/rmw_connextdds Signed-off-by: Andrea Sorbini --- .../rclcpp/strategies/test_allocator_memory_strategy.cpp | 5 ++++- 1 file changed, 4 insertions(+), 1 deletion(-) diff --git a/rclcpp/test/rclcpp/strategies/test_allocator_memory_strategy.cpp b/rclcpp/test/rclcpp/strategies/test_allocator_memory_strategy.cpp index b0951c9218..b401d6d046 100644 --- a/rclcpp/test/rclcpp/strategies/test_allocator_memory_strategy.cpp +++ b/rclcpp/test/rclcpp/strategies/test_allocator_memory_strategy.cpp @@ -511,7 +511,10 @@ TEST_F(TestAllocatorMemoryStrategy, number_of_entities_with_subscription) { RclWaitSetSizes expected_sizes = {}; expected_sizes.size_of_subscriptions = 1; const std::string implementation_identifier = rmw_get_implementation_identifier(); - if (implementation_identifier == "rmw_cyclonedds_cpp") { + // TODO(asorbini): Remove Connext exception once ros2/rmw_connext is deprecated. + if (implementation_identifier == "rmw_connext_cpp" || + implementation_identifier == "rmw_cyclonedds_cpp") + { // For cyclonedds, a subscription will also add an event and waitable expected_sizes.size_of_events += 1; expected_sizes.size_of_waitables += 1;