-
Notifications
You must be signed in to change notification settings - Fork 431
Add controller manager services #139
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
Changes from all commits
5deeaf6
f00ecec
91e1767
bef08e2
27206ea
47b170f
81a8d21
160a7c7
4c6a609
2b81d5d
e4e226c
64ff2b2
3481676
62df3aa
bc63827
3650c38
9c05a5e
0967122
2805044
ef1187f
0067623
b6ea360
2fba683
dd02801
ae10741
9aff325
528d1c9
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -1,4 +1,4 @@ | ||
| // Copyright 2017 Open Source Robotics Foundation, Inc. | ||
| // 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. | ||
|
|
@@ -23,7 +23,14 @@ | |
| #include "controller_interface/controller_interface.hpp" | ||
|
|
||
| #include "controller_manager/controller_loader_interface.hpp" | ||
| #include "controller_manager/controller_spec.hpp" | ||
| #include "controller_manager/visibility_control.h" | ||
| #include "controller_manager_msgs/srv/list_controllers.hpp" | ||
| #include "controller_manager_msgs/srv/list_controller_types.hpp" | ||
| #include "controller_manager_msgs/srv/load_controller.hpp" | ||
| #include "controller_manager_msgs/srv/reload_controller_libraries.hpp" | ||
| #include "controller_manager_msgs/srv/switch_controller.hpp" | ||
| #include "controller_manager_msgs/srv/unload_controller.hpp" | ||
|
|
||
| #include "hardware_interface/robot_hardware.hpp" | ||
|
|
||
|
|
@@ -36,6 +43,9 @@ namespace controller_manager | |
| class ControllerManager : public rclcpp::Node | ||
| { | ||
| public: | ||
| static constexpr bool WAIT_FOR_ALL_RESOURCES = false; | ||
| static constexpr double INFINITE_TIMEOUT = 0.0; | ||
|
|
||
| CONTROLLER_MANAGER_PUBLIC | ||
| ControllerManager( | ||
| std::shared_ptr<hardware_interface::RobotHardware> hw, | ||
|
|
@@ -47,20 +57,25 @@ class ControllerManager : public rclcpp::Node | |
| ~ControllerManager() = default; | ||
|
|
||
| CONTROLLER_MANAGER_PUBLIC | ||
| std::shared_ptr<controller_interface::ControllerInterface> | ||
| controller_interface::ControllerInterfaceSharedPtr | ||
| load_controller( | ||
| const std::string & controller_name, | ||
| const std::string & controller_type); | ||
|
|
||
| /** | ||
| * @brief load_controller loads a controller by name, the type must be defined in the parameter server | ||
| */ | ||
| CONTROLLER_MANAGER_PUBLIC | ||
| controller_interface::ControllerInterfaceSharedPtr | ||
| load_controller( | ||
| const std::string & controller_name); | ||
|
|
||
| CONTROLLER_MANAGER_PUBLIC | ||
| std::vector<std::shared_ptr<controller_interface::ControllerInterface>> | ||
| get_loaded_controllers() const; | ||
| controller_interface::return_type unload_controller( | ||
| const std::string & controller_name); | ||
|
|
||
| [[deprecated( | ||
| "get_loaded_controller is deprecated, it has been renamed to get_loaded_controllers")]] | ||
| CONTROLLER_MANAGER_PUBLIC | ||
| std::vector<std::shared_ptr<controller_interface::ControllerInterface>> | ||
| get_loaded_controller() const; | ||
| std::vector<ControllerSpec> get_loaded_controllers() const; | ||
|
|
||
| CONTROLLER_MANAGER_PUBLIC | ||
| void register_controller_loader(ControllerLoaderInterfaceSharedPtr loader); | ||
|
|
@@ -69,44 +84,196 @@ class ControllerManager : public rclcpp::Node | |
| typename T, | ||
| typename std::enable_if<std::is_convertible< | ||
| T *, controller_interface::ControllerInterface *>::value, T>::type * = nullptr> | ||
| std::shared_ptr<controller_interface::ControllerInterface> | ||
| add_controller(std::shared_ptr<T> controller, std::string controller_name) | ||
| controller_interface::ControllerInterfaceSharedPtr | ||
| add_controller( | ||
| std::shared_ptr<T> controller, const std::string & controller_name, | ||
| const std::string & controller_type) | ||
| { | ||
| return add_controller_impl(controller, controller_name); | ||
| ControllerSpec controller_spec; | ||
| controller_spec.c = controller; | ||
| controller_spec.info.name = controller_name; | ||
| controller_spec.info.type = controller_type; | ||
| return add_controller_impl(controller_spec); | ||
| } | ||
|
|
||
| /** | ||
| * @brief switch_controller Stops some controllers and others. | ||
v-lopez marked this conversation as resolved.
Show resolved
Hide resolved
|
||
| * @see Documentation in controller_manager_msgs/SwitchController.srv | ||
| */ | ||
| CONTROLLER_MANAGER_PUBLIC | ||
| controller_interface::return_type | ||
| update(); | ||
| switch_controller( | ||
| const std::vector<std::string> & start_controllers, | ||
| const std::vector<std::string> & stop_controllers, | ||
| int strictness, | ||
| bool start_asap = WAIT_FOR_ALL_RESOURCES, | ||
| const rclcpp::Duration & timeout = rclcpp::Duration(INFINITE_TIMEOUT)); | ||
|
|
||
| CONTROLLER_MANAGER_PUBLIC | ||
| controller_interface::return_type | ||
| configure() const; | ||
| update(); | ||
|
|
||
| protected: | ||
| CONTROLLER_MANAGER_PUBLIC | ||
| controller_interface::return_type | ||
| activate() const; | ||
| controller_interface::ControllerInterfaceSharedPtr | ||
| add_controller_impl(const ControllerSpec & controller); | ||
|
|
||
| CONTROLLER_MANAGER_PUBLIC | ||
| controller_interface::return_type | ||
| deactivate() const; | ||
| void manage_switch(); | ||
|
|
||
| CONTROLLER_MANAGER_PUBLIC | ||
| controller_interface::return_type | ||
| cleanup() const; | ||
| void stop_controllers(); | ||
|
|
||
| protected: | ||
| CONTROLLER_MANAGER_PUBLIC | ||
| std::shared_ptr<controller_interface::ControllerInterface> | ||
| add_controller_impl( | ||
| std::shared_ptr<controller_interface::ControllerInterface> controller, | ||
| const std::string & controller_name); | ||
| void start_controllers(); | ||
|
|
||
| CONTROLLER_MANAGER_PUBLIC | ||
| void start_controllers_asap(); | ||
v-lopez marked this conversation as resolved.
Outdated
Show resolved
Hide resolved
|
||
|
|
||
| CONTROLLER_MANAGER_PUBLIC | ||
| void list_controllers_srv_cb( | ||
| const std::shared_ptr<controller_manager_msgs::srv::ListControllers::Request> request, | ||
| std::shared_ptr<controller_manager_msgs::srv::ListControllers::Response> response); | ||
|
|
||
| CONTROLLER_MANAGER_PUBLIC | ||
| void list_controller_types_srv_cb( | ||
| const std::shared_ptr<controller_manager_msgs::srv::ListControllerTypes::Request> request, | ||
| std::shared_ptr<controller_manager_msgs::srv::ListControllerTypes::Response> response); | ||
|
|
||
| CONTROLLER_MANAGER_PUBLIC | ||
| void load_controller_service_cb( | ||
| const std::shared_ptr<controller_manager_msgs::srv::LoadController::Request> request, | ||
| std::shared_ptr<controller_manager_msgs::srv::LoadController::Response> response); | ||
|
|
||
| CONTROLLER_MANAGER_PUBLIC | ||
| void reload_controller_libraries_service_cb( | ||
| const std::shared_ptr<controller_manager_msgs::srv::ReloadControllerLibraries::Request> request, | ||
| std::shared_ptr<controller_manager_msgs::srv::ReloadControllerLibraries::Response> response); | ||
|
|
||
| CONTROLLER_MANAGER_PUBLIC | ||
| void switch_controller_service_cb( | ||
| const std::shared_ptr<controller_manager_msgs::srv::SwitchController::Request> request, | ||
| std::shared_ptr<controller_manager_msgs::srv::SwitchController::Response> response); | ||
|
|
||
| CONTROLLER_MANAGER_PUBLIC | ||
| void unload_controller_service_cb( | ||
| const std::shared_ptr<controller_manager_msgs::srv::UnloadController::Request> request, | ||
| std::shared_ptr<controller_manager_msgs::srv::UnloadController::Response> response); | ||
|
|
||
| private: | ||
| std::vector<std::string> get_controller_names(); | ||
|
|
||
v-lopez marked this conversation as resolved.
Show resolved
Hide resolved
|
||
| std::shared_ptr<hardware_interface::RobotHardware> hw_; | ||
| std::shared_ptr<rclcpp::Executor> executor_; | ||
| std::vector<ControllerLoaderInterfaceSharedPtr> loaders_; | ||
| std::vector<std::shared_ptr<controller_interface::ControllerInterface>> loaded_controllers_; | ||
|
|
||
| /** | ||
| * @brief The RTControllerListWrapper class wraps a double-buffered list of controllers | ||
| * to avoid needing to lock the real-time thread when switching controllers in | ||
| * the non-real-time thread. | ||
| * | ||
| * There's always an "updated" list and an "outdated" one | ||
| * There's always an "used by rt" list and an "unused by rt" list | ||
| * | ||
| * The updated state changes on the switch_updated_list() | ||
| * The rt usage state changes on the update_and_get_used_by_rt_list() | ||
| */ | ||
| class RTControllerListWrapper | ||
| { | ||
| // *INDENT-OFF* | ||
| public: | ||
| // *INDENT-ON* | ||
| /** | ||
| * @brief update_and_get_used_by_rt_list Makes the "updated" list the "used by rt" list | ||
| * @warning Should only be called by the RT thread, no one should modify the | ||
| * updated list while it's being used | ||
| * @return reference to the updated list | ||
| */ | ||
| std::vector<ControllerSpec> & update_and_get_used_by_rt_list(); | ||
|
|
||
| /** | ||
| * @brief get_unused_list Waits until the "outdated" and "unused by rt" | ||
| * lists match and returns a reference to it | ||
| * This referenced list can be modified safely until switch_updated_controller_list() | ||
| * is called, at this point the RT thread may start using it at any time | ||
| * @param guard Guard needed to make sure the caller is the only one accessing the unused by rt list | ||
| */ | ||
| std::vector<ControllerSpec> & get_unused_list( | ||
| const std::lock_guard<std::recursive_mutex> & guard); | ||
|
|
||
| /** | ||
| * @brief get_updated_list Returns a const reference to the most updated list, | ||
| * @warning May or may not being used by the realtime thread, read-only reference for safety | ||
| * @param guard Guard needed to make sure the caller is the only one accessing the unused by rt list | ||
bmagyar marked this conversation as resolved.
Show resolved
Hide resolved
|
||
| */ | ||
| const std::vector<ControllerSpec> & get_updated_list( | ||
| const std::lock_guard<std::recursive_mutex> & guard) const; | ||
|
|
||
| /** | ||
| * @brief switch_updated_list Switches the "updated" and "outdated" lists, and waits | ||
| * until the RT thread is using the new "updated" list. | ||
| * @param guard Guard needed to make sure the caller is the only one accessing the unused by rt list | ||
bmagyar marked this conversation as resolved.
Show resolved
Hide resolved
|
||
| */ | ||
| void switch_updated_list(const std::lock_guard<std::recursive_mutex> & guard); | ||
|
|
||
v-lopez marked this conversation as resolved.
Show resolved
Hide resolved
|
||
| // Mutex protecting the controllers list | ||
| // must be acquired before using any list other than the "used by rt" | ||
| mutable std::recursive_mutex controllers_lock_; | ||
|
|
||
| // *INDENT-OFF* | ||
| private: | ||
| // *INDENT-ON* | ||
| /** | ||
| * @brief get_other_list get the list not pointed by index | ||
| */ | ||
| int get_other_list(int index) const; | ||
bmagyar marked this conversation as resolved.
Show resolved
Hide resolved
|
||
|
|
||
| void wait_until_rt_not_using( | ||
| int index, | ||
| std::chrono::microseconds sleep_delay = std::chrono::microseconds(200)) const; | ||
|
|
||
| std::vector<ControllerSpec> controllers_lists_[2]; | ||
| /// The index of the controller list with the most updated information | ||
| int updated_controllers_index_ = 0; | ||
| /// The index of the controllers list being used in the real-time thread. | ||
| int used_by_realtime_controllers_index_ = -1; | ||
|
Comment on lines
+237
to
+239
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I find this naming kind of confusing. If I understand this correctly, these indices are used for indicating which of the two lists within the double buffered lists are active, is that right? That also would imply that they must not be pointing to the same index. What about having only one variable (maybe call it
Contributor
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. No, they can be pointing to the same index. From the documentation above: What is missing in that documentation is an explicit mention that these "states" can overlap. I will add this You can have an:
That's why we need two variables to track this. Yes, they could be two booleans, but right now at least there's one usage for the
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Sorry, don't want to make this tedious, but I still don't quite get this. What's the purpose of "update + unused_by_rt" and "outdated + unused_by_rt". I understand that there's a phase where the controller list is getting updated while not being used, but that shouldn't influence the current state of the Why can't you just use a pointer or iterator to the currently used list in the
Contributor
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Yeah, we can use two iterators instead of two ints, but it's always two variables to indicate:
I may be missing something, but I believe we always need this "acknowledge" from the RT thread, because the non-RT thread is waiting for it to use the new list and cleanup the old one, and that requires two variables, or at least a more expressive variable. But again, coming to my point for not rewriting this from scratch... This works, and has worked for the last decade, and is efficient. We have a lot of pending work on other fronts. Yes it's ugly and there's probably better solutions to it, but I feel like we're investing a lot of time for some code that will be forgotten like the old version was, because it works. At least I suggest we move it to an issue and look at it again after this PR is done.
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more.
ok |
||
| }; | ||
|
|
||
| RTControllerListWrapper rt_controllers_wrapper_; | ||
| /// mutex copied from ROS1 Control, protects service callbacks | ||
| /// not needed if we're guaranteed that the callbacks don't come from multiple threads | ||
| std::mutex services_lock_; | ||
| rclcpp::Service<controller_manager_msgs::srv::ListControllers>::SharedPtr | ||
| list_controllers_service_; | ||
| rclcpp::Service<controller_manager_msgs::srv::ListControllerTypes>::SharedPtr | ||
| list_controller_types_service_; | ||
| rclcpp::Service<controller_manager_msgs::srv::LoadController>::SharedPtr | ||
| load_controller_service_; | ||
| rclcpp::Service<controller_manager_msgs::srv::ReloadControllerLibraries>::SharedPtr | ||
| reload_controller_libraries_service_; | ||
| rclcpp::Service<controller_manager_msgs::srv::SwitchController>::SharedPtr | ||
| switch_controller_service_; | ||
| rclcpp::Service<controller_manager_msgs::srv::UnloadController>::SharedPtr | ||
| unload_controller_service_; | ||
|
|
||
| std::vector<std::string> start_request_, stop_request_; | ||
| #ifdef TODO_IMPLEMENT_RESOURCE_CHECKING | ||
| // std::list<hardware_interface::ControllerInfo> switch_start_list_, switch_stop_list_; | ||
| #endif | ||
|
|
||
| struct SwitchParams | ||
| { | ||
| bool do_switch = {false}; | ||
| bool started = {false}; | ||
| rclcpp::Time init_time = {rclcpp::Time::max()}; | ||
|
|
||
| // Switch options | ||
| int strictness = {0}; | ||
| bool start_asap = {false}; | ||
| rclcpp::Duration timeout = rclcpp::Duration{0, 0}; | ||
| }; | ||
|
|
||
| SwitchParams switch_params_; | ||
| }; | ||
|
|
||
| } // namespace controller_manager | ||
|
|
||
Uh oh!
There was an error while loading. Please reload this page.