valhalla
valhalla copied to clipboard
Show distance limit in error message
Issue
Show distance limit in error message when distance is too high, for example distance limit for bicycle is 500000 meters which is 500 km. It will now display that the max limit is 500 km.
Tasklist
- [ ] Add tests
- [ ] Add #fixes with the issue number that this PR addresses
- [ ] Update the docs with any new request parameters or changes to behavior described
- [ ] Update the changelog
- [x] If you made changes to the lua files, update the taginfo too.
Requirements / Relations
Link any requirements here. Other pull requests this PR is based on?
Sorry about that, I didn't know that I declared it fixed, I'm brand new at open source so I'm not completely sure how to do things. To build it on my end after making a change, I just follow the "Building from Source - Windows" heading and this will let me know if the change I made actually works?
yes you have to pass it as the extra parameter, and you'll have to change the tests to match it (loki_service)
honestly im not a huge fan of us changing only a single error message to expose the limit while the bulk of them do not do this. the right thing to do would be either to have them all omit the limit or all show it. would you be game for the latter @reillywynn ? the change is pretty mechanical. find all the places where these types of errors are thrown and std::to_string the integer. we might want to reword some of the messages so that they read better. a quick grep shows the following sources have error codes in the 100s, not all of them will have errors that need this treatment though:
src/loki/isochrone_action.cc
src/loki/locate_action.cc
src/loki/matrix_action.cc
src/loki/polygon_search.cc
src/loki/route_action.cc
src/loki/status_action.cc
src/loki/trace_route_action.cc
src/loki/transit_available_action.cc
src/loki/worker.cc
src/sif/dynamiccost.cc
src/tyr/actor.cc
src/worker.cc
@kevinkreiser yes, I can work on that. Just to clarify, rather than just changing 154 to show the distance limit, anything that has a max limit of something should be changed. (and reworded if need be)? So for example, I would change the error code for max locations also correct? edit: actually some already seem to pass in the max limit
@reillywynn yep that's right! Thank you!
@kevinkreiser I'm having trouble locating where exactly the last portion of the error message is printed when it says "for bicycles" "OSM: Path distance exceeds the max distance limit for bicycle.". The solution I'm working on reads as follows "Path distance exceeds the max distance limit of [max_distance] kilometers". will the "for bikes" populate in there automatically? and also, I think the max distance is in meters correct? If so, would it be appropriate to convert to kilometers before passing the value into valhalla_exception_t or keep it as meters?
regarding the units, everything is in meters in the configs so the messages should match in my opinion.
regarding this blurb:
"OSM: Path distance exceeds the max distance limit for bicycle."
i dont think our software creates this. that "OSM:" prefix is a clue in my opinion. it looks to me that you are getting this from somewhere else rather than directly from valhalla. its totally possible that the thing you are getting it from wraps the valhalla message in "OSM: " and " for bicycle"
okay I will keep it in meters then. That makes sense with the error message because I couldn't find it anywhere. It seems like it pulls the error message and then wraps it with that in openStreetMap.
it looks to me that you are getting this from somewhere else rather than directly from valhalla
Jep, our Valhalla web app running on osm.de adds those bits:) (we also do TomTom & HERE, that’s why we specify “OSM”)
superseded by #3779