itinero / routing

The routing core of itinero.
Apache License 2.0
221 stars 69 forks source link

Exception with GenerateInstructions #194

Closed TomQv closed 6 years ago

TomQv commented 6 years ago

var instructions = _route.GenerateInstructions();

results in:

System.Collections.Generic.KeyNotFoundException HResult=0x80131577 Message=Key not found in dictionary. Source=mscorlib StackTrace: at System.ThrowHelper.ThrowKeyNotFoundException()

for every route, Germany, Namibia, short, long, doesn't matter. Profile is just Car.Fastest.

xivk commented 6 years ago

I still can't reproduce this.

Do you have a full stack trace? What platform is this on, or is this an issue everywhere? I need more details, I already tried the following to reproduce:

TomQv commented 6 years ago

The problem with all these unittests and functional tests is, its not real life. I'm using other machines, maybe other windows-version, different localisation, maybe different dependencies etc.

In this case, I'm just using your github-package, not the source-code, so I don't have a stack trace, only my first posting.

But will try to figure out more.

xivk commented 6 years ago

Do you do translations, perhaps a string is not in the translation table... I haven't tested this yet but that could be it. I'll change this part of the code later to report on this properly when this happens/

https://github.com/itinero/routing/blob/develop/src/Itinero/Navigation/Instructions/DynamicUnimodalInstructionGenerator.cs#L175

That should give you a proper message, and the fall back could be the default English instead of failing.

TomQv commented 6 years ago

I also thought of this, but even with no or default ILanguageReference the error occurs. Attached is a ToGeoJson of the route, maybe this helps to reproduce the issue. route.zip

var json = _calculatedRoute.ToGeoJson();
var instructions = _calculatedRoute.GenerateInstructions(  );
xivk commented 6 years ago

Ah that's a good idea to reproduce, could you do ToJson() instead of ToGeoJson()? The geojson leaves out branches needed to generate instructions.

TomQv commented 6 years ago

Here it is: route2.zip

But how do you get the original route back from that json?

xivk commented 6 years ago

I found the issue, this is happening because the profile used to generate the route wasn't globally registered in Itinero. Itinero used to, by default, globally register routing profiles once a routerdb was loaded, that was bad because it made it hard work with multiple routerdb's so that was removed but I forgot to anticipate the instructions-case.

Quickfix

So calling this for each profile you use should fix things:

Itinero.Profiles.Profile.Register(yourProfile)

Updates to Itinero

I've updated the code to make it easier to explicitly define a profile when generating instructions or to provide the routerdb that was used to generate the route so it can get the profiles from there. That should be the way to go from now on, removing the need to globally register profiles, they will now either live on their own or in the routerdb. Obviously you can still register them globally but when moving to 2.0 that won't be possible anymore.

Changes done to support this:

What needs to change in your code

starting @ Itinero 1.4.0-pre61

When you still have the routedb around, you best call:

var instructions = route.GenerateInstructions(routerDb);

or

var instructions = route.GenerateInstructions(routerDb, languageRef);

When you don't have the routerdb, you can call:

var instructions = route.GenerateInstructions(profile);

or

var instructions = route.GenerateInstructions(profile, languageRef);

The profile has to match the profile the route was generated with. Generating instructions without the profile is impossible. I can't remove the old way of doing this because that would a breaking change.

xivk commented 6 years ago

Feel free to reopen if not fixed.

TomQv commented 6 years ago

Sorry for late response, was out of office for a couple of days. Thanks for the fix, its working now. I still have the routerdb so my preferred usage is var instructions = route.GenerateInstructions(routerDb, languageRef); This is leading now to the next issue, the instructions.