Remove domain_id and localhost_only from node_options (#708)

Signed-off-by: Ivan Santiago Paunovic <ivanpauno@ekumenlabs.com>

rcl/include/rcl/context.h

@@ -210,7 +210,7 @@ rcl_context_fini(rcl_context_t * context);
 RCL_PUBLIC
 RCL_WARN_UNUSED
 const rcl_init_options_t *
-rcl_context_get_init_options(rcl_context_t * context);
+rcl_context_get_init_options(const rcl_context_t * context);
 
 /// Returns an unsigned integer that is unique to the given context, or `0` if invalid.
 /**

rcl/include/rcl/init_options.h

@@ -147,7 +147,7 @@ rcl_init_options_fini(rcl_init_options_t * init_options);
 RCL_PUBLIC
 RCL_WARN_UNUSED
 rcl_ret_t
-rcl_init_options_get_domain_id(rcl_init_options_t * init_options, size_t * domain_id);
+rcl_init_options_get_domain_id(const rcl_init_options_t * init_options, size_t * domain_id);
 
 /// Set a domain id in the init options provided.
 /**

rcl/include/rcl/node_options.h

@@ -38,19 +38,6 @@ typedef struct rcl_node_options_t
   /// If true, no parameter infrastructure will be setup.
   // bool no_parameters;
 
-  /// If set, then this value overrides the ROS_DOMAIN_ID environment variable.
-  /**
-   * It defaults to RCL_NODE_OPTIONS_DEFAULT_DOMAIN_ID, which will cause the
-   * node to use the ROS domain ID set in the ROS_DOMAIN_ID environment
-   * variable, or on some systems 0 if the environment variable is not set.
-   *
-   * \todo TODO(wjwwood):
-   *   Should we put a limit on the ROS_DOMAIN_ID value, that way we can have
-   *   a safe value for the default RCL_NODE_OPTIONS_DEFAULT_DOMAIN_ID?
-   *   (currently max size_t)
-   */
-  size_t domain_id;
-
   /// Custom allocator used for internal allocations.
   rcl_allocator_t allocator;
 

rcl/src/rcl/context.c

@@ -63,7 +63,7 @@ rcl_context_fini(rcl_context_t * context)
 // See `rcl_shutdown()` for invalidation of the context.
 
 const rcl_init_options_t *
-rcl_context_get_init_options(rcl_context_t * context)
+rcl_context_get_init_options(const rcl_context_t * context)
 {
   RCL_CHECK_ARGUMENT_FOR_NULL(context, NULL);
   RCL_CHECK_FOR_NULL_WITH_MSG(context->impl, "context is zero-initialized", return NULL);

rcl/src/rcl/init_options.c

@@ -132,7 +132,7 @@ rcl_init_options_fini(rcl_init_options_t * init_options)
 }
 
 rcl_ret_t
-rcl_init_options_get_domain_id(rcl_init_options_t * init_options, size_t * domain_id)
+rcl_init_options_get_domain_id(const rcl_init_options_t * init_options, size_t * domain_id)
 {
   RCL_CHECK_ARGUMENT_FOR_NULL(init_options, RCL_RET_INVALID_ARGUMENT);
   RCL_CHECK_ARGUMENT_FOR_NULL(init_options->impl, RCL_RET_INVALID_ARGUMENT);

rcl/src/rcl/node.c

@@ -26,7 +26,7 @@ extern "C"
 
 #include "rcl/arguments.h"
 #include "rcl/error_handling.h"
-#include "rcl/domain_id.h"
+#include "rcl/init_options.h"
 #include "rcl/localhost.h"
 #include "rcl/logging.h"
 #include "rcl/logging_rosout.h"
@@ -121,7 +121,6 @@ rcl_node_init(
   const rcl_node_options_t * options)
 {
   size_t domain_id = 0;
-  rmw_localhost_only_t localhost_only = RMW_LOCALHOST_ONLY_DEFAULT;
   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();
@@ -254,21 +253,16 @@ rcl_node_init(
   RCL_CHECK_FOR_NULL_WITH_MSG(
     node->impl->logger_name, "creating logger name failed", goto fail);
 
-  domain_id = node->impl->options.domain_id;
-  if (RCL_DEFAULT_DOMAIN_ID == domain_id) {
-    if (RCL_RET_OK != rcl_get_default_domain_id(&domain_id)) {
-      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;
 
-  localhost_only = context->impl->init_options.impl->rmw_init_options.localhost_only;
-
   node->impl->rmw_node_handle = rmw_create_node(
     &(node->context->impl->rmw_context),
-    name, local_namespace_, domain_id,
-    localhost_only == RMW_LOCALHOST_ONLY_ENABLED);
+    name, local_namespace_);
 
   RCL_CHECK_FOR_NULL_WITH_MSG(
     node->impl->rmw_node_handle, rmw_get_error_string().str, goto fail);

rcl/src/rcl/node_options.c

@@ -29,7 +29,6 @@ rcl_node_get_default_options()
 {
   // !!! MAKE SURE THAT CHANGES TO THESE DEFAULTS ARE REFLECTED IN THE HEADER DOC STRING
   static rcl_node_options_t default_options = {
-    .domain_id = RCL_NODE_OPTIONS_DEFAULT_DOMAIN_ID,
     .use_global_arguments = true,
     .enable_rosout = true,
   };
@@ -54,7 +53,6 @@ rcl_node_options_copy(
     RCL_SET_ERROR_MSG("Options out must be zero initialized");
     return RCL_RET_INVALID_ARGUMENT;
   }
-  options_out->domain_id = options->domain_id;
   options_out->allocator = options->allocator;
   options_out->use_global_arguments = options->use_global_arguments;
   options_out->enable_rosout = options->enable_rosout;

rcl/test/rcl/test_node.cpp

@@ -94,6 +94,7 @@ TEST_F(CLASSNAME(TestNodeFixture, RMW_IMPLEMENTATION), test_rcl_node_accessors)
   {
     EXPECT_EQ(RCL_RET_OK, rcl_init_options_fini(&init_options)) << rcl_get_error_string().str;
   });
+  EXPECT_EQ(RCL_RET_OK, rcl_init_options_set_domain_id(&init_options, 42));
   rcl_context_t invalid_context = rcl_get_zero_initialized_context();
   ret = rcl_init(0, nullptr, &init_options, &invalid_context);
   ASSERT_EQ(RCL_RET_OK, ret);  // Shutdown later after invalid node.
@@ -103,19 +104,8 @@ TEST_F(CLASSNAME(TestNodeFixture, RMW_IMPLEMENTATION), test_rcl_node_accessors)
   const char * namespace_ = "/ns";
   const char * fq_name = "/ns/test_rcl_node_accessors_node";
   rcl_node_options_t default_options = rcl_node_get_default_options();
-  default_options.domain_id = 42;  // Set the domain id to something explicit.
   ret = rcl_node_init(&invalid_node, name, namespace_, &invalid_context, &default_options);
-  if (is_windows && is_opensplice) {
-    // On Windows with OpenSplice, setting the domain id is not expected to work.
-    ASSERT_NE(RCL_RET_OK, ret);
-    // So retry with the default domain id setting (uses the environment as is).
-    default_options.domain_id = rcl_node_get_default_options().domain_id;
-    ret = rcl_node_init(&invalid_node, name, namespace_, &invalid_context, &default_options);
-    ASSERT_EQ(RCL_RET_OK, ret);
-  } else {
-    // This is the normal check (not windows and windows if not opensplice)
-    ASSERT_EQ(RCL_RET_OK, ret);
-  }
+  ASSERT_EQ(RCL_RET_OK, ret) << rcl_get_error_string().str;
   OSRF_TESTING_TOOLS_CPP_SCOPE_EXIT(
   {
     osrf_testing_tools_cpp::memory_tools::disable_monitoring_in_all_threads();
@@ -251,7 +241,6 @@ TEST_F(CLASSNAME(TestNodeFixture, RMW_IMPLEMENTATION), test_rcl_node_accessors)
   EXPECT_NE(nullptr, actual_options);
   if (actual_options) {
     EXPECT_EQ(default_options.allocator.allocate, actual_options->allocator.allocate);
-    EXPECT_EQ(default_options.domain_id, actual_options->domain_id);
   }
   rcl_reset_error();
   EXPECT_NO_MEMORY_OPERATIONS(
@@ -261,7 +250,6 @@ TEST_F(CLASSNAME(TestNodeFixture, RMW_IMPLEMENTATION), test_rcl_node_accessors)
   EXPECT_NE(nullptr, actual_options);
   if (actual_options) {
     EXPECT_EQ(default_options.allocator.allocate, actual_options->allocator.allocate);
-    EXPECT_EQ(default_options.domain_id, actual_options->domain_id);
   }
   // Test rcl_node_get_domain_id().
   size_t actual_domain_id;
@@ -281,10 +269,7 @@ TEST_F(CLASSNAME(TestNodeFixture, RMW_IMPLEMENTATION), test_rcl_node_accessors)
     ret = rcl_node_get_domain_id(&node, &actual_domain_id);
   });
   EXPECT_EQ(RCL_RET_OK, ret);
-  if (RCL_RET_OK == ret && (!is_windows || !is_opensplice)) {
-    // Can only expect the domain id to be 42 if not windows or not opensplice.
-    EXPECT_EQ(42u, actual_domain_id);
-  }
+  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);
@@ -427,19 +412,6 @@ TEST_F(CLASSNAME(TestNodeFixture, RMW_IMPLEMENTATION), test_rcl_node_life_cycle)
   EXPECT_EQ(RCL_RET_OK, ret);
   ret = rcl_node_fini(&node);
   EXPECT_EQ(RCL_RET_OK, ret);
-  // Try with a specific domain id.
-  rcl_node_options_t options_with_custom_domain_id = rcl_node_get_default_options();
-  options_with_custom_domain_id.domain_id = 42;
-  ret = rcl_node_init(&node, name, namespace_, &context, &options_with_custom_domain_id);
-  if (is_windows && is_opensplice) {
-    // A custom domain id is not expected to work on Windows with Opensplice.
-    EXPECT_NE(RCL_RET_OK, ret);
-  } else {
-    // This is the normal check.
-    EXPECT_EQ(RCL_RET_OK, ret);
-    ret = rcl_node_fini(&node);
-    EXPECT_EQ(RCL_RET_OK, ret);
-  }
 }
 
 /* Tests the node name restrictions enforcement.
@@ -754,7 +726,6 @@ TEST_F(CLASSNAME(TestNodeFixture, RMW_IMPLEMENTATION), test_rcl_node_options) {
 
   EXPECT_TRUE(default_options.use_global_arguments);
   EXPECT_TRUE(default_options.enable_rosout);
-  EXPECT_EQ(RCL_NODE_OPTIONS_DEFAULT_DOMAIN_ID, default_options.domain_id);
   EXPECT_TRUE(rcutils_allocator_is_valid(&(default_options.allocator)));
 
   EXPECT_EQ(RCL_RET_INVALID_ARGUMENT, rcl_node_options_copy(nullptr, &default_options));
@@ -767,11 +738,9 @@ TEST_F(CLASSNAME(TestNodeFixture, RMW_IMPLEMENTATION), test_rcl_node_options) {
   EXPECT_EQ(
     RCL_RET_OK,
     rcl_parse_arguments(argc, argv, default_options.allocator, &(default_options.arguments)));
-  default_options.domain_id = 42u;
   default_options.use_global_arguments = false;
   default_options.enable_rosout = false;
   EXPECT_EQ(RCL_RET_OK, rcl_node_options_copy(&default_options, &not_ini_options));
-  EXPECT_EQ(42u, not_ini_options.domain_id);
   EXPECT_FALSE(not_ini_options.use_global_arguments);
   EXPECT_FALSE(not_ini_options.enable_rosout);
   EXPECT_EQ(