RouteResultBuilder Issue

[marq24]

Hi,

as I started in the google groups
https://groups.google.com/d/msg/openrouteservice/o5-ZTUZVP-I/vU8C2rvuBwAJ

I believe there is an issue with the route instruction builder... I have collected few more examples:

Here BOTH of the roundabout turning instructions will be removed

https://maps.openrouteservice.org/directions?n1=51.830152&n2=8.446485&n3=17&a=51.831362,8.448905,51.828949,8.444045&b=0&c=0&k1=en-US&k2=km

while here

https://maps.openrouteservice.org/directions?n1=51.830152&n2=8.446485&n3=17&a=51.831362,8.448905,51.828995,8.445407&b=0&c=0&k1=en-US&k2=km

they will be keept...

I am not 100% sure, but I believe that the root cause of this issue can be found in the heigit.ors.routing.RouteResultBuilder - starting from line 220

// merge route steps with similar names
here TurnInstructions of the same type will be merged, if the "name1" contains "name2" or vice versa - in the code there is an example URL which should demonstrate why this code can be useful - unfortunately I am not able to check/understand this route [it's quite long] - so might be the code is removing some turn instructions with a good reason - but the check

if (prevStep != null && instrType == prevInstrType && canMergeInstructions(instr.getName(), prevInstr.getName()))
is IMHO not correct (or way to less) - in my initial case two right turns will be merged into one - or in my second example the roundabout turn instructions will be merged...

So IF this merging is required (in the given example URL) then you should add additional checks for specific turn types ONLY - and second check which might be helpful is the distance between the the "possible merge targets" ?

Thanks for spending time with this issue report - if you want/need more examples - please let me know ;-)

Cheers
Matthias

[marq24]

I finally manages to debug the development branch in my windows IDE...

So I was able to verify my initial assumption - RouteResultBuilder is the source of the issue. Simply skipping the "canBeMerged" check and always perform the "else" returning the expected turn instructions.

http://localhost:8080/openrouteservice-4.5.0/routes?profile=driving-car&coordinates=8.448905,51.831362%7C8.444045,51.8289496&prettify_instructions=true

And even in the given example URL from the source code I see, that four times a turn instruction will be merged with a previous one... (I just have evaluated the first one)

First the Instruction:

prevInstr (-2,Görresstraße,85.684,20564) points (49.393344098180776,8.691466793972257,122.0)

will be merged with

instr (-2,Görresstraße,107.235,25736) points (49.39411467449624,8.6914662351787,122.0), (49.394175582993974,8.690967232532067,120.0), (49.394283057621486,8.690034233555963,120.0)

I am sorry to say - but this is an error - I tried to illustrate it here:

https://maps.openrouteservice.org/directions?n1=49.393816&n2=8.690749&n3=17&a=49.393344,8.691467,49.394115,8.691466,49.394176,8.690967,49.394283,8.690034&b=0&c=0&k1=en-US&k2=km

where we se (as A) the prevIntr point(s) and as 1,2 & B the locations of the 'Instr' points

to travel from the Hohe Gasse (via Görrerstraße) to Rohrbacher-Straße you need to make to times are left turn!

Merging these instructions is an error.

@Rungee can you remember what was your initial thought, when you started with this section of the ResultBuilder? - IMHO the complete if(){...} should be removed

[rabidllama]

Hi @marq24 - thanks, I will have a look into the logic behind things and see if I can see if there was a reason for the merging of instructions

@TimMcCauley - do you know the reasoning behind the previous logic?

[marq24]

It could be the case, that in the past GH had returned duplicated turn instructions for some reasons - IF this was the case, then the implemented check was/is insufficient - nevertheless, IF it was a GH issue then this should had been addressed within the GH sources...

It might be that case, that the code was trying to address issues like this one here:

https://maps.openrouteservice.org/directions?n1=51.763262&n2=8.722731&n3=17&a=51.763099,8.719132,51.762953,8.720258&b=0&c=0&k1=en-US&k2=km

where you could get two "slight right turns" shortly after each other...

but again - then the distance between the two merged turn information is IMHO essential...

in the routing example from the source code (Görrerstraße) the merging of the two right turns is for sure not correct...

As you might have noticed I have already made a PR where I removed the merging completely

[TimMcCauley]

@rabidllama no idea, sorry..

[marq24]

:-D Yeah - I'll fix that ;*)