Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

[Iron] Add new node interface TypeDescriptionsInterface to provide GetTypeDescription service (backport #2224) #2236

Merged
merged 5 commits into from
Jul 18, 2023
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
1 change: 1 addition & 0 deletions rclcpp/CMakeLists.txt
Original file line number Diff line number Diff line change
Expand Up @@ -92,6 +92,7 @@ set(${PROJECT_NAME}_SRCS
src/rclcpp/node_interfaces/node_time_source.cpp
src/rclcpp/node_interfaces/node_timers.cpp
src/rclcpp/node_interfaces/node_topics.cpp
src/rclcpp/node_interfaces/node_type_descriptions.cpp
src/rclcpp/node_interfaces/node_waitables.cpp
src/rclcpp/node_options.cpp
src/rclcpp/parameter.cpp
Expand Down
11 changes: 11 additions & 0 deletions rclcpp/include/rclcpp/node.hpp
Original file line number Diff line number Diff line change
Expand Up @@ -56,6 +56,7 @@
#include "rclcpp/node_interfaces/node_time_source_interface.hpp"
#include "rclcpp/node_interfaces/node_timers_interface.hpp"
#include "rclcpp/node_interfaces/node_topics_interface.hpp"
#include "rclcpp/node_interfaces/node_type_descriptions_interface.hpp"
#include "rclcpp/node_interfaces/node_waitables_interface.hpp"
#include "rclcpp/node_options.hpp"
#include "rclcpp/parameter.hpp"
Expand Down Expand Up @@ -1454,6 +1455,11 @@ class Node : public std::enable_shared_from_this<Node>
rclcpp::node_interfaces::NodeTimeSourceInterface::SharedPtr
get_node_time_source_interface();

/// Return the Node's internal NodeTypeDescriptionsInterface implementation.
RCLCPP_PUBLIC
rclcpp::node_interfaces::NodeTypeDescriptionsInterface::SharedPtr
get_node_type_descriptions_interface();
emersonknapp marked this conversation as resolved.
Show resolved Hide resolved

/// Return the sub-namespace, if this is a sub-node, otherwise an empty string.
/**
* The returned sub-namespace is either the accumulated sub-namespaces which
Expand Down Expand Up @@ -1591,6 +1597,11 @@ class Node : public std::enable_shared_from_this<Node>
const rclcpp::NodeOptions node_options_;
const std::string sub_namespace_;
const std::string effective_namespace_;

/// Static map(s) containing extra member variables for Node without changing its ABI.
// See node.cpp for more details.
class BackportMembers;
static BackportMembers backport_members_;
};

} // namespace rclcpp
Expand Down
2 changes: 2 additions & 0 deletions rclcpp/include/rclcpp/node_interfaces/node_interfaces.hpp
Original file line number Diff line number Diff line change
Expand Up @@ -30,6 +30,7 @@
rclcpp::node_interfaces::NodeTimeSourceInterface, \
rclcpp::node_interfaces::NodeTimersInterface, \
rclcpp::node_interfaces::NodeTopicsInterface, \
rclcpp::node_interfaces::NodeTypeDescriptionsInterface, \
rclcpp::node_interfaces::NodeWaitablesInterface


Expand Down Expand Up @@ -118,6 +119,7 @@ class NodeInterfaces
* - rclcpp::node_interfaces::NodeTimeSourceInterface
* - rclcpp::node_interfaces::NodeTimersInterface
* - rclcpp::node_interfaces::NodeTopicsInterface
* - rclcpp::node_interfaces::NodeTypeDescriptionsInterface
* - rclcpp::node_interfaces::NodeWaitablesInterface
*
* Or you use custom interfaces as long as you make a template specialization
Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1,63 @@
// Copyright 2023 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.

#ifndef RCLCPP__NODE_INTERFACES__NODE_TYPE_DESCRIPTIONS_HPP_
#define RCLCPP__NODE_INTERFACES__NODE_TYPE_DESCRIPTIONS_HPP_

#include <memory>

#include "rclcpp/macros.hpp"
#include "rclcpp/node_interfaces/node_base_interface.hpp"
#include "rclcpp/node_interfaces/node_logging_interface.hpp"
#include "rclcpp/node_interfaces/node_parameters_interface.hpp"
#include "rclcpp/node_interfaces/node_services_interface.hpp"
#include "rclcpp/node_interfaces/node_topics_interface.hpp"
#include "rclcpp/node_interfaces/node_type_descriptions_interface.hpp"
#include "rclcpp/visibility_control.hpp"

namespace rclcpp
{
namespace node_interfaces
{

/// Implementation of the NodeTypeDescriptions part of the Node API.
class NodeTypeDescriptions : public NodeTypeDescriptionsInterface
{
public:
RCLCPP_SMART_PTR_ALIASES_ONLY(NodeTypeDescriptions)

RCLCPP_PUBLIC
explicit NodeTypeDescriptions(
NodeBaseInterface::SharedPtr node_base,
NodeLoggingInterface::SharedPtr node_logging,
NodeParametersInterface::SharedPtr node_parameters,
NodeServicesInterface::SharedPtr node_services);

RCLCPP_PUBLIC
virtual
~NodeTypeDescriptions();

private:
RCLCPP_DISABLE_COPY(NodeTypeDescriptions)

// Pimpl hides helper types and functions used for wrapping a C service, which would be
// awkward to expose in this header.
class NodeTypeDescriptionsImpl;
std::unique_ptr<NodeTypeDescriptionsImpl> impl_;
};

} // namespace node_interfaces
} // namespace rclcpp

#endif // RCLCPP__NODE_INTERFACES__NODE_TYPE_DESCRIPTIONS_HPP_
Original file line number Diff line number Diff line change
@@ -0,0 +1,44 @@
// Copyright 2023 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.

#ifndef RCLCPP__NODE_INTERFACES__NODE_TYPE_DESCRIPTIONS_INTERFACE_HPP_
#define RCLCPP__NODE_INTERFACES__NODE_TYPE_DESCRIPTIONS_INTERFACE_HPP_

#include "rclcpp/macros.hpp"
#include "rclcpp/node_interfaces/detail/node_interfaces_helpers.hpp"
#include "rclcpp/visibility_control.hpp"

namespace rclcpp
{
namespace node_interfaces
{

/// Pure virtual interface class for the NodeTypeDescriptions part of the Node API.
class NodeTypeDescriptionsInterface
{
public:
RCLCPP_SMART_PTR_ALIASES_ONLY(NodeTypeDescriptionsInterface)

RCLCPP_PUBLIC
virtual
~NodeTypeDescriptionsInterface() = default;
};

} // namespace node_interfaces
} // namespace rclcpp

RCLCPP_NODE_INTERFACE_HELPERS_SUPPORT(
rclcpp::node_interfaces::NodeTypeDescriptionsInterface, type_descriptions)

#endif // RCLCPP__NODE_INTERFACES__NODE_TYPE_DESCRIPTIONS_INTERFACE_HPP_
79 changes: 79 additions & 0 deletions rclcpp/src/rclcpp/node.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -17,7 +17,10 @@
#include <limits>
#include <map>
#include <memory>
#include <mutex>
#include <shared_mutex>
#include <string>
#include <unordered_map>
#include <utility>
#include <vector>

Expand All @@ -36,6 +39,7 @@
#include "rclcpp/node_interfaces/node_time_source.hpp"
#include "rclcpp/node_interfaces/node_timers.hpp"
#include "rclcpp/node_interfaces/node_topics.hpp"
#include "rclcpp/node_interfaces/node_type_descriptions.hpp"
#include "rclcpp/node_interfaces/node_waitables.hpp"
#include "rclcpp/qos_overriding_options.hpp"

Expand Down Expand Up @@ -109,6 +113,72 @@ create_effective_namespace(const std::string & node_namespace, const std::string

} // namespace

/// \brief Associate new extra member variables with instances of Node without changing ABI.
/**
* It is used only for bugfixes or backported features that require new members.
* Atomically constructs/destroys all extra members.
* Node instance will register and remove itself, and use its methods to retrieve members.
* Note for performance consideration that accessing these members uses a map lookup.
*/
class Node::BackportMembers
{
public:
BackportMembers() = default;
~BackportMembers() = default;

/// \brief Add all backported members for a new Node.
/**
* \param[in] key Raw pointer to the Node instance that will use new members.
*/
void add(Node * key)
{
// Adding a new instance to the maps requires exclusive access
std::unique_lock lock(map_access_mutex_);
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Nit: #include <mutex> at the top of the file for this.

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This is a really good eye you have for these - do you know of any include-what-you-use linters that could handle this so you don't have to?

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

There is iwyu: https://include-what-you-use.org/ . It's even packaged for Ubuntu under the same name. The major issue with it (from an automation standpoint) is that it relies on clang, so we could only conceivably run it in our clang CI jobs.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I'm surprised that cpplint.py doesn't catch this, it used to catch things like this. Maybe its coverage is just not as good.

type_descriptions_map_.emplace(
key,
std::make_shared<rclcpp::node_interfaces::NodeTypeDescriptions>(
key->get_node_base_interface(),
key->get_node_logging_interface(),
key->get_node_parameters_interface(),
key->get_node_services_interface()));
}

/// \brief Remove the members for an instance of Node
/**
* \param[in] key Raw pointer to the Node
*/
void remove(const Node * key)
{
// Removing an instance from the maps requires exclusive access
std::unique_lock lock(map_access_mutex_);
type_descriptions_map_.erase(key);
}

/// \brief Retrieve the NodeTypeDescriptionsInterface for a Node.
/**
* \param[in] key Raw pointer to an instance of Node.
* \return A shared ptr to this Node's NodeTypeDescriptionsInterface instance.
*/
rclcpp::node_interfaces::NodeTypeDescriptionsInterface::SharedPtr
get_node_type_descriptions_interface(const Node * key) const
{
// Multiple threads can retrieve from the maps at the same time
std::shared_lock lock(map_access_mutex_);
emersonknapp marked this conversation as resolved.
Show resolved Hide resolved
return type_descriptions_map_.at(key);
}

private:
/// \brief Map that stored TypeDescriptionsInterface members
std::unordered_map<
emersonknapp marked this conversation as resolved.
Show resolved Hide resolved
const Node *, rclcpp::node_interfaces::NodeTypeDescriptionsInterface::SharedPtr
> type_descriptions_map_;

/// \brief Controls access to all private maps
mutable std::shared_mutex map_access_mutex_;
};
// Definition of static member declaration
Node::BackportMembers Node::backport_members_;

Node::Node(
const std::string & node_name,
const NodeOptions & options)
Expand Down Expand Up @@ -211,6 +281,8 @@ Node::Node(
sub_namespace_(""),
effective_namespace_(create_effective_namespace(this->get_namespace(), sub_namespace_))
{
backport_members_.add(this);

// we have got what we wanted directly from the overrides,
// but declare the parameters anyway so they are visible.
rclcpp::detail::declare_qos_parameters(
Expand Down Expand Up @@ -272,6 +344,7 @@ Node::Node(
Node::~Node()
{
// release sub-interfaces in an order that allows them to consult with node_base during tear-down
backport_members_.remove(this);
node_waitables_.reset();
node_time_source_.reset();
node_parameters_.reset();
Expand Down Expand Up @@ -591,6 +664,12 @@ Node::get_node_topics_interface()
return node_topics_;
}

rclcpp::node_interfaces::NodeTypeDescriptionsInterface::SharedPtr
Node::get_node_type_descriptions_interface()
{
return backport_members_.get_node_type_descriptions_interface(this);
}

rclcpp::node_interfaces::NodeServicesInterface::SharedPtr
Node::get_node_services_interface()
{
Expand Down
Loading