Skip to content

Use the new declare_or_get_parameter API for nav2_behavior_tree#5552

Merged
SteveMacenski merged 2 commits intoros-navigation:mainfrom
leander-dsouza:new_param_api-nav2_behavior_tree
Oct 2, 2025
Merged

Use the new declare_or_get_parameter API for nav2_behavior_tree#5552
SteveMacenski merged 2 commits intoros-navigation:mainfrom
leander-dsouza:new_param_api-nav2_behavior_tree

Conversation

@leander-dsouza
Copy link
Copy Markdown
Contributor

@leander-dsouza leander-dsouza commented Sep 26, 2025

Basic Info

Info Please fill out this column
Ticket(s) this addresses #5299
Primary OS tested on Ubuntu
Robotic platform tested on N/A
Does this PR contain AI-generated software? No
Was this PR description generated by AI software? N/A

Description of contribution in a few bullet points

  • Migrated the get_parameter and declare_parameter fields to use the new declare_or_get_parameter API for nav2_behavior_tree.

Description of documentation updates required from your changes

  • N/A

Description of how this change was tested

  • Performed linting validation using pre-commit run --all and testing locally using colcon test

Future work that may be required in bullet points

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-*.

@leander-dsouza leander-dsouza marked this pull request as ready for review September 26, 2025 01:00
@leander-dsouza leander-dsouza force-pushed the new_param_api-nav2_behavior_tree branch 2 times, most recently from e899f63 to 97893c5 Compare September 26, 2025 06:09
@codecov
Copy link
Copy Markdown

codecov bot commented Sep 26, 2025 •
edited
Loading

Codecov Report

✅ All modified and coverable lines are covered by tests.

Files with missing lines Coverage Δ
...clude/nav2_behavior_tree/bt_action_server_impl.hpp 90.39% <100.00%> (+1.44%) ⬆️
..._tree/plugins/condition/goal_reached_condition.cpp 92.59% <100.00%> (-0.52%) ⬇️

... and 11 files with indirect coverage changes

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

SteveMacenski
SteveMacenski requested changes Sep 29, 2025
View reviewed changes
Copy link
Copy Markdown
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.

This is admittedly a hard node to start this project out with, possibly the most complex one with the most trade offs that need to be made for how parameters are processed.

Does this completely handle all parameter calls? I feel like there are alot more in this package

@leander-dsouza leander-dsouza force-pushed the new_param_api-nav2_behavior_tree branch from 97893c5 to 6e517c6 Compare September 30, 2025 18:01
@leander-dsouza
Copy link
Copy Markdown
Contributor Author

leander-dsouza commented Sep 30, 2025

This is admittedly a hard node to start this project out with, possibly the most complex one with the most trade offs that need to be made for how parameters are processed.

Yes, it indeed was a tricky one to start with.

Does this completely handle all parameter calls? I feel like there are alot more in this package

Yes, I have replaced all the parameter calls which have declare_parameter and get_parameter methods in the same file.

@leander-dsouza leander-dsouza force-pushed the new_param_api-nav2_behavior_tree branch from 6e517c6 to 80c5402 Compare September 30, 2025 22:04
@leander-dsouza
Copy link
Copy Markdown
Contributor Author

leander-dsouza commented Sep 30, 2025

I am not sure why all the tests for the costmap_filters are failing.
Namely - test_keepout_filter, test_speed_filter_global, and test_speed_filter_local.

SteveMacenski
SteveMacenski reviewed Oct 1, 2025
View reviewed changes
Copy link
Copy Markdown
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.

Can error_code_name_prefixes be included? I think this could be simplified and use the new API as well.

@leander-dsouza
Use the new declare_or_get_parameter API for nav2_behavior_tree
76dc8a3 
Signed-off-by: Leander Stephen D'Souza <leanderdsouza1234@gmail.com>
@leander-dsouza
Simplified the parameter implemenation for error_code_name_prefixes.
b3dc5d9 
Signed-off-by: Leander Stephen D'Souza <leanderdsouza1234@gmail.com>
@leander-dsouza leander-dsouza force-pushed the new_param_api-nav2_behavior_tree branch from 80c5402 to b3dc5d9 Compare October 2, 2025 12:59
@leander-dsouza
Copy link
Copy Markdown
Contributor Author

leander-dsouza commented Oct 2, 2025

Can error_code_name_prefixes be included? I think this could be simplified and use the new API as well.

I have simplified the section to match the earlier implementation.

@SteveMacenski SteveMacenski merged commit 2a2a709 into ros-navigation:main Oct 2, 2025
16 checks passed
@SteveMacenski SteveMacenski mentioned this pull request Oct 2, 2025
Closed
SakshayMahna pushed a commit to SakshayMahna/navigation2 that referenced this pull request Oct 4, 2025
@leander-dsouza @SakshayMahna
Use the new declare_or_get_parameter API for nav2_behavior_tree (ros-…
2a13f40 
…navigation#5552)

* Use the new declare_or_get_parameter API for nav2_behavior_tree

Signed-off-by: Leander Stephen D'Souza <leanderdsouza1234@gmail.com>

* Simplified the parameter implemenation for error_code_name_prefixes.

Signed-off-by: Leander Stephen D'Souza <leanderdsouza1234@gmail.com>

---------

Signed-off-by: Leander Stephen D'Souza <leanderdsouza1234@gmail.com>
silanus23 pushed a commit to silanus23/navigation2 that referenced this pull request Oct 10, 2025
@leander-dsouza @silanus23
Use the new declare_or_get_parameter API for nav2_behavior_tree (ros-…
e6632eb 
…navigation#5552)

* Use the new declare_or_get_parameter API for nav2_behavior_tree

Signed-off-by: Leander Stephen D'Souza <leanderdsouza1234@gmail.com>

* Simplified the parameter implemenation for error_code_name_prefixes.

Signed-off-by: Leander Stephen D'Souza <leanderdsouza1234@gmail.com>

---------

Signed-off-by: Leander Stephen D'Souza <leanderdsouza1234@gmail.com>
silanus23 pushed a commit to silanus23/navigation2 that referenced this pull request Oct 11, 2025
@leander-dsouza @silanus23
Use the new declare_or_get_parameter API for nav2_behavior_tree (ros-…
a245298 
…navigation#5552)

* Use the new declare_or_get_parameter API for nav2_behavior_tree

Signed-off-by: Leander Stephen D'Souza <leanderdsouza1234@gmail.com>

* Simplified the parameter implemenation for error_code_name_prefixes.

Signed-off-by: Leander Stephen D'Souza <leanderdsouza1234@gmail.com>

---------

Signed-off-by: Leander Stephen D'Souza <leanderdsouza1234@gmail.com>
decwest pushed a commit to decwest/navigation2 that referenced this pull request Dec 11, 2025
@leander-dsouza @decwest
Use the new declare_or_get_parameter API for nav2_behavior_tree (ros-…
6a62c0d 
…navigation#5552)

* Use the new declare_or_get_parameter API for nav2_behavior_tree

Signed-off-by: Leander Stephen D'Souza <leanderdsouza1234@gmail.com>

* Simplified the parameter implemenation for error_code_name_prefixes.

Signed-off-by: Leander Stephen D'Souza <leanderdsouza1234@gmail.com>

---------

Signed-off-by: Leander Stephen D'Souza <leanderdsouza1234@gmail.com>
Signed-off-by: Decwest <fumiyaonishi1016@gmail.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@SteveMacenski SteveMacenski SteveMacenski approved these changes

Assignees

No one assigned

Labels

None yet

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

2 participants

@leander-dsouza @SteveMacenski