[GraphHopper] Understanding GraphHopper Instructions
Matthias Marquardt
marquardt24 at gmail.com
Sat Jun 28 17:26:27 UTC 2014
I total I have adjusted 3 classes in the core - before I am going to start
a "pull" request I would ask for your opinion - if you would consider to
add this modification - OR if there are good reasons why you implemented
GraphHopper in the way you just did ?!
Normally I would use a static constant for the prefix '@@@_' - but since I
am not familiar
with the code yet, I did not really knew where would be an appropriate
place.
1) Ensure that each edge will have a unique name [when will be parsed from
the OSM-Data]
the 'genFakeUUID()' is just as example - it's very simple and you might not
like that
kind of code in the graphhopper sources - but I am lazy too :-)
file com/graphhopper/routing/uti/EncodingManager
ORIGINAL:
public void applyWayTags( OSMWay way, EdgeIteratorState edge )
{
// storing the road name does not yet depend on the flagEncoder so
manage it directly
if (enableInstructions)
{
// String wayInfo = carFlagEncoder.getWayInfo(way);
// http://wiki.openstreetmap.org/wiki/Key:name
String name = fixWayName(way.getTag("name"));
// http://wiki.openstreetmap.org/wiki/Key:ref
String refName = fixWayName(way.getTag("ref"));
if (!Helper.isEmpty(refName))
{
if (Helper.isEmpty(name))
name = refName;
else
name += ", " + refName;
}
edge.setName(name);
}
for (AbstractFlagEncoder encoder : edgeEncoders)
{
encoder.applyWayTags(way, edge);
}
}
ADJUSTMENT:
public void applyWayTags( OSMWay way, EdgeIteratorState edge )
{
// storing the road name does not yet depend on the flagEncoder so
manage it directly
if (enableInstructions)
{
// String wayInfo = carFlagEncoder.getWayInfo(way);
// http://wiki.openstreetmap.org/wiki/Key:name
String name = fixWayName(way.getTag("name"));
// http://wiki.openstreetmap.org/wiki/Key:ref
String refName = fixWayName(way.getTag("ref"));
if (!Helper.isEmpty(refName))
{
if (Helper.isEmpty(name))
name = refName;
else
name += ", " + refName;
}
if(Helper.isEmpty(name)){
name = "@@@_"+genFakeUUID();
}
edge.setName(name);
}
for (AbstractFlagEncoder encoder : edgeEncoders)
{
encoder.applyWayTags(way, edge);
}
}
private String lastUsedId = "";
private String genFakeUUID(){
// for sure this UUID generator might be "overkill" cause of
// performance issues
// return java.util.UUID.randomUUID().toString();
// this is for sure not ThreadSafe!
String ret = Long.toString(System.currentTimeMillis());
if(ret.equals(lastUsedId)){
ret = lastUsedId+"X";
}
lastUsedId = ret;
return ret;
}
2) Simple adjustment in order to ensure, that our "fake names" will be
never displayed
in the instruction list
file: com/graphhopper/util/Instruction.java
ORIGINAL:
public Instruction( int sign, String name, InstructionAnnotation ia,
PointList pl )
{
this.sign = sign;
this.name = name;
this.points = pl;
this.annotation = ia;
}
ADJUSTMENT:
public Instruction( int sign, String name, InstructionAnnotation ia,
PointList pl )
{
this.sign = sign;
if(!name.startsWith("@@@_")){
this.name = name;
}else{
this.name = "";
}
this.points = pl;
this.annotation = ia;
}
3) the final adjustment in the Instruction generation process - since the
data is already
adjusted, that each edge will have a name, the instructions are more or
less fine. The
only reason why I found it necessary to modify the list generation is, that
I wanted to
get rid of the 'Straight ahead' instructions when traveling from one
'fakedName' section
to another... -> as long as it's straight ahead, there is IMHO no need for
an instruction.
Please note, that I have moved the creation of the points object into the
if(!(name.startsWith("@@@_") ... section
file: com/graphhopper/routing/Path.java
ORIGINAL:
public InstructionList calcInstructions( final Translation tr )
{
...
forEveryEdge(new EdgeVisitor()
{
...
@Override
public void next( EdgeIteratorState edge, int index )
{
...
double orientation = ac.calcOrientation(prevLat, prevLon,
latitude, longitude);
if (name == null)
{
...
} else
{
double tmpOrientation =
ac.alignOrientation(prevOrientation, orientation);
String tmpName = edge.getName();
InstructionAnnotation tmpAnnotation =
encoder.getAnnotation(flags, tr);
if ((!name.equals(tmpName))
|| (!annotation.equals(tmpAnnotation)))
{
points = new PointList(10, nodeAccess.is3D());
name = tmpName;
annotation = tmpAnnotation;
...
prevInstruction = new Instruction(sign, name,
annotation, points);
cachedWays.add(prevInstruction);
}
updatePointsAndInstruction(edge, wayGeo);
}
...
}
...
});
return cachedWays;
}
ADJUSTMENT:
public InstructionList calcInstructions( final Translation tr )
{
...
forEveryEdge(new EdgeVisitor()
{
...
@Override
public void next( EdgeIteratorState edge, int index )
{
...
double orientation = ac.calcOrientation(prevLat, prevLon,
latitude, longitude);
if (name == null)
{
...
} else
{
double tmpOrientation =
ac.alignOrientation(prevOrientation, orientation);
String tmpName = edge.getName();
InstructionAnnotation tmpAnnotation =
encoder.getAnnotation(flags, tr);
if ((!name.equals(tmpName))
|| (!annotation.equals(tmpAnnotation)))
{
name = tmpName;
annotation = tmpAnnotation;
...
// do not add the instruction, if we are traveling
from one
// 'no-name' path to another 'no-name' path and
it's just
// straight ahead (no turn required - it might make
sense
// to make this configurable?!)
if(!(name.startsWith("@@@_") &&
sign==Instruction.CONTINUE_ON_STREET) ){
points = new PointList(10,
nodeAccess.is3D());
prevInstruction = new Instruction(sign, name,
annotation, points);
cachedWays.add(prevInstruction);
}
}
updatePointsAndInstruction(edge, wayGeo);
}
...
}
...
});
return cachedWays;
}
2014-06-28 19:22 GMT+02:00 Matthias Marquardt <marquardt24 at gmail.com>:
> Peter - would you mind, to review my suggestion for an adjustment in the
> Instruction calculation? - With this changes I was able to create the
> expected TurnByTurn Instructions I am looking forward to receive from
> GraphHopper once I am offroad
>
>
> 2014-06-28 16:20 GMT+02:00 Matthias Marquardt <marquardt24 at gmail.com>:
>
> ... I think I have already found it :-)
>>
>> com.graphhopper.routing.util.EncodingManager.applyWayTags(...)
>>
>> right?!
>>
>>
>> 2014-06-28 16:04 GMT+02:00 Matthias Marquardt <marquardt24 at gmail.com>:
>>
>> Hello,
>>>
>>> my name is Matthias - 43 year old guy from the Bielefeld area. I want to
>>> make use of your offer 'do not hesitate to ask questions' - I am at the
>>> begin of a none commercial outdoor project - before I am going into details
>>> [what I plan to do, and why I expect that GraphHopper is just the 'right'
>>> thing (beside the fact that's pure Java and I am lazy too!)] - I am
>>> currently focusing on the instruction list functionality.
>>>
>>> At the beginning I had difficulties to understand why they are, like
>>> they are - when it comes to short trails like this one:
>>>
>>>
>>> http://graphhopper.com/maps/?point=51.271743%2C8.648402&point=51.274045%2C8.646782&vehicle=bike2&elevation=true
>>>
>>> I am missing at least one (IMHO essential) turn in the resulting
>>> instruction list (in the last section of the path (shortly after leaving
>>> the woods) - without knowing too much details about the used OSD from that
>>> specific area I am even more confused when I move the endpoint to the other
>>> path...
>>>
>>>
>>> http://graphhopper.com/maps/?point=51.271743%2C8.648402&point=51.274421%2C8.648252&vehicle=bike2&elevation=true
>>>
>>> Still only two turns - and the (IMHO essential) instruction is missing -
>>> actually my "expectation" would be, that I would get four (or even five)
>>> turning instructions for this small route.
>>>
>>> Lucky enough the GraphHopper sources are available, so I made my way
>>> though the calls (while having the code running in my IDE debugger) and
>>> realized the root of my "issue" is in the com.graphhopper.routing.Path
>>> class - to be more precise in the lines 416, 417 and 418 (in the
>>> 'forEveryEdge' code)
>>>
>>> if ((!name.equals(tmpName))
>>> || (!annotation.equals(tmpAnnotation)))
>>> {
>>>
>>> While the graph in the given example has six edges the resulting list of
>>> instructions will be so short simply cause the edge objects does not have
>>> any names AND no essential differences in their annotations - this even
>>> explains instantly, why when switching over from bike to foot, that I get
>>> zero turning instructions at all, since the foot encoder does not not make
>>> any differences in the annotations...
>>>
>>> So with other words (moving away from the java object world back to the
>>> real world) - the current GraphHopper version give you only instructions,
>>> IF there is a significant change between the edges [like street name
>>> change, or change from tarmac to gravel] - I do not doubt, that the chosen
>>> implementation is the right way by default - a quick test (removing the
>>> name & annotation check) showed me that with my hacked code, the
>>> instruction list result is finally matching "my expectation".
>>>
>>> My question right now is, since I have realized, that the extracted data
>>> is IMHO the root cause of the issue, in which area of the code I have to
>>> dig for the portion, that is responsible for creating the base data from
>>> which later the edges are created - since my next attempt would be to give
>>> every path that will be extracted from the OSM data a unique name (if not
>>> present) - or would that be a silly idea?! [I think I will add a prefix, so
>>> that when the name later will be used, it can be detected, that this is
>>> just a "internal name" - and that instructions like 'turn left to
>>> MyUUIDPrefix_243211' could be avoided']
>>>
>>> TIA - Matthias
>>>
>>
>>
>>
>> --
>> Matthias Marquardt
>> http://about.me/matthiasmarquardt
>>
>> [FileScout | iMazing | Iconify | TOMPlayer | LittleBrother | GPSLogger II
>> | GPSiesConnect]
>> http://www.emacberry.com
>> <http://about.me/matthiasmarquardt>
>>
>
>
>
> --
> Matthias Marquardt
> http://about.me/matthiasmarquardt
>
> [FileScout | iMazing | Iconify | TOMPlayer | LittleBrother | GPSLogger II
> | GPSiesConnect]
> http://www.emacberry.com
> <http://about.me/matthiasmarquardt>
>
--
Matthias Marquardt
http://about.me/matthiasmarquardt
[FileScout | iMazing | Iconify | TOMPlayer | LittleBrother | GPSLogger II |
GPSiesConnect]
http://www.emacberry.com
<http://about.me/matthiasmarquardt>
-------------- n�chster Teil --------------
Ein Dateianhang mit HTML-Daten wurde abgetrennt...
URL: <http://lists.openstreetmap.org/pipermail/graphhopper/attachments/20140628/fe135339/attachment.html>
More information about the GraphHopper
mailing list