From de8edbecf64118f5526c7e1e03de0e69bb5374fb Mon Sep 17 00:00:00 2001 From: "mergify[bot]" <37929162+mergify[bot]@users.noreply.github.com> Date: Thu, 9 Nov 2023 14:01:22 -0500 Subject: [PATCH] Set disable loan to on by default. (#1110) (#1117) It is currently not safe to use; see the executor code in rclcpp for more information. Signed-off-by: Chris Lalancette (cherry picked from commit 1b01127ae2b9bcec4fef7c733fc7f680aacf5c7f) Co-authored-by: Chris Lalancette --- rcl/include/rcl/subscription.h | 2 +- rcl/src/rcl/subscription.c | 21 +++++++++++++-------- rcl/test/rcl/test_subscription.cpp | 9 +++++++-- 3 files changed, 21 insertions(+), 11 deletions(-) diff --git a/rcl/include/rcl/subscription.h b/rcl/include/rcl/subscription.h index 0c3db4c6d..d1d67e881 100644 --- a/rcl/include/rcl/subscription.h +++ b/rcl/include/rcl/subscription.h @@ -209,7 +209,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 + * - disable_loaned_message = true, false only if ROS_DISABLE_LOANED_MESSAGES=0 * * \return A structure containing the default options for a subscription. */ diff --git a/rcl/src/rcl/subscription.c b/rcl/src/rcl/subscription.c index ba7217dc0..6ccf686bc 100644 --- a/rcl/src/rcl/subscription.c +++ b/rcl/src/rcl/subscription.c @@ -24,6 +24,7 @@ extern "C" #include "rcl/error_handling.h" #include "rcl/node.h" #include "rcl/node_type_cache.h" +#include "rcutils/env.h" #include "rcutils/logging_macros.h" #include "rcutils/strdup.h" #include "rcutils/types/string_array.h" @@ -233,15 +234,19 @@ rcl_subscription_get_default_options() 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 { + // TODO(clalancette): This is kind of a copy of rcl_get_disable_loaned_message(), but we need + // more information than that function provides. + default_options.disable_loaned_message = true; + + const char * env_val = NULL; + const char * env_error_str = rcutils_get_env(RCL_DISABLE_LOANED_MESSAGES_ENV_VAR, &env_val); + if (NULL != env_error_str) { 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; + RCUTILS_SAFE_FWRITE_TO_STDERR_WITH_FORMAT_STRING( + "Error getting env var: '" RCUTILS_STRINGIFY(RCL_DISABLE_LOANED_MESSAGES_ENV_VAR) "': %s\n", + env_error_str); + } else { + default_options.disable_loaned_message = !(strcmp(env_val, "0") == 0); } return default_options; diff --git a/rcl/test/rcl/test_subscription.cpp b/rcl/test/rcl/test_subscription.cpp index c1610a74b..fdbda7ac2 100644 --- a/rcl/test/rcl/test_subscription.cpp +++ b/rcl/test/rcl/test_subscription.cpp @@ -720,7 +720,7 @@ 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); + EXPECT_TRUE(subscription_options.disable_loaned_message); } { ASSERT_TRUE(rcutils_set_env("ROS_DISABLE_LOANED_MESSAGES", "1")); @@ -730,11 +730,16 @@ TEST_F(CLASSNAME(TestSubscriptionFixture, RMW_IMPLEMENTATION), test_subscription { 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); + EXPECT_TRUE(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_TRUE(subscription_options.disable_loaned_message); + } + { + ASSERT_TRUE(rcutils_set_env("ROS_DISABLE_LOANED_MESSAGES", "0")); + rcl_subscription_options_t subscription_options = rcl_subscription_get_default_options(); EXPECT_FALSE(subscription_options.disable_loaned_message); } }