Skip to content

Comments

Modified nav2_behavior_tree and nav2_bt_navigator#3653

Merged
SteveMacenski merged 14 commits intoros-navigation:mainfrom
HarunTeper:main
Jul 28, 2023
Merged

Modified nav2_behavior_tree and nav2_bt_navigator#3653
SteveMacenski merged 14 commits intoros-navigation:mainfrom
HarunTeper:main

Conversation

@HarunTeper
Copy link
Contributor

@HarunTeper HarunTeper commented Jun 24, 2023


Basic Info

Info Please fill out this column
Ticket(s) this addresses #3574
Primary OS tested on Ubuntu
Robotic platform tested on Gazebo

Description of contribution in a few bullet points

For navigators in nav2_bt_navigator, added parameter pass through from bt_navigator to bt_navigator/client_node_name for robot_base_frame and global_frame.
For plugins in nav2_behavior_tree, removed default parameters for behavior tree input ports for robot_base_frame and global_frame, and added the following behavior:

Check behavior tree for user-specified values in behavior tree.
If those are not available, check client_node parameters.
If those are not available, use default values.

Description of documentation updates required from your changes

The parameters global_frame and robot_base frame can now be used by the navigators, instead of the behavior tree specification.


Future work that may be required in bullet points

This could be extended for other parameters or adapted for more plugins/navigators.
For future plugins, the same procedure as in this commit needs to be applied to enable these functionalities.

For Maintainers:

  • Check that any new parameters added are updated in navigation.ros.org
  • Check that any significant change is added to the migration guide
  • Check that any new features OR changes to existing behaviors are reflected in the tuning guide
  • Check that any new functions have Doxygen added
  • Check that any new features have test coverage
  • Check that any new plugins is added to the plugins page
  • If BT Node, Additionally: add to BT's XML index of nodes for groot, BT package's readme table, and BT library lists

… parameters as backup for behavior tree parameters
@mergify
Copy link
Contributor

mergify bot commented Jun 24, 2023

@HarunTeper, your PR has failed to build. Please check CI outputs and resolve issues.
You may need to rebase or pull in main due to API changes (or your contribution genuinely fails).

@SteveMacenski SteveMacenski linked an issue Jun 26, 2023 that may be closed by this pull request
Copy link
Member

@SteveMacenski SteveMacenski left a comment

Choose a reason for hiding this comment

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

Good first stab at it!

@mergify
Copy link
Contributor

mergify bot commented Jul 10, 2023

@HarunTeper, your PR has failed to build. Please check CI outputs and resolve issues.
You may need to rebase or pull in main due to API changes (or your contribution genuinely fails).

vboxuser and others added 2 commits July 10, 2023 19:37
Signed-off-by: vboxuser <vboxuser@ros2-humble.myguest.virtualbox.org>
@HarunTeper
Copy link
Contributor Author

What should I do about the test errors, and the error about the missing bt_conversions.hpp code coverage test.

@SteveMacenski
Copy link
Member

What should I do about the test errors, and the error about the missing bt_conversions.hpp code coverage test.

Look into why they're failing, these tests should not be failing if your PR did not introduce breaking changes and/or caused potentially unintended problem.

W.r.t. adding new tests, we need test coverage for all new features - so the deconflictPortAndParamFrame function needs some tests

Copy link
Member

@SteveMacenski SteveMacenski left a comment

Choose a reason for hiding this comment

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

Just a couple of small things!

@HarunTeper
Copy link
Contributor Author

The last error occurs because I renamed bt_conversions.hpp to bt_utils hpp, and now the code coverage cannot find bt_conversions anymore. Did I forget to rename it somewhere?

@SteveMacenski
Copy link
Member

Try incrementing any instances of v14 to v15 to break the cache https://github.com/ros-planning/navigation2/blob/main/.circleci/config.yml -- you need to do all 3 of them as the same thing (v15)

@HarunTeper
Copy link
Contributor Author

Did that, but now a test didn't generate a results file.

@SteveMacenski
Copy link
Member

Retriggering

@SteveMacenski SteveMacenski merged commit 418fc80 into ros-navigation:main Jul 28, 2023
@SteveMacenski
Copy link
Member

@HarunTeper thanks for your contribution here, this is a really nice update! I appreciate your responsiveness and diligence during this!

enricosutera pushed a commit to enricosutera/navigation2 that referenced this pull request May 19, 2024
* Modified nav2_behavior_tree and nav2_bt_navigator to use bt_navigator parameters as backup for behavior tree parameters

* Modified nav2_behavior_tree and nav2_bt_navigator to use bt_navigator parameters as backup for behavior tree parameters

* implemented feedback from pull request

* fixed RCLCPP_DEBUG error

Signed-off-by: vboxuser <vboxuser@ros2-humble.myguest.virtualbox.org>

* fixed hpp layout, removed previous code line in distance_controller

* fixed all CI/CD errors, modified tests

* fixed bt_navigator coding style. Modified behavior trees for updated nodes

* implemented comments, fixed distance in distance_controller

* updated circleci v14 to v15

---------

Signed-off-by: vboxuser <vboxuser@ros2-humble.myguest.virtualbox.org>
Co-authored-by: vboxuser <vboxuser@ros2-humble.myguest.virtualbox.org>
Signed-off-by: enricosutera <enricosutera@outlook.com>
Marc-Morcos pushed a commit to Marc-Morcos/navigation2 that referenced this pull request Jul 4, 2024
* Modified nav2_behavior_tree and nav2_bt_navigator to use bt_navigator parameters as backup for behavior tree parameters

* Modified nav2_behavior_tree and nav2_bt_navigator to use bt_navigator parameters as backup for behavior tree parameters

* implemented feedback from pull request

* fixed RCLCPP_DEBUG error

Signed-off-by: vboxuser <vboxuser@ros2-humble.myguest.virtualbox.org>

* fixed hpp layout, removed previous code line in distance_controller

* fixed all CI/CD errors, modified tests

* fixed bt_navigator coding style. Modified behavior trees for updated nodes

* implemented comments, fixed distance in distance_controller

* updated circleci v14 to v15

---------

Signed-off-by: vboxuser <vboxuser@ros2-humble.myguest.virtualbox.org>
Co-authored-by: vboxuser <vboxuser@ros2-humble.myguest.virtualbox.org>
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.

RemovePassedGoals Plugin frame parameter inheritance

2 participants