Skip to content

Commit 3546c2e

Browse files
otbutzThomas Butzkarussell
authored
Improve tag extraction (graphhopper#2091)
* Add support for parsing short tons * Extend unittest to check for short ton conversion * Move speed parser logic into OSMValueExtractor * Use Double.NaN for unparseable speed values * Add a test for speedlimit "none" * Use the same Delta for all tests * Add tag extractor test utility * Ignore non-positive speed values Co-authored-by: Thomas Butz <[email protected]> Co-authored-by: Peter <[email protected]>
1 parent b2185be commit 3546c2e

File tree

15 files changed

+278
-144
lines changed

15 files changed

+278
-144
lines changed

‎core/src/main/java/com/graphhopper/routing/util/AbstractFlagEncoder.java

Lines changed: 16 additions & 61 deletions
Original file line numberDiff line numberDiff line change
@@ -23,6 +23,7 @@
2323
import com.graphhopper.reader.osm.conditional.ConditionalOSMTagInspector;
2424
import com.graphhopper.reader.osm.conditional.DateRangeParser;
2525
import com.graphhopper.routing.ev.*;
26+
import com.graphhopper.routing.util.parsers.helpers.OSMValueExtractor;
2627
import com.graphhopper.storage.IntsRef;
2728
import com.graphhopper.util.*;
2829
import org.slf4j.Logger;
@@ -222,20 +223,27 @@ public double getMaxSpeed() {
222223
}
223224

224225
/**
225-
* @return -1 if no maxspeed found
226+
* @return {@link Double#NaN} if no maxspeed found
226227
*/
227228
protected double getMaxSpeed(ReaderWay way) {
228-
double maxSpeed = parseSpeed(way.getTag("maxspeed"));
229-
double fwdSpeed = parseSpeed(way.getTag("maxspeed:forward"));
230-
if (fwdSpeed >= 0 && (maxSpeed < 0 || fwdSpeed < maxSpeed))
229+
double maxSpeed = OSMValueExtractor.stringToKmh(way.getTag("maxspeed"));
230+
double fwdSpeed = OSMValueExtractor.stringToKmh(way.getTag("maxspeed:forward"));
231+
if (isValidSpeed(fwdSpeed) && (!isValidSpeed(maxSpeed) || fwdSpeed < maxSpeed))
231232
maxSpeed = fwdSpeed;
232233

233-
double backSpeed = parseSpeed(way.getTag("maxspeed:backward"));
234-
if (backSpeed >= 0 && (maxSpeed < 0 || backSpeed < maxSpeed))
234+
double backSpeed = OSMValueExtractor.stringToKmh(way.getTag("maxspeed:backward"));
235+
if (isValidSpeed(backSpeed) && (!isValidSpeed(maxSpeed) || backSpeed < maxSpeed))
235236
maxSpeed = backSpeed;
236237

237238
return maxSpeed;
238239
}
240+
241+
/**
242+
* @return <i>true</i> if the given speed is not {@link Double#NaN}
243+
*/
244+
protected boolean isValidSpeed(double speed) {
245+
return !Double.isNaN(speed);
246+
}
239247

240248
@Override
241249
public int hashCode() {
@@ -256,59 +264,6 @@ public boolean equals(Object obj) {
256264
return toString().equals(afe.toString()) && encoderBit == afe.encoderBit && accessEnc.equals(afe.accessEnc);
257265
}
258266

259-
/**
260-
* @return the speed in km/h
261-
*/
262-
public static double parseSpeed(String str) {
263-
if (Helper.isEmpty(str))
264-
return -1;
265-
266-
// on some German autobahns and a very few other places
267-
if ("none".equals(str))
268-
return MaxSpeed.UNLIMITED_SIGN_SPEED;
269-
270-
if (str.endsWith(":rural") || str.endsWith(":trunk"))
271-
return 80;
272-
273-
if (str.endsWith(":urban"))
274-
return 50;
275-
276-
if (str.equals("walk") || str.endsWith(":living_street"))
277-
return 6;
278-
279-
try {
280-
int val;
281-
// see https://en.wikipedia.org/wiki/Knot_%28unit%29#Definitions
282-
int mpInteger = str.indexOf("mp");
283-
if (mpInteger > 0) {
284-
str = str.substring(0, mpInteger).trim();
285-
val = Integer.parseInt(str);
286-
return val * DistanceCalcEarth.KM_MILE;
287-
}
288-
289-
int knotInteger = str.indexOf("knots");
290-
if (knotInteger > 0) {
291-
str = str.substring(0, knotInteger).trim();
292-
val = Integer.parseInt(str);
293-
return val * 1.852;
294-
}
295-
296-
int kmInteger = str.indexOf("km");
297-
if (kmInteger > 0) {
298-
str = str.substring(0, kmInteger).trim();
299-
} else {
300-
kmInteger = str.indexOf("kph");
301-
if (kmInteger > 0) {
302-
str = str.substring(0, kmInteger).trim();
303-
}
304-
}
305-
306-
return Integer.parseInt(str);
307-
} catch (Exception ex) {
308-
return -1;
309-
}
310-
}
311-
312267
/**
313268
* Second parsing step. Invoked after splitting the edges. Currently used to offer a hook to
314269
* calculate precise speed values based on elevation data stored in the specified edge.
@@ -394,7 +349,7 @@ public final BooleanEncodedValue getAccessEnc() {
394349
* @Deprecated
395350
*/
396351
protected void setSpeed(boolean reverse, IntsRef edgeFlags, double speed) {
397-
if (speed < 0 || Double.isNaN(speed))
352+
if (!isValidSpeed(speed))
398353
throw new IllegalArgumentException("Speed cannot be negative or NaN: " + speed + ", flags:" + BitUtil.LITTLE.toBitString(edgeFlags));
399354

400355
if (speed < speedFactor / 2) {
@@ -429,7 +384,7 @@ protected void setSpeed(boolean reverse, IntsRef edgeFlags, double speed) {
429384
protected double applyMaxSpeed(ReaderWay way, double speed) {
430385
double maxSpeed = getMaxSpeed(way);
431386
// We obey speed limits
432-
if (maxSpeed >= 0) {
387+
if (isValidSpeed(maxSpeed)) {
433388
// We assume that the average speed is 90% of the allowed maximum
434389
return maxSpeed * 0.9;
435390
}

‎core/src/main/java/com/graphhopper/routing/util/BikeCommonFlagEncoder.java

Lines changed: 7 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -289,12 +289,11 @@ boolean isSacScaleAllowed(String sacScale) {
289289
@Override
290290
protected double applyMaxSpeed(ReaderWay way, double speed) {
291291
double maxSpeed = getMaxSpeed(way);
292-
if (maxSpeed >= 0) {
293-
// We strictly obey speed limits, see #600
294-
if (speed > maxSpeed)
295-
return maxSpeed;
292+
// We strictly obey speed limits, see #600
293+
if (isValidSpeed(maxSpeed) && speed > maxSpeed) {
294+
return maxSpeed;
296295
}
297-
if (speed > maxPossibleSpeed)
296+
if (isValidSpeed(speed) && speed > maxPossibleSpeed)
298297
return maxPossibleSpeed;
299298
return speed;
300299
}
@@ -453,14 +452,14 @@ void collect(ReaderWay way, double wayTypeSpeed, TreeMap<Double, Integer> weight
453452
}
454453

455454
double maxSpeed = getMaxSpeed(way);
456-
if (preferHighwayTags.contains(highway) || maxSpeed > 0 && maxSpeed <= 30) {
457-
if (maxSpeed < avoidSpeedLimit) {
455+
if (preferHighwayTags.contains(highway) || (isValidSpeed(maxSpeed) && maxSpeed <= 30)) {
456+
if (!isValidSpeed(maxSpeed) || maxSpeed < avoidSpeedLimit) {
458457
weightToPrioMap.put(40d, PREFER.getValue());
459458
if (way.hasTag("tunnel", intendedValues))
460459
weightToPrioMap.put(40d, UNCHANGED.getValue());
461460
}
462461
} else if (avoidHighwayTags.contains(highway)
463-
|| maxSpeed >= avoidSpeedLimit && !"track".equals(highway)) {
462+
|| isValidSpeed(maxSpeed) && maxSpeed >= avoidSpeedLimit && !"track".equals(highway)) {
464463
weightToPrioMap.put(50d, REACH_DEST.getValue());
465464
if (way.hasTag("tunnel", intendedValues))
466465
weightToPrioMap.put(50d, AVOID_AT_ALL_COSTS.getValue());

‎core/src/main/java/com/graphhopper/routing/util/CarFlagEncoder.java

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -305,7 +305,7 @@ protected boolean isOneway(ReaderWay way) {
305305
*/
306306
protected double applyBadSurfaceSpeed(ReaderWay way, double speed) {
307307
// limit speed if bad surface
308-
if (badSurfaceSpeed > 0 && speed > badSurfaceSpeed && way.hasTag("surface", badSurfaceSpeedMap))
308+
if (badSurfaceSpeed > 0 && isValidSpeed(speed) && speed > badSurfaceSpeed && way.hasTag("surface", badSurfaceSpeedMap))
309309
speed = badSurfaceSpeed;
310310
return speed;
311311
}

‎core/src/main/java/com/graphhopper/routing/util/FootFlagEncoder.java

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -279,15 +279,15 @@ void collect(ReaderWay way, TreeMap<Double, Integer> weightToPrioMap) {
279279
weightToPrioMap.put(100d, PREFER.getValue());
280280

281281
double maxSpeed = getMaxSpeed(way);
282-
if (safeHighwayTags.contains(highway) || maxSpeed > 0 && maxSpeed <= 20) {
282+
if (safeHighwayTags.contains(highway) || (isValidSpeed(maxSpeed) && maxSpeed <= 20)) {
283283
weightToPrioMap.put(40d, PREFER.getValue());
284284
if (way.hasTag("tunnel", intendedValues)) {
285285
if (way.hasTag("sidewalk", sidewalksNoValues))
286286
weightToPrioMap.put(40d, AVOID_IF_POSSIBLE.getValue());
287287
else
288288
weightToPrioMap.put(40d, UNCHANGED.getValue());
289289
}
290-
} else if (maxSpeed > 50 || avoidHighwayTags.contains(highway)) {
290+
} else if ((isValidSpeed(maxSpeed) && maxSpeed > 50) || avoidHighwayTags.contains(highway)) {
291291
if (!way.hasTag("sidewalk", sidewalkValues))
292292
weightToPrioMap.put(45d, AVOID_IF_POSSIBLE.getValue());
293293
}

‎core/src/main/java/com/graphhopper/routing/util/HikeFlagEncoder.java

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -73,15 +73,15 @@ void collect(ReaderWay way, TreeMap<Double, Integer> weightToPrioMap) {
7373
weightToPrioMap.put(100d, PREFER.getValue());
7474

7575
double maxSpeed = getMaxSpeed(way);
76-
if (safeHighwayTags.contains(highway) || maxSpeed > 0 && maxSpeed <= 20) {
76+
if (safeHighwayTags.contains(highway) || (isValidSpeed(maxSpeed) && maxSpeed <= 20)) {
7777
weightToPrioMap.put(40d, PREFER.getValue());
7878
if (way.hasTag("tunnel", intendedValues)) {
7979
if (way.hasTag("sidewalk", sidewalksNoValues))
8080
weightToPrioMap.put(40d, REACH_DEST.getValue());
8181
else
8282
weightToPrioMap.put(40d, UNCHANGED.getValue());
8383
}
84-
} else if (maxSpeed > 50 || avoidHighwayTags.contains(highway)) {
84+
} else if ((isValidSpeed(maxSpeed) && maxSpeed > 50) || avoidHighwayTags.contains(highway)) {
8585
if (way.hasTag("sidewalk", sidewalksNoValues))
8686
weightToPrioMap.put(45d, WORST.getValue());
8787
else

‎core/src/main/java/com/graphhopper/routing/util/MotorcycleFlagEncoder.java

Lines changed: 5 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -20,7 +20,9 @@
2020
import com.graphhopper.reader.ReaderWay;
2121
import com.graphhopper.routing.ev.DecimalEncodedValue;
2222
import com.graphhopper.routing.ev.EncodedValue;
23+
import com.graphhopper.routing.ev.MaxSpeed;
2324
import com.graphhopper.routing.ev.UnsignedDecimalEncodedValue;
25+
import com.graphhopper.routing.util.parsers.helpers.OSMValueExtractor;
2426
import com.graphhopper.routing.weighting.CurvatureWeighting;
2527
import com.graphhopper.routing.weighting.PriorityWeighting;
2628
import com.graphhopper.storage.IntsRef;
@@ -177,12 +179,12 @@ public IntsRef handleWayTags(IntsRef edgeFlags, ReaderWay way, EncodingManager.A
177179
double speed = getSpeed(way);
178180
speed = applyMaxSpeed(way, speed);
179181

180-
double maxMCSpeed = parseSpeed(way.getTag("maxspeed:motorcycle"));
181-
if (maxMCSpeed > 0 && maxMCSpeed < speed)
182+
double maxMCSpeed = OSMValueExtractor.stringToKmh(way.getTag("maxspeed:motorcycle"));
183+
if (isValidSpeed(maxMCSpeed) && maxMCSpeed < speed)
182184
speed = maxMCSpeed * 0.9;
183185

184186
// limit speed to max 30 km/h if bad surface
185-
if (speed > 30 && way.hasTag("surface", badSurfaceSpeedMap))
187+
if (isValidSpeed(speed) && speed > 30 && way.hasTag("surface", badSurfaceSpeedMap))
186188
speed = 30;
187189

188190
boolean isRoundabout = roundaboutEnc.getBool(false, edgeFlags);

‎core/src/main/java/com/graphhopper/routing/util/parsers/OSMMaxSpeedParser.java

Lines changed: 17 additions & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -23,7 +23,7 @@
2323
import com.graphhopper.routing.ev.EncodedValueLookup;
2424
import com.graphhopper.routing.ev.MaxSpeed;
2525
import com.graphhopper.routing.ev.RoadClass;
26-
import com.graphhopper.routing.util.AbstractFlagEncoder;
26+
import com.graphhopper.routing.util.parsers.helpers.OSMValueExtractor;
2727
import com.graphhopper.routing.util.spatialrules.SpatialRuleSet;
2828
import com.graphhopper.routing.util.spatialrules.TransportationMode;
2929
import com.graphhopper.storage.IntsRef;
@@ -54,34 +54,41 @@ public void createEncodedValues(EncodedValueLookup lookup, List<EncodedValue> li
5454

5555
@Override
5656
public IntsRef handleWayTags(IntsRef edgeFlags, ReaderWay way, boolean ferry, IntsRef relationFlags) {
57-
double maxSpeed = AbstractFlagEncoder.parseSpeed(way.getTag("maxspeed"));
57+
double maxSpeed = OSMValueExtractor.stringToKmh(way.getTag("maxspeed"));
5858

5959
SpatialRuleSet spatialRuleSet = way.getTag("spatial_rule_set", null);
6060
if (spatialRuleSet != null && spatialRuleSet != SpatialRuleSet.EMPTY) {
6161
RoadClass roadClass = RoadClass.find(way.getTag("highway", ""));
6262
maxSpeed = spatialRuleSet.getMaxSpeed(roadClass, TransportationMode.MOTOR_VEHICLE, maxSpeed);
6363
}
6464

65-
double fwdSpeed = AbstractFlagEncoder.parseSpeed(way.getTag("maxspeed:forward"));
66-
if (fwdSpeed < 0 && maxSpeed > 0)
65+
double fwdSpeed = OSMValueExtractor.stringToKmh(way.getTag("maxspeed:forward"));
66+
if (!isValidSpeed(fwdSpeed) && isValidSpeed(maxSpeed))
6767
fwdSpeed = maxSpeed;
6868
double maxPossibleSpeed = MaxSpeed.UNLIMITED_SIGN_SPEED;
69-
if (fwdSpeed > maxPossibleSpeed)
69+
if (isValidSpeed(fwdSpeed) && fwdSpeed > maxPossibleSpeed)
7070
fwdSpeed = maxPossibleSpeed;
7171

72-
double bwdSpeed = AbstractFlagEncoder.parseSpeed(way.getTag("maxspeed:backward"));
73-
if (bwdSpeed < 0 && maxSpeed > 0)
72+
double bwdSpeed = OSMValueExtractor.stringToKmh(way.getTag("maxspeed:backward"));
73+
if (!isValidSpeed(bwdSpeed) && isValidSpeed(maxSpeed))
7474
bwdSpeed = maxSpeed;
75-
if (bwdSpeed > maxPossibleSpeed)
75+
if (isValidSpeed(bwdSpeed) && bwdSpeed > maxPossibleSpeed)
7676
bwdSpeed = maxPossibleSpeed;
7777

78-
if (fwdSpeed <= 0)
78+
if (!isValidSpeed(fwdSpeed))
7979
fwdSpeed = UNSET_SPEED;
8080
carMaxSpeedEnc.setDecimal(false, edgeFlags, fwdSpeed);
8181

82-
if (bwdSpeed <= 0)
82+
if (!isValidSpeed(bwdSpeed))
8383
bwdSpeed = UNSET_SPEED;
8484
carMaxSpeedEnc.setDecimal(true, edgeFlags, bwdSpeed);
8585
return edgeFlags;
8686
}
87+
88+
/**
89+
* @return <i>true</i> if the given speed is not {@link Double#NaN}
90+
*/
91+
private boolean isValidSpeed(double speed) {
92+
return !Double.isNaN(speed);
93+
}
8794
}

‎core/src/main/java/com/graphhopper/routing/util/parsers/helpers/OSMValueExtractor.java

Lines changed: 62 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -7,7 +7,10 @@
77

88
import com.graphhopper.reader.ReaderWay;
99
import com.graphhopper.routing.ev.DecimalEncodedValue;
10+
import com.graphhopper.routing.ev.MaxSpeed;
1011
import com.graphhopper.storage.IntsRef;
12+
import com.graphhopper.util.DistanceCalcEarth;
13+
import com.graphhopper.util.Helper;
1114

1215
public class OSMValueExtractor {
1316

@@ -43,7 +46,10 @@ public static double stringToTons(String value) {
4346
return Double.NaN;
4447

4548
double factor = 1;
46-
if (value.endsWith("t")) {
49+
if (value.endsWith("st")) {
50+
value = value.substring(0, value.length() - 2);
51+
factor = 0.907194048807;
52+
} else if (value.endsWith("t")) {
4753
value = value.substring(0, value.length() - 1);
4854
} else if (value.endsWith("lbs")) {
4955
value = value.substring(0, value.length() - 3);
@@ -136,4 +142,59 @@ public static boolean isInvalidValue(String value) {
136142
// only support '.' and no German decimals
137143
|| value.contains(",");
138144
}
145+
146+
/**
147+
* @return the speed in km/h
148+
*/
149+
public static double stringToKmh(String str) {
150+
if (Helper.isEmpty(str))
151+
return Double.NaN;
152+
153+
// on some German autobahns and a very few other places
154+
if ("none".equals(str))
155+
return MaxSpeed.UNLIMITED_SIGN_SPEED;
156+
157+
if (str.endsWith(":rural") || str.endsWith(":trunk"))
158+
return 80;
159+
160+
if (str.endsWith(":urban"))
161+
return 50;
162+
163+
if (str.equals("walk") || str.endsWith(":living_street"))
164+
return 6;
165+
166+
int mpInteger = str.indexOf("mp");
167+
int knotInteger = str.indexOf("knots");
168+
int kmInteger = str.indexOf("km");
169+
int kphInteger = str.indexOf("kph");
170+
171+
double factor;
172+
if (mpInteger > 0) {
173+
str = str.substring(0, mpInteger).trim();
174+
factor = DistanceCalcEarth.KM_MILE;
175+
} else if (knotInteger > 0) {
176+
str = str.substring(0, knotInteger).trim();
177+
factor = 1.852; // see https://en.wikipedia.org/wiki/Knot_%28unit%29#Definitions
178+
} else {
179+
if (kmInteger > 0) {
180+
str = str.substring(0, kmInteger).trim();
181+
} else if (kphInteger > 0) {
182+
str = str.substring(0, kphInteger).trim();
183+
}
184+
factor = 1;
185+
}
186+
187+
double value;
188+
try {
189+
value = Integer.parseInt(str) * factor;
190+
} catch (Exception ex) {
191+
return Double.NaN;
192+
}
193+
194+
if (value <= 0) {
195+
return Double.NaN;
196+
}
197+
198+
return value;
199+
}
139200
}

‎core/src/main/java/com/graphhopper/routing/util/spatialrules/SpatialRule.java

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -39,7 +39,7 @@ public interface SpatialRule {
3939
*
4040
* @param roadClass The highway type, e.g. {@link RoadClass#MOTORWAY}
4141
* @param transport The mode of transportation
42-
* @param currentMaxSpeed The current max speed value or -1 if no value has been set yet
42+
* @param currentMaxSpeed The current max speed value or {@link Double#NaN} if no value has been set yet
4343
* @return the maximum speed value to be used
4444
*/
4545
double getMaxSpeed(RoadClass roadClass, TransportationMode transport, double currentMaxSpeed);

‎core/src/main/java/com/graphhopper/routing/util/spatialrules/SpatialRuleSet.java

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -48,7 +48,7 @@ public SpatialRuleSet(List<SpatialRule> rules, int spatialId) {
4848
*
4949
* @param roadClass The highway type, e.g. {@link RoadClass#MOTORWAY}
5050
* @param transport The mode of transportation
51-
* @param currentMaxSpeed The current max speed value or -1 if no value has been set yet
51+
* @param currentMaxSpeed The current max speed value or {@link Double#NaN} if no value has been set yet
5252
* @return the maximum speed value to be used
5353
*/
5454
public double getMaxSpeed(RoadClass roadClass, TransportationMode transport, double currentMaxSpeed) {

0 commit comments

Comments
 (0)