Coderabbit suggestions

husarion · Oct 30, 2024 · d184572 · d184572
1 parent e882033
commit d184572
Show file tree

Hide file tree

Showing 8 changed files with 37 additions and 69 deletions.
diff --git 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 $<BUILD_INTERFACE:${CMAKE_CURRENT_SOURCE_DIR}/include>
            $<INSTALL_INTERFACE:include>)
-  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 $<BUILD_INTERFACE:${CMAKE_CURRENT_SOURCE_DIR}/include>
            $<INSTALL_INTERFACE:include>)
-  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 $<BUILD_INTERFACE:${CMAKE_CURRENT_SOURCE_DIR}/test>
            $<INSTALL_INTERFACE:include>)
-  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 $<BUILD_INTERFACE:${CMAKE_CURRENT_SOURCE_DIR}/include>
            $<INSTALL_INTERFACE:include>)
-  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 $<BUILD_INTERFACE:${CMAKE_CURRENT_SOURCE_DIR}/test>
            $<INSTALL_INTERFACE:include>)
-  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
@@ -3,20 +3,6 @@
     <include path="docking.xml"/>
     <!-- Description of Node Models (used by Groot) -->
     <TreeNodesModel>
-        <Action ID="CallSetLedAnimationService" editable="true">
-            <input_port name="id">animation ID</input_port>
-            <input_port name="param">optional parameter</input_port>
-            <input_port name="repeating" default="false">indicates if animation should repeat</input_port>
-            <input_port name="service_name">ROS service name</input_port>
-        </Action>
-        <Action ID="CallTriggerService" editable="true">
-            <input_port name="service_name">ROS service name</input_port>
-        </Action>
-        <Condition ID="CheckBoolMsg" editable="true">
-            <input_port name="topic_name">
-Topic name for sensor_msgs/Joy message type</input_port>
-            <input_port name="data">Vector of provided axis inputs</input_port>
-        </Condition>
         <Condition ID="CheckJoyMsg" editable="true">
             <input_port name="topic_name">
 Topic name for sensor_msgs/Joy message type</input_port>
@@ -33,9 +19,6 @@ Topic name for sensor_msgs/Joy message type</input_port>
                 staging pose</input_port>
             <input_port name="action_name">action name to call</input_port>
         </Action>
-        <Decorator ID="TickAfterTimeout" editable="true">
-            <input_port name="timeout">time in s to wait before ticking child again</input_port>
-        </Decorator>
         <Action ID="UndockRobot" editable="true">
             <input_port name="dock_type">type of the dock</input_port>
             <input_port name="max_undocking_time">maximum time to get back to the staging pose</input_port>

diff --git 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
@@ -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
@@ -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
@@ -42,9 +42,10 @@ DockingManagerNode::DockingManagerNode(
 
   DeclareParameters();
   const std::map<std::string, std::any> docking_initial_bb = {};
+  const int bt_server_port = this->get_parameter("bt_server_port").as_int();
 
   docking_tree_manager_ = std::make_unique<BehaviorTreeManager>(
-    "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<double>("ros_communication_timeout.response", 1.0);
 
   this->declare_parameter<float>("timer_frequency", 20.0);
+  this->declare_parameter<int>("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
@@ -48,8 +48,10 @@ LightsManagerNode::LightsManagerNode(
   battery_percent_ma_ = std::make_unique<panther_utils::MovingAverage<double>>(
     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<BehaviorTreeManager>("Lights", initial_blackboard, 5555);
+  lights_tree_manager_ = std::make_unique<BehaviorTreeManager>(
+    "Lights", initial_blackboard, bt_server_port);
 
   RCLCPP_INFO(this->get_logger(), "Node constructed successfully.");
 }
@@ -100,6 +102,7 @@ void LightsManagerNode::DeclareParameters()
   this->declare_parameter<float>("battery.animation_period.critical", 15.0);
   this->declare_parameter<float>("battery.charging_anim_step", 0.1);
   this->declare_parameter<float>("timer_frequency", 10.0);
+  this->declare_parameter<int>("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
@@ -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;
   }