-
Notifications
You must be signed in to change notification settings - Fork 87
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
Migrate plansys2_bt_actions
to BehaviorTree.ROS2
wrappers
#304
base: rolling
Are you sure you want to change the base?
Conversation
Failing. The workflow env seems to be missing |
Yes, I opened #306 , but something strange is happening here... |
CI seems to be fixed, but it is applying old pipeline. Can you rebase/merge? |
I updated to current rolling. No merge conflicts. Error: https://github.com/PlanSys2/ros2_planning_system/actions/runs/8506248137/job/23296137377#step:4:227 Related to setting up ROS2 env. |
Now, the errors are related to the library you included:
|
I am not completely happy including a new dependency. I would rather prefer to use parameters and avoiding |
I used json to parse argument pairs. Because I wanted to be able to specify the action/service/topic names. Like this: move_cmd = Node(
package='plansys2_bt_actions',
executable='bt_action_node',
name='move',
namespace=namespace,
output='screen',
parameters=[
{
'action_name': 'move',
'bt_xml_file': example_dir + '/behavior_trees_xml/move.xml',
'plugins': json.dumps([
{'name': 'plansys2_NODE_A_bt_node', 'ast': 'topic_a'},
{'name': 'plansys2_NODE_B_bt_node', 'ast': 'topic_b'},
{'name': 'plansys2_NODE_C_bt_node', 'ast': 'topic_c'} ])
}
]) I have altered it and removed the json dependency. I now use a double array: move_cmd = Node(
package='plansys2_bt_actions',
executable='bt_action_node',
name='move',
namespace=namespace,
output='screen',
parameters=[
{
'action_name': 'move',
'bt_xml_file': example_dir + '/behavior_trees_xml/move.xml',
'plugin_names': [
'plansys2_NODE_A_bt_node',
'plansys2_NODE_B_bt_node',
'plansys2_NODE_C_bt_node'
],
'plugin_asts': [
'action_A',
'service_A',
'topic_A'
]
}
]
) NOTE: the namespace specified during launch is added as prefix to AST's, e.g. to enable multiple name-spaced robots. Does this look acceptable to you? And do you agree this feature should be added? I also will create an example in the plansys2 examples when this PR is approved or pre-approved. Also, I just looked at the BehaviorTree.ROS2 repo for some examples. |
…lanning_system into behaviortree_ros2
It is still failing:
I think |
If you did a marge with `rolling,' BT.CPP 4 should be in the CI workflow. May you check it? |
I did a merge with
I build from source |
…lanning_system into behaviortree_ros2
NOTE: Work in progress, needs testing!
Changelog
Added
behaviortree_ros2/ros_node_params.hpp
,behaviortree_ros2/plugins.hpp
node_namespace_
and methodcreateFullyQualifiedName
inBTAction
to handle namespace qualification inplansys2_bt_actions/src/plansys2_bt_actions/BTAction.cpp
.BTAction::on_configure
to handle plugin configurations dynamically.You can now specify the server node names used by the wrappers (
ast
stands for action, service or topic). I also chose to drop theconfig/params.yaml
for readability in my application (I will change theplansys2_bt_example
later).Changed
BTAction::BTAction
constructor to use a single string for theplugins
parameter instead of a vector of strings inplansys2_bt_actions/src/plansys2_bt_actions/BTAction.cpp
.BTAction::on_configure
for enhanced flexibility and error handling, including support for both node and ROS node registrations from plugins.Notes
test
directory in the CMakeLists conditional compilation block is now commented out. Tests should be changed to reflect the changes made in this PR.ros2_planning_system_examples/plansys2_bt_example
has not been adapted for these changes yet. I have testedaction
,service
andtopic sub
wrappers and they seem to work perfectly.