Skip to content

Conversation

@mergify
Copy link
Contributor

@mergify mergify bot commented May 29, 2025

Basic Info

Info Please fill out this column
Primary OS tested on Ubuntu,)
Robotic platform tested on None
Does this PR contain AI generated software? No

Description of contribution in a few bullet points

As previously discussed in #5154, as most of the parameters in the stack are associated to default values, it is currently difficult for code maintainers to spot parameters that are not loaded because of typos, wrong namespaces, or because of code changes.

The problem can be solved associating a full definition of the parameters in the yaml configuration files with a mechanism that warns when an association fails.
As this would likely result in a lot of spam for users without a full yaml definition, the current change proposes optionally enabling the feature for a node through a parameter.
This would leave the default policy to not-warning while allowing the user to enable it without modifying source-code.

Description of how this change was tested

Unit tests

Future work that may be required in bullet points

  • Applying the utility functions in the code base to simplify it and enable the functionality

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

This is an automatic backport of pull request #5189 done by [Mergify](https://mergify.com).
…arameter warnings (#5189)

* Adding a parameter warn_when_defaulting_parameters to control default parameter warnings instead of using a flag

Signed-off-by: Marco Bassa <marco.bassa@idealworks.com>

* Adding parameter strict_param_loading for optionally throwing an exception if parameter overrides are missing

Signed-off-by: Marco Bassa <marco.bassa@idealworks.com>

* Using default false declaration instead of declare_or_get in param util

Signed-off-by: Marco Bassa <marco.bassa@idealworks.com>

---------

Signed-off-by: Marco Bassa <marco.bassa@idealworks.com>
(cherry picked from commit 7b5e7e0)
@mergify
Copy link
Contributor Author

mergify bot commented May 29, 2025

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

@SteveMacenski SteveMacenski merged commit 71132a2 into kilted May 29, 2025
11 of 12 checks passed
@mergify mergify bot deleted the mergify/bp/kilted/pr-5189 branch May 29, 2025 04:08
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

3 participants