Skip to content

Commit

Permalink
Add mutex to reloadDbCb to fail if called during docking (#56)
Browse files Browse the repository at this point in the history
  • Loading branch information
redvinaa authored Jul 29, 2024
1 parent 4a63900 commit 0485059
Show file tree
Hide file tree
Showing 4 changed files with 23 additions and 8 deletions.
4 changes: 3 additions & 1 deletion opennav_docking/include/opennav_docking/dock_database.hpp
Original file line number Diff line number Diff line change
Expand Up @@ -18,6 +18,7 @@
#include <vector>
#include <memory>
#include <string>
#include <mutex>

#include "rclcpp/rclcpp.hpp"
#include "pluginlib/class_loader.hpp"
Expand All @@ -42,7 +43,7 @@ class DockDatabase
/**
* @brief A constructor for opennav_docking::DockDatabase
*/
DockDatabase();
explicit DockDatabase(std::shared_ptr<std::mutex> mutex = std::make_shared<std::mutex>());

/**
* @brief A setup function to populate database
Expand Down Expand Up @@ -127,6 +128,7 @@ class DockDatabase
std::shared_ptr<opennav_docking_msgs::srv::ReloadDatabase::Response> response);

rclcpp_lifecycle::LifecycleNode::WeakPtr node_;
std::shared_ptr<std::mutex> mutex_; // Don't reload database while actively docking
DockPluginMap dock_plugins_;
DockMap dock_instances_;
pluginlib::ClassLoader<opennav_docking_core::ChargingDock> dock_loader_;
Expand Down
5 changes: 4 additions & 1 deletion opennav_docking/include/opennav_docking/docking_server.hpp
Original file line number Diff line number Diff line change
Expand Up @@ -18,6 +18,7 @@
#include <vector>
#include <memory>
#include <string>
#include <mutex>

#include "rclcpp/rclcpp.hpp"
#include "nav2_util/lifecycle_node.hpp"
Expand Down Expand Up @@ -217,7 +218,9 @@ class DockingServer : public nav2_util::LifecycleNode

// Dynamic parameters handler
rclcpp::node_interfaces::OnSetParametersCallbackHandle::SharedPtr dyn_params_handler_;
std::mutex dynamic_params_lock_;

// Mutex for dynamic parameters and dock database
std::shared_ptr<std::mutex> mutex_;

// Frequency to run control loops
double controller_frequency_;
Expand Down
13 changes: 11 additions & 2 deletions opennav_docking/src/dock_database.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -17,8 +17,9 @@
namespace opennav_docking
{

DockDatabase::DockDatabase()
: dock_loader_("opennav_docking_core", "opennav_docking_core::ChargingDock")
DockDatabase::DockDatabase(std::shared_ptr<std::mutex> mutex)
: mutex_(mutex),
dock_loader_("opennav_docking_core", "opennav_docking_core::ChargingDock")
{}

DockDatabase::~DockDatabase()
Expand Down Expand Up @@ -71,13 +72,21 @@ void DockDatabase::reloadDbCb(
const std::shared_ptr<opennav_docking_msgs::srv::ReloadDatabase::Request> request,
std::shared_ptr<opennav_docking_msgs::srv::ReloadDatabase::Response> response)
{
if (!mutex_->try_lock()) {
RCLCPP_ERROR(node_.lock()->get_logger(), "Cannot reload database while docking!");
response->success = false;
return;
}

DockMap dock_instances;
if (utils::parseDockFile(request->filepath, node_.lock(), dock_instances)) {
dock_instances_ = dock_instances;
response->success = true;
mutex_->unlock();
return;
}
response->success = false;
mutex_->unlock();
}

Dock * DockDatabase::findDock(const std::string & dock_id)
Expand Down
9 changes: 5 additions & 4 deletions opennav_docking/src/docking_server.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -85,9 +85,10 @@ DockingServer::on_configure(const rclcpp_lifecycle::State & /*state*/)
true, server_options);

// Create composed utilities
mutex_ = std::make_shared<std::mutex>();
controller_ = std::make_unique<Controller>(node);
navigator_ = std::make_unique<Navigator>(node);
dock_db_ = std::make_unique<DockDatabase>();
dock_db_ = std::make_unique<DockDatabase>(mutex_);
if (!dock_db_->initialize(node, tf2_buffer_)) {
return nav2_util::CallbackReturn::FAILURE;
}
Expand Down Expand Up @@ -198,7 +199,7 @@ bool DockingServer::checkAndWarnIfPreempted(

void DockingServer::dockRobot()
{
std::lock_guard<std::mutex> lock(dynamic_params_lock_);
std::lock_guard<std::mutex> lock(*mutex_);
action_start_time_ = this->now();
rclcpp::Rate loop_rate(controller_frequency_);

Expand Down Expand Up @@ -540,7 +541,7 @@ bool DockingServer::getCommandToPose(

void DockingServer::undockRobot()
{
std::lock_guard<std::mutex> lock(dynamic_params_lock_);
std::lock_guard<std::mutex> lock(*mutex_);
action_start_time_ = this->now();
rclcpp::Rate loop_rate(controller_frequency_);

Expand Down Expand Up @@ -682,7 +683,7 @@ void DockingServer::publishDockingFeedback(uint16_t state)
rcl_interfaces::msg::SetParametersResult
DockingServer::dynamicParametersCallback(std::vector<rclcpp::Parameter> parameters)
{
std::lock_guard<std::mutex> lock(dynamic_params_lock_);
std::lock_guard<std::mutex> lock(*mutex_);

rcl_interfaces::msg::SetParametersResult result;
for (auto parameter : parameters) {
Expand Down

0 comments on commit 0485059

Please sign in to comment.