Cache disable flag to avoid reading environmental variable. (ros2#1029)

* Cache disable flag to avoid reading environmental variable. Signed-off-by: Tomoya Fujita <[email protected]> * Revert "Cache disable flag to avoid reading environmental variable." This reverts commit 2ed9454. Signed-off-by: Tomoya Fujita <[email protected]> * extend pub/sub option to save loaned message disable flag. Signed-off-by: Tomoya Fujita <[email protected]> * add more test cases. Signed-off-by: Tomoya Fujita <[email protected]> * set env variable explicitly during loaned message test. Signed-off-by: Tomoya Fujita <[email protected]> * add comments and error handling. Signed-off-by: Tomoya Fujita <[email protected]> --------- Signed-off-by: Tomoya Fujita <[email protected]>
danthony06 · Jun 14, 2023 · 79cb1e6 · 79cb1e6
1 parent e3eccbe
commit 79cb1e6
Show file tree

Hide file tree

Showing 6 changed files with 147 additions and 56 deletions.
diff --git 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
@@ -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
@@ -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
@@ -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
@@ -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
@@ -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));
+    }
   }
 }