[WIP / Re-Opened] Add functions to return formatted Node Name-Namespace strings (#698)

* Add functions to return formatted Node Name-Namespace strings

Signed-off-by: Jacob Hassold <jhassold@dcscorp.com>

* Changed get_node_names to return fully qualified names, removed namespace method
Signed-off-by: Oswin So <oswinso@gmail.com>

* Removed unnecessary capture-by-reference

Signed-off-by: Jacob Hassold <jhassold@dcscorp.com>

* Added first draft of tests

Signed-off-by: Jacob Hassold <jhassold@dcscorp.com>

* Fixed bug creating phantom empty name/namespaces

Signed-off-by: Jacob Hassold <jhassold@dcscorp.com>

* Re-ordered includes

Signed-off-by: Jacob Hassold <jhassold@dcscorp.com>

* Swap checks to see if name is in set

Signed-off-by: Jacob Hassold <jhassold@dcscorp.com>

* Fixed style errors from uncrustify

Signed-off-by: Jacob Hassold <jhassold@dcscorp.com>

* Swapped to unordered_set

Signed-off-by: Jacob Hassold <jhassold@dcscorp.com>

* Re-ordered includes again

Signed-off-by: Jacob Hassold <jhassold@dcscorp.com>

* nitpick: minimize vertical whitespace

see: https://google.github.io/styleguide/cppguide.html#Vertical_Whitespace

Signed-off-by: William Woodall <william@osrfoundation.org>

* Add API documentation for added get_node_names function

Signed-off-by: Jacob Hassold <jhassold@dcscorp.com>

* Revert to last known semi-working point

Signed-off-by: Jacob Hassold <jhassold@dcscorp.com>

* Modified expected test results

A fully-qualified name is "namespace"/"name". If namespace is set to be "/" (as they are in these tests), we would expect a qualified name of "//name"
Signed-off-by: Jacob Hassold <jhassold@dcscorp.com>

* Have get_node_names determine if central slash needed or not

Signed-off-by: Jacob Hassold <jhassold@dcscorp.com>

* Corrected tests to not accept double slashes

Signed-off-by: Jacob Hassold <jhassold@dcscorp.com>

* Undo changes to .gitignore

Signed-off-by: Jacob Hassold <jhassold@dcscorp.com>

* Change qualified string construction to better handle invalid slashes

Signed-off-by: Jacob Hassold <jhassold@dcscorp.com>

* Removed debugging statements

Signed-off-by: Jacob Hassold <jhassold@dcscorp.com>

* Simplified slash-checking logic

Signed-off-by: Jacob Hassold <jhassold@dcscorp.com>

rclcpp/include/rclcpp/node.hpp

@@ -916,6 +916,12 @@ public:
   void
   register_param_change_callback(CallbackT && callback);
 
+
+  /// Get the fully-qualified names of all available nodes.
+  /**
+   * The fully-qualified name includes the local namespace and name of the node.
+   * \return A vector of fully-qualified names of nodes.
+   */
   RCLCPP_PUBLIC
   std::vector<std::string>
   get_node_names() const;

rclcpp/src/rclcpp/node_interfaces/node_graph.cpp

@@ -14,6 +14,7 @@
 
 #include "rclcpp/node_interfaces/node_graph.hpp"
 
+#include <algorithm>
 #include <map>
 #include <string>
 #include <utility>
@@ -136,9 +137,24 @@ NodeGraph::get_node_names() const
   std::vector<std::string> nodes;
   auto names_and_namespaces = get_node_names_and_namespaces();
 
-  for (const auto & it : names_and_namespaces) {
+  std::transform(names_and_namespaces.begin(),
-    nodes.push_back(it.first);
+    names_and_namespaces.end(),
-  }
+    std::back_inserter(nodes),
+    [](std::pair<std::string, std::string> nns) {
+      std::string return_string;
+      if (nns.second.back() == '/') {
+        return_string = nns.second + nns.first;
+      } else {
+        return_string = nns.second + '/' + nns.first;
+      }
+      // Quick check to make sure that we start with a slash
+      // Since fully-qualified strings need to
+      if (return_string.front() != '/') {
+        return_string = "/" + return_string;
+      }
+      return return_string;
+    }
+  );
   return nodes;
 }
 
@@ -173,10 +189,12 @@ NodeGraph::get_node_names_and_namespaces() const
     throw std::runtime_error(error_msg);
   }
 
-  std::vector<std::pair<std::string, std::string>> node_names(node_names_c.size);
+
+  std::vector<std::pair<std::string, std::string>> node_names;
+  node_names.reserve(node_names_c.size);
   for (size_t i = 0; i < node_names_c.size; ++i) {
     if (node_names_c.data[i] && node_namespaces_c.data[i]) {
-      node_names.push_back(std::make_pair(node_names_c.data[i], node_namespaces_c.data[i]));
+      node_names.emplace_back(node_names_c.data[i], node_namespaces_c.data[i]);
     }
   }
 

rclcpp/test/test_node.cpp

@@ -14,9 +14,10 @@
 
 #include <gtest/gtest.h>
 
-#include <string>
 #include <map>
 #include <memory>
+#include <string>
+#include <unordered_set>
 #include <vector>
 
 #include "rclcpp/exceptions.hpp"
@@ -99,6 +100,23 @@ TEST_F(TestNode, get_name_and_namespace) {
     EXPECT_STREQ("/my/ns", node->get_namespace());
     EXPECT_STREQ("/my/ns/my_node", node->get_fully_qualified_name());
   }
+  {
+    auto node1 = std::make_shared<rclcpp::Node>("my_node1", "my/ns");
+    auto node2 = std::make_shared<rclcpp::Node>("my_node2", "my/ns");
+    auto node3 = std::make_shared<rclcpp::Node>("my_node3", "/ns2");
+    auto node4 = std::make_shared<rclcpp::Node>("my_node4", "my/ns3");
+    auto names_and_namespaces = node1->get_node_names();
+    auto name_namespace_set = std::unordered_set<std::string>(names_and_namespaces.begin(),
+        names_and_namespaces.end());
+    std::function<bool(std::string)> Set_Contains = [&](std::string string_key)
+      {
+        return name_namespace_set.find(string_key) != name_namespace_set.end();
+      };
+    EXPECT_TRUE(Set_Contains("/my/ns/my_node1"));
+    EXPECT_TRUE(Set_Contains("/my/ns/my_node2"));
+    EXPECT_TRUE(Set_Contains("/ns2/my_node3"));
+    EXPECT_TRUE(Set_Contains("/my/ns3/my_node4"));
+  }
 }
 
 TEST_F(TestNode, subnode_get_name_and_namespace) {

rclcpp_components/test/test_component_manager_api.cpp

@@ -117,10 +117,10 @@ TEST_F(TestComponentManager, load_components)
       return std::find(node_names.begin(), node_names.end(), name) != node_names.end();
     };
 
-  EXPECT_TRUE(find_in_nodes("test_component_foo"));
+  EXPECT_TRUE(find_in_nodes("/test_component_foo"));
-  EXPECT_TRUE(find_in_nodes("test_component_bar"));
+  EXPECT_TRUE(find_in_nodes("/test_component_bar"));
-  EXPECT_TRUE(find_in_nodes("test_component_baz"));
+  EXPECT_TRUE(find_in_nodes("/test_component_baz"));
-  EXPECT_TRUE(find_in_nodes("test_component_bing"));
+  EXPECT_TRUE(find_in_nodes("/ns/test_component_bing"));
 }
 
 TEST_F(TestComponentManager, load_invalid_components)
@@ -263,7 +263,7 @@ TEST_F(TestComponentManager, unload_component)
   auto find_in_nodes = [node_names](std::string name) {
       return std::find(node_names.begin(), node_names.end(), name) != node_names.end();
     };
-  EXPECT_TRUE(find_in_nodes("test_component_foo"));
+  EXPECT_TRUE(find_in_nodes("/test_component_foo"));
 
   {
     auto client = node->create_client<composition_interfaces::srv::UnloadNode>(