From 50d500e84e3967b0e5b6c367395e7156b0f358f9 Mon Sep 17 00:00:00 2001 From: Karsten Knese Date: Wed, 15 Apr 2020 19:08:04 -0700 Subject: [PATCH] Make Component Manager public (#1065) * make functions public & virtual Signed-off-by: Karsten Knese * flexible resource index for cmake macros Signed-off-by: Karsten Knese * review comments Signed-off-by: Karsten Knese * remove superfluous include Signed-off-by: Karsten Knese * remove wrong dllexort Signed-off-by: Karsten Knese * check for empty plugin & executable args Signed-off-by: Karsten Knese * remove commented lines Signed-off-by: Karsten Knese * fix typo Signed-off-by: Karsten Knese * relax macro constraints Signed-off-by: Karsten Knese --- rclcpp_components/CMakeLists.txt | 6 +- .../rclcpp_components_package_hook.cmake | 7 ++- .../rclcpp_components_register_node.cmake | 25 +++++++-- .../rclcpp_components_register_nodes.cmake | 16 +++++- .../rclcpp_components}/component_manager.hpp | 51 +++++++++++------ .../rclcpp_components/visibility_control.hpp | 56 +++++++++++++++++++ rclcpp_components/src/component_container.cpp | 2 +- .../src/component_container_mt.cpp | 2 +- rclcpp_components/src/component_manager.cpp | 17 ++++-- .../test/test_component_manager.cpp | 2 +- .../test/test_component_manager_api.cpp | 2 +- 11 files changed, 146 insertions(+), 40 deletions(-) rename rclcpp_components/{src => include/rclcpp_components}/component_manager.hpp (73%) create mode 100644 rclcpp_components/include/rclcpp_components/visibility_control.hpp diff --git a/rclcpp_components/CMakeLists.txt b/rclcpp_components/CMakeLists.txt index e7eacd6..d2a0de7 100644 --- a/rclcpp_components/CMakeLists.txt +++ b/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() diff --git a/rclcpp_components/cmake/rclcpp_components_package_hook.cmake b/rclcpp_components/cmake/rclcpp_components_package_hook.cmake index 4833d02..268bc5b 100644 --- a/rclcpp_components/cmake/rclcpp_components_package_hook.cmake +++ b/rclcpp_components/cmake/rclcpp_components_package_hook.cmake @@ -13,6 +13,9 @@ # limitations under the License. # register node plugins -ament_index_register_resource( - "rclcpp_components" CONTENT "${_RCLCPP_COMPONENTS__NODES}") +list(REMOVE_DUPLICATES _RCLCPP_COMPONENTS_PACKAGE_RESOURCE_INDICES) +foreach(resource_index ${_RCLCPP_COMPONENTS_PACKAGE_RESOURCE_INDICES}) + ament_index_register_resource( + ${resource_index} CONTENT "${_RCLCPP_COMPONENTS_${resource_index}__NODES}") +endforeach() diff --git a/rclcpp_components/cmake/rclcpp_components_register_node.cmake b/rclcpp_components/cmake/rclcpp_components_register_node.cmake index e7fa3e2..1a9d0cb 100644 --- a/rclcpp_components/cmake/rclcpp_components_register_node.cmake +++ b/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 - "${_RCLCPP_COMPONENTS__NODES}${component};${_path}/$\n") + set(_RCLCPP_COMPONENTS_${resource_index}__NODES + "${_RCLCPP_COMPONENTS_${resource_index}__NODES}${component};${_path}/$\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 diff --git a/rclcpp_components/cmake/rclcpp_components_register_nodes.cmake b/rclcpp_components/cmake/rclcpp_components_register_nodes.cmake index eac3841..e80550a 100644 --- a/rclcpp_components/cmake/rclcpp_components_register_nodes.cmake +++ b/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 - "${_RCLCPP_COMPONENTS__NODES}${_arg};${_path}/$\n") + set(_RCLCPP_COMPONENTS_${resource_index}__NODES + "${_RCLCPP_COMPONENTS_${resource_index}__NODES}${_arg};${_path}/$\n") + list(APPEND _RCLCPP_COMPONENTS_PACKAGE_RESOURCE_INDICES ${resource_index}) endforeach() endif() endmacro() diff --git a/rclcpp_components/src/component_manager.hpp b/rclcpp_components/include/rclcpp_components/component_manager.hpp similarity index 73% rename from rclcpp_components/src/component_manager.hpp rename to rclcpp_components/include/rclcpp_components/component_manager.hpp index 62d7e1e..0003b07 100644 --- a/rclcpp_components/src/component_manager.hpp +++ b/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__ -#define COMPONENT_MANAGER_HPP__ +#ifndef RCLCPP_COMPONENTS__COMPONENT_MANAGER_HPP__ +#define RCLCPP_COMPONENTS__COMPONENT_MANAGER_HPP__ #include #include @@ -21,17 +21,21 @@ #include #include -#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; + RCLCPP_COMPONENTS_PUBLIC ComponentManager( - std::weak_ptr executor); + std::weak_ptr 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 - get_component_resources(const std::string & package_name) const; + RCLCPP_COMPONENTS_PUBLIC + virtual std::vector + get_component_resources( + const std::string & package_name, + const std::string & resource_index = "rclcpp_components") const; - std::shared_ptr + RCLCPP_COMPONENTS_PUBLIC + virtual std::shared_ptr create_component_factory(const ComponentResource & resource); -private: - void +protected: + RCLCPP_COMPONENTS_PUBLIC + virtual void OnLoadNode( const std::shared_ptr request_header, const std::shared_ptr request, std::shared_ptr response); - void + RCLCPP_COMPONENTS_PUBLIC + virtual void OnUnloadNode( const std::shared_ptr request_header, const std::shared_ptr request, std::shared_ptr response); - void + RCLCPP_COMPONENTS_PUBLIC + virtual void OnListNodes( const std::shared_ptr request_header, const std::shared_ptr request, @@ -90,7 +105,7 @@ private: private: std::weak_ptr executor_; - uint64_t unique_id {1}; + uint64_t unique_id_ {1}; std::map> loaders_; std::map node_wrappers_; @@ -101,4 +116,4 @@ private: } // namespace rclcpp_components -#endif // COMPONENT_MANAGER_HPP__ +#endif // RCLCPP_COMPONENTS__COMPONENT_MANAGER_HPP__ diff --git a/rclcpp_components/include/rclcpp_components/visibility_control.hpp b/rclcpp_components/include/rclcpp_components/visibility_control.hpp new file mode 100644 index 0000000..fff5cdd --- /dev/null +++ b/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_ diff --git a/rclcpp_components/src/component_container.cpp b/rclcpp_components/src/component_container.cpp index e1f3eaf..6408391 100644 --- a/rclcpp_components/src/component_container.cpp +++ b/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[]) { diff --git a/rclcpp_components/src/component_container_mt.cpp b/rclcpp_components/src/component_container_mt.cpp index af8e3a0..aa4784e 100644 --- a/rclcpp_components/src/component_container_mt.cpp +++ b/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[]) { diff --git a/rclcpp_components/src/component_manager.cpp b/rclcpp_components/src/component_manager.cpp index 05bfa9d..720d713 100644 --- a/rclcpp_components/src/component_manager.cpp +++ b/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 #include #include +#include #include #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 executor) -: Node("ComponentManager"), + std::weak_ptr executor, + std::string node_name, + const rclcpp::NodeOptions & node_options) +: Node(std::move(node_name), node_options), executor_(executor) { loadNode_srv_ = create_service( @@ -57,13 +61,14 @@ ComponentManager::~ComponentManager() } std::vector -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. diff --git a/rclcpp_components/test/test_component_manager.cpp b/rclcpp_components/test/test_component_manager.cpp index a86bbe5..a19fdab 100644 --- a/rclcpp_components/test/test_component_manager.cpp +++ b/rclcpp_components/test/test_component_manager.cpp @@ -16,7 +16,7 @@ #include -#include "component_manager.hpp" +#include "rclcpp_components/component_manager.hpp" #include "rcpputils/filesystem_helper.hpp" diff --git a/rclcpp_components/test/test_component_manager_api.cpp b/rclcpp_components/test/test_component_manager_api.cpp index 5baac26..1d39b04 100644 --- a/rclcpp_components/test/test_component_manager_api.cpp +++ b/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;