-
Notifications
You must be signed in to change notification settings - Fork 57
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
Ambiguous error in rmf_fleet_adapter::agv::parse_graph, no file found when given nav_graph #257
Comments
This is a vexing behavior and I certainly sympathize with your frustration, but I'm not totally convinced that having RMF internally interpret a leading In general if I need to refer to the home directory in a path name that I'm passing into an application, I'll use the
So my current stance on this is "won't fix" but I'll leave this issue open for now in case people think my stance is unreasonable and want to insist that the behavior is changed. |
Oh, I agree, I don't think you should add any hacky parsing to rmf for paths.
I'd be willing to make a PR, if there's a way you prefer. |
I definitely like option (2). It's always good to be loud and clear about failure modes. Option (1) would be good to do in addition to (2). If you're willing to open a PR for that I'll be more than happy to give it a quick review and merge. |
rmf_ros2/rmf_fleet_adapter/src/rmf_fleet_adapter/agv/parse_graph.cpp
Line 30 in 3c17991
Passing in a valid path, of the form
~/path/to/nav_graph/0.yaml
to parse_graph resulted in a raised exception from yaml-cpp that no file was found.This was tested on self-compiled main, ubuntu 20, ros foxy, using the full_control fleet adapter launch file.
The solution/realization for me was that yaml-cpp was not performing tilde expansion, so a form of
/home/myself/path/to/nav_graph/0.yaml
was sufficient.The main problem is the cryptic-ness of not accepting a shell-valid filepath, when it may get passed in via the shell cmdline args.
I've only tested/suffered the tilde expansion, but symbolic links and relative paths are also common cmdline path inputs.
Maybe this could be short-term fixed with either a comment by the line, though a cleaner long-term interface would resolve the filepath before passing to yaml-cpp, &/or catching the raised no-file-found to give a more specific error msg.
On a deeper note, this seems symptomatic of relying on yaml-cpp to do input (file-format) sanitization, but not doing input (path) sanitization to yaml-cpp. If there's any other deep-ish files that take raw user input, identifying them and listing them somewhere so that they can be checked for appropriate input sanitization would be nice project enhancement.
The text was updated successfully, but these errors were encountered: