Keep domain id if ROS_DOMAIN_ID is invalid. (#689)

Including domain id tests.

Signed-off-by: Michel Hidalgo <michel@ekumenlabs.com>

rcl/src/rcl/domain_id.c

@@ -14,6 +14,7 @@
 
 #include "rcl/domain_id.h"
 
+#include <errno.h>
 #include <limits.h>
 
 #include "rcutils/get_env.h"
@@ -40,10 +41,15 @@ rcl_get_default_domain_id(size_t * domain_id)
       get_env_error_str);
     return RCL_RET_ERROR;
   }
-  if (ros_domain_id) {
-    unsigned long number = strtoul(ros_domain_id, NULL, 0);  // NOLINT(runtime/int)
-    if (number == ULONG_MAX) {
-      RCL_SET_ERROR_MSG("failed to interpret ROS_DOMAIN_ID as integral number");
+  if (ros_domain_id && strcmp(ros_domain_id, "") != 0) {
+    char * end = NULL;
+    unsigned long number = strtoul(ros_domain_id, &end, 0);  // NOLINT(runtime/int)
+    if (number == 0UL && *end != '\0') {
+      RCL_SET_ERROR_MSG("ROS_DOMAIN_ID is not an integral number");
+      return RCL_RET_ERROR;
+    }
+    if ((number == ULONG_MAX && errno == ERANGE) || number > SIZE_MAX) {
+      RCL_SET_ERROR_MSG("ROS_DOMAIN_ID is out of range");
       return RCL_RET_ERROR;
     }
     *domain_id = (size_t)number;

rcl/test/CMakeLists.txt

@@ -332,6 +332,12 @@ rcl_add_custom_gtest(test_validate_topic_name
   LIBRARIES ${PROJECT_NAME}
 )
 
+rcl_add_custom_gtest(test_domain_id
+  SRCS rcl/test_domain_id.cpp
+  APPEND_LIBRARY_DIRS ${extra_lib_dirs}
+  LIBRARIES ${PROJECT_NAME}
+)
+
 rcl_add_custom_gtest(test_expand_topic_name
   SRCS rcl/test_expand_topic_name.cpp
   APPEND_LIBRARY_DIRS ${extra_lib_dirs}

rcl/test/rcl/test_domain_id.cpp

@@ -0,0 +1,52 @@
+// Copyright 2020 Open Source Robotics Foundation, Inc.
+//
+// Licensed under the Apache License, Version 2.0 (the "License");
+// you may not use this file except in compliance with the License.
+// You may obtain a copy of the License at
+//
+//     http://www.apache.org/licenses/LICENSE-2.0
+//
+// Unless required by applicable law or agreed to in writing, software
+// distributed under the License is distributed on an "AS IS" BASIS,
+// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+// See the License for the specific language governing permissions and
+// limitations under the License.
+
+#include <gtest/gtest.h>
+
+#include "rcl/rcl.h"
+
+#include "rcl/domain_id.h"
+#include "rcl/error_handling.h"
+#include "rcutils/env.h"
+
+TEST(TestGetDomainId, test_nominal) {
+  ASSERT_TRUE(rcutils_set_env("ROS_DOMAIN_ID", "42"));
+  size_t domain_id = RCL_DEFAULT_DOMAIN_ID;
+  EXPECT_EQ(RCL_RET_OK, rcl_get_default_domain_id(&domain_id));
+  EXPECT_EQ(42u, domain_id);
+
+  ASSERT_TRUE(rcutils_set_env("ROS_DOMAIN_ID", ""));
+  domain_id = RCL_DEFAULT_DOMAIN_ID;
+  EXPECT_EQ(RCL_RET_OK, rcl_get_default_domain_id(&domain_id));
+  EXPECT_EQ(RCL_DEFAULT_DOMAIN_ID, domain_id);
+
+  ASSERT_TRUE(rcutils_set_env("ROS_DOMAIN_ID", "0000"));
+  domain_id = RCL_DEFAULT_DOMAIN_ID;
+  EXPECT_EQ(RCL_RET_OK, rcl_get_default_domain_id(&domain_id));
+  EXPECT_EQ(0u, domain_id);
+
+  ASSERT_TRUE(rcutils_set_env("ROS_DOMAIN_ID", "0   not really"));
+  domain_id = RCL_DEFAULT_DOMAIN_ID;
+  EXPECT_EQ(RCL_RET_ERROR, rcl_get_default_domain_id(&domain_id));
+  rcl_reset_error();
+  EXPECT_EQ(RCL_DEFAULT_DOMAIN_ID, domain_id);
+
+  ASSERT_TRUE(rcutils_set_env("ROS_DOMAIN_ID", "998446744073709551615"));
+  domain_id = RCL_DEFAULT_DOMAIN_ID;
+  EXPECT_EQ(RCL_RET_ERROR, rcl_get_default_domain_id(&domain_id));
+  rcl_reset_error();
+  EXPECT_EQ(RCL_DEFAULT_DOMAIN_ID, domain_id);
+
+  EXPECT_EQ(RCL_RET_INVALID_ARGUMENT, rcl_get_default_domain_id(nullptr));
+}