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

Make main behavior tree configurable #47

Merged
merged 8 commits into from
Jan 6, 2025
Merged

Make main behavior tree configurable #47

merged 8 commits into from
Jan 6, 2025

Conversation

luca-della-vedova
Copy link
Member

@luca-della-vedova luca-della-vedova commented Dec 31, 2024

This PR makes the main behavior tree configurable through a parameter since it was found in #42 that it is not generic enough that it can be used to execute different classes of work orders.

Overview:

  • A new parameter main_bt_filename has been introduced, which takes just the name of a behavior tree that is contained in the bt_path parameter.
  • The bt_path system orchestrator parameter now just points to the folder containing behavior trees.
  • The main behavior tree to be used will just be a path concatenation of bt_path and main_bt_filename.
  • The main_bt_filename parameter is editable and a parameter callback has been added. Trying to set it to a non existing file will fail and return an error to the user.

Signed-off-by: Luca Della Vedova <lucadv@intrinsic.ai>
Signed-off-by: Luca Della Vedova <lucadv@intrinsic.ai>
Signed-off-by: Luca Della Vedova <lucadv@intrinsic.ai>
@luca-della-vedova luca-della-vedova marked this pull request as ready for review January 3, 2025 07:52
Signed-off-by: Luca Della Vedova <lucadv@intrinsic.ai>
Signed-off-by: Luca Della Vedova <lucadv@intrinsic.ai>
Signed-off-by: Luca Della Vedova <lucadv@intrinsic.ai>
@luca-della-vedova luca-della-vedova changed the title Remove main behavior tree Make main behavior tree configurable Jan 6, 2025
Signed-off-by: Luca Della Vedova <lucadv@intrinsic.ai>
Copy link
Member

@Yadunund Yadunund left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM except for one minor thing.

@@ -132,6 +132,7 @@ def launch_setup(context, *args, **kwargs):
"""{
pick_and_place: [place_on_conveyor, pick_from_conveyor],
}""",
"main_bt_filename": "main.xml",
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Please DeclareLaunchArgument and get the value via LaunchConfiguration instead of hardcoding here. That way, parent launch files like launch.py can overwrite the value if needed.

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Ah totally forgot, 99c0c69. It's not declared in the root launch.py but it is still passed to the control_center.launch.py, I can declare it in both places if it makes it clearer

Signed-off-by: Luca Della Vedova <lucadv@intrinsic.ai>
@Yadunund Yadunund merged commit 7d77a81 into main Jan 6, 2025
2 of 3 checks passed
@Yadunund Yadunund deleted the luca/remove_main_bt branch January 6, 2025 06:05
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