Skip to content

Groot monitoring support back ported to jazzy#5762

Merged
SteveMacenski merged 15 commits intoros-navigation:jazzyfrom
mikeslembrouckZYGO:feature/jazzy/groot-monitoring
Dec 10, 2025
Merged

Groot monitoring support back ported to jazzy#5762
SteveMacenski merged 15 commits intoros-navigation:jazzyfrom
mikeslembrouckZYGO:feature/jazzy/groot-monitoring

Conversation

@mikeslembrouckZYGO
Copy link

@mikeslembrouckZYGO mikeslembrouckZYGO commented Dec 5, 2025


Basic Info

Info Please fill out this column
Ticket(s) this addresses (add tickets here #1)
Primary OS tested on Ubuntu
Robotic platform tested on Desktop
Does this PR contain AI generated software? No
Was this PR description generated by AI software? No

Description of contribution in a few bullet points

  • Added Groot monitoring support to the Jazzy branch.
  • All changes were copied from the Groot monitoring support feature in the Kilted branch.

Description of documentation updates required from your changes

  • Two new parameters were added to the default nav2_params.yaml configuration:
    • enable_groot_monitoring: false
    • groot_server_port: 1667

Description of how this change was tested

  • I've tested this feature locally on a robot by starting a navigation task using Groot2 Monitor v1.6.1.
  • I've verified that the behavior tree can be monitored live.
  • I've confirmed that the blackboard values can be viewed.

Future work that may be required in bullet points

  • Nothing that I'm aware of

For Maintainers:

  • Check that any new parameters added are updated in docs.nav2.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
  • Should this be backported to current distributions? If so, tag with backport-*.

@mikeslembrouckZYGO mikeslembrouckZYGO changed the title WIP: Groot monitoring support back ported to jazzy Groot monitoring support back ported to jazzy Dec 5, 2025
@mergify
Copy link
Contributor

mergify bot commented Dec 5, 2025

@mikeslembrouckZYGO, all pull requests must be targeted towards the main development branch.
Once merged into main, it is possible to backport to @jazzy, but it must be in main
to have these changes reflected into new distributions.

@mini-1235
Copy link
Collaborator

mini-1235 commented Dec 5, 2025

@mikeslembrouckZYGO, is this ready for review? Also, I am assuming you have fully tested this feature locally, yes?

Check CI, build is failing https://app.circleci.com/pipelines/github/ros-navigation/navigation2/17032/workflows/0544ffeb-6829-43f9-9729-7b577c72a917/jobs/50044

@codecov
Copy link

codecov bot commented Dec 8, 2025

Codecov Report

❌ Patch coverage is 98.59155% with 1 line in your changes missing coverage. Please review.

Files with missing lines Patch % Lines
...ior_tree/include/nav2_behavior_tree/json_utils.hpp 96.96% 1 Missing ⚠️
Files with missing lines Coverage Δ
...nclude/nav2_behavior_tree/behavior_tree_engine.hpp 100.00% <ø> (ø)
...tree/include/nav2_behavior_tree/bt_action_node.hpp 86.04% <ø> (ø)
...ee/include/nav2_behavior_tree/bt_action_server.hpp 83.33% <ø> (ø)
...clude/nav2_behavior_tree/bt_action_server_impl.hpp 88.15% <100.00%> (+0.57%) ⬆️
...ree/include/nav2_behavior_tree/bt_service_node.hpp 81.81% <ø> (-1.26%) ⬇️
...avior_tree/include/nav2_behavior_tree/bt_utils.hpp 100.00% <100.00%> (ø)
...ugins/action/compute_path_through_poses_action.hpp 100.00% <ø> (ø)
...ree/plugins/action/compute_path_to_pose_action.hpp 100.00% <ø> (ø)
...ehavior_tree/plugins/action/follow_path_action.hpp 100.00% <ø> (ø)
..._tree/plugins/action/get_pose_from_path_action.hpp 100.00% <ø> (ø)
... and 19 more

... and 19 files with indirect coverage changes

🚀 New features to boost your workflow:
  • ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.

Signed-off-by: Mike Slembrouck <mike.slembrouck@zygo.be>
Signed-off-by: mikeslembrouckZYGO <mike.slembrouck@zygo.be>
Signed-off-by: Mike Slembrouck <mike.slembrouck@zygo.be>
Removed tests related to WaypointStatus from JsonTest class.

Signed-off-by: mikeslembrouckZYGO <mike.slembrouck@zygo.be>
Signed-off-by: Mike Slembrouck <mike.slembrouck@zygo.be>
Signed-off-by: Mike Slembrouck <mike.slembrouck@zygo.be>
Signed-off-by: Mike Slembrouck <mike.slembrouck@zygo.be>
Signed-off-by: Mike Slembrouck <mike.slembrouck@zygo.be>
Signed-off-by: Mike Slembrouck <mike.slembrouck@zygo.be>
@mikeslembrouckZYGO mikeslembrouckZYGO force-pushed the feature/jazzy/groot-monitoring branch from 4323626 to 36b8585 Compare December 8, 2025 10:42
@mikeslembrouckZYGO
Copy link
Author

mikeslembrouckZYGO commented Dec 8, 2025

@mikeslembrouckZYGO, is this ready for review? Also, I am assuming you have fully tested this feature locally, yes?

Check CI, build is failing https://app.circleci.com/pipelines/github/ros-navigation/navigation2/17032/workflows/0544ffeb-6829-43f9-9729-7b577c72a917/jobs/50044

I've tested the feature locally and the build succeed, so it's ready for review yes. Thx!

@mikeslembrouckZYGO mikeslembrouckZYGO marked this pull request as ready for review December 8, 2025 11:15
Copy link
Collaborator

@mini-1235 mini-1235 left a comment

Choose a reason for hiding this comment

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

Please fix the following item.

In the main branch, we also do BT::RegisterJsonDefinition, is that not needed in this PR?

Signed-off-by: Mike Slembrouck <mike.slembrouck@zygo.be>
@mini-1235
Copy link
Collaborator

You can actually keep the params in nav2_system_tests/src/system/nav2_system_params.yaml, I actually requested to revert the newline in L365

Signed-off-by: Mike Slembrouck <mike.slembrouck@zygo.be>
@mikeslembrouckZYGO
Copy link
Author

Please fix the following item.

In the main branch, we also do BT::RegisterJsonDefinition, is that not needed in this PR?

Ah yes, It should be ok now

Copy link
Collaborator

@mini-1235 mini-1235 left a comment

Choose a reason for hiding this comment

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

I don't think nav_msgs::msg::Goals is the correct type, could you please go through all bt nodes and make sure it register the correct type?

Otherwise LGTM

@@ -362,4 +364,3 @@ docking_server:
k_delta: 2.0
v_linear_min: 0.15
v_linear_max: 0.15
Copy link
Collaborator

Choose a reason for hiding this comment

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

Oh, I think there was a space here instead of newline, please add the line back without space

@mini-1235
Copy link
Collaborator

Also, per https://www.behaviortree.dev/docs/tutorial-basics/tutorial_11_groot2, let's include behaviortree_cpp/json_export.h in the files that you do BT::RegisterJsonDefinition

…:PoseStamped>` instead of `nav_msgs::msg::Goals`

Signed-off-by: Mike Slembrouck <mike.slembrouck@zygo.be>
…export functionality

Signed-off-by: Mike Slembrouck <mike.slembrouck@zygo.be>
@mini-1235
Copy link
Collaborator

Please don't touch other header files, keep the original order

@mikeslembrouckZYGO
Copy link
Author

Please don't touch other header files, keep the original order

Sure, I just followed the include order of the kilted branch, which seemed at bit more cleaned up. Should I revert the order?

@mini-1235
Copy link
Collaborator

Yes, please revert

Signed-off-by: Mike Slembrouck <mike.slembrouck@zygo.be>
Signed-off-by: mikeslembrouckZYGO <mike.slembrouck@zygo.be>
Signed-off-by: Mike Slembrouck <mike.slembrouck@zygo.be>
…lembrouckZYGO/navigation2.git into feature/jazzy/groot-monitoring
@mikeslembrouckZYGO
Copy link
Author

Please don't touch other header files, keep the original order

Done

Copy link
Collaborator

@mini-1235 mini-1235 left a comment

Choose a reason for hiding this comment

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

LGTM. Thanks for reiterating

@SteveMacenski SteveMacenski merged commit 31a9911 into ros-navigation:jazzy Dec 10, 2025
11 checks passed
redvinaa pushed a commit to EnjoyRobotics/navigation2 that referenced this pull request Mar 2, 2026
* Groot monitoring support back ported to jazzy

Signed-off-by: Mike Slembrouck <mike.slembrouck@zygo.be>

* add test_json_utils to CMakeLists.txt

Signed-off-by: mikeslembrouckZYGO <mike.slembrouck@zygo.be>
Signed-off-by: Mike Slembrouck <mike.slembrouck@zygo.be>

* Remove WaypointStatus tests from JsonTest

Removed tests related to WaypointStatus from JsonTest class.

Signed-off-by: mikeslembrouckZYGO <mike.slembrouck@zygo.be>
Signed-off-by: Mike Slembrouck <mike.slembrouck@zygo.be>

* Add JSON parsing support for key conversion in bt_utils

Signed-off-by: Mike Slembrouck <mike.slembrouck@zygo.be>

* Add Groot monitoring support in system tests and parameters

Signed-off-by: Mike Slembrouck <mike.slembrouck@zygo.be>

* Fix uncrustify and flake8 errors

Signed-off-by: Mike Slembrouck <mike.slembrouck@zygo.be>

* Fix extra blank line in nav_to_pose_tester_node.py

Signed-off-by: Mike Slembrouck <mike.slembrouck@zygo.be>

* Remove unused Groot-related code and parameters

Signed-off-by: Mike Slembrouck <mike.slembrouck@zygo.be>

* Add JSON type registration for ports in behavior tree plugins

Signed-off-by: Mike Slembrouck <mike.slembrouck@zygo.be>

* Update JSON type registration to use `std::vector<geometry_msgs::msg::PoseStamped>` instead of `nav_msgs::msg::Goals`

Signed-off-by: Mike Slembrouck <mike.slembrouck@zygo.be>

* Include `json_export` header in behavior tree plugins to enable JSON export functionality

Signed-off-by: Mike Slembrouck <mike.slembrouck@zygo.be>

* Revert header include order

Signed-off-by: Mike Slembrouck <mike.slembrouck@zygo.be>

* Remove unnecessary blank line in header file

Signed-off-by: mikeslembrouckZYGO <mike.slembrouck@zygo.be>

* Remove extra blank lines

Signed-off-by: Mike Slembrouck <mike.slembrouck@zygo.be>

---------

Signed-off-by: Mike Slembrouck <mike.slembrouck@zygo.be>
Signed-off-by: mikeslembrouckZYGO <mike.slembrouck@zygo.be>
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.

3 participants