-
Notifications
You must be signed in to change notification settings - Fork 1.2k
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.
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
User misconfiguration of parameters may cause instantaneous crashs #4005
Comments
Thanks for the report - in general its not planned for us to protect users from themselves with every parameter if they choose something impossible. We try where we can, but the codebase would balloon to 2x the size for checking every single parameter for every way that it could be misinterpreted. I don't disagree with you that you could make the system crash on initialization if you set the width/height to a negative value. I put that in the category of "fair" since its not a crash mid-execution, non-deterministically, or during operations. That is a crash you would see immediately when you tried to bring the system up, every time, that could be debugged before proceeding. I take crashes very seriously, but for parameter misconfiguration of this nature, its not realistic today that we're going to protect users from making errors on every one of a thousand parameters. However, that's not an unreasonable ask, so I'm going to rename the ticket in line with that and it'll stay in the tracker so that perhaps in the future when we're better resourced we can potentially have some interns do this, but no promises / commitments. |
@GoesM perhaps your extra memory problems are related to your misconfigurations 😉 Ask on Robotics stack exchange with your config file changes and I'll follow up there on that topic. I don't want tickets to become > 1 topic, so I don't want to pollute this thread with that discussion. I hope that makes sense |
I've forwarded this issue to the Robotics stack exchange stack: question 106008. |
Keep an eye on this thread for potential solutions to add parameter validation without ballooning the code. |
It is already possible with parameter descriptions. It is more a matter of time / inclination to set bounds on all N params in the stack (and make sure the dynamic parameter callbacks are respecting them too). The blockage isn’t the tech, it’s the value proposition of time. You should see the amount of code is required per-parameter in this case. Certainly some can be abstracted into Utils, but still. |
Another example #4157 & impl note: #4157 (comment) |
Bug report
Required Info:
Steps to reproduce issue
launch the navigation2 within defaulted
tb3_simulation_launch.py
as following commandsthere's only one difference between
my_nav2_params.yaml
and defaultednav2_params.yaml
, (we wrongly set the height of local_costmap as -20), as following:and then , try to set an /initial_pose in rviz by
2D_estimate_pose
or send a msg into /initial_pose in another terminalExpected behavior
Actual behavior
process crashed when launching
specifically, during the startup process of nav2, there may be computer lag, terminal crashes, or even a blue screen
abnormal memory usage
we found that nav2_controller process occupies a very large amount of memory (up to 10G, sometimes even exceeding 20G, which is very abnormal), as following:
here's a normal memory usage when we use the defaulted
nav2_params.yaml
, which is for comparision.the max memory usage of whole nav2 is below 1G.
The text was updated successfully, but these errors were encountered: