Conversation
Summary of ChangesHello @Seongoo, I'm Gemini Code Assist1! I'm currently reviewing this pull request and will post my feedback shortly. In the meantime, here's a summary to help you and other reviewers quickly get up to speed! This pull request introduces a behavior tree system for physical AI robots, enabling complex task sequences to be executed. It includes a new ROS 2 package with action and control nodes, XML-based tree configuration, and integration with the existing physical_ai_server for controlling inference and action publishing. Highlights
🧠 New Feature in Public Preview: You can now enable Memory to help Gemini Code Assist learn from your team's feedback. This makes future code reviews more consistent and personalized to your project's style. Click here to enable Memory in your admin console. Ignored Files
Using Gemini Code AssistThe full guide for Gemini Code Assist can be found on our documentation page, here are some quick tips. Invoking Gemini You can request assistance from Gemini at any point by creating a comment using either
Customization To customize Gemini Code Assist for GitHub experience, repository maintainers can create a configuration file and/or provide a custom code review style guide (such as PEP-8 for Python) by creating and adding files to a Limitations & Feedback Gemini Code Assist may make mistakes. Please leave feedback on any instances where its feedback is incorrect or counter productive. You can react with 👍 and 👎 on @gemini-code-assist comments. If you're interested in giving your feedback about your experience with Gemini Code Assist for Github and other Google products, sign up here. You can also get AI-powered code generation, chat, as well as code reviews directly in the IDE at no cost with the Gemini Code Assist IDE Extension. Footnotes
|
There was a problem hiding this comment.
Pull request overview
This PR introduces the physical_ai_bt package, a Python-based behavior tree framework for executing hierarchical task sequences on physical AI robots. It provides XML-based tree definitions, tick-based execution at configurable rates, and support for multiple robot types.
Changes:
- Added new
physical_ai_btpackage with core behavior tree functionality - Modified
physical_ai_serverto support action publishing control during inference - Updated version numbers across packages to 0.8.0
Reviewed changes
Copilot reviewed 36 out of 39 changed files in this pull request and generated 1 comment.
Show a summary per file
| File | Description |
|---|---|
| physical_ai_bt/physical_ai_bt/bt_node.py | Main ROS 2 node for loading and executing behavior trees with automatic shutdown |
| physical_ai_bt/physical_ai_bt/bt_nodes_loader.py | XML parser for loading behavior tree definitions and instantiating nodes |
| physical_ai_bt/physical_ai_bt/actions/base_action.py | Base classes for behavior tree nodes (BTNode, BaseAction, BaseControl) |
| physical_ai_bt/physical_ai_bt/actions/rotate.py | Action node for rotating mobile base using odometry feedback |
| physical_ai_bt/physical_ai_bt/actions/move_arms.py | Action node for moving both robot arms to target joint positions |
| physical_ai_bt/physical_ai_bt/actions/move_head.py | Action node for moving robot head joints |
| physical_ai_bt/physical_ai_bt/actions/move_lift.py | Action node for moving robot lift to specified position |
| physical_ai_bt/physical_ai_bt/controls/sequence.py | Sequence control node for sequential child execution |
| physical_ai_bt/physical_ai_bt/blackboard.py | Singleton blackboard for sharing data across nodes |
| physical_ai_server/physical_ai_server/physical_ai_server.py | Added inference control service and action publish pause functionality |
| physical_ai_server/physical_ai_server/communication/communicator.py | Added action publish enable/disable flag |
| physical_ai_server/config/omx_f_config.yaml | Updated camera topic configuration |
| physical_ai_server/config/ffw_sg2_rev1_config.yaml | Fixed camera topic and trailing whitespace |
| physical_ai_tools/package.xml | Added physical_ai_bt dependency |
| rosbag_recorder/CHANGELOG.rst | Updated changelog with release date |
| physical_ai_bt/trees/ffw_sg2_rev1.xml | Example behavior tree definition for FFW SG2 Rev1 robot |
Files not reviewed (1)
- physical_ai_manager/package-lock.json: Language not supported
Comments suppressed due to low confidence (3)
rosbag_recorder/CHANGELOG.rst:1
- The changelog entry uses a date of 2026-01-19, which is in the future. The current date is January 23, 2026, so this date is actually in the past. However, it appears this should reference 2025-01-19 based on typical versioning patterns.
physical_ai_tools/CHANGELOG.rst:1 - The changelog entry uses a date of 2026-01-19, which is in the future. The current date is January 23, 2026, so this date is actually in the past. However, it appears this should reference 2025-01-19 based on typical versioning patterns.
physical_ai_bt/physical_ai_bt/bt_node.py:1 - The default tree_xml parameter is 'ffw_test.xml', but this file doesn't appear to exist in the trees directory. Only 'ffw_sg2_rev1.xml' is present. Consider updating the default to match an existing file or ensuring ffw_test.xml is included.
#!/usr/bin/env python3
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
![gemini-code-assist[bot]](https://avatars.githubusercontent.com/in/956858?s=60&v=4){kind=small}
There was a problem hiding this comment.
Code Review
This pull request introduces a comprehensive behavior tree framework in the physical_ai_bt package, which is a significant and well-structured addition. The design, including the use of threads for non-blocking actions and XML-based tree definitions, is solid. I've provided several comments to address a high-severity bug in the MoveHead action, improve consistency across the action node implementations, and suggest some code cleanup and refactoring for better maintainability. The integration with physical_ai_server to allow the behavior tree to control the robot appears to be correctly implemented.
Seongoo
requested review from
kimtaehyeong99 and
ola31
and removed request for
kimtaehyeong99 and
ola31
Seongoo
force-pushed
the
feature-behavior-tree
branch
from
dbb72b1 to
8cf08d9
Compare
Seongoo
force-pushed
the
feature-behavior-tree
branch
from
b960ea6 to
7eda1f0
Compare
Signed-off-by: Seongwoo Kim <kimsw@robotis.com>
Signed-off-by: Seongwoo Kim <kimsw@robotis.com>
- Add comprehensive docstrings to all modules, classes, and methods - Fix import ordering (stdlib, third-party, local imports) - Change double quotes to single quotes throughout - Fix line length issues (max 99 characters) - Add noqa comments for unavoidable style warnings (I100, A003) - Create test infrastructure with test_flake8.py and test_pep257.py - Add test dependencies to package.xml - Create .flake8 configuration file - Update setup.cfg with flake8 settings All files now pass ament_flake8 and ament_pep257 checks. Signed-off-by: Seongwoo Kim <kimsw@robotis.com>
Automatically shutdown the BT node when behavior tree execution completes (either SUCCESS or FAILURE). This allows the node to exit cleanly after finishing its task instead of running indefinitely. Signed-off-by: Seongwoo Kim <kimsw@robotis.com>
Signed-off-by: Seongwoo Kim <kimsw@robotis.com>
Signed-off-by: Seongwoo Kim <kimsw@robotis.com>
- Add missing else clause when joint not found in /joint_states - Add warning logs for missing joints in MoveArms, MoveHead, MoveLift - Prevent premature success when joints are missing from state message Signed-off-by: Seongwoo Kim <kimsw@robotis.com>
- Remove Isaac-GR00T submodule from git tracking - Add Isaac-GR00T/, install/, log/ to .gitignore - Keep physical directory for local use but exclude from version control Signed-off-by: Seongwoo Kim <kimsw@robotis.com>
Combine left and right arm joint checking into a single loop by merging joint_names and positions lists. This eliminates code duplication and improves maintainability. Signed-off-by: Seongwoo Kim <kimsw@robotis.com>
The Rotate action uses Twist messages for mobile base rotation, not JointTrajectory messages. Removed the unused trajectory_pub and its import to improve code clarity. Signed-off-by: Seongwoo Kim <kimsw@robotis.com>
- Add AGENTS.md to .gitignore for project documentation - Remove redundant bt_node entry point from setup.py The physical_ai_bt entry point is sufficient and more descriptive than the generic bt_node name. Signed-off-by: Seongwoo Kim <kimsw@robotis.com>
Add else clause to _load_node method to raise ValueError when encountering unknown node types or IDs. This prevents silent failures and makes debugging easier. Signed-off-by: Seongwoo Kim <kimsw@robotis.com>
Signed-off-by: Seongwoo Kim <kimsw@robotis.com>
Signed-off-by: Seongwoo Kim <kimsw@robotis.com>
Reorganize behavior tree base classes into clearer file structure: - Create bt_core.py for BTNode and NodeStatus (core classes) - Update actions/base_action.py to only contain BaseAction - Create controls/base_control.py for BaseControl and BaseDecorator - Update all imports across action and control files This improves code maintainability by placing each class in an appropriately named location that reflects its purpose. Signed-off-by: Seongwoo Kim <kimsw@robotis.com>
- Create physical_ai_bt/constants.py with descriptive constant names - Update all action files (move_arms, move_head, move_lift, rotate) to use constants - Improve code self-documentation and maintainability - No functional changes, purely refactoring Signed-off-by: Seongwoo Kim <kimsw@robotis.com>
- Add DEFAULT joint name constants to MoveArms, MoveHead, MoveLift - Update action constructors to accept optional joint_names parameters - Add TreeLoader helper to extract joint names from topic_config - Pass configured joint names from TreeLoader to action instances - Maintain backward compatibility with hardcoded defaults - Support different robot types through configuration alone Signed-off-by: Seongwoo Kim <kimsw@robotis.com>
- Add threading.Lock to protect shared result state - Add threading.Event for clean thread signaling and shutdown - Replace _thread_done/_thread_success with lock-protected _result - Ensure thread-safe access in tick(), _control_loop(), and reset() - Apply fix to all action classes (MoveArms, MoveHead, MoveLift, Rotate) - Prevent torn reads, stale values, and undefined behavior Signed-off-by: Seongwoo Kim <kimsw@robotis.com>
Signed-off-by: Seongwoo Kim <kimsw@robotis.com>
Signed-off-by: Seongwoo Kim <kimsw@robotis.com>
- Remove control_action_publish_callback method (never registered as service) - Remove action_publish_enabled flag (always True, never changed) - Remove inference_paused flag (no service to control it) - Simplify publish_action method to always publish - No functional impact as BT never uses this functionality Signed-off-by: Seongwoo Kim <kimsw@robotis.com>
Signed-off-by: Seongwoo Kim <kimsw@robotis.com>
Signed-off-by: Seongwoo Kim <kimsw@robotis.com>
Signed-off-by: Seongwoo Kim <kimsw@robotis.com>
Seongoo
force-pushed
the
feature-behavior-tree
branch
from
a34dab5 to
95547a2
Compare
3 participants
Add physical_ai_bt package for behavior tree execution
This commit introduces the physical_ai_bt package, a Python-based behavior tree
framework for executing hierarchical task sequences on physical AI robots.
Package Overview:
Key Components: