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

Async controller lifecycle #932

Merged
merged 14 commits into from
Mar 21, 2023
1 change: 1 addition & 0 deletions controller_interface/src/controller_interface_base.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -129,5 +129,6 @@ std::shared_ptr<rclcpp_lifecycle::LifecycleNode> ControllerInterfaceBase::get_no
}

unsigned int ControllerInterfaceBase::get_update_rate() const { return update_rate_; }
bool ControllerInterfaceBase::is_async() const { return is_async_; }
VX792 marked this conversation as resolved.
Show resolved Hide resolved
VX792 marked this conversation as resolved.
Show resolved Hide resolved

} // namespace controller_interface
Original file line number Diff line number Diff line change
Expand Up @@ -510,6 +510,64 @@ class ControllerManager : public rclcpp::Node
};

SwitchParams switch_params_;

class ControllerThreadWrapper
Copy link
Member

Choose a reason for hiding this comment

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

I am not sure that this is the right place to add this class. Wound't make more sense to have this in a separate file in "controller_interface" package?

Also, about the name. I would call this "AsyncController" or something similar.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Good point! I've been thinking about this for a while and my initial decision to put this in the controller manager was indeed because it results in less lines of code.

However, the reason I'm reluctant to create a ControllerInterface for this is that async controllers aren't inherently different from regular controllers, the only difference is how they are handled by thr ControllerManager - and that is really nothing more than excluding them from the control loop and accommodating the stored thread's lifecycle.

Also, if we are to implement an AsyncControllerInterface then we'd have to have an async version of the available controllers, as iirc they all inherit from the ControllerInterface class, which might be a bit tedious

{
public:
ControllerThreadWrapper(
std::shared_ptr<controller_interface::ControllerInterfaceBase> & controller,
int cm_update_rate)
: terminated_(false), controller_(controller), thread_{}, cm_update_rate_(cm_update_rate)
{
}
VX792 marked this conversation as resolved.
Show resolved Hide resolved
ControllerThreadWrapper(const ControllerThreadWrapper & t) = delete;
ControllerThreadWrapper(ControllerThreadWrapper && t) = default;
~ControllerThreadWrapper()
{
terminated_.store(true, std::memory_order_seq_cst);
if (thread_.joinable())
{
thread_.join();
}
}

void start() { thread_ = std::thread(&ControllerThreadWrapper::call_controller_update, this); }
VX792 marked this conversation as resolved.
Show resolved Hide resolved

void call_controller_update()
{
rclcpp::Time previous_time = controller_->get_node()->now();
VX792 marked this conversation as resolved.
Show resolved Hide resolved

while (!terminated_.load(std::memory_order_relaxed))
{
auto const period = std::chrono::nanoseconds(1'000'000'000 / cm_update_rate_);
VX792 marked this conversation as resolved.
Show resolved Hide resolved
std::chrono::system_clock::time_point next_iteration_time =
VX792 marked this conversation as resolved.
Show resolved Hide resolved
std::chrono::system_clock::time_point(
std::chrono::nanoseconds(controller_->get_node()->now().nanoseconds()));

if (controller_->get_state().id() == lifecycle_msgs::msg::State::PRIMARY_STATE_ACTIVE)
VX792 marked this conversation as resolved.
Show resolved Hide resolved
{
// critical section, not implemented yet
}

next_iteration_time += period;
std::this_thread::sleep_until(next_iteration_time);
}
}

std::shared_ptr<controller_interface::ControllerInterfaceBase> get_controller()
{
return controller_;
}

private:
std::atomic<bool> terminated_;
std::shared_ptr<controller_interface::ControllerInterfaceBase> controller_;
VX792 marked this conversation as resolved.
Show resolved Hide resolved
std::thread thread_;
unsigned int cm_update_rate_;
};

std::unordered_map<std::string, std::unique_ptr<ControllerThreadWrapper>>
async_controller_threads_;
};

} // namespace controller_manager
Expand Down
23 changes: 22 additions & 1 deletion controller_manager/src/controller_manager.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -388,6 +388,13 @@ controller_interface::return_type ControllerManager::unload_controller(
controller_name.c_str());
return controller_interface::return_type::ERROR;
}
if (controller.c->is_async())
{
RCLCPP_DEBUG(
get_logger(), "Removing controller '%s' from the list of async controllers",
controller_name.c_str());
async_controller_threads_.erase(controller_name);
}

RCLCPP_DEBUG(get_logger(), "Cleanup controller");
// TODO(destogl): remove reference interface if chainable; i.e., add a separate method for
Expand All @@ -405,6 +412,7 @@ controller_interface::return_type ControllerManager::unload_controller(
RCLCPP_DEBUG(get_logger(), "Destruct controller finished");

RCLCPP_DEBUG(get_logger(), "Successfully unloaded controller '%s'", controller_name.c_str());

return controller_interface::return_type::OK;
}

Expand Down Expand Up @@ -472,6 +480,13 @@ controller_interface::return_type ControllerManager::configure_controller(
return controller_interface::return_type::ERROR;
}

// ASYNCHRONOUS CONTROLLERS: Start background thread for update
if (controller->is_async())
{
async_controller_threads_.emplace(
controller_name, std::make_unique<ControllerThreadWrapper>(controller, update_rate_));
}

// CHAINABLE CONTROLLERS: get reference interfaces from chainable controllers
if (controller->is_chainable())
{
Expand Down Expand Up @@ -1163,6 +1178,7 @@ void ControllerManager::activate_controllers(
continue;
}
auto controller = found_it->c;
auto controller_name = found_it->info.name;

bool assignment_successful = true;
// assign command interfaces to the controller
Expand Down Expand Up @@ -1251,6 +1267,11 @@ void ControllerManager::activate_controllers(
get_logger(), "After activating, controller '%s' is in state '%s', expected Active",
controller->get_node()->get_name(), new_state.label().c_str());
}

if (controller->is_async())
{
async_controller_threads_.at(controller_name)->start();
}
}
// All controllers activated, switching done
switch_params_.do_switch = false;
Expand Down Expand Up @@ -1761,7 +1782,7 @@ controller_interface::return_type ControllerManager::update(
{
// TODO(v-lopez) we could cache this information
// https://github.com/ros-controls/ros2_control/issues/153
if (is_controller_active(*loaded_controller.c))
if (!loaded_controller.c->is_async() && is_controller_active(*loaded_controller.c))
{
const auto controller_update_rate = loaded_controller.c->get_update_rate();

Expand Down