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

Migrate plansys2_bt_actions to BehaviorTree.ROS2 wrappers #304

Open
wants to merge 10 commits into
base: rolling
Choose a base branch
from

Conversation

robodrome
Copy link
Contributor

@robodrome robodrome commented Mar 14, 2024

NOTE: Work in progress, needs testing!

Changelog

Added

  • Include statements for behaviortree_ros2/ros_node_params.hpp, behaviortree_ros2/plugins.hpp
  • New private member node_namespace_ and method createFullyQualifiedName in BTAction to handle namespace qualification in plansys2_bt_actions/src/plansys2_bt_actions/BTAction.cpp.
  • Dual array parsing for plugins in 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 the config/params.yaml for readability in my application (I will change the plansys2_bt_example later).

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'
            ]
        }
    ]
)

Changed

  • Updated BTAction::BTAction constructor to use a single string for the plugins parameter instead of a vector of strings in plansys2_bt_actions/src/plansys2_bt_actions/BTAction.cpp.
  • Refactored plugin loading logic in BTAction::on_configure for enhanced flexibility and error handling, including support for both node and ROS node registrations from plugins.

Notes

  • The new plugin loading approach allows for a more flexible and error-tolerant system, accommodating different types of BehaviorTree.CPP plugins with better diagnostics.
  • The direct addition of the test directory in the CMakeLists conditional compilation block is now commented out. Tests should be changed to reflect the changes made in this PR.
  • The ros2_planning_system_examples/plansys2_bt_example has not been adapted for these changes yet. I have tested action, service and topic sub wrappers and they seem to work perfectly.
  • Please test it yourself before merging.
  • Removed the proposed json dependency.

@robodrome
Copy link
Contributor Author

Failing. The workflow env seems to be missing behaviortree_cpp package

@fmrico
Copy link
Contributor

fmrico commented Mar 19, 2024

Yes, I opened #306 , but something strange is happening here...

@fmrico
Copy link
Contributor

fmrico commented Mar 31, 2024

CI seems to be fixed, but it is applying old pipeline. Can you rebase/merge?

@robodrome
Copy link
Contributor Author

robodrome commented Apr 1, 2024

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.

@fmrico
Copy link
Contributor

fmrico commented Apr 5, 2024

Now, the errors are related to the library you included:

2024-04-01T09:36:35.3583120Z CMake Error at CMakeLists.txt:15 (find_package):
2024-04-01T09:36:35.3584039Z   By not providing "Findnlohmann_json.cmake" in CMAKE_MODULE_PATH this
2024-04-01T09:36:35.3585062Z   project has asked CMake to find a package configuration file provided by
2024-04-01T09:36:35.3585914Z   "nlohmann_json", but CMake did not find one.
2024-04-01T09:36:35.3586348Z 
2024-04-01T09:36:35.3586754Z   Could not find a package configuration file provided by "nlohmann_json"
2024-04-01T09:36:35.3587552Z   with any of the following names:
2024-04-01T09:36:35.3587901Z 
2024-04-01T09:36:35.3588062Z     nlohmann_jsonConfig.cmake
2024-04-01T09:36:35.3588610Z     nlohmann_json-config.cmake
2024-04-01T09:36:35.3588936Z 
2024-04-01T09:36:35.3589375Z   Add the installation prefix of "nlohmann_json" to CMAKE_PREFIX_PATH or set
2024-04-01T09:36:35.3590475Z   "nlohmann_json_DIR" to a directory containing one of the above files.  If
2024-04-01T09:36:35.3591506Z   "nlohmann_json" provides a separate development package or SDK, be sure it
2024-04-01T09:36:35.3592280Z   has been installed.

@fmrico
Copy link
Contributor

fmrico commented Apr 5, 2024

I am not completely happy including a new dependency. I would rather prefer to use parameters and avoiding nlohmann_json.
I also need a better explanation of the utility of this PR :-/

@robodrome
Copy link
Contributor Author

robodrome commented Apr 5, 2024

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.

@robodrome
Copy link
Contributor Author

It is still failing:

-- Build files have been written to: /home/runner/work/ros2_planning_system/ros2_planning_system/ros_ws/build/plansys2_bt_actions
  [ 25%] Building CXX object CMakeFiles/plansys2_bt_actions.dir/src/plansys2_bt_actions/BTAction.cpp.o
  In file included from /home/runner/work/ros2_planning_system/ros2_planning_system/ros_ws/src/b9q00w3heb6/ros2_planning_system/plansys2_bt_actions/src/plansys2_bt_actions/BTAction.cpp:26:
  /home/runner/work/ros2_planning_system/ros2_planning_system/ros_ws/src/b9q00w3heb6/ros2_planning_system/plansys2_bt_actions/include/plansys2_bt_actions/BTAction.hpp:31:10: fatal error: behaviortree_ros2/ros_node_params.hpp: No such file or directory
     31 | #include "behaviortree_ros2/ros_node_params.hpp"
        |          ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~

I think BehaviorTree.ROS2 should be added to the workflow environment, correct?

@fmrico
Copy link
Contributor

fmrico commented Apr 15, 2024

If you did a marge with `rolling,' BT.CPP 4 should be in the CI workflow. May you check it?

@robodrome
Copy link
Contributor Author

robodrome commented Apr 15, 2024

I did a merge with rolling. I update my branch to rolling now. It was behind. Same issue, it can't find:

behaviortree_ros2/ros_node_params.hpp

I build from source humble.

@robodrome robodrome closed this Apr 15, 2024
@robodrome robodrome reopened this Apr 15, 2024
robodrome and others added 5 commits April 15, 2024 09:07
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