Make Component Manager public (#1065)

* make functions public & virtual

Signed-off-by: Karsten Knese <karsten@openrobotics.org>

* flexible resource index for cmake macros

Signed-off-by: Karsten Knese <karsten@openrobotics.org>

* review comments

Signed-off-by: Karsten Knese <karsten@openrobotics.org>

* remove superfluous include

Signed-off-by: Karsten Knese <karsten@openrobotics.org>

* remove wrong dllexort

Signed-off-by: Karsten Knese <karsten@openrobotics.org>

* check for empty plugin & executable args

Signed-off-by: Karsten Knese <karsten@openrobotics.org>

* remove commented lines

Signed-off-by: Karsten Knese <karsten@openrobotics.org>

* fix typo

Signed-off-by: Karsten Knese <karsten@openrobotics.org>

* relax macro constraints

Signed-off-by: Karsten Knese <karsten@openrobotics.org>

rclcpp_components/CMakeLists.txt

@@ -21,7 +21,7 @@ include_directories(include)
 
 add_library(
   component_manager
-  STATIC
+  SHARED
   src/component_manager.cpp
 )
 ament_target_dependencies(component_manager
@@ -31,6 +31,8 @@ ament_target_dependencies(component_manager
   "rclcpp"
   "rcpputils"
 )
+target_compile_definitions(component_manager
+  PRIVATE "RCLCPP_COMPONENTS_BUILDING_LIBRARY")
 
 add_executable(
   component_container
@@ -89,7 +91,6 @@ if(BUILD_TESTING)
     APPEND_LIBRARY_DIRS "${append_library_dirs}")
   if(TARGET test_component_manager)
     target_link_libraries(test_component_manager component_manager)
-    target_include_directories(test_component_manager PRIVATE src)
   endif()
 
   ament_add_gtest(test_component_manager_api test/test_component_manager_api.cpp
@@ -97,7 +98,6 @@ if(BUILD_TESTING)
     APPEND_LIBRARY_DIRS "${append_library_dirs}")
   if(TARGET test_component_manager_api)
     target_link_libraries(test_component_manager_api component_manager)
-    target_include_directories(test_component_manager_api PRIVATE src)
   endif()
 endif()
 

rclcpp_components/cmake/rclcpp_components_package_hook.cmake

@@ -13,6 +13,9 @@
 # limitations under the License.
 
 # register node plugins
-ament_index_register_resource(
+list(REMOVE_DUPLICATES _RCLCPP_COMPONENTS_PACKAGE_RESOURCE_INDICES)
-  "rclcpp_components" CONTENT "${_RCLCPP_COMPONENTS__NODES}")
+foreach(resource_index ${_RCLCPP_COMPONENTS_PACKAGE_RESOURCE_INDICES})
+  ament_index_register_resource(
+    ${resource_index} CONTENT "${_RCLCPP_COMPONENTS_${resource_index}__NODES}")
+endforeach()
 

rclcpp_components/cmake/rclcpp_components_register_node.cmake

@@ -22,15 +22,30 @@
 # :type target: string
 # :param PLUGIN: the plugin name
 # :type PLUGIN: string
-# :type EXECUTABLE: the node's executable name
+# :param EXECUTABLE: the node's executable name
 # :type EXECUTABLE: string
+# :param RESOURCE_INDEX: the ament resource index to register the components
+# :type RESOURCE_INDEX: string
 #
 macro(rclcpp_components_register_node target)
-  cmake_parse_arguments(ARGS "" "PLUGIN;EXECUTABLE" "" ${ARGN})
+  cmake_parse_arguments(ARGS "" "PLUGIN;EXECUTABLE;RESOURCE_INDEX" "" ${ARGN})
   if(ARGS_UNPARSED_ARGUMENTS)
     message(FATAL_ERROR "rclcpp_components_register_node() called with unused "
       "arguments: ${ARGS_UNPARSED_ARGUMENTS}")
   endif()
+  if("${ARGS_PLUGIN}" STREQUAL "")
+    message(FATAL_ERROR "rclcpp_components_register_node macro requires a PLUGIN argument for target ${target}")
+  endif()
+  if("${ARGS_EXECUTABLE}" STREQUAL "")
+    message(FATAL_ERROR "rclcpp_components_register_node macro requires a EXECUTABLE argument for target ${target}")
+  endif()
+  # default to rclcpp_components if not specified otherwise
+  set(resource_index "rclcpp_components")
+  if(NOT "${ARGS_RESOURCE_INDEX}" STREQUAL "")
+    set(resource_index ${ARGS_RESOURCE_INDEX})
+    message(STATUS "Setting component resource index to non-default value ${resource_index}")
+  endif()
+
   set(component ${ARGS_PLUGIN})
   set(node ${ARGS_EXECUTABLE})
   _rclcpp_components_register_package_hook()
@@ -39,8 +54,10 @@ macro(rclcpp_components_register_node target)
   if(WIN32)
     set(_path "bin")
   endif()
-  set(_RCLCPP_COMPONENTS__NODES
+  set(_RCLCPP_COMPONENTS_${resource_index}__NODES
-    "${_RCLCPP_COMPONENTS__NODES}${component};${_path}/$<TARGET_FILE_NAME:${target}>\n")
+    "${_RCLCPP_COMPONENTS_${resource_index}__NODES}${component};${_path}/$<TARGET_FILE_NAME:${target}>\n")
+  list(APPEND _RCLCPP_COMPONENTS_PACKAGE_RESOURCE_INDICES ${resource_index})
+
   configure_file(${rclcpp_components_NODE_TEMPLATE}
     ${PROJECT_BINARY_DIR}/rclcpp_components/node_main_configured_${node}.cpp.in)
   file(GENERATE OUTPUT ${PROJECT_BINARY_DIR}/rclcpp_components/node_main_${node}.cpp

rclcpp_components/cmake/rclcpp_components_register_nodes.cmake

@@ -21,6 +21,8 @@
 # :type target: string
 # :param ARGN: the unique plugin names being exported using class_loader
 # :type ARGN: list of strings
+# :param RESOURCE_INDEX: the ament resource index to register the components
+# :type RESOURCE_INDEX: string
 #
 macro(rclcpp_components_register_nodes target)
   if(NOT TARGET ${target})
@@ -29,6 +31,13 @@ macro(rclcpp_components_register_nodes target)
       "rclcpp_components_register_nodes() first argument "
       "'${target}' is not a target")
   endif()
+  cmake_parse_arguments(ARGS "" "RESOURCE_INDEX" "" ${ARGN})
+  # default to rclcpp_components if not specified otherwise
+  set(resource_index "rclcpp_components")
+  if(NOT "${ARGS_RESOURCE_INDEX}" STREQUAL "")
+    set(resource_index ${ARGS_RESOURCE_INDEX})
+    message(STATUS "Setting component resource index to non-default value ${resource_index}")
+  endif()
   get_target_property(_target_type ${target} TYPE)
   if(NOT _target_type STREQUAL "SHARED_LIBRARY")
     message(
@@ -40,7 +49,7 @@ macro(rclcpp_components_register_nodes target)
   if(${ARGC} GREATER 0)
     _rclcpp_components_register_package_hook()
     set(_unique_names)
-    foreach(_arg ${ARGN})
+    foreach(_arg ${ARGS_UNPARSED_ARGUMENTS})
       if(_arg IN_LIST _unique_names)
         message(
           FATAL_ERROR
@@ -54,8 +63,9 @@ macro(rclcpp_components_register_nodes target)
       else()
         set(_path "lib")
       endif()
-      set(_RCLCPP_COMPONENTS__NODES
+      set(_RCLCPP_COMPONENTS_${resource_index}__NODES
-        "${_RCLCPP_COMPONENTS__NODES}${_arg};${_path}/$<TARGET_FILE_NAME:${target}>\n")
+        "${_RCLCPP_COMPONENTS_${resource_index}__NODES}${_arg};${_path}/$<TARGET_FILE_NAME:${target}>\n")
+      list(APPEND _RCLCPP_COMPONENTS_PACKAGE_RESOURCE_INDICES ${resource_index})
     endforeach()
   endif()
 endmacro()

rclcpp_components/include/rclcpp_components/component_manager.hpp

@@ -12,8 +12,8 @@
 // See the License for the specific language governing permissions and
 // limitations under the License.
 
-#ifndef COMPONENT_MANAGER_HPP__
+#ifndef RCLCPP_COMPONENTS__COMPONENT_MANAGER_HPP__
-#define COMPONENT_MANAGER_HPP__
+#define RCLCPP_COMPONENTS__COMPONENT_MANAGER_HPP__
 
 #include <map>
 #include <memory>
@@ -21,17 +21,21 @@
 #include <utility>
 #include <vector>
 
-#include "class_loader/class_loader.hpp"
+#include "composition_interfaces/srv/load_node.hpp"
+#include "composition_interfaces/srv/unload_node.hpp"
+#include "composition_interfaces/srv/list_nodes.hpp"
 
 #include "rclcpp/executor.hpp"
 #include "rclcpp/node_options.hpp"
 #include "rclcpp/rclcpp.hpp"
 
-#include "composition_interfaces/srv/load_node.hpp"
-#include "composition_interfaces/srv/unload_node.hpp"
-#include "composition_interfaces/srv/list_nodes.hpp"
-
 #include "rclcpp_components/node_factory.hpp"
+#include "rclcpp_components/visibility_control.hpp"
+
+namespace class_loader
+{
+class ClassLoader;
+}  // namespace class_loader
 
 namespace rclcpp_components
 {
@@ -56,32 +60,43 @@ public:
    */
   using ComponentResource = std::pair<std::string, std::string>;
 
+  RCLCPP_COMPONENTS_PUBLIC
   ComponentManager(
-    std::weak_ptr<rclcpp::executor::Executor> executor);
+    std::weak_ptr<rclcpp::executor::Executor> executor,
+    std::string node_name = "ComponentManager",
+    const rclcpp::NodeOptions & node_options = rclcpp::NodeOptions());
 
-  ~ComponentManager();
+  RCLCPP_COMPONENTS_PUBLIC
+  virtual ~ComponentManager();
 
   /// Return a list of valid loadable components in a given package.
-  std::vector<ComponentResource>
+  RCLCPP_COMPONENTS_PUBLIC
-  get_component_resources(const std::string & package_name) const;
+  virtual std::vector<ComponentResource>
+  get_component_resources(
+    const std::string & package_name,
+    const std::string & resource_index = "rclcpp_components") const;
 
-  std::shared_ptr<rclcpp_components::NodeFactory>
+  RCLCPP_COMPONENTS_PUBLIC
+  virtual std::shared_ptr<rclcpp_components::NodeFactory>
   create_component_factory(const ComponentResource & resource);
 
-private:
+protected:
-  void
+  RCLCPP_COMPONENTS_PUBLIC
+  virtual void
   OnLoadNode(
     const std::shared_ptr<rmw_request_id_t> request_header,
     const std::shared_ptr<LoadNode::Request> request,
     std::shared_ptr<LoadNode::Response> response);
 
-  void
+  RCLCPP_COMPONENTS_PUBLIC
+  virtual void
   OnUnloadNode(
     const std::shared_ptr<rmw_request_id_t> request_header,
     const std::shared_ptr<UnloadNode::Request> request,
     std::shared_ptr<UnloadNode::Response> response);
 
-  void
+  RCLCPP_COMPONENTS_PUBLIC
+  virtual void
   OnListNodes(
     const std::shared_ptr<rmw_request_id_t> request_header,
     const std::shared_ptr<ListNodes::Request> request,
@@ -90,7 +105,7 @@ private:
 private:
   std::weak_ptr<rclcpp::executor::Executor> executor_;
 
-  uint64_t unique_id {1};
+  uint64_t unique_id_ {1};
   std::map<std::string, std::unique_ptr<class_loader::ClassLoader>> loaders_;
   std::map<uint64_t, rclcpp_components::NodeInstanceWrapper> node_wrappers_;
 
@@ -101,4 +116,4 @@ private:
 
 }  // namespace rclcpp_components
 
-#endif  // COMPONENT_MANAGER_HPP__
+#endif  // RCLCPP_COMPONENTS__COMPONENT_MANAGER_HPP__

rclcpp_components/include/rclcpp_components/visibility_control.hpp

@@ -0,0 +1,56 @@
+// 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.
+
+/* This header must be included by all rclcpp headers which declare symbols
+ * which are defined in the rclcpp library. When not building the rclcpp
+ * library, i.e. when using the headers in other package's code, the contents
+ * of this header change the visibility of certain symbols which the rclcpp
+ * library cannot have, but the consuming code must have inorder to link.
+ */
+
+#ifndef RCLCPP_COMPONENTS__VISIBILITY_CONTROL_HPP_
+#define RCLCPP_COMPONENTS__VISIBILITY_CONTROL_HPP_
+
+// This logic was borrowed (then namespaced) from the examples on the gcc wiki:
+//     https://gcc.gnu.org/wiki/Visibility
+
+#if defined _WIN32 || defined __CYGWIN__
+  #ifdef __GNUC__
+    #define RCLCPP_COMPONENTS_EXPORT __attribute__ ((dllexport))
+    #define RCLCPP_COMPONENTS_IMPORT __attribute__ ((dllimport))
+  #else
+    #define RCLCPP_COMPONENTS_EXPORT __declspec(dllexport)
+    #define RCLCPP_COMPONENTS_IMPORT __declspec(dllimport)
+  #endif
+  #ifdef RCLCPP_COMPONENTS_BUILDING_LIBRARY
+    #define RCLCPP_COMPONENTS_PUBLIC RCLCPP_COMPONENTS_EXPORT
+  #else
+    #define RCLCPP_COMPONENTS_PUBLIC RCLCPP_COMPONENTS_IMPORT
+  #endif
+  #define RCLCPP_COMPONENTS_PUBLIC_TYPE RCLCPP_COMPONENTS_PUBLIC
+  #define RCLCPP_COMPONENTS_LOCAL
+#else
+  #define RCLCPP_COMPONENTS_EXPORT __attribute__ ((visibility("default")))
+  #define RCLCPP_COMPONENTS_IMPORT
+  #if __GNUC__ >= 4
+    #define RCLCPP_COMPONENTS_PUBLIC __attribute__ ((visibility("default")))
+    #define RCLCPP_COMPONENTS_LOCAL  __attribute__ ((visibility("hidden")))
+  #else
+    #define RCLCPP_COMPONENTS_PUBLIC
+    #define RCLCPP_COMPONENTS_LOCAL
+  #endif
+  #define RCLCPP_COMPONENTS_PUBLIC_TYPE
+#endif
+
+#endif  // RCLCPP_COMPONENTS__VISIBILITY_CONTROL_HPP_

rclcpp_components/src/component_container.cpp

@@ -16,7 +16,7 @@
 
 #include "rclcpp/rclcpp.hpp"
 
-#include "component_manager.hpp"
+#include "rclcpp_components/component_manager.hpp"
 
 int main(int argc, char * argv[])
 {

rclcpp_components/src/component_container_mt.cpp

@@ -16,7 +16,7 @@
 
 #include "rclcpp/rclcpp.hpp"
 
-#include "component_manager.hpp"
+#include "rclcpp_components/component_manager.hpp"
 
 int main(int argc, char * argv[])
 {

rclcpp_components/src/component_manager.cpp

@@ -12,14 +12,16 @@
 // See the License for the specific language governing permissions and
 // limitations under the License.
 
-#include "component_manager.hpp"
+#include "rclcpp_components/component_manager.hpp"
 
 #include <functional>
 #include <memory>
 #include <string>
+#include <utility>
 #include <vector>
 
 #include "ament_index_cpp/get_resource.hpp"
+#include "class_loader/class_loader.hpp"
 #include "rcpputils/filesystem_helper.hpp"
 #include "rcpputils/split.hpp"
 
@@ -29,8 +31,10 @@ namespace rclcpp_components
 {
 
 ComponentManager::ComponentManager(
-  std::weak_ptr<rclcpp::executor::Executor> executor)
+  std::weak_ptr<rclcpp::executor::Executor> executor,
-: Node("ComponentManager"),
+  std::string node_name,
+  const rclcpp::NodeOptions & node_options)
+: Node(std::move(node_name), node_options),
   executor_(executor)
 {
   loadNode_srv_ = create_service<LoadNode>(
@@ -57,13 +61,14 @@ ComponentManager::~ComponentManager()
 }
 
 std::vector<ComponentManager::ComponentResource>
-ComponentManager::get_component_resources(const std::string & package_name) const
+ComponentManager::get_component_resources(
+  const std::string & package_name, const std::string & resource_index) const
 {
   std::string content;
   std::string base_path;
   if (
     !ament_index_cpp::get_resource(
-      "rclcpp_components", package_name, content, &base_path))
+      resource_index, package_name, content, &base_path))
   {
     throw ComponentManagerException("Could not find requested resource in ament index");
   }
@@ -176,7 +181,7 @@ ComponentManager::OnLoadNode(
         }
       }
 
-      auto node_id = unique_id++;
+      auto node_id = unique_id_++;
 
       if (0 == node_id) {
         // This puts a technical limit on the number of times you can add a component.

rclcpp_components/test/test_component_manager.cpp

@@ -16,7 +16,7 @@
 
 #include <memory>
 
-#include "component_manager.hpp"
+#include "rclcpp_components/component_manager.hpp"
 
 #include "rcpputils/filesystem_helper.hpp"
 

rclcpp_components/test/test_component_manager_api.cpp

@@ -21,7 +21,7 @@
 #include "composition_interfaces/srv/unload_node.hpp"
 #include "composition_interfaces/srv/list_nodes.hpp"
 
-#include "component_manager.hpp"
+#include "rclcpp_components/component_manager.hpp"
 
 using namespace std::chrono_literals;