From 79cb1e6e9fd9316631fa0425411ae7f96e6829af Mon Sep 17 00:00:00 2001 From: Tomoya Fujita Date: Mon, 27 Feb 2023 14:04:38 -0800 Subject: [PATCH] Cache disable flag to avoid reading environmental variable. (#1029) * Cache disable flag to avoid reading environmental variable. Signed-off-by: Tomoya Fujita * Revert "Cache disable flag to avoid reading environmental variable." This reverts commit 2ed9454228ca944b543b4decfd68977668fee100. Signed-off-by: Tomoya Fujita * extend pub/sub option to save loaned message disable flag. Signed-off-by: Tomoya Fujita * add more test cases. Signed-off-by: Tomoya Fujita * set env variable explicitly during loaned message test. Signed-off-by: Tomoya Fujita * add comments and error handling. Signed-off-by: Tomoya Fujita --------- Signed-off-by: Tomoya Fujita --- rcl/include/rcl/publisher.h | 3 ++ rcl/include/rcl/subscription.h | 3 ++ rcl/src/rcl/publisher.c | 17 ++++-- rcl/src/rcl/subscription.c | 17 ++++-- rcl/test/rcl/test_publisher.cpp | 84 +++++++++++++++++++++--------- rcl/test/rcl/test_subscription.cpp | 79 +++++++++++++++++++--------- 6 files changed, 147 insertions(+), 56 deletions(-) diff --git a/rcl/include/rcl/publisher.h b/rcl/include/rcl/publisher.h index c39f5cfc5..8736c78c0 100644 --- a/rcl/include/rcl/publisher.h +++ b/rcl/include/rcl/publisher.h @@ -49,6 +49,8 @@ typedef struct rcl_publisher_options_s rcl_allocator_t allocator; /// rmw specific publisher options, e.g. the rmw implementation specific payload. rmw_publisher_options_t rmw_publisher_options; + /// Disable flag to LoanedMessage, initialized via environmental variable. + bool disable_loaned_message; } rcl_publisher_options_t; /// Return a rcl_publisher_t struct with members set to `NULL`. @@ -194,6 +196,7 @@ rcl_publisher_fini(rcl_publisher_t * publisher, rcl_node_t * node); * - qos = rmw_qos_profile_default * - allocator = rcl_get_default_allocator() * - rmw_publisher_options = rmw_get_default_publisher_options() + * - disable_loaned_message = false, true only if ROS_DISABLE_LOANED_MESSAGES=1 * * \return A structure with the default publisher options. */ diff --git a/rcl/include/rcl/subscription.h b/rcl/include/rcl/subscription.h index 28bbf1713..1244174e5 100644 --- a/rcl/include/rcl/subscription.h +++ b/rcl/include/rcl/subscription.h @@ -51,6 +51,8 @@ typedef struct rcl_subscription_options_s rcl_allocator_t allocator; /// rmw specific subscription options, e.g. the rmw implementation specific payload. rmw_subscription_options_t rmw_subscription_options; + /// Disable flag to LoanedMessage, initialized via environmental variable. + bool disable_loaned_message; } rcl_subscription_options_t; typedef struct rcl_subscription_content_filter_options_s @@ -206,6 +208,7 @@ rcl_subscription_fini(rcl_subscription_t * subscription, rcl_node_t * node); * - qos = rmw_qos_profile_default * - allocator = rcl_get_default_allocator() * - rmw_subscription_options = rmw_get_default_subscription_options(); + * - disable_loaned_message = false, true only if ROS_DISABLE_LOANED_MESSAGES=1 * * \return A structure containing the default options for a subscription. */ diff --git a/rcl/src/rcl/publisher.c b/rcl/src/rcl/publisher.c index 47582da3d..f61f5ab75 100644 --- a/rcl/src/rcl/publisher.c +++ b/rcl/src/rcl/publisher.c @@ -203,6 +203,19 @@ rcl_publisher_get_default_options() default_options.qos = rmw_qos_profile_default; default_options.allocator = rcl_get_default_allocator(); default_options.rmw_publisher_options = rmw_get_default_publisher_options(); + + // Load disable flag to LoanedMessage via environmental variable. + bool disable_loaned_message = false; + rcl_ret_t ret = rcl_get_disable_loaned_message(&disable_loaned_message); + if (ret == RCL_RET_OK) { + default_options.disable_loaned_message = disable_loaned_message; + } else { + RCUTILS_SAFE_FWRITE_TO_STDERR("Failed to get disable_loaned_message: "); + RCUTILS_SAFE_FWRITE_TO_STDERR(rcl_get_error_string().str); + rcl_reset_error(); + default_options.disable_loaned_message = false; + } + return default_options; } @@ -441,9 +454,7 @@ rcl_publisher_can_loan_messages(const rcl_publisher_t * publisher) return false; // error message already set } - bool disable_loaned_message = false; - rcl_ret_t ret = rcl_get_disable_loaned_message(&disable_loaned_message); - if (ret == RCL_RET_OK && disable_loaned_message) { + if (publisher->impl->options.disable_loaned_message) { return false; } diff --git a/rcl/src/rcl/subscription.c b/rcl/src/rcl/subscription.c index fd5984ded..cc6d9cd53 100644 --- a/rcl/src/rcl/subscription.c +++ b/rcl/src/rcl/subscription.c @@ -206,6 +206,19 @@ rcl_subscription_get_default_options() default_options.qos = rmw_qos_profile_default; default_options.allocator = rcl_get_default_allocator(); default_options.rmw_subscription_options = rmw_get_default_subscription_options(); + + // Load disable flag to LoanedMessage via environmental variable. + bool disable_loaned_message = false; + rcl_ret_t ret = rcl_get_disable_loaned_message(&disable_loaned_message); + if (ret == RCL_RET_OK) { + default_options.disable_loaned_message = disable_loaned_message; + } else { + RCUTILS_SAFE_FWRITE_TO_STDERR("Failed to get disable_loaned_message: "); + RCUTILS_SAFE_FWRITE_TO_STDERR(rcl_get_error_string().str); + rcl_reset_error(); + default_options.disable_loaned_message = false; + } + return default_options; } @@ -736,9 +749,7 @@ rcl_subscription_can_loan_messages(const rcl_subscription_t * subscription) return false; // error message already set } - bool disable_loaned_message = false; - rcl_ret_t ret = rcl_get_disable_loaned_message(&disable_loaned_message); - if (ret == RCL_RET_OK && disable_loaned_message) { + if (subscription->impl->options.disable_loaned_message) { return false; } diff --git a/rcl/test/rcl/test_publisher.cpp b/rcl/test/rcl/test_publisher.cpp index 15bd520c3..0a4249218 100644 --- a/rcl/test/rcl/test_publisher.cpp +++ b/rcl/test/rcl/test_publisher.cpp @@ -372,39 +372,73 @@ TEST_F(CLASSNAME(TestPublisherFixture, RMW_IMPLEMENTATION), test_publisher_loan) } } +TEST_F(CLASSNAME(TestPublisherFixture, RMW_IMPLEMENTATION), test_publisher_option) { + { + rcl_publisher_options_t publisher_options = rcl_publisher_get_default_options(); + EXPECT_FALSE(publisher_options.disable_loaned_message); + } + { + ASSERT_TRUE(rcutils_set_env("ROS_DISABLE_LOANED_MESSAGES", "0")); + rcl_publisher_options_t publisher_options = rcl_publisher_get_default_options(); + EXPECT_FALSE(publisher_options.disable_loaned_message); + } + { + ASSERT_TRUE(rcutils_set_env("ROS_DISABLE_LOANED_MESSAGES", "1")); + rcl_publisher_options_t publisher_options = rcl_publisher_get_default_options(); + EXPECT_TRUE(publisher_options.disable_loaned_message); + } + { + ASSERT_TRUE(rcutils_set_env("ROS_DISABLE_LOANED_MESSAGES", "2")); + rcl_publisher_options_t publisher_options = rcl_publisher_get_default_options(); + EXPECT_FALSE(publisher_options.disable_loaned_message); + } + { + ASSERT_TRUE(rcutils_set_env("ROS_DISABLE_LOANED_MESSAGES", "Unexpected")); + rcl_publisher_options_t publisher_options = rcl_publisher_get_default_options(); + EXPECT_FALSE(publisher_options.disable_loaned_message); + } +} + TEST_F(CLASSNAME(TestPublisherFixture, RMW_IMPLEMENTATION), test_publisher_loan_disable) { - rcl_publisher_t publisher = rcl_get_zero_initialized_publisher(); + bool is_fastdds = (std::string(rmw_get_implementation_identifier()).find("rmw_fastrtps") == 0); const rosidl_message_type_support_t * ts = ROSIDL_GET_MSG_TYPE_SUPPORT(test_msgs, msg, BasicTypes); constexpr char topic_name[] = "pod_msg"; - rcl_publisher_options_t publisher_options = rcl_publisher_get_default_options(); - rcl_ret_t ret = - rcl_publisher_init(&publisher, this->node_ptr, ts, topic_name, &publisher_options); - ASSERT_EQ(RCL_RET_OK, ret) << rcl_get_error_string().str; - OSRF_TESTING_TOOLS_CPP_SCOPE_EXIT( - { - rcl_ret_t ret = rcl_publisher_fini(&publisher, this->node_ptr); - EXPECT_EQ(RCL_RET_OK, ret) << rcl_get_error_string().str; - }); - if (rcl_publisher_can_loan_messages(&publisher)) { - ASSERT_TRUE(rcutils_set_env("ROS_DISABLE_LOANED_MESSAGES", "0")); - EXPECT_TRUE(rcl_publisher_can_loan_messages(&publisher)); + { ASSERT_TRUE(rcutils_set_env("ROS_DISABLE_LOANED_MESSAGES", "1")); + rcl_publisher_t publisher = rcl_get_zero_initialized_publisher(); + rcl_publisher_options_t publisher_options = rcl_publisher_get_default_options(); + EXPECT_TRUE(publisher_options.disable_loaned_message); + rcl_ret_t ret = + rcl_publisher_init(&publisher, this->node_ptr, ts, topic_name, &publisher_options); + ASSERT_EQ(RCL_RET_OK, ret) << rcl_get_error_string().str; + OSRF_TESTING_TOOLS_CPP_SCOPE_EXIT( + { + rcl_ret_t ret = rcl_publisher_fini(&publisher, this->node_ptr); + EXPECT_EQ(RCL_RET_OK, ret) << rcl_get_error_string().str; + }); EXPECT_FALSE(rcl_publisher_can_loan_messages(&publisher)); - ASSERT_TRUE(rcutils_set_env("ROS_DISABLE_LOANED_MESSAGES", "2")); - EXPECT_TRUE(rcl_publisher_can_loan_messages(&publisher)); - ASSERT_TRUE(rcutils_set_env("ROS_DISABLE_LOANED_MESSAGES", "Unexpected")); - EXPECT_TRUE(rcl_publisher_can_loan_messages(&publisher)); - } else { + } + + { ASSERT_TRUE(rcutils_set_env("ROS_DISABLE_LOANED_MESSAGES", "0")); - EXPECT_FALSE(rcl_publisher_can_loan_messages(&publisher)); - ASSERT_TRUE(rcutils_set_env("ROS_DISABLE_LOANED_MESSAGES", "1")); - EXPECT_FALSE(rcl_publisher_can_loan_messages(&publisher)); - ASSERT_TRUE(rcutils_set_env("ROS_DISABLE_LOANED_MESSAGES", "2")); - EXPECT_FALSE(rcl_publisher_can_loan_messages(&publisher)); - ASSERT_TRUE(rcutils_set_env("ROS_DISABLE_LOANED_MESSAGES", "Unexpected")); - EXPECT_FALSE(rcl_publisher_can_loan_messages(&publisher)); + rcl_publisher_t publisher = rcl_get_zero_initialized_publisher(); + rcl_publisher_options_t publisher_options = rcl_publisher_get_default_options(); + EXPECT_FALSE(publisher_options.disable_loaned_message); + rcl_ret_t ret = + rcl_publisher_init(&publisher, this->node_ptr, ts, topic_name, &publisher_options); + ASSERT_EQ(RCL_RET_OK, ret) << rcl_get_error_string().str; + OSRF_TESTING_TOOLS_CPP_SCOPE_EXIT( + { + rcl_ret_t ret = rcl_publisher_fini(&publisher, this->node_ptr); + EXPECT_EQ(RCL_RET_OK, ret) << rcl_get_error_string().str; + }); + if (is_fastdds) { + EXPECT_TRUE(rcl_publisher_can_loan_messages(&publisher)); + } else { + EXPECT_FALSE(rcl_publisher_can_loan_messages(&publisher)); + } } } diff --git a/rcl/test/rcl/test_subscription.cpp b/rcl/test/rcl/test_subscription.cpp index 500497dcb..c1610a74b 100644 --- a/rcl/test/rcl/test_subscription.cpp +++ b/rcl/test/rcl/test_subscription.cpp @@ -717,39 +717,68 @@ TEST_F(CLASSNAME(TestSubscriptionFixture, RMW_IMPLEMENTATION), test_subscription } } +TEST_F(CLASSNAME(TestSubscriptionFixture, RMW_IMPLEMENTATION), test_subscription_option) { + { + rcl_subscription_options_t subscription_options = rcl_subscription_get_default_options(); + EXPECT_FALSE(subscription_options.disable_loaned_message); + } + { + ASSERT_TRUE(rcutils_set_env("ROS_DISABLE_LOANED_MESSAGES", "1")); + rcl_subscription_options_t subscription_options = rcl_subscription_get_default_options(); + EXPECT_TRUE(subscription_options.disable_loaned_message); + } + { + ASSERT_TRUE(rcutils_set_env("ROS_DISABLE_LOANED_MESSAGES", "2")); + rcl_subscription_options_t subscription_options = rcl_subscription_get_default_options(); + EXPECT_FALSE(subscription_options.disable_loaned_message); + } + { + ASSERT_TRUE(rcutils_set_env("ROS_DISABLE_LOANED_MESSAGES", "Unexpected")); + rcl_subscription_options_t subscription_options = rcl_subscription_get_default_options(); + EXPECT_FALSE(subscription_options.disable_loaned_message); + } +} + TEST_F(CLASSNAME(TestSubscriptionFixture, RMW_IMPLEMENTATION), test_subscription_loan_disable) { - rcl_subscription_t subscription = rcl_get_zero_initialized_subscription(); + bool is_fastdds = (std::string(rmw_get_implementation_identifier()).find("rmw_fastrtps") == 0); const rosidl_message_type_support_t * ts = ROSIDL_GET_MSG_TYPE_SUPPORT(test_msgs, msg, BasicTypes); constexpr char topic[] = "pod_msg"; - rcl_subscription_options_t subscription_options = rcl_subscription_get_default_options(); - rcl_ret_t ret = - rcl_subscription_init(&subscription, this->node_ptr, ts, topic, &subscription_options); - ASSERT_EQ(RCL_RET_OK, ret) << rcl_get_error_string().str; - OSRF_TESTING_TOOLS_CPP_SCOPE_EXIT( - { - rcl_ret_t ret = rcl_subscription_fini(&subscription, this->node_ptr); - EXPECT_EQ(RCL_RET_OK, ret) << rcl_get_error_string().str; - }); - if (rcl_subscription_can_loan_messages(&subscription)) { - ASSERT_TRUE(rcutils_set_env("ROS_DISABLE_LOANED_MESSAGES", "0")); - EXPECT_TRUE(rcl_subscription_can_loan_messages(&subscription)); + { ASSERT_TRUE(rcutils_set_env("ROS_DISABLE_LOANED_MESSAGES", "1")); + rcl_subscription_t subscription = rcl_get_zero_initialized_subscription(); + rcl_subscription_options_t subscription_options = rcl_subscription_get_default_options(); + EXPECT_TRUE(subscription_options.disable_loaned_message); + rcl_ret_t ret = + rcl_subscription_init(&subscription, this->node_ptr, ts, topic, &subscription_options); + ASSERT_EQ(RCL_RET_OK, ret) << rcl_get_error_string().str; + OSRF_TESTING_TOOLS_CPP_SCOPE_EXIT( + { + rcl_ret_t ret = rcl_subscription_fini(&subscription, this->node_ptr); + EXPECT_EQ(RCL_RET_OK, ret) << rcl_get_error_string().str; + }); EXPECT_FALSE(rcl_subscription_can_loan_messages(&subscription)); - ASSERT_TRUE(rcutils_set_env("ROS_DISABLE_LOANED_MESSAGES", "2")); - EXPECT_TRUE(rcl_subscription_can_loan_messages(&subscription)); - ASSERT_TRUE(rcutils_set_env("ROS_DISABLE_LOANED_MESSAGES", "Unexpected")); - EXPECT_TRUE(rcl_subscription_can_loan_messages(&subscription)); - } else { + } + + { ASSERT_TRUE(rcutils_set_env("ROS_DISABLE_LOANED_MESSAGES", "0")); - EXPECT_FALSE(rcl_subscription_can_loan_messages(&subscription)); - ASSERT_TRUE(rcutils_set_env("ROS_DISABLE_LOANED_MESSAGES", "1")); - EXPECT_FALSE(rcl_subscription_can_loan_messages(&subscription)); - ASSERT_TRUE(rcutils_set_env("ROS_DISABLE_LOANED_MESSAGES", "2")); - EXPECT_FALSE(rcl_subscription_can_loan_messages(&subscription)); - ASSERT_TRUE(rcutils_set_env("ROS_DISABLE_LOANED_MESSAGES", "Unexpected")); - EXPECT_FALSE(rcl_subscription_can_loan_messages(&subscription)); + rcl_subscription_t subscription = rcl_get_zero_initialized_subscription(); + rcl_subscription_options_t subscription_options = rcl_subscription_get_default_options(); + EXPECT_FALSE(subscription_options.disable_loaned_message); + rcl_ret_t ret = + rcl_subscription_init(&subscription, this->node_ptr, ts, topic, &subscription_options); + ASSERT_EQ(RCL_RET_OK, ret) << rcl_get_error_string().str; + OSRF_TESTING_TOOLS_CPP_SCOPE_EXIT( + { + rcl_ret_t ret = rcl_subscription_fini(&subscription, this->node_ptr); + EXPECT_EQ(RCL_RET_OK, ret) << rcl_get_error_string().str; + }); + if (is_fastdds) { + EXPECT_TRUE(rcl_subscription_can_loan_messages(&subscription)); + } else { + EXPECT_FALSE(rcl_subscription_can_loan_messages(&subscription)); + } } }