Skip to content

Cleanup the static executor API #1334

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.

Sign up for GitHub

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

Merged
clalancette merged 2 commits into from
Sep 28, 2020
Merged

Cleanup the static executor API #1334

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
056b95b
Const-ify more of the static executor.
clalancette Sep 24, 2020
28117e9
Make more of the static entity collector API private.
clalancette Sep 24, 2020
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
37 changes: 16 additions & 21 deletions rclcpp/include/rclcpp/executors/static_executor_entities_collector.hpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -71,22 +71,6 @@ class StaticExecutorEntitiesCollector final
void
execute() override;

RCLCPP_PUBLIC
void
fill_memory_strategy();

RCLCPP_PUBLIC
void
fill_executable_list();

/// Function to reallocate space for entities in the wait set.
/**
* \throws std::runtime_error if wait set couldn't be cleared or resized.
*/
RCLCPP_PUBLIC
void
prepare_wait_set();

/// Function to add_handles_to_wait_set and wait for work and
/**
* block until the wait set is ready or until the timeout has been exceeded.
Expand Down Expand Up @@ -282,6 +266,20 @@ class StaticExecutorEntitiesCollector final
rclcpp::Waitable::SharedPtr
get_waitable(size_t i) {return exec_list_.waitable[i];}

private:
/// Function to reallocate space for entities in the wait set.
/**
* \throws std::runtime_error if wait set couldn't be cleared or resized.
*/
void
prepare_wait_set();

void
fill_executable_list();

void
fill_memory_strategy();

/// Return true if the node belongs to the collector
/**
* \param[in] group_ptr a node base interface shared pointer
Expand All @@ -290,22 +288,19 @@ class StaticExecutorEntitiesCollector final
bool
has_node(
const rclcpp::node_interfaces::NodeBaseInterface::SharedPtr node_ptr,
WeakCallbackGroupsToNodesMap weak_groups_to_nodes) const;
const WeakCallbackGroupsToNodesMap & weak_groups_to_nodes) const;

private:
/// Add all callback groups that can be automatically added by any executor
/// and is not already associated with an executor from nodes
/// that are associated with executor
/**
* \see rclcpp::Executor::add_callback_groups_from_nodes_associated_to_executor()
*/
RCLCPP_PUBLIC
void
add_callback_groups_from_nodes_associated_to_executor();

RCLCPP_PUBLIC
void
fill_executable_list_from_map(WeakCallbackGroupsToNodesMap weak_groups_to_nodes);
fill_executable_list_from_map(const WeakCallbackGroupsToNodesMap & weak_groups_to_nodes);

/// Memory strategy: an interface for handling user-defined memory allocation strategies.
rclcpp::memory_strategy::MemoryStrategy::SharedPtr memory_strategy_;
Expand Down
28 changes: 14 additions & 14 deletions rclcpp/src/rclcpp/executors/static_executor_entities_collector.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -29,22 +29,22 @@ using rclcpp::executors::StaticExecutorEntitiesCollector;
StaticExecutorEntitiesCollector::~StaticExecutorEntitiesCollector()
{
// Disassociate all callback groups and thus nodes.
for (auto & pair : weak_groups_associated_with_executor_to_nodes_) {
for (const auto & pair : weak_groups_associated_with_executor_to_nodes_) {
auto group = pair.first.lock();
if (group) {
std::atomic_bool & has_executor = group->get_associated_with_executor_atomic();
has_executor.store(false);
}
}
for (auto & pair : weak_groups_to_nodes_associated_with_executor_) {
for (const auto & pair : weak_groups_to_nodes_associated_with_executor_) {
auto group = pair.first.lock();
if (group) {
std::atomic_bool & has_executor = group->get_associated_with_executor_atomic();
has_executor.store(false);
}
}
// Disassociate all nodes
for (auto & weak_node : weak_nodes_) {
for (const auto & weak_node : weak_nodes_) {
auto node = weak_node.lock();
if (node) {
std::atomic_bool & has_executor = node->get_associated_with_executor_atomic();
Expand Down Expand Up @@ -100,7 +100,7 @@ StaticExecutorEntitiesCollector::fill_memory_strategy()
// Clean up any invalid nodes, if they were detected
if (has_invalid_weak_groups_or_nodes) {
std::vector<rclcpp::CallbackGroup::WeakPtr> invalid_group_ptrs;
for (auto & pair : weak_groups_to_nodes_associated_with_executor_) {
for (const auto & pair : weak_groups_to_nodes_associated_with_executor_) {
auto & weak_group_ptr = pair.first;
auto & weak_node_ptr = pair.second;
if (weak_group_ptr.expired() || weak_node_ptr.expired()) {
Expand All @@ -118,9 +118,9 @@ StaticExecutorEntitiesCollector::fill_memory_strategy()
// Clean up any invalid nodes, if they were detected
if (has_invalid_weak_groups_or_nodes) {
std::vector<rclcpp::CallbackGroup::WeakPtr> invalid_group_ptrs;
for (auto & pair : weak_groups_associated_with_executor_to_nodes_) {
for (const auto & pair : weak_groups_associated_with_executor_to_nodes_) {
auto & weak_group_ptr = pair.first;
auto & weak_node_ptr = pair.second;
const auto & weak_node_ptr = pair.second;
if (weak_group_ptr.expired() || weak_node_ptr.expired()) {
invalid_group_ptrs.push_back(weak_group_ptr);
}
Expand Down Expand Up @@ -148,7 +148,7 @@ StaticExecutorEntitiesCollector::fill_executable_list()
}
void
StaticExecutorEntitiesCollector::fill_executable_list_from_map(
WeakCallbackGroupsToNodesMap weak_groups_to_nodes)
const WeakCallbackGroupsToNodesMap & weak_groups_to_nodes)
{
for (const auto & pair : weak_groups_to_nodes) {
auto group = pair.first.lock();
Expand Down Expand Up @@ -270,7 +270,7 @@ StaticExecutorEntitiesCollector::add_node(
if (has_executor.exchange(true)) {
throw std::runtime_error("Node has already been added to an executor.");
}
for (auto & weak_group : node_ptr->get_callback_groups()) {
for (const auto & weak_group : node_ptr->get_callback_groups()) {
auto group_ptr = weak_group.lock();
if (group_ptr != nullptr && !group_ptr->get_associated_with_executor_atomic().load() &&
group_ptr->automatically_add_to_executor_with_node())
Expand Down Expand Up @@ -430,7 +430,7 @@ StaticExecutorEntitiesCollector::is_ready(rcl_wait_set_t * p_wait_set)
bool
StaticExecutorEntitiesCollector::has_node(
const rclcpp::node_interfaces::NodeBaseInterface::SharedPtr node_ptr,
WeakCallbackGroupsToNodesMap weak_groups_to_nodes) const
const WeakCallbackGroupsToNodesMap & weak_groups_to_nodes) const
{
return std::find_if(
weak_groups_to_nodes.begin(),
Expand All @@ -444,7 +444,7 @@ StaticExecutorEntitiesCollector::has_node(
void
StaticExecutorEntitiesCollector::add_callback_groups_from_nodes_associated_to_executor()
{
for (auto & weak_node : weak_nodes_) {
for (const auto & weak_node : weak_nodes_) {
auto node = weak_node.lock();
if (node) {
auto group_ptrs = node->get_callback_groups();
Expand All @@ -470,10 +470,10 @@ std::vector<rclcpp::CallbackGroup::WeakPtr>
StaticExecutorEntitiesCollector::get_all_callback_groups()
{
std::vector<rclcpp::CallbackGroup::WeakPtr> groups;
for (auto const & group_node_ptr : weak_groups_associated_with_executor_to_nodes_) {
for (const auto & group_node_ptr : weak_groups_associated_with_executor_to_nodes_) {
groups.push_back(group_node_ptr.first);
}
for (auto const & group_node_ptr : weak_groups_to_nodes_associated_with_executor_) {
for (const auto & group_node_ptr : weak_groups_to_nodes_associated_with_executor_) {
groups.push_back(group_node_ptr.first);
}
return groups;
Expand All @@ -483,7 +483,7 @@ std::vector<rclcpp::CallbackGroup::WeakPtr>
StaticExecutorEntitiesCollector::get_manually_added_callback_groups()
{
std::vector<rclcpp::CallbackGroup::WeakPtr> groups;
for (auto const & group_node_ptr : weak_groups_associated_with_executor_to_nodes_) {
for (const auto & group_node_ptr : weak_groups_associated_with_executor_to_nodes_) {
groups.push_back(group_node_ptr.first);
}
return groups;
Expand All @@ -493,7 +493,7 @@ std::vector<rclcpp::CallbackGroup::WeakPtr>
StaticExecutorEntitiesCollector::get_automatically_added_callback_groups_from_nodes()
{
std::vector<rclcpp::CallbackGroup::WeakPtr> groups;
for (auto const & group_node_ptr : weak_groups_to_nodes_associated_with_executor_) {
for (const auto & group_node_ptr : weak_groups_to_nodes_associated_with_executor_) {
groups.push_back(group_node_ptr.first);
}
return groups;
Expand Down