Skip to content

Use declare_or_get_param API#5372

Merged
SteveMacenski merged 3 commits intoros-navigation:mainfrom
elsayedelsheikh:use_new_param_api_bt_navigator
Jul 23, 2025
Merged

Use declare_or_get_param API#5372
SteveMacenski merged 3 commits intoros-navigation:mainfrom
elsayedelsheikh:use_new_param_api_bt_navigator

Conversation

@elsayedelsheikh
Copy link
Contributor

@elsayedelsheikh elsayedelsheikh commented Jul 20, 2025 •
edited
Loading

Basic Info

Info Please fill out this column
Ticket(s) this addresses #5299
Primary OS tested on Ubuntu
Robotic platform tested on Gazebo
Does this PR contain AI generated software? No

Description of contribution in a few bullet points

Addresses #5299
Use declare_or_get_param API instead of declare_parameter - get_parameter

Target packages:

  • nav2_bt_navigator

Description of documentation updates required from your changes

Nothing

Description of how this change was tested

  • Unit tests

Future work

  • Add declare_parameter_if_not_declared to nav2::LifecycleNode so that we could do node->declare_parameter_if_not_declared

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

@elsayedelsheikh elsayedelsheikh mentioned this pull request Jul 20, 2025
Closed
@mergify
Copy link
Contributor

mergify bot commented Jul 20, 2025

@elsayedelsheikh, 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).

1 similar comment
@mergify
Copy link
Contributor

mergify bot commented Jul 20, 2025

@elsayedelsheikh, 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 requested changes Jul 22, 2025
View reviewed changes
@elsayedelsheikh elsayedelsheikh marked this pull request as draft July 22, 2025 16:28
@elsayedelsheikh elsayedelsheikh force-pushed the use_new_param_api_bt_navigator branch from 3fbf08d to de9a12f Compare July 22, 2025 18:49
@mergify
Copy link
Contributor

mergify bot commented Jul 22, 2025

@elsayedelsheikh, 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).

@elsayedelsheikh elsayedelsheikh marked this pull request as ready for review July 22, 2025 19:54
@mergify
Copy link
Contributor

mergify bot commented Jul 22, 2025

@elsayedelsheikh, 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).

@elsayedelsheikh
Copy link
Contributor Author

elsayedelsheikh commented Jul 22, 2025

@SteveMacenski
ci/circleci Build timed out

[59min 11.7s] [33/37 complete] [2 ongoing] ...
context deadline exceeded

@SteveMacenski
Copy link
Member

SteveMacenski commented Jul 23, 2025

I just rekicked it. Let me know if it fails again

@codecov
Copy link

codecov bot commented Jul 23, 2025 •
edited
Loading

Codecov Report

All modified and coverable lines are covered by tests ✅

Files with missing lines Coverage Δ
nav2_bt_navigator/src/bt_navigator.cpp 91.80% <100.00%> (-1.54%) ⬇️
...avigator/src/navigators/navigate_through_poses.cpp 87.91% <100.00%> (-0.33%) ⬇️
...2_bt_navigator/src/navigators/navigate_to_pose.cpp 81.01% <100.00%> (-2.14%) ⬇️

... and 5 files with indirect coverage changes

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

@elsayedelsheikh
Rebase
7d4f229 
Signed-off-by: ElSayed ElSheikh <elsayed.elsheikh97@gmail.com>
@elsayedelsheikh
Fix
423f99b 
Signed-off-by: ElSayed ElSheikh <elsayed.elsheikh97@gmail.com>
@elsayedelsheikh
Feedback
0a8b887 
Signed-off-by: ElSayed ElSheikh <elsayed.elsheikh97@gmail.com>
@elsayedelsheikh elsayedelsheikh force-pushed the use_new_param_api_bt_navigator branch from 28f1f8d to 0a8b887 Compare July 23, 2025 09:52
@elsayedelsheikh
Copy link
Contributor Author

elsayedelsheikh commented Jul 23, 2025

@SteveMacenski Ready to merge!

@SteveMacenski SteveMacenski merged commit 3e4a7a3 into ros-navigation:main Jul 23, 2025
16 checks passed
SakshayMahna pushed a commit to SakshayMahna/navigation2 that referenced this pull request Aug 8, 2025
@elsayedelsheikh @SakshayMahna
Use declare_or_get_param API (ros-navigation#5372)
856ce8e 
* Rebase

Signed-off-by: ElSayed ElSheikh <elsayed.elsheikh97@gmail.com>

* Fix

Signed-off-by: ElSayed ElSheikh <elsayed.elsheikh97@gmail.com>

* Feedback

Signed-off-by: ElSayed ElSheikh <elsayed.elsheikh97@gmail.com>

---------

Signed-off-by: ElSayed ElSheikh <elsayed.elsheikh97@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

@elsayedelsheikh @SteveMacenski