Move actual domain id from node to context (#718)

Signed-off-by: Ivan Santiago Paunovic <ivanpauno@ekumenlabs.com>
ros2 · Jul 22, 2020 · fb1010f · fb1010f
1 parent bfc0da9
commit fb1010f
Show file tree

Hide file tree

Showing 5 changed files with 76 additions and 13 deletions.
diff --git a/rcl/include/rcl/context.h b/rcl/include/rcl/context.h
@@ -240,6 +240,32 @@ RCL_WARN_UNUSED
 rcl_context_instance_id_t
 rcl_context_get_instance_id(rcl_context_t * context);
 
+/// Returns the context domain id.
+/**
+ * \pre If context is uninitialized, then it is undefined behavior.
+ *
+ * <hr>
+ * Attribute          | Adherence
+ * ------------------ | -------------
+ * Allocates Memory   | No
+ * Thread-Safe        | Yes [1]
+ * Uses Atomics       | No
+ * Lock-Free          | No
+ *
+ * <i>[1] Calling the function asynchronously with `rcl_init` or `rcl_shutdown` can result
+ *  in the function sometimes succeeding and sometimes returning `RCL_RET_INVALID_ARGUMENT`.
+ *
+ * \param[in] context from which the domain id should be retrieved.
+ * \param[out] domain_id output variable where the domain id will be returned.
+ * \return RCL_RET_INVALID_ARGUMENT if `context` is invalid \ref `rcl_context_is_valid`, or
+ * \return RCL_RET_INVALID_ARGUMENT if `domain_id` is `NULL`, or
+ * \return RCL_RET_OK if the domain id was correctly retrieved.
+ */
+RCL_PUBLIC
+RCL_WARN_UNUSED
+rcl_ret_t
+rcl_context_get_domain_id(rcl_context_t * context, size_t * domain_id);
+
 /// Return `true` if the given context is currently valid, otherwise `false`.
 /**
  * If context is `NULL`, then `false` is returned.

diff --git a/rcl/src/rcl/context.c b/rcl/src/rcl/context.c
@@ -77,6 +77,17 @@ rcl_context_get_instance_id(rcl_context_t * context)
   return rcutils_atomic_load_uint64_t((atomic_uint_least64_t *)(&context->instance_id_storage));
 }
 
+rcl_ret_t
+rcl_context_get_domain_id(rcl_context_t * context, size_t * domain_id)
+{
+  if (!rcl_context_is_valid(context)) {
+    return RCL_RET_INVALID_ARGUMENT;
+  }
+  RCL_CHECK_ARGUMENT_FOR_NULL(domain_id, RCL_RET_INVALID_ARGUMENT);
+  *domain_id = context->impl->rmw_context.actual_domain_id;
+  return RCL_RET_OK;
+}
+
 bool
 rcl_context_is_valid(rcl_context_t * context)
 {

diff --git a/rcl/src/rcl/node.c b/rcl/src/rcl/node.c
@@ -56,7 +56,6 @@ extern "C"
 typedef struct rcl_node_impl_t
 {
   rcl_node_options_t options;
-  size_t actual_domain_id;
   rmw_node_t * rmw_node_handle;
   rcl_guard_condition_t * graph_guard_condition;
   const char * logger_name;
@@ -120,7 +119,6 @@ rcl_node_init(
   rcl_context_t * context,
   const rcl_node_options_t * options)
 {
-  size_t domain_id = 0;
   const rmw_guard_condition_t * rmw_graph_guard_condition = NULL;
   rcl_guard_condition_options_t graph_guard_condition_options =
     rcl_guard_condition_get_default_options();
@@ -253,12 +251,8 @@ rcl_node_init(
   RCL_CHECK_FOR_NULL_WITH_MSG(
     node->impl->logger_name, "creating logger name failed", goto fail);
 
-  ret = rcl_init_options_get_domain_id(rcl_context_get_init_options(context), &domain_id);
-  if (RCL_RET_OK != ret) {
-    goto fail;
-  }
-  RCUTILS_LOG_DEBUG_NAMED(ROS_PACKAGE_NAME, "Using domain ID of '%zu'", domain_id);
-  node->impl->actual_domain_id = domain_id;
+  RCUTILS_LOG_DEBUG_NAMED(
+    ROS_PACKAGE_NAME, "Using domain ID of '%zu'", context->impl->rmw_context.actual_domain_id);
 
   node->impl->rmw_node_handle = rmw_create_node(
     &(node->context->impl->rmw_context),
@@ -474,12 +468,14 @@ rcl_node_get_options(const rcl_node_t * node)
 rcl_ret_t
 rcl_node_get_domain_id(const rcl_node_t * node, size_t * domain_id)
 {
-  const rcl_node_options_t * node_options = rcl_node_get_options(node);
-  if (!node_options) {
-    return RCL_RET_NODE_INVALID;  // error already set
+  if (!rcl_node_is_valid(node)) {
+    return RCL_RET_NODE_INVALID;
   }
   RCL_CHECK_ARGUMENT_FOR_NULL(domain_id, RCL_RET_INVALID_ARGUMENT);
-  *domain_id = node->impl->actual_domain_id;
+  rcl_ret_t ret = rcl_context_get_domain_id(node->context, domain_id);
+  if (RCL_RET_OK != ret) {
+    return ret;
+  }
   return RCL_RET_OK;
 }
 

diff --git a/rcl/test/rcl/test_context.cpp b/rcl/test/rcl/test_context.cpp
@@ -87,6 +87,28 @@ TEST_F(CLASSNAME(TestContextFixture, RMW_IMPLEMENTATION), nominal) {
   EXPECT_NE(instance_id, 0UL) << rcl_get_error_string().str;
   rcl_reset_error();
 
+  // test rcl_context_get_domain_id
+  size_t domain_id;
+
+  EXPECT_NO_MEMORY_OPERATIONS(
+  {
+    EXPECT_EQ(RCL_RET_INVALID_ARGUMENT, rcl_context_get_domain_id(&context, nullptr));
+  });
+  EXPECT_TRUE(rcl_error_is_set());
+  rcl_reset_error();
+
+  EXPECT_NO_MEMORY_OPERATIONS(
+  {
+    EXPECT_EQ(RCL_RET_INVALID_ARGUMENT, rcl_context_get_domain_id(nullptr, &domain_id));
+  });
+  EXPECT_TRUE(rcl_error_is_set());
+  rcl_reset_error();
+
+  EXPECT_NO_MEMORY_OPERATIONS(
+  {
+    EXPECT_EQ(RCL_RET_OK, rcl_context_get_domain_id(&context, &domain_id));
+  });
+
   // test rcl_context_is_valid
   bool is_valid;
   EXPECT_NO_MEMORY_OPERATIONS(

diff --git a/rcl/test/rcl/test_node.cpp b/rcl/test/rcl/test_node.cpp
@@ -254,14 +254,22 @@ TEST_F(CLASSNAME(TestNodeFixture, RMW_IMPLEMENTATION), test_rcl_node_accessors)
   ASSERT_TRUE(rcl_error_is_set());
   rcl_reset_error();
   ret = rcl_node_get_domain_id(&invalid_node, &actual_domain_id);
-  EXPECT_EQ(RCL_RET_OK, ret);
+  EXPECT_EQ(RCL_RET_NODE_INVALID, ret);
   rcl_reset_error();
   EXPECT_NO_MEMORY_OPERATIONS(
   {
     ret = rcl_node_get_domain_id(&node, &actual_domain_id);
   });
   EXPECT_EQ(RCL_RET_OK, ret);
   EXPECT_EQ(42u, actual_domain_id);
+  actual_domain_id = 0u;
+  EXPECT_NO_MEMORY_OPERATIONS(
+  {
+    ret = rcl_context_get_domain_id(&context, &actual_domain_id);
+  });
+  EXPECT_EQ(RCL_RET_OK, ret);
+  EXPECT_EQ(42u, actual_domain_id);
+
   // Test rcl_node_get_rmw_handle().
   rmw_node_t * node_handle;
   node_handle = rcl_node_get_rmw_handle(nullptr);