Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Add encoded value for mountainbike route relation handling #2904

Merged
merged 4 commits into from
Nov 8, 2023
Merged
Show file tree
Hide file tree
Changes from 2 commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
2 changes: 2 additions & 0 deletions core/src/main/java/com/graphhopper/GraphHopper.java
Original file line number Diff line number Diff line change
Expand Up @@ -698,6 +698,8 @@ protected OSMParsers buildOSMParsers(Map<String, String> vehiclesByName, List<St
if (tagParser instanceof BikeCommonAccessParser) {
if (encodingManager.hasEncodedValue(BikeNetwork.KEY) && added.add(BikeNetwork.KEY))
osmParsers.addRelationTagParser(relConfig -> new OSMBikeNetworkTagParser(encodingManager.getEnumEncodedValue(BikeNetwork.KEY, RouteNetwork.class), relConfig));
if (encodingManager.hasEncodedValue(MtbNetwork.KEY) && added.add(MtbNetwork.KEY))
osmParsers.addRelationTagParser(relConfig -> new OSMMtbNetworkTagParser(encodingManager.getEnumEncodedValue(MtbNetwork.KEY, RouteNetwork.class), relConfig));
if (encodingManager.hasEncodedValue(Smoothness.KEY) && added.add(Smoothness.KEY))
osmParsers.addWayTagParser(new OSMSmoothnessParser(encodingManager.getEnumEncodedValue(Smoothness.KEY, Smoothness.class)));
} else if (tagParser instanceof FootAccessParser) {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -61,7 +61,7 @@ public EncodedValue create(String name, PMap properties) {
return Toll.create();
} else if (TrackType.KEY.equals(name)) {
return TrackType.create();
} else if (BikeNetwork.KEY.equals(name) || FootNetwork.KEY.equals(name)) {
} else if (BikeNetwork.KEY.equals(name) || MtbNetwork.KEY.equals(name) || FootNetwork.KEY.equals(name)) {
return RouteNetwork.create(name);
} else if (Hazmat.KEY.equals(name)) {
return Hazmat.create();
Expand Down
5 changes: 5 additions & 0 deletions core/src/main/java/com/graphhopper/routing/ev/MtbNetwork.java
Original file line number Diff line number Diff line change
@@ -0,0 +1,5 @@
package com.graphhopper.routing.ev;

public class MtbNetwork {
public static final String KEY = RouteNetwork.key("mtb");
}
Original file line number Diff line number Diff line change
Expand Up @@ -190,6 +190,7 @@ private void addDefaultEncodedValues() {
));
if (em.getVehicles().stream().anyMatch(vehicle -> vehicle.contains("bike") || vehicle.contains("mtb") || vehicle.contains("racingbike"))) {
keys.add(BikeNetwork.KEY);
keys.add(MtbNetwork.KEY);
keys.add(GetOffBike.KEY);
keys.add(Smoothness.KEY);
}
Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1,19 @@
package com.graphhopper.routing.util.parsers;

import com.graphhopper.routing.ev.RouteNetwork;

public class BikeNetworkParserHelper {
static RouteNetwork determine(String tag) {
RouteNetwork newBikeNetwork = RouteNetwork.LOCAL;
if ("lcn".equals(tag)) {
newBikeNetwork = RouteNetwork.LOCAL;
} else if ("rcn".equals(tag)) {
newBikeNetwork = RouteNetwork.REGIONAL;
} else if ("ncn".equals(tag)) {
newBikeNetwork = RouteNetwork.NATIONAL;
} else if ("icn".equals(tag)) {
newBikeNetwork = RouteNetwork.INTERNATIONAL;
}
return newBikeNetwork;
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -15,16 +15,16 @@ public class MountainBikePriorityParser extends BikeCommonPriorityParser {
public MountainBikePriorityParser(EncodedValueLookup lookup, PMap properties) {
this(lookup.getDecimalEncodedValue(VehicleSpeed.key(properties.getString("name", "mtb"))),
lookup.getDecimalEncodedValue(VehiclePriority.key(properties.getString("name", "mtb"))),
lookup.getEnumEncodedValue(BikeNetwork.KEY, RouteNetwork.class));
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yes. Usage is supposed to be in a custom model.

But have a look into this change. Shouldn't we keep using BikeNetwork here then? Otherwise normal (and more frequent) route=bicycle tags will be ignored.

Copy link
Contributor Author

@ratrun ratrun Nov 7, 2023

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Now that you ask I agree. I was unsure about it when writing this line. But it makes sense to keep the current behaviour in regards of prioritization for the more frequent route=bicycle tags. I changed it in the PR.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Ok, thanks for your explanation! Can you also revert the change in MountainBikePriorityParser or is this somehow required?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thank you, I forgot that. Now I reverted the network priority changes in MountainBikePriorityParser.

lookup.getEnumEncodedValue(MtbNetwork.KEY, RouteNetwork.class));
}

protected MountainBikePriorityParser(DecimalEncodedValue speedEnc, DecimalEncodedValue priorityEnc,
EnumEncodedValue<RouteNetwork> bikeRouteEnc) {
super(priorityEnc, speedEnc, bikeRouteEnc);

routeMap.put(INTERNATIONAL, PREFER.getValue());
routeMap.put(NATIONAL, PREFER.getValue());
routeMap.put(REGIONAL, PREFER.getValue());
routeMap.put(INTERNATIONAL, BEST.getValue());
routeMap.put(NATIONAL, BEST.getValue());
routeMap.put(REGIONAL, BEST.getValue());
routeMap.put(LOCAL, BEST.getValue());

preferHighwayTags.add("road");
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -41,16 +41,7 @@ public void handleRelationTags(IntsRef relFlags, ReaderRelation relation) {
RouteNetwork oldBikeNetwork = transformerRouteRelEnc.getEnum(false, -1, relIntAccess);
if (relation.hasTag("route", "bicycle")) {
String tag = Helper.toLowerCase(relation.getTag("network", ""));
RouteNetwork newBikeNetwork = RouteNetwork.LOCAL;
if ("lcn".equals(tag)) {
newBikeNetwork = RouteNetwork.LOCAL;
} else if ("rcn".equals(tag)) {
newBikeNetwork = RouteNetwork.REGIONAL;
} else if ("ncn".equals(tag)) {
newBikeNetwork = RouteNetwork.NATIONAL;
} else if ("icn".equals(tag)) {
newBikeNetwork = RouteNetwork.INTERNATIONAL;
}
RouteNetwork newBikeNetwork = BikeNetworkParserHelper.determine(tag);
if (oldBikeNetwork == RouteNetwork.MISSING || oldBikeNetwork.ordinal() > newBikeNetwork.ordinal())
transformerRouteRelEnc.setEnum(false, -1, relIntAccess, newBikeNetwork);
}
Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1,61 @@
/*
* Licensed to GraphHopper GmbH under one or more contributor
* license agreements. See the NOTICE file distributed with this work for
* additional information regarding copyright ownership.
*
* GraphHopper GmbH licenses this file to you under the Apache License,
* Version 2.0 (the "License"); you may not use this file except in
* compliance with the License. You may obtain a copy of the License at
*
* http://www.apache.org/licenses/LICENSE-2.0
*
* Unless required by applicable law or agreed to in writing, software
* distributed under the License is distributed on an "AS IS" BASIS,
* WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
* See the License for the specific language governing permissions and
* limitations under the License.
*/
package com.graphhopper.routing.util.parsers;

import com.graphhopper.reader.ReaderRelation;
import com.graphhopper.reader.ReaderWay;
import com.graphhopper.routing.ev.*;
import com.graphhopper.storage.IntsRef;
import com.graphhopper.util.Helper;

import static com.graphhopper.routing.util.EncodingManager.getKey;

public class OSMMtbNetworkTagParser implements RelationTagParser {
private final EnumEncodedValue<RouteNetwork> bikeRouteEnc;
// used only for internal transformation from relations into edge flags
private final EnumEncodedValue<RouteNetwork> transformerRouteRelEnc = new EnumEncodedValue<>(getKey("mtb", "route_relation"), RouteNetwork.class);

public OSMMtbNetworkTagParser(EnumEncodedValue<RouteNetwork> bikeRouteEnc, EncodedValue.InitializerConfig relConfig) {
this.bikeRouteEnc = bikeRouteEnc;
this.transformerRouteRelEnc.init(relConfig);
}

@Override
public void handleRelationTags(IntsRef relFlags, ReaderRelation relation) {
IntsRefEdgeIntAccess relIntAccess = new IntsRefEdgeIntAccess(relFlags);
RouteNetwork oldBikeNetwork = transformerRouteRelEnc.getEnum(false, -1, relIntAccess);
if (relation.hasTag("route", "mtb")) {
String tag = Helper.toLowerCase(relation.getTag("network", ""));
RouteNetwork newBikeNetwork = BikeNetworkParserHelper.determine(tag);
if (oldBikeNetwork == RouteNetwork.MISSING || oldBikeNetwork.ordinal() > newBikeNetwork.ordinal())
transformerRouteRelEnc.setEnum(false, -1, relIntAccess, newBikeNetwork);
}
}

@Override
public void handleWayTags(int edgeId, EdgeIntAccess edgeIntAccess, ReaderWay way, IntsRef relationFlags) {
// just copy value into different bit range
IntsRefEdgeIntAccess relIntAccess = new IntsRefEdgeIntAccess(relationFlags);
RouteNetwork routeNetwork = transformerRouteRelEnc.getEnum(false, -1, relIntAccess);
bikeRouteEnc.setEnum(false, edgeId, edgeIntAccess, routeNetwork);
}

public EnumEncodedValue<RouteNetwork> getTransformerRouteRelEnc() {
return transformerRouteRelEnc;
}
}
34 changes: 31 additions & 3 deletions core/src/test/java/com/graphhopper/reader/osm/OSMReaderTest.java
Original file line number Diff line number Diff line change
Expand Up @@ -478,11 +478,14 @@ public void testBarriersOnTowerNodes() {
}

@Test
public void testRelation() {
public void testBikeAndMtbRelation() {
EnumEncodedValue<RouteNetwork> bikeNetworkEnc = new EnumEncodedValue<>(BikeNetwork.KEY, RouteNetwork.class);
EncodingManager manager = new EncodingManager.Builder().add(bikeNetworkEnc).build();
EnumEncodedValue<RouteNetwork> mtbNetworkEnc = new EnumEncodedValue<>(MtbNetwork.KEY, RouteNetwork.class);
EncodingManager manager = new EncodingManager.Builder().add(mtbNetworkEnc).add(bikeNetworkEnc).build();
OSMParsers osmParsers = new OSMParsers()
.addRelationTagParser(relConf -> new OSMBikeNetworkTagParser(bikeNetworkEnc, relConf));
.addRelationTagParser(relConf -> new OSMBikeNetworkTagParser(bikeNetworkEnc, relConf))
.addRelationTagParser(relConf -> new OSMMtbNetworkTagParser(mtbNetworkEnc, relConf));

ReaderRelation osmRel = new ReaderRelation(1);
osmRel.add(new ReaderRelation.Member(ReaderElement.Type.WAY, 1, ""));
osmRel.add(new ReaderRelation.Member(ReaderElement.Type.WAY, 2, ""));
Expand All @@ -494,6 +497,7 @@ public void testRelation() {
osmRel.setTag("route", "bicycle");
osmRel.setTag("network", "lcn");

// bikeNetworkEnc and MtbNetworkEnc share the same instance of the relFlags
IntsRef relFlags = manager.createRelationFlags();
IntsRefEdgeIntAccess intAccess = new IntsRefEdgeIntAccess(relFlags);
int edgeId = 0;
Expand All @@ -512,6 +516,30 @@ public void testRelation() {
osmParsers.handleRelationTags(osmRel, relFlags);
assertEquals(RouteNetwork.NATIONAL, transformEnc.getEnum(false, edgeId, intAccess));
assertNotEquals(before, relFlags);

// The further tests below are for an edge which is part of a bike and another mountainbike relation
osmRel.clearTags();

// this is pretty ugly: the mtb network parser writes to the edge flags we pass into it, but at a location we
// don't know, so we need to get the internal enc to read the flags below
transformEnc = ((OSMMtbNetworkTagParser) osmParsers.getRelationTagParsers().get(1)).getTransformerRouteRelEnc();

osmRel.setTag("route", "mtb");
osmRel.setTag("network", "lcn");

osmParsers.handleRelationTags(osmRel, relFlags);
assertEquals(RouteNetwork.LOCAL, transformEnc.getEnum(false, edgeId, intAccess));

// unchanged network
osmParsers.handleRelationTags(osmRel, relFlags);
assertEquals(RouteNetwork.LOCAL, transformEnc.getEnum(false, edgeId, intAccess));
assertEquals(RouteNetwork.LOCAL, transformEnc.getEnum(false, edgeId, intAccess));

// overwrite network
osmRel.setTag("network", "ncn");
osmParsers.handleRelationTags(osmRel, relFlags);
assertEquals(RouteNetwork.NATIONAL, transformEnc.getEnum(false, edgeId, intAccess));
assertNotEquals(before, relFlags);
}

@Test
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -60,6 +60,7 @@ public void setUp() {
priorityParser = (BikeCommonPriorityParser) parsers.getPriorityParser();
osmParsers = new OSMParsers()
.addRelationTagParser(relConfig -> new OSMBikeNetworkTagParser(encodingManager.getEnumEncodedValue(BikeNetwork.KEY, RouteNetwork.class), relConfig))
.addRelationTagParser(relConfig -> new OSMMtbNetworkTagParser(encodingManager.getEnumEncodedValue(MtbNetwork.KEY, RouteNetwork.class), relConfig))
.addWayTagParser(new OSMSmoothnessParser(encodingManager.getEnumEncodedValue(Smoothness.KEY, Smoothness.class)))
.addWayTagParser(accessParser).addWayTagParser(speedParser).addWayTagParser(priorityParser);
priorityEnc = priorityParser.getPriorityEnc();
Expand Down Expand Up @@ -664,7 +665,6 @@ public void testOneway() {
private void assertAccess(ReaderWay way, boolean fwd, boolean bwd) {
EdgeIntAccess edgeIntAccess = new ArrayEdgeIntAccess(1);
int edge = 0;
IntsRef edgeFlags = new IntsRef(1);
IntsRef relationFlags = new IntsRef(1);
accessParser.handleWayTags(edge, edgeIntAccess, way, relationFlags);
if (fwd) assertTrue(accessEnc.getBool(false, edge, edgeIntAccess));
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -134,26 +134,26 @@ public void testSacScale() {
}

@Test
public void testHandleWayTagsInfluencedByRelation() {
public void testHandleWayTagsInfluencedByBikeAndMtbRelation() {
ReaderWay osmWay = new ReaderWay(1);
osmWay.setTag("highway", "track");

ReaderRelation osmRel = new ReaderRelation(1);
// unchanged
assertPriorityAndSpeed(PREFER, 18, osmWay);
assertPriorityAndSpeed(PREFER, 18, osmWay, osmRel);

// relation code is PREFER
osmRel.setTag("route", "bicycle");
osmRel.setTag("network", "lcn");
assertPriorityAndSpeed(PREFER, 18, osmWay);
assertPriorityAndSpeed(PREFER, 18, osmWay, osmRel);

// relation code is PREFER
osmRel.setTag("network", "rcn");
assertPriorityAndSpeed(PREFER, 18, osmWay);
assertPriorityAndSpeed(PREFER, 18, osmWay, osmRel);

// relation code is PREFER
osmRel.setTag("network", "ncn");
assertPriorityAndSpeed(PREFER, 18, osmWay);
assertPriorityAndSpeed(PREFER, 18, osmWay, osmRel);

// PREFER relation, but tertiary road
// => no pushing section but road wayTypeCode and faster
Expand All @@ -162,10 +162,33 @@ public void testHandleWayTagsInfluencedByRelation() {

osmRel.setTag("route", "bicycle");
osmRel.setTag("network", "lcn");
assertPriorityAndSpeed(PREFER, 18, osmWay);
assertPriorityAndSpeed(PREFER, 18, osmWay, osmRel);

osmWay.clearTags();
osmRel.clearTags();
osmWay.setTag("highway", "track");
// unchanged
assertPriorityAndSpeed(PREFER, 18, osmWay, osmRel);

osmRel.setTag("route", "mtb");
osmRel.setTag("network", "lcn");
assertPriorityAndSpeed(BEST, 18, osmWay, osmRel);

osmRel.setTag("network", "rcn");
assertPriorityAndSpeed(BEST, 18, osmWay, osmRel);

osmRel.setTag("network", "ncn");
assertPriorityAndSpeed(BEST, 18, osmWay, osmRel);

osmWay.clearTags();
osmWay.setTag("highway", "tertiary");

osmRel.setTag("route", "mtb");
osmRel.setTag("network", "lcn");
assertPriorityAndSpeed(BEST, 18, osmWay, osmRel);
}

// Issue 407 : Always block kissing_gate execpt for mountainbikes
// Issue 407 : Always block kissing_gate except for mountainbikes
@Test
@Override
public void testBarrierAccess() {
Expand Down