fix(cie_thread_configurator): make spawn_non_ros2_thread node name unique using thread_name (#1243)

* fix(cie_thread_configurator): make spawn_non_ros2_thread node name unique using thread_name

Signed-off-by: atsushi421 <atsushi.yano.2@tier4.jp>

* fix: address review findings

- Add doc comment to sanitize_node_name explaining its behavior
- Add unit tests for sanitize_node_name (5 cases)

Signed-off-by: atsushi421 <atsushi.yano.2@tier4.jp>

* fix(cie_thread_configurator): address review findings

- Replace locale-dependent std::isalnum with explicit ASCII range checks
  in sanitize_node_name() to avoid non-ASCII bytes passing through.
- Add missing ament_cmake_gtest test_depend to package.xml.

Signed-off-by: atsushi421 <atsushi.yano.2@tier4.jp>

---------

Signed-off-by: atsushi421 <atsushi.yano.2@tier4.jp>

Author: atsushi421

src/agnocast_cie_thread_configurator/CMakeLists.txt

@@ -45,4 +45,10 @@ install(TARGETS thread_configurator_node DESTINATION lib/${PROJECT_NAME})
 ament_export_targets(export_${PROJECT_NAME} HAS_LIBRARY_TARGET)
 ament_export_include_directories(include)
 
+if(BUILD_TESTING)
+  find_package(ament_cmake_gtest REQUIRED)
+  ament_add_gtest(test_${PROJECT_NAME} test/test_sanitize_node_name.cpp)
+  target_link_libraries(test_${PROJECT_NAME} thread_configurator)
+endif()
+
 ament_package()

src/agnocast_cie_thread_configurator/include/agnocast_cie_thread_configurator/cie_thread_configurator.hpp

@@ -26,6 +26,20 @@ size_t get_default_domain_id();
 // Create a node for a different domain
 rclcpp::Node::SharedPtr create_node_for_domain(size_t domain_id);
 
+// Replace characters that are not alphanumeric or '_' with '_',
+// producing a valid ROS 2 node name token.
+inline std::string sanitize_node_name(const std::string & name)
+{
+  std::string result;
+  result.reserve(name.size());
+  for (char c : name) {
+    const bool is_alnum =
+      (c >= 'a' && c <= 'z') || (c >= 'A' && c <= 'Z') || (c >= '0' && c <= '9');
+    result += (is_alnum || c == '_') ? c : '_';
+  }
+  return result;
+}
+
 /// Spawn a thread whose scheduling policy can be managed through
 /// cie_thread_configurator.
 /// Caution: the `thread_name` must be unique among threads managed by
@@ -45,7 +59,8 @@ std::thread spawn_non_ros2_thread(const char * thread_name, F && f, Args &&... a
     rclcpp::NodeOptions options;
     options.context(context);
     auto node = std::make_shared<rclcpp::Node>(
-      "cie_thread_client", "/agnocast_cie_thread_configurator", options);
+      "cie_thread_client_" + sanitize_node_name(thread_name), "/agnocast_cie_thread_configurator",
+      options);
 
     auto publisher = node->create_publisher<agnocast_cie_config_msgs::msg::NonRosThreadInfo>(
       "/agnocast_cie_thread_configurator/non_ros_thread_info", rclcpp::QoS(5000).reliable());

src/agnocast_cie_thread_configurator/package.xml

@@ -16,6 +16,7 @@
 
   <buildtool_depend>ament_cmake</buildtool_depend>
 
+  <test_depend>ament_cmake_gtest</test_depend>
   <test_depend>ament_lint_auto</test_depend>
   <test_depend>ament_lint_common</test_depend>
 

src/agnocast_cie_thread_configurator/test/test_sanitize_node_name.cpp

@@ -0,0 +1,30 @@
+#include "agnocast_cie_thread_configurator/cie_thread_configurator.hpp"
+
+#include <gtest/gtest.h>
+
+using agnocast_cie_thread_configurator::sanitize_node_name;
+
+TEST(SanitizeNodeName, AlphanumericPassthrough)
+{
+  EXPECT_EQ(sanitize_node_name("abc123"), "abc123");
+}
+
+TEST(SanitizeNodeName, UnderscorePreserved)
+{
+  EXPECT_EQ(sanitize_node_name("my_thread"), "my_thread");
+}
+
+TEST(SanitizeNodeName, SpecialCharsReplaced)
+{
+  EXPECT_EQ(sanitize_node_name("my-thread.1"), "my_thread_1");
+}
+
+TEST(SanitizeNodeName, AllSpecialChars)
+{
+  EXPECT_EQ(sanitize_node_name("---"), "___");
+}
+
+TEST(SanitizeNodeName, EmptyString)
+{
+  EXPECT_EQ(sanitize_node_name(""), "");
+}