Skip to content

Commit

Permalink
Remove (anyway broken) pass_through support for CH, remove ch.force_h…
Browse files Browse the repository at this point in the history
…eading parameter, fix #1763
  • Loading branch information
easbar committed Dec 5, 2019
1 parent c13ea6b commit cfc9766
Show file tree
Hide file tree
Showing 4 changed files with 24 additions and 31 deletions.
5 changes: 0 additions & 5 deletions api/src/main/java/com/graphhopper/util/Parameters.java
Original file line number Diff line number Diff line change
Expand Up @@ -153,11 +153,6 @@ public static final class CH {
* This property name configures at start if the DISABLE parameter can have an effect.
*/
public static final String INIT_DISABLING_ALLOWED = ROUTING_INIT_PREFIX + "ch.disabling_allowed";
/**
* The property name in HintsMap if heading should be used for CH regardless of the possible
* routing errors.
*/
public static final String FORCE_HEADING = "ch.force_heading";
}

/**
Expand Down
1 change: 1 addition & 0 deletions core/files/changelog.txt
Original file line number Diff line number Diff line change
Expand Up @@ -5,6 +5,7 @@
added new `curbside` feature, #1697
moved QueryGraph to new routing.querygraph package
removed GraphExtension, #1783, renamed TurnCostExtension to TurnCostStorage
removed `heading`, `pass_through` and `ch.force_heading` parameters for speed mode/CH, #1763

0.13
removed docker compose file
Expand Down
8 changes: 5 additions & 3 deletions core/src/main/java/com/graphhopper/GraphHopper.java
Original file line number Diff line number Diff line change
Expand Up @@ -1052,10 +1052,12 @@ else if (ALT_ROUTE.equalsIgnoreCase(algoStr))
QueryGraph queryGraph;

if (chFactoryDecorator.isEnabled() && !disableCH) {
boolean forceCHHeading = hints.getBool(CH.FORCE_HEADING, false);
if (!forceCHHeading && request.hasFavoredHeading(0))
throw new IllegalArgumentException("Heading is not (fully) supported for CHGraph. See issue #483");
if (request.hasFavoredHeading(0))
throw new IllegalArgumentException("The 'heading' parameter is currently not supported for speed mode, you need to disable speed mode with `ch.disable=true`. See issue #483");

if (request.getHints().getBool(Routing.PASS_THROUGH, false)) {
throw new IllegalArgumentException("The '" + Parameters.Routing.PASS_THROUGH + "' parameter is currently not supported for speed mode, you need to disable speed mode with `ch.disable=true`. See issue #1765");
}
// if LM is enabled we have the LMFactory with the CH algo!
RoutingAlgorithmFactory chAlgoFactory = tmpAlgoFactory;
if (tmpAlgoFactory instanceof LMAlgoFactoryDecorator.LMRAFactory)
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -31,6 +31,7 @@
import com.graphhopper.util.CmdArgs;
import com.graphhopper.util.Helper;
import com.graphhopper.util.InstructionList;
import com.graphhopper.util.Parameters;
import com.graphhopper.util.details.PathDetail;
import com.graphhopper.util.exceptions.PointOutOfBoundsException;
import com.graphhopper.util.shapes.GHPoint;
Expand Down Expand Up @@ -114,20 +115,6 @@ public void testWrongPointFormat() {
assertTrue("There should be an error " + json.get("message"), json.get("message").asText().contains("Cannot parse point '1234'"));
}

@Test
public void testQueryWithDirections() {
// Note, in general specifying directions does not work with CH, but this is an example where it works
final Response response = app.client().target("http://localhost:8080/route?" + "point=42.496696,1.499323&point=42.497257,1.501501&heading=240&heading=240&ch.force_heading=true").request().buildGet().invoke();
assertEquals(200, response.getStatus());
JsonNode json = response.readEntity(JsonNode.class);
JsonNode infoJson = json.get("info");
assertFalse(infoJson.has("errors"));
JsonNode path = json.get("paths").get(0);
double distance = path.get("distance").asDouble();
assertTrue("distance wasn't correct:" + distance, distance > 960);
assertTrue("distance wasn't correct:" + distance, distance < 970);
}

@Test
public void testQueryWithoutInstructions() {
final Response response = app.client().target("http://localhost:8080/route?point=42.554851,1.536198&point=42.510071,1.548128&instructions=false").request().buildGet().invoke();
Expand All @@ -142,17 +129,25 @@ public void testQueryWithoutInstructions() {
}

@Test
public void testQueryWithStraightVia() {
// Note, in general specifying pass_through does not work with CH, but this is an example where it works
public void testCHWithHeading_error() {
// There are special cases where heading works with node-based CH, but generally it leads to wrong results -> we expect an error
final Response response = app.client().target("http://localhost:8080/route?" + "point=42.496696,1.499323&point=42.497257,1.501501&heading=240&heading=240").request().buildGet().invoke();
assertEquals(400, response.getStatus());
JsonNode json = response.readEntity(JsonNode.class);
assertTrue("There should have been an error response", json.has("message"));
String expected = "The 'heading' parameter is currently not supported for speed mode, you need to disable speed mode with `ch.disable=true`. See issue #483";
assertTrue("There should be an error containing " + expected + ", but got: " + json.get("message"), json.get("message").asText().contains(expected));
}

@Test
public void testCHWithPassThrough_error() {
// There are special cases where pass_through works with node-based CH, but generally it leads to wrong results -> we expect an error
final Response response = app.client().target("http://localhost:8080/route?point=42.534133,1.581473&point=42.534781,1.582149&point=42.535042,1.582514&pass_through=true").request().buildGet().invoke();
assertEquals(200, response.getStatus());
assertEquals(400, response.getStatus());
JsonNode json = response.readEntity(JsonNode.class);
JsonNode infoJson = json.get("info");
assertFalse(infoJson.has("errors"));
JsonNode path = json.get("paths").get(0);
double distance = path.get("distance").asDouble();
assertTrue("distance wasn't correct:" + distance, distance > 320);
assertTrue("distance wasn't correct:" + distance, distance < 325);
assertTrue("There should have been an error response", json.has("message"));
String expected = "The '" + Parameters.Routing.PASS_THROUGH + "' parameter is currently not supported for speed mode, you need to disable speed mode with `ch.disable=true`. See issue #1765";
assertTrue("There should be an error containing " + expected + ", but got: " + json.get("message"), json.get("message").asText().contains(expected));
}

@Test
Expand Down

0 comments on commit cfc9766

Please sign in to comment.