From 0e5094693e1f78898cf20f8667ae80cac3ea8c85 Mon Sep 17 00:00:00 2001 From: Jackie Kay Date: Mon, 26 Oct 2015 12:27:31 -0700 Subject: [PATCH] Remove declaration of friendship for Executor in callback_group, client, node, service, subscription, timer --- rclcpp/include/rclcpp/callback_group.hpp | 39 +++++++-- rclcpp/include/rclcpp/client.hpp | 7 -- rclcpp/include/rclcpp/executor.hpp | 101 ++++++++++++----------- rclcpp/include/rclcpp/node.hpp | 8 +- rclcpp/include/rclcpp/node_impl.hpp | 7 ++ rclcpp/include/rclcpp/service.hpp | 7 -- rclcpp/include/rclcpp/subscription.hpp | 17 ++-- rclcpp/include/rclcpp/timer.hpp | 18 ++-- 8 files changed, 114 insertions(+), 90 deletions(-) diff --git a/rclcpp/include/rclcpp/callback_group.hpp b/rclcpp/include/rclcpp/callback_group.hpp index 03a3262..ca050fa 100644 --- a/rclcpp/include/rclcpp/callback_group.hpp +++ b/rclcpp/include/rclcpp/callback_group.hpp @@ -32,10 +32,6 @@ namespace node { class Node; } // namespace node -namespace executor -{ -class Executor; -} // namespace executor namespace callback_group { @@ -49,7 +45,6 @@ enum class CallbackGroupType class CallbackGroup { friend class rclcpp::node::Node; - friend class rclcpp::executor::Executor; public: RCLCPP_SMART_PTR_DEFINITIONS(CallbackGroup); @@ -58,6 +53,40 @@ public: : type_(group_type), can_be_taken_from_(true) {} + const std::vector & + get_subscription_ptrs() const + { + return subscription_ptrs_; + } + + const std::vector & + get_timer_ptrs() const + { + return timer_ptrs_; + } + + const std::vector & + get_service_ptrs() const + { + return service_ptrs_; + } + + const std::vector & + get_client_ptrs() const + { + return client_ptrs_; + } + + std::atomic_bool & can_be_taken_from() + { + return can_be_taken_from_; + } + + const CallbackGroupType & type() const + { + return type_; + } + private: RCLCPP_DISABLE_COPY(CallbackGroup); diff --git a/rclcpp/include/rclcpp/client.hpp b/rclcpp/include/rclcpp/client.hpp index 4b70998..7037363 100644 --- a/rclcpp/include/rclcpp/client.hpp +++ b/rclcpp/include/rclcpp/client.hpp @@ -31,18 +31,11 @@ namespace rclcpp { -// Forward declaration for friend statement -namespace executor -{ -class Executor; -} // namespace executor - namespace client { class ClientBase { - friend class rclcpp::executor::Executor; public: RCLCPP_SMART_PTR_DEFINITIONS_NOT_COPYABLE(ClientBase); diff --git a/rclcpp/include/rclcpp/executor.hpp b/rclcpp/include/rclcpp/executor.hpp index d2d1e62..9a1658b 100644 --- a/rclcpp/include/rclcpp/executor.hpp +++ b/rclcpp/include/rclcpp/executor.hpp @@ -228,7 +228,7 @@ protected: execute_client(any_exec->client); } // Reset the callback_group, regardless of type - any_exec->callback_group->can_be_taken_from_.store(true); + any_exec->callback_group->can_be_taken_from().store(true); // Wake the wait, because it may need to be recalculated or work that // was previously blocked is now available. rmw_ret_t status = rmw_trigger_guard_condition(interrupt_guard_condition_); @@ -245,7 +245,8 @@ protected: bool taken = false; rmw_message_info_t message_info; auto ret = - rmw_take_with_info(subscription->subscription_handle_, message.get(), &taken, &message_info); + rmw_take_with_info(subscription->get_subscription_handle(), + message.get(), &taken, &message_info); if (ret == RMW_RET_OK) { if (taken) { message_info.from_intra_process = false; @@ -267,7 +268,7 @@ protected: bool taken = false; rmw_message_info_t message_info; rmw_ret_t status = rmw_take_with_info( - subscription->intra_process_subscription_handle_, + subscription->get_intra_process_subscription_handle(), &ipm, &taken, &message_info); @@ -287,7 +288,7 @@ protected: execute_timer( rclcpp::timer::TimerBase::SharedPtr timer) { - timer->callback_(); + timer->execute_callback(); } static void @@ -298,7 +299,7 @@ protected: std::shared_ptr request = service->create_request(); bool taken = false; rmw_ret_t status = rmw_take_request( - service->service_handle_, + service->get_service_handle(), request_header.get(), request.get(), &taken); @@ -321,7 +322,7 @@ protected: std::shared_ptr response = client->create_response(); bool taken = false; rmw_ret_t status = rmw_take_response( - client->client_handle_, + client->get_client_handle(), request_header.get(), response.get(), &taken); @@ -353,21 +354,21 @@ protected: has_invalid_weak_nodes = false; continue; } - for (auto & weak_group : node->callback_groups_) { + for (auto & weak_group : node->get_callback_groups()) { auto group = weak_group.lock(); - if (!group || !group->can_be_taken_from_.load()) { + if (!group || !group->can_be_taken_from().load()) { continue; } - for (auto & weak_subscription : group->subscription_ptrs_) { + for (auto & weak_subscription : group->get_subscription_ptrs()) { auto subscription = weak_subscription.lock(); if (subscription) { memory_strategy_->subs.push_back(subscription); } } - for (auto & service : group->service_ptrs_) { + for (auto & service : group->get_service_ptrs()) { memory_strategy_->services.push_back(service); } - for (auto & client : group->client_ptrs_) { + for (auto & client : group->get_client_ptrs()) { memory_strategy_->clients.push_back(client); } } @@ -397,10 +398,10 @@ protected: // Then fill the SubscriberHandles with ready subscriptions for (auto & subscription : memory_strategy_->subs) { subscriber_handles.subscribers[subscriber_handles.subscriber_count++] = - subscription->subscription_handle_->data; - if (subscription->intra_process_subscription_handle_) { + subscription->get_subscription_handle()->data; + if (subscription->get_intra_process_subscription_handle()) { subscriber_handles.subscribers[subscriber_handles.subscriber_count++] = - subscription->intra_process_subscription_handle_->data; + subscription->get_intra_process_subscription_handle()->data; } } @@ -420,7 +421,7 @@ protected: size_t service_handle_index = 0; for (auto & service : memory_strategy_->services) { service_handles.services[service_handle_index] = \ - service->service_handle_->data; + service->get_service_handle()->data; service_handle_index += 1; } @@ -438,7 +439,7 @@ protected: size_t client_handle_index = 0; for (auto & client : memory_strategy_->clients) { client_handles.clients[client_handle_index] = \ - client->client_handle_->data; + client->get_client_handle()->data; client_handle_index += 1; } @@ -556,19 +557,19 @@ protected: if (!node) { continue; } - for (auto & weak_group : node->callback_groups_) { + for (auto & weak_group : node->get_callback_groups()) { auto group = weak_group.lock(); if (!group) { continue; } - for (auto & weak_subscription : group->subscription_ptrs_) { + for (auto & weak_subscription : group->get_subscription_ptrs()) { auto subscription = weak_subscription.lock(); if (subscription) { - if (subscription->subscription_handle_->data == subscriber_handle) { + if (subscription->get_subscription_handle()->data == subscriber_handle) { return subscription; } - if (subscription->intra_process_subscription_handle_ && - subscription->intra_process_subscription_handle_->data == subscriber_handle) + if (subscription->get_intra_process_subscription_handle() && + subscription->get_intra_process_subscription_handle()->data == subscriber_handle) { return subscription; } @@ -587,13 +588,13 @@ protected: if (!node) { continue; } - for (auto & weak_group : node->callback_groups_) { + for (auto & weak_group : node->get_callback_groups()) { auto group = weak_group.lock(); if (!group) { continue; } - for (auto & service : group->service_ptrs_) { - if (service->service_handle_->data == service_handle) { + for (auto & service : group->get_service_ptrs()) { + if (service->get_service_handle()->data == service_handle) { return service; } } @@ -610,13 +611,13 @@ protected: if (!node) { continue; } - for (auto & weak_group : node->callback_groups_) { + for (auto & weak_group : node->get_callback_groups()) { auto group = weak_group.lock(); if (!group) { continue; } - for (auto & client : group->client_ptrs_) { - if (client->client_handle_->data == client_handle) { + for (auto & client : group->get_client_ptrs()) { + if (client->get_client_handle()->data == client_handle) { return client; } } @@ -636,7 +637,7 @@ protected: if (!node) { continue; } - for (auto & weak_group : node->callback_groups_) { + for (auto & weak_group : node->get_callback_groups()) { auto callback_group = weak_group.lock(); if (callback_group == group) { return node; @@ -655,12 +656,12 @@ protected: if (!node) { continue; } - for (auto & weak_group : node->callback_groups_) { + for (auto & weak_group : node->get_callback_groups()) { auto group = weak_group.lock(); if (!group) { continue; } - for (auto & weak_timer : group->timer_ptrs_) { + for (auto & weak_timer : group->get_timer_ptrs()) { auto t = weak_timer.lock(); if (t == timer) { return group; @@ -679,12 +680,12 @@ protected: if (!node) { continue; } - for (auto & weak_group : node->callback_groups_) { + for (auto & weak_group : node->get_callback_groups()) { auto group = weak_group.lock(); - if (!group || !group->can_be_taken_from_.load()) { + if (!group || !group->can_be_taken_from().load()) { continue; } - for (auto & timer_ref : group->timer_ptrs_) { + for (auto & timer_ref : group->get_timer_ptrs()) { auto timer = timer_ref.lock(); if (timer && timer->check_and_trigger()) { any_exec->timer = timer; @@ -707,12 +708,12 @@ protected: if (!node) { continue; } - for (auto & weak_group : node->callback_groups_) { + for (auto & weak_group : node->get_callback_groups()) { auto group = weak_group.lock(); - if (!group || !group->can_be_taken_from_.load()) { + if (!group || !group->can_be_taken_from().load()) { continue; } - for (auto & timer_ref : group->timer_ptrs_) { + for (auto & timer_ref : group->get_timer_ptrs()) { timers_empty = false; // Check the expected trigger time auto timer = timer_ref.lock(); @@ -737,12 +738,12 @@ protected: if (!node) { continue; } - for (auto & weak_group : node->callback_groups_) { + for (auto & weak_group : node->get_callback_groups()) { auto group = weak_group.lock(); if (!group) { continue; } - for (auto & weak_sub : group->subscription_ptrs_) { + for (auto & weak_sub : group->get_subscription_ptrs()) { auto sub = weak_sub.lock(); if (sub == subscription) { return group; @@ -762,8 +763,8 @@ protected: if (subscription) { // Figure out if this is for intra-process or not. bool is_intra_process = false; - if (subscription->intra_process_subscription_handle_) { - is_intra_process = subscription->intra_process_subscription_handle_->data == *it; + if (subscription->get_intra_process_subscription_handle()) { + is_intra_process = subscription->get_intra_process_subscription_handle()->data == *it; } // Find the group for this handle and see if it can be serviced auto group = get_group_by_subscription(subscription); @@ -773,7 +774,7 @@ protected: subscriber_handles_.erase(it++); continue; } - if (!group->can_be_taken_from_.load()) { + if (!group->can_be_taken_from().load()) { // Group is mutually exclusive and is being used, so skip it for now // Leave it to be checked next time, but continue searching ++it; @@ -804,12 +805,12 @@ protected: if (!node) { continue; } - for (auto & weak_group : node->callback_groups_) { + for (auto & weak_group : node->get_callback_groups()) { auto group = weak_group.lock(); if (!group) { continue; } - for (auto & serv : group->service_ptrs_) { + for (auto & serv : group->get_service_ptrs()) { if (serv == service) { return group; } @@ -834,7 +835,7 @@ protected: service_handles_.erase(it++); continue; } - if (!group->can_be_taken_from_.load()) { + if (!group->can_be_taken_from().load()) { // Group is mutually exclusive and is being used, so skip it for now // Leave it to be checked next time, but continue searching ++it; @@ -861,12 +862,12 @@ protected: if (!node) { continue; } - for (auto & weak_group : node->callback_groups_) { + for (auto & weak_group : node->get_callback_groups()) { auto group = weak_group.lock(); if (!group) { continue; } - for (auto & cli : group->client_ptrs_) { + for (auto & cli : group->get_client_ptrs()) { if (cli == client) { return group; } @@ -891,7 +892,7 @@ protected: client_handles_.erase(it++); continue; } - if (!group->can_be_taken_from_.load()) { + if (!group->can_be_taken_from().load()) { // Group is mutually exclusive and is being used, so skip it for now // Leave it to be checked next time, but continue searching ++it; @@ -957,13 +958,13 @@ protected: if (any_exec) { // If it is valid, check to see if the group is mutually exclusive or // not, then mark it accordingly - if (any_exec->callback_group && any_exec->callback_group->type_ == \ + if (any_exec->callback_group && any_exec->callback_group->type() == \ callback_group::CallbackGroupType::MutuallyExclusive) { // It should not have been taken otherwise - assert(any_exec->callback_group->can_be_taken_from_.load()); + assert(any_exec->callback_group->can_be_taken_from().load()); // Set to false to indicate something is being run from this group - any_exec->callback_group->can_be_taken_from_.store(false); + any_exec->callback_group->can_be_taken_from().store(false); } } return any_exec; diff --git a/rclcpp/include/rclcpp/node.hpp b/rclcpp/include/rclcpp/node.hpp index bdac682..6b64bec 100644 --- a/rclcpp/include/rclcpp/node.hpp +++ b/rclcpp/include/rclcpp/node.hpp @@ -47,18 +47,12 @@ struct rmw_node_t; namespace rclcpp { -// Forward declaration for friend statement -namespace executor -{ -class Executor; -} // namespace executor namespace node { /// Node is the single point of entry for creating publishers and subscribers. class Node { - friend class rclcpp::executor::Executor; public: RCLCPP_SMART_PTR_DEFINITIONS(Node); @@ -232,6 +226,8 @@ public: size_t count_subscribers(const std::string & topic_name) const; + const CallbackGroupWeakPtrList & get_callback_groups() const; + private: RCLCPP_DISABLE_COPY(Node); diff --git a/rclcpp/include/rclcpp/node_impl.hpp b/rclcpp/include/rclcpp/node_impl.hpp index f66afad..02f08b2 100644 --- a/rclcpp/include/rclcpp/node_impl.hpp +++ b/rclcpp/include/rclcpp/node_impl.hpp @@ -643,4 +643,11 @@ Node::count_subscribers(const std::string & topic_name) const return count; } + +const Node::CallbackGroupWeakPtrList & +Node::get_callback_groups() const +{ + return callback_groups_; +} + #endif /* RCLCPP_RCLCPP_NODE_IMPL_HPP_ */ diff --git a/rclcpp/include/rclcpp/service.hpp b/rclcpp/include/rclcpp/service.hpp index 8848a85..2bfbb09 100644 --- a/rclcpp/include/rclcpp/service.hpp +++ b/rclcpp/include/rclcpp/service.hpp @@ -30,18 +30,11 @@ namespace rclcpp { -// Forward declaration for friend statement -namespace executor -{ -class Executor; -} // namespace executor - namespace service { class ServiceBase { - friend class rclcpp::executor::Executor; public: RCLCPP_SMART_PTR_DEFINITIONS_NOT_COPYABLE(ServiceBase); diff --git a/rclcpp/include/rclcpp/subscription.hpp b/rclcpp/include/rclcpp/subscription.hpp index ee757ac..b7250e6 100644 --- a/rclcpp/include/rclcpp/subscription.hpp +++ b/rclcpp/include/rclcpp/subscription.hpp @@ -32,12 +32,6 @@ namespace rclcpp { -// Forward declaration is for friend statement in SubscriptionBase -namespace executor -{ -class Executor; -} // namespace executor - namespace node { class Node; @@ -50,7 +44,6 @@ namespace subscription /// specializations of Subscription, among other things. class SubscriptionBase { - friend class rclcpp::executor::Executor; public: RCLCPP_SMART_PTR_DEFINITIONS_NOT_COPYABLE(SubscriptionBase); @@ -104,6 +97,16 @@ public: return this->topic_name_; } + const rmw_subscription_t * get_subscription_handle() const + { + return subscription_handle_; + } + + const rmw_subscription_t * get_intra_process_subscription_handle() const + { + return intra_process_subscription_handle_; + } + /// Borrow a new message. // \return Shared pointer to the fresh message. virtual std::shared_ptr create_message() = 0; diff --git a/rclcpp/include/rclcpp/timer.hpp b/rclcpp/include/rclcpp/timer.hpp index b68d954..9a69cb8 100644 --- a/rclcpp/include/rclcpp/timer.hpp +++ b/rclcpp/include/rclcpp/timer.hpp @@ -31,12 +31,6 @@ namespace rclcpp { -// Forward declaration is for friend statement in GenericTimer -namespace executor -{ -class Executor; -} // namespace executor - namespace timer { @@ -44,7 +38,6 @@ using CallbackType = std::function; class TimerBase { - friend class rclcpp::executor::Executor; public: RCLCPP_SMART_PTR_DEFINITIONS_NOT_COPYABLE(TimerBase); @@ -66,6 +59,16 @@ public: this->canceled_ = true; } + void execute_callback() const + { + callback_(); + } + + const CallbackType & get_callback() const + { + return callback_; + } + /// Check how long the timer has until its next scheduled callback. // \return A std::chrono::duration representing the relative time until the next callback. virtual std::chrono::nanoseconds @@ -95,7 +98,6 @@ protected: template class GenericTimer : public TimerBase { - friend class rclcpp::executor::Executor; public: RCLCPP_SMART_PTR_DEFINITIONS(GenericTimer);