Skip to content

Bug in parameter configuration in controller. #5100

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom
Open
ajtudela opened this issue Apr 23, 2025 · 2 comments · May be fixed by #5106
Open

Bug in parameter configuration in controller. #5100

ajtudela opened this issue Apr 23, 2025 · 2 comments · May be fixed by #5106

Comments

@ajtudela
Copy link
Contributor

I think I have found a bug related to parameter configuration in the controller.

I started the robot as usual this morning and it gave me this configuration error on startup.

Image

It says that The value of parameter 'MPPI.vx_min' is incorrectly set to -0.350000, it should be >=0. Ignoring parameter update..

I looked at the documentation and this is the default value: https://docs.nav2.org/configuration/packages/configuring-mppic.html

I also looked at this part of the code and I found it in the RPP controller!!!

navigation2/nav2_regulated_pure_pursuit_controller/src/parameter_handler.cpp

Lines 230 to 234 in 5423eba

RCLCPP_WARN(
logger_, "The value of parameter '%s' is incorrectly set to %f, "
"it should be >=0. Ignoring parameter update.",
name.c_str(), parameter.as_double());
result.successful = false;

Maybe this is related to this PR #4971
@SteveMacenski
Copy link
Member

SteveMacenski commented Apr 23, 2025 •
edited
Loading

I ... am getting annoyed by the dynamic parameter APIs.

Here's what we do. Check if the parameter name starts with plugin_name_. If it doesn't, then continue in the loop. That way we skip any non-this-plugin parameters set.

Heck, that might also be the solution for MPPI with the non-MPPI parameter changes triggering resets we've been talking about over Slack. Lets do that in MPPI too.

Then, we can rearrange this ticket to updating all Plugins to do the same, since all plugin's parameters are or should be under the plugin's root namespace. I think this is a good global change.

@SteveMacenski SteveMacenski mentioned this issue Apr 24, 2025
Open
@Nils-ChristianIseke
Copy link
Contributor

Nils-ChristianIseke commented Apr 25, 2025 •
edited
Loading

@ajtudela Thank you for catching this. I indeed introduced this bug in #4971. Sry for that! This PR should hopefully fix it: #5106.

@ajtudela ajtudela linked a pull request Apr 25, 2025 that will close this issue
Draft
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
3 participants
@ajtudela @SteveMacenski @Nils-ChristianIseke