husarion / panther_ros

ROS package for Panther autonomous mobile robot
Apache License 2.0
29 stars 10 forks source link

Robot States #426

Open rafal-gorecki opened 1 month ago

rafal-gorecki commented 1 month ago

Related with: https://github.com/husarion/panther_msgs/pull/82

Description

Requirements

Tests 🧪

delihus commented 1 month ago

I started making the review but in my opinion the improvements, what you derived, fall outside the scope of the docking project.

Regarding to the feature of RobotState. It touches every section of the robot even the battery management. If you would like to implement RobotState that way let's meet with whole team and brainstorm about that. So for now, I recommend to:

  1. plan new task what would implement RobotState and robot_state_manager (you have done most of it),
  2. apply this feature to the main develop branch of panther_ros,
  3. sync the docking project with the main develop branch,
  4. add new state for Docking and Undocking actions to the docking project.

About managing behavior trees. The RobotState should be read with BT::RosTopicSubNode<> as same as should be published with BT::RosTopicPubNode<>. It will make a tree more readable and will show when exactly RobotState is read. We want to get rid of creating ROS instances inside managers in the future so we should change the way of your implementation.

delihus commented 1 month ago

According to our yesterday's meeting this should be closed, right @rafal-gorecki ?