From d1845722d9bf41dd36f015ef4d4e1393e1a55cd0 Mon Sep 17 00:00:00 2001 From: rafal-gorecki Date: Wed, 30 Oct 2024 17:08:26 +0100 Subject: [PATCH] Coderabbit suggestions --- panther_manager/CMakeLists.txt | 70 ++++++------------- .../behavior_trees/DockingBT.btproj | 17 ----- panther_manager/config/docking_manager.yaml | 1 + panther_manager/config/lights_manager.yaml | 1 + .../panther_manager/docking_manager_node.hpp | 6 +- panther_manager/src/docking_manager_node.cpp | 4 +- panther_manager/src/lights_manager_node.cpp | 5 +- .../src/plugins/condition/check_joy_msg.cpp | 2 +- 8 files changed, 37 insertions(+), 69 deletions(-) diff --git a/panther_manager/CMakeLists.txt b/panther_manager/CMakeLists.txt index 2c901e20..e70fe2e9 100644 --- a/panther_manager/CMakeLists.txt +++ b/panther_manager/CMakeLists.txt @@ -225,15 +225,26 @@ if(BUILD_TESTING) target_link_libraries(${bt_node_test} ${LIBSSH_LIBRARIES} yaml-cpp) endforeach() + set(TEST_TREE_DEPENDENCIES + behaviortree_cpp + behaviortree_ros2 + geometry_msgs + panther_msgs + panther_utils + rclcpp + sensor_msgs + std_msgs + std_srvs + tf2_geometry_msgs) + ament_add_gtest(${PROJECT_NAME}_test_behavior_tree_utils test/test_behavior_tree_utils.cpp) target_include_directories( ${PROJECT_NAME}_test_behavior_tree_utils PUBLIC $ $) - ament_target_dependencies( - ${PROJECT_NAME}_test_behavior_tree_utils behaviortree_cpp behaviortree_ros2 - geometry_msgs panther_utils tf2_geometry_msgs) + ament_target_dependencies(${PROJECT_NAME}_test_behavior_tree_utils + ${TEST_TREE_DEPENDENCIES}) ament_add_gtest( ${PROJECT_NAME}_test_behavior_tree_manager @@ -252,17 +263,8 @@ if(BUILD_TESTING) ${PROJECT_NAME}_test_lights_manager_node PUBLIC $ $) - ament_target_dependencies( - ${PROJECT_NAME}_test_lights_manager_node - behaviortree_cpp - behaviortree_ros2 - geometry_msgs - panther_msgs - panther_utils - rclcpp - sensor_msgs - tf2_geometry_msgs - std_msgs) + ament_target_dependencies(${PROJECT_NAME}_test_lights_manager_node + ${TEST_TREE_DEPENDENCIES}) ament_add_gtest( ${PROJECT_NAME}_test_lights_behavior_tree @@ -272,17 +274,8 @@ if(BUILD_TESTING) ${PROJECT_NAME}_test_lights_behavior_tree PUBLIC $ $) - ament_target_dependencies( - ${PROJECT_NAME}_test_lights_behavior_tree - behaviortree_cpp - behaviortree_ros2 - geometry_msgs - panther_msgs - panther_utils - rclcpp - sensor_msgs - std_msgs - tf2_geometry_msgs) + ament_target_dependencies(${PROJECT_NAME}_test_lights_behavior_tree + ${TEST_TREE_DEPENDENCIES}) ament_add_gtest( ${PROJECT_NAME}_test_safety_manager_node test/test_safety_manager_node.cpp @@ -291,17 +284,8 @@ if(BUILD_TESTING) ${PROJECT_NAME}_test_safety_manager_node PUBLIC $ $) - ament_target_dependencies( - ${PROJECT_NAME}_test_safety_manager_node - behaviortree_cpp - behaviortree_ros2 - geometry_msgs - panther_msgs - panther_utils - rclcpp - sensor_msgs - std_msgs - tf2_geometry_msgs) + ament_target_dependencies(${PROJECT_NAME}_test_safety_manager_node + ${TEST_TREE_DEPENDENCIES}) ament_add_gtest( ${PROJECT_NAME}_test_safety_behavior_tree @@ -311,18 +295,8 @@ if(BUILD_TESTING) ${PROJECT_NAME}_test_safety_behavior_tree PUBLIC $ $) - ament_target_dependencies( - ${PROJECT_NAME}_test_safety_behavior_tree - behaviortree_cpp - behaviortree_ros2 - geometry_msgs - panther_msgs - panther_utils - rclcpp - sensor_msgs - std_msgs - std_srvs - tf2_geometry_msgs) + ament_target_dependencies(${PROJECT_NAME}_test_safety_behavior_tree + ${TEST_TREE_DEPENDENCIES}) endif() ament_package() diff --git a/panther_manager/behavior_trees/DockingBT.btproj b/panther_manager/behavior_trees/DockingBT.btproj index ab7e8e91..2752c0d3 100644 --- a/panther_manager/behavior_trees/DockingBT.btproj +++ b/panther_manager/behavior_trees/DockingBT.btproj @@ -3,20 +3,6 @@ - - animation ID - optional parameter - indicates if animation should repeat - ROS service name - - - ROS service name - - - -Topic name for sensor_msgs/Joy message type - Vector of provided axis inputs - Topic name for sensor_msgs/Joy message type @@ -33,9 +19,6 @@ Topic name for sensor_msgs/Joy message type staging pose action name to call - - time in s to wait before ticking child again - type of the dock maximum time to get back to the staging pose diff --git a/panther_manager/config/docking_manager.yaml b/panther_manager/config/docking_manager.yaml index aedc2082..28879a8b 100644 --- a/panther_manager/config/docking_manager.yaml +++ b/panther_manager/config/docking_manager.yaml @@ -2,6 +2,7 @@ docking_manager: ros__parameters: timer_frequency: 50.0 + bt_server_port: 4444 ros_communication_timeout: availability: 2.0 response: 1.0 diff --git a/panther_manager/config/lights_manager.yaml b/panther_manager/config/lights_manager.yaml index cea2a5e4..f9033f68 100644 --- a/panther_manager/config/lights_manager.yaml +++ b/panther_manager/config/lights_manager.yaml @@ -2,6 +2,7 @@ lights_manager: ros__parameters: timer_frequency: 10.0 + bt_server_port: 5555 battery: percent: window_len: 6 diff --git a/panther_manager/include/panther_manager/docking_manager_node.hpp b/panther_manager/include/panther_manager/docking_manager_node.hpp index 9ddc1db9..d258078c 100644 --- a/panther_manager/include/panther_manager/docking_manager_node.hpp +++ b/panther_manager/include/panther_manager/docking_manager_node.hpp @@ -41,8 +41,12 @@ class DockingManagerNode : public rclcpp::Node public: DockingManagerNode( const std::string & node_name, const rclcpp::NodeOptions & options = rclcpp::NodeOptions()); - ~DockingManagerNode() {} + ~DockingManagerNode() = default; + /** + * @brief Initializes the docking manager, setting up parameters and behavior tree. + * @throws std::runtime_error if initialization fails + */ void Initialize(); protected: diff --git a/panther_manager/src/docking_manager_node.cpp b/panther_manager/src/docking_manager_node.cpp index b4925f6d..91ba15d6 100644 --- a/panther_manager/src/docking_manager_node.cpp +++ b/panther_manager/src/docking_manager_node.cpp @@ -42,9 +42,10 @@ DockingManagerNode::DockingManagerNode( DeclareParameters(); const std::map docking_initial_bb = {}; + const int bt_server_port = this->get_parameter("bt_server_port").as_int(); docking_tree_manager_ = std::make_unique( - "Docking", docking_initial_bb, 4444); + "Docking", docking_initial_bb, bt_server_port); RCLCPP_INFO(this->get_logger(), "Node constructed successfully."); } @@ -83,6 +84,7 @@ void DockingManagerNode::DeclareParameters() this->declare_parameter("ros_communication_timeout.response", 1.0); this->declare_parameter("timer_frequency", 20.0); + this->declare_parameter("bt_server_port", 4444); } void DockingManagerNode::RegisterBehaviorTree() diff --git a/panther_manager/src/lights_manager_node.cpp b/panther_manager/src/lights_manager_node.cpp index b4485a9e..ad784457 100644 --- a/panther_manager/src/lights_manager_node.cpp +++ b/panther_manager/src/lights_manager_node.cpp @@ -48,8 +48,10 @@ LightsManagerNode::LightsManagerNode( battery_percent_ma_ = std::make_unique>( battery_percent_window_len, 1.0); + const auto bt_server_port = this->get_parameter("bt_server_port").as_int(); const auto initial_blackboard = CreateLightsInitialBlackboard(); - lights_tree_manager_ = std::make_unique("Lights", initial_blackboard, 5555); + lights_tree_manager_ = std::make_unique( + "Lights", initial_blackboard, bt_server_port); RCLCPP_INFO(this->get_logger(), "Node constructed successfully."); } @@ -100,6 +102,7 @@ void LightsManagerNode::DeclareParameters() this->declare_parameter("battery.animation_period.critical", 15.0); this->declare_parameter("battery.charging_anim_step", 0.1); this->declare_parameter("timer_frequency", 10.0); + this->declare_parameter("bt_server_port", 5555); } void LightsManagerNode::RegisterBehaviorTree() diff --git a/panther_manager/src/plugins/condition/check_joy_msg.cpp b/panther_manager/src/plugins/condition/check_joy_msg.cpp index f99f476a..30d059d3 100644 --- a/panther_manager/src/plugins/condition/check_joy_msg.cpp +++ b/panther_manager/src/plugins/condition/check_joy_msg.cpp @@ -58,7 +58,7 @@ bool CheckJoyMsg::checkButtons(const JoyMsg::SharedPtr & last_msg) if (last_msg->buttons.size() != expected_buttons.size()) { RCLCPP_WARN_STREAM( this->logger(), GetLoggerPrefix(name()) - << "Joy message has " << last_msg->axes.size() + << "Joy message has " << last_msg->buttons.size() << " axes, expected at least " << expected_buttons.size()); return false; }