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

Robot States #426

Draft
wants to merge 13 commits into
base: ros2-docking
Choose a base branch
from
Draft

Robot States #426

wants to merge 13 commits into from

Conversation

rafal-gorecki
Copy link
Contributor

@rafal-gorecki rafal-gorecki commented Oct 16, 2024

Related with: husarion/panther_msgs#82

Description

  • Adds BT for managing the robot states
  • Change lights tree to read robot_state
  • Add use_docking to simulation.launch.py
  • Rename are_buttons_pressed -> check_joy_msg and extend functionalities
  • Add right now unused check_bool_msg plugin (to use it we need to modify BT code to be able to set up custom QOS setting)

Requirements

  • Code style guidelines followed
  • Documentation updated

Tests 🧪

  • Robot
  • Container
  • Simulation

@rafal-gorecki rafal-gorecki marked this pull request as ready for review October 18, 2024 10:36
@delihus
Copy link
Contributor

delihus commented Oct 18, 2024

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
Copy link
Contributor

delihus commented Oct 30, 2024

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

@rafal-gorecki rafal-gorecki changed the title Ros2-docking-manager-rafal Robot States Oct 31, 2024
@rafal-gorecki rafal-gorecki marked this pull request as draft October 31, 2024 09:22
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants