From c81081c8811e5699b4676ba49ff9372b9bd2bd77 Mon Sep 17 00:00:00 2001 From: Christian Beiwinkel Date: Thu, 22 Feb 2024 18:31:35 +0100 Subject: [PATCH 01/14] added param, simple test --- proto/options.proto | 3 ++ src/sif/dynamiccost.cc | 20 +++++----- src/sif/truckcost.cc | 3 +- test/gurka/test_conditional_restrictions.cc | 5 ++- test/gurka/test_truck_restrictions.cc | 42 +++++++++++++++++++++ valhalla/sif/dynamiccost.h | 5 ++- 6 files changed, 63 insertions(+), 15 deletions(-) diff --git a/proto/options.proto b/proto/options.proto index f6c4ca3194..571ada7730 100644 --- a/proto/options.proto +++ b/proto/options.proto @@ -312,6 +312,9 @@ message Costing { uint32 axle_count = 81; float use_lit = 82; bool disable_hierarchy_pruning = 83; + oneof has_ignore_common_restrictions { + bool ignore_common_restrictions = 84; + } } oneof has_options { diff --git a/src/sif/dynamiccost.cc b/src/sif/dynamiccost.cc index 5416fb7de9..008babef37 100644 --- a/src/sif/dynamiccost.cc +++ b/src/sif/dynamiccost.cc @@ -122,8 +122,8 @@ BaseCostingOptionsConfig::BaseCostingOptionsConfig() : dest_only_penalty_{0.f, kDefaultDestinationOnlyPenalty, kMaxPenalty}, maneuver_penalty_{0.f, kDefaultManeuverPenalty, kMaxPenalty}, alley_penalty_{0.f, kDefaultAlleyPenalty, kMaxPenalty}, - gate_cost_{0.f, kDefaultGateCost, kMaxPenalty}, gate_penalty_{0.f, kDefaultGatePenalty, - kMaxPenalty}, + gate_cost_{0.f, kDefaultGateCost, kMaxPenalty}, + gate_penalty_{0.f, kDefaultGatePenalty, kMaxPenalty}, private_access_penalty_{0.f, kDefaultPrivateAccessPenalty, kMaxPenalty}, country_crossing_cost_{0.f, kDefaultCountryCrossingCost, kMaxPenalty}, country_crossing_penalty_{0.f, kDefaultCountryCrossingPenalty, kMaxPenalty}, @@ -131,15 +131,13 @@ BaseCostingOptionsConfig::BaseCostingOptionsConfig() toll_booth_penalty_{0.f, kDefaultTollBoothPenalty, kMaxPenalty}, ferry_cost_{0.f, kDefaultFerryCost, kMaxPenalty}, use_ferry_{0.f, kDefaultUseFerry, 1.f}, rail_ferry_cost_{0.f, kDefaultRailFerryCost, kMaxPenalty}, - use_rail_ferry_{0.f, kDefaultUseRailFerry, 1.f}, service_penalty_{0.f, kDefaultServicePenalty, - kMaxPenalty}, - service_factor_{kMinFactor, kDefaultServiceFactor, kMaxFactor}, use_tracks_{0.f, - kDefaultUseTracks, - 1.f}, + use_rail_ferry_{0.f, kDefaultUseRailFerry, 1.f}, + service_penalty_{0.f, kDefaultServicePenalty, kMaxPenalty}, + service_factor_{kMinFactor, kDefaultServiceFactor, kMaxFactor}, + use_tracks_{0.f, kDefaultUseTracks, 1.f}, use_living_streets_{0.f, kDefaultUseLivingStreets, 1.f}, use_lit_{0.f, kDefaultUseLit, 1.f}, - closure_factor_{kClosureFactorRange}, exclude_unpaved_(false), - exclude_cash_only_tolls_(false), include_hot_{false}, include_hov2_{false}, include_hov3_{ - false} { + closure_factor_{kClosureFactorRange}, exclude_unpaved_(false), exclude_cash_only_tolls_(false), + include_hot_{false}, include_hov2_{false}, include_hov3_{false} { } DynamicCost::DynamicCost(const Costing& costing, @@ -151,6 +149,7 @@ DynamicCost::DynamicCost(const Costing& costing, closure_factor_(kDefaultClosureFactor), flow_mask_(kDefaultFlowMask), shortest_(costing.options().shortest()), ignore_restrictions_(costing.options().ignore_restrictions()), + ignore_common_restrictions_(costing.options().ignore_common_restrictions()), ignore_oneways_(costing.options().ignore_oneways()), ignore_access_(costing.options().ignore_access()), ignore_closures_(costing.options().ignore_closures()), @@ -389,6 +388,7 @@ void ParseBaseCostOptions(const rapidjson::Value& json, JSON_PBF_DEFAULT(co, false, json, "/ignore_oneways", ignore_oneways); JSON_PBF_DEFAULT(co, false, json, "/ignore_access", ignore_access); JSON_PBF_DEFAULT(co, false, json, "/ignore_closures", ignore_closures); + JSON_PBF_DEFAULT(co, false, json, "/ignore_common_restrictions", ignore_common_restrictions); // shortest JSON_PBF_DEFAULT(co, false, json, "/shortest", shortest); diff --git a/src/sif/truckcost.cc b/src/sif/truckcost.cc index 52eec75ab2..5dac1d43b4 100644 --- a/src/sif/truckcost.cc +++ b/src/sif/truckcost.cc @@ -439,7 +439,8 @@ inline bool TruckCost::Allowed(const baldr::DirectedEdge* edge, uint8_t& restriction_idx) const { // Check access, U-turn, and simple turn restriction. if (!IsAccessible(edge) || (!pred.deadend() && pred.opp_local_idx() == edge->localedgeidx()) || - ((pred.restrictions() & (1 << edge->localedgeidx())) && !ignore_restrictions_) || + ((pred.restrictions() & (1 << edge->localedgeidx())) && + (!ignore_restrictions_ && !ignore_common_restrictions_)) || edge->surface() == Surface::kImpassable || IsUserAvoidEdge(edgeid) || (!allow_destination_only_ && !pred.destonly() && edge->destonly_hgv()) || (pred.closure_pruning() && IsClosed(edge, tile)) || diff --git a/test/gurka/test_conditional_restrictions.cc b/test/gurka/test_conditional_restrictions.cc index d161168d6b..390c3a48d1 100644 --- a/test/gurka/test_conditional_restrictions.cc +++ b/test/gurka/test_conditional_restrictions.cc @@ -97,8 +97,9 @@ class ConditionalRestrictions : public ::testing::Test { }; const auto layout = gurka::detail::map_to_coordinates(ascii_map, grid_size_meters); - map = gurka::buildtiles(layout, ways, {}, {}, "test/data/conditional_restrictions", - {{"mjolnir.timezone", {"test/data/tz.sqlite"}}}); + map = gurka::buildtiles(layout, ways, {}, {}, + VALHALLA_BUILD_DIR "test/data/conditional_restrictions", + {{"mjolnir.timezone", {VALHALLA_BUILD_DIR "test/data/tz.sqlite"}}}); } }; diff --git a/test/gurka/test_truck_restrictions.cc b/test/gurka/test_truck_restrictions.cc index 7368bb8dbf..30dc42be8a 100644 --- a/test/gurka/test_truck_restrictions.cc +++ b/test/gurka/test_truck_restrictions.cc @@ -165,3 +165,45 @@ TEST(TruckSpeed, MaxTruckSpeed) { // expect lower traffic speeds (< kMaxAssumedTruckSpeed ) to lead to a lower duration ASSERT_LT(traffic_time, traffic_low_speed_time); } + +TEST(TruckSpeed, IgnoreCommonRestrictions) { + constexpr double gridsize = 500; + + const std::string ascii_map = R"( + A----------B-----C----D + )"; + + const gurka::ways ways = { + {"AB", {{"highway", "secondary"}}}, + {"BC", {{"highway", "secondary"}}}, + {"CD", {{"highway", "secondary"}}}, + }; + const gurka::relations relations = {{{ + {gurka::way_member, "AB", "from"}, + {gurka::way_member, "BC", "to"}, + {gurka::node_member, "B", "via"}, + }, + {{"type", "restriction"}, {"restriction", "no_straight_on"}}}}; + + const auto layout = gurka::detail::map_to_coordinates(ascii_map, gridsize); + gurka::map map = + gurka::buildtiles(layout, ways, {}, relations, "test/data/truck_ignore_common_restrictions"); + + try { + valhalla::Api route = gurka::do_action(valhalla::Options::route, map, {"A", "D"}, "truck", {}); + FAIL() << "Expected valhalla_exception_t."; + } catch (const valhalla_exception_t& err) { EXPECT_EQ(err.code, 442); } catch (...) { + FAIL() << "Expected valhalla_exception_t."; + } + + valhalla::Api route = + gurka::do_action(valhalla::Options::route, map, {"A", "D"}, "truck", + { + {"/costing_options/truck/ignore_common_restrictions", "1"}, + {"/costing_options/truck/width", "2"}, // lil' cube truck + {"/costing_options/truck/height", "2"}, + {"/costing_options/truck/length", "2"}, + {"/costing_options/truck/weight", "2"}, + }); + gurka::assert::raw::expect_path(route, {"AB", "BC", "CD"}); +} diff --git a/valhalla/sif/dynamiccost.h b/valhalla/sif/dynamiccost.h index fb7ad7eaf0..f07de2cbb4 100644 --- a/valhalla/sif/dynamiccost.h +++ b/valhalla/sif/dynamiccost.h @@ -605,7 +605,7 @@ class DynamicCost { if (access_type == baldr::AccessType::kTimedAllowed) time_allowed = true; - if (current_time == 0) { + if (current_time == 0 || ignore_common_restrictions_) { // No time supplied so ignore time-based restrictions // (but mark the edge (`has_time_restrictions`) continue; @@ -635,7 +635,7 @@ class DynamicCost { // if we have time allowed restrictions then these restrictions are // the only time we can route here. Meaning all other time is restricted. // We looped over all the time allowed restrictions and we were never in range. - return !time_allowed || (current_time == 0); + return (!time_allowed || (current_time == 0)) || ignore_common_restrictions_; } /** @@ -997,6 +997,7 @@ class DynamicCost { bool shortest_; bool ignore_restrictions_{false}; + bool ignore_common_restrictions_{false}; bool ignore_oneways_{false}; bool ignore_access_{false}; bool ignore_closures_{false}; From 260e1f8f2c3c4528e661a17bb26305f2185811fd Mon Sep 17 00:00:00 2001 From: Christian Beiwinkel Date: Fri, 23 Feb 2024 10:51:20 +0100 Subject: [PATCH 02/14] time deny test case --- test/gurka/test_truck_restrictions.cc | 38 +++++++++++++++++++++------ 1 file changed, 30 insertions(+), 8 deletions(-) diff --git a/test/gurka/test_truck_restrictions.cc b/test/gurka/test_truck_restrictions.cc index 30dc42be8a..9c0a84a0e9 100644 --- a/test/gurka/test_truck_restrictions.cc +++ b/test/gurka/test_truck_restrictions.cc @@ -171,12 +171,19 @@ TEST(TruckSpeed, IgnoreCommonRestrictions) { const std::string ascii_map = R"( A----------B-----C----D + | + E + | + | + F )"; const gurka::ways ways = { {"AB", {{"highway", "secondary"}}}, {"BC", {{"highway", "secondary"}}}, {"CD", {{"highway", "secondary"}}}, + {"AE", {{"highway", "secondary"}}}, + {"EF", {{"highway", "secondary"}, {"hgv:conditional", "no @ (09:00-18:00)"}}}, }; const gurka::relations relations = {{{ {gurka::way_member, "AB", "from"}, @@ -187,8 +194,10 @@ TEST(TruckSpeed, IgnoreCommonRestrictions) { const auto layout = gurka::detail::map_to_coordinates(ascii_map, gridsize); gurka::map map = - gurka::buildtiles(layout, ways, {}, relations, "test/data/truck_ignore_common_restrictions"); + gurka::buildtiles(layout, ways, {}, relations, "test/data/truck_ignore_common_restrictions", + {{"mjolnir.timezone", {VALHALLA_BUILD_DIR "test/data/tz.sqlite"}}}); + // first, route through turn restriction, should fail... try { valhalla::Api route = gurka::do_action(valhalla::Options::route, map, {"A", "D"}, "truck", {}); FAIL() << "Expected valhalla_exception_t."; @@ -196,14 +205,27 @@ TEST(TruckSpeed, IgnoreCommonRestrictions) { FAIL() << "Expected valhalla_exception_t."; } + // ...but succeed with ignore_common_restrictions valhalla::Api route = gurka::do_action(valhalla::Options::route, map, {"A", "D"}, "truck", - { - {"/costing_options/truck/ignore_common_restrictions", "1"}, - {"/costing_options/truck/width", "2"}, // lil' cube truck - {"/costing_options/truck/height", "2"}, - {"/costing_options/truck/length", "2"}, - {"/costing_options/truck/weight", "2"}, - }); + {{"/costing_options/truck/ignore_common_restrictions", "1"}}); gurka::assert::raw::expect_path(route, {"AB", "BC", "CD"}); + + // second, route through time based access restrictions, should fail... + try { + valhalla::Api route = + gurka::do_action(valhalla::Options::route, map, {"A", "F"}, "truck", + {{"/date_time/type", "1"}, {"/date_time/value", "2020-10-10T13:00"}}); + FAIL() << "Expected route to fail."; + } catch (const valhalla_exception_t& err) { EXPECT_EQ(err.code, 442); } catch (...) { + FAIL() << "Expected different error code."; + } + + //...but succeed with ignore_common_restrictions + valhalla::Api route_succeed = + gurka::do_action(valhalla::Options::route, map, {"A", "F"}, "truck", + {{"/costing_options/truck/ignore_common_restrictions", "1"}, + {"/date_time/type", "1"}, + {"/date_time/value", "2020-10-10T13:00"}}); + gurka::assert::raw::expect_path(route_succeed, {"AE", "EF"}); } From 076f07bbc04a52a3a1b44519350be8910256a136 Mon Sep 17 00:00:00 2001 From: Christian Beiwinkel Date: Fri, 23 Feb 2024 11:08:09 +0100 Subject: [PATCH 03/14] forgot maxaxle --- src/sif/truckcost.cc | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/sif/truckcost.cc b/src/sif/truckcost.cc index 5dac1d43b4..a86dc845c8 100644 --- a/src/sif/truckcost.cc +++ b/src/sif/truckcost.cc @@ -398,7 +398,7 @@ bool TruckCost::ModeSpecificAllowed(const baldr::AccessRestriction& restriction) } break; case AccessType::kMaxAxles: - if (axle_count_ > static_cast(restriction.value())) { + if (!ignore_common_restrictions_ && (axle_count_ > static_cast(restriction.value()))) { return false; } break; From fc9bcc8adab9139d5d92a6d6f5c2ebdcccbc4acf Mon Sep 17 00:00:00 2001 From: Christian Beiwinkel Date: Sat, 24 Feb 2024 10:35:33 +0100 Subject: [PATCH 04/14] changelog --- CHANGELOG.md | 1 + 1 file changed, 1 insertion(+) diff --git a/CHANGELOG.md b/CHANGELOG.md index da0c684181..ac47a13a07 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -93,6 +93,7 @@ * ADDED: Improved instructions for blind users [#3694](https://github.com/valhalla/valhalla/pull/3694) * FIXED: Fixed roundoff issue in Tiles Row and Col methods [#4585](https://github.com/valhalla/valhalla/pull/4585) * ADDED: isochrone proper polygon support & pbf output for isochrone [#4575](https://github.com/valhalla/valhalla/pull/4575) + * ADDED: `ignore_common_restrictions` parameter for truck costing [#4606](https://github.com/valhalla/valhalla/pull/4606) ## Release Date: 2023-05-11 Valhalla 3.4.0 * **Removed** From ad940fc98f8f9b75a34df4b5c759999b2edc614e Mon Sep 17 00:00:00 2001 From: Christian Beiwinkel Date: Sat, 24 Feb 2024 10:36:30 +0100 Subject: [PATCH 05/14] format --- src/sif/dynamiccost.cc | 18 ++++++++++-------- 1 file changed, 10 insertions(+), 8 deletions(-) diff --git a/src/sif/dynamiccost.cc b/src/sif/dynamiccost.cc index 008babef37..9826dd0082 100644 --- a/src/sif/dynamiccost.cc +++ b/src/sif/dynamiccost.cc @@ -122,8 +122,8 @@ BaseCostingOptionsConfig::BaseCostingOptionsConfig() : dest_only_penalty_{0.f, kDefaultDestinationOnlyPenalty, kMaxPenalty}, maneuver_penalty_{0.f, kDefaultManeuverPenalty, kMaxPenalty}, alley_penalty_{0.f, kDefaultAlleyPenalty, kMaxPenalty}, - gate_cost_{0.f, kDefaultGateCost, kMaxPenalty}, - gate_penalty_{0.f, kDefaultGatePenalty, kMaxPenalty}, + gate_cost_{0.f, kDefaultGateCost, kMaxPenalty}, gate_penalty_{0.f, kDefaultGatePenalty, + kMaxPenalty}, private_access_penalty_{0.f, kDefaultPrivateAccessPenalty, kMaxPenalty}, country_crossing_cost_{0.f, kDefaultCountryCrossingCost, kMaxPenalty}, country_crossing_penalty_{0.f, kDefaultCountryCrossingPenalty, kMaxPenalty}, @@ -131,13 +131,15 @@ BaseCostingOptionsConfig::BaseCostingOptionsConfig() toll_booth_penalty_{0.f, kDefaultTollBoothPenalty, kMaxPenalty}, ferry_cost_{0.f, kDefaultFerryCost, kMaxPenalty}, use_ferry_{0.f, kDefaultUseFerry, 1.f}, rail_ferry_cost_{0.f, kDefaultRailFerryCost, kMaxPenalty}, - use_rail_ferry_{0.f, kDefaultUseRailFerry, 1.f}, - service_penalty_{0.f, kDefaultServicePenalty, kMaxPenalty}, - service_factor_{kMinFactor, kDefaultServiceFactor, kMaxFactor}, - use_tracks_{0.f, kDefaultUseTracks, 1.f}, + use_rail_ferry_{0.f, kDefaultUseRailFerry, 1.f}, service_penalty_{0.f, kDefaultServicePenalty, + kMaxPenalty}, + service_factor_{kMinFactor, kDefaultServiceFactor, kMaxFactor}, use_tracks_{0.f, + kDefaultUseTracks, + 1.f}, use_living_streets_{0.f, kDefaultUseLivingStreets, 1.f}, use_lit_{0.f, kDefaultUseLit, 1.f}, - closure_factor_{kClosureFactorRange}, exclude_unpaved_(false), exclude_cash_only_tolls_(false), - include_hot_{false}, include_hov2_{false}, include_hov3_{false} { + closure_factor_{kClosureFactorRange}, exclude_unpaved_(false), + exclude_cash_only_tolls_(false), include_hot_{false}, include_hov2_{false}, include_hov3_{ + false} { } DynamicCost::DynamicCost(const Costing& costing, From ca7504eae4641ceaffbcf668269bd44217a9d6dd Mon Sep 17 00:00:00 2001 From: Christian Beiwinkel Date: Mon, 26 Feb 2024 15:33:38 +0100 Subject: [PATCH 06/14] comments: add parameterized test, make available to more costing modes, etc. --- docs/docs/api/turn-by-turn/api-reference.md | 2 + src/sif/autocost.cc | 8 +- src/sif/dynamiccost.cc | 20 ++-- src/sif/motorcyclecost.cc | 4 +- src/sif/truckcost.cc | 5 +- test/gurka/test_ignore_restrictions.cc | 104 ++++++++++++++++++++ test/gurka/test_truck_restrictions.cc | 64 ------------ valhalla/sif/dynamiccost.h | 4 + 8 files changed, 128 insertions(+), 83 deletions(-) create mode 100644 test/gurka/test_ignore_restrictions.cc diff --git a/docs/docs/api/turn-by-turn/api-reference.md b/docs/docs/api/turn-by-turn/api-reference.md index 18c131d0a6..f31e7326e4 100644 --- a/docs/docs/api/turn-by-turn/api-reference.md +++ b/docs/docs/api/turn-by-turn/api-reference.md @@ -121,6 +121,8 @@ These options are available for `auto`, `bus`, and `truck` costing methods. | `fixed_speed` | Fixed speed the vehicle can go. Used to override the calculated speed. Can be useful if speed of vehicle is known. `fixed_speed` must be between 1 and 252 KPH. The default value is 0 KPH which disables fixed speed and falls back to the standard calculated speed based on the road attribution. | | `ignore_closures` | If set to `true`, ignores all closures, marked due to live traffic closures, during routing. **Note:** This option cannot be set if `location.search_filter.exclude_closures` is also specified in the request and will return an error if it is | | `closure_factor` | A factor that penalizes the cost when traversing a closed edge (eg: if `search_filter.exclude_closures` is `false` for origin and/or destination location and the route starts/ends on closed edges). Its value can range from `1.0` - don't penalize closed edges, to `10.0` - apply high cost penalty to closed edges. Default value is `9.0`. **Note:** This factor is applicable only for motorized modes of transport, i.e `auto`, `motorcycle`, `motor_scooter`, `bus`, `truck` & `taxi`. | +| `ignore_restrictions` | If set to `true`, ignores any restrictions (e.g. turn/dimensional/conditional restrictions). Especially useful for matching GPS traces to the road network regardless of restrictions. Default is `false`. | +| `ignore_turn_conditional_restrictions` | Similar to `ignore_restrictions`, but will respect restrictions that impact vehicle safety, such as weight and size restrictions. | ###### Other costing options The following options are available for `auto`, `bus`, `taxi`, and `truck` costing methods. diff --git a/src/sif/autocost.cc b/src/sif/autocost.cc index 34662ab292..93a3dbd648 100644 --- a/src/sif/autocost.cc +++ b/src/sif/autocost.cc @@ -421,7 +421,7 @@ bool AutoCost::Allowed(const baldr::DirectedEdge* edge, // a not thru region and a heading selected an edge entering the // region. if (!IsAccessible(edge) || (!pred.deadend() && pred.opp_local_idx() == edge->localedgeidx()) || - ((pred.restrictions() & (1 << edge->localedgeidx())) && !ignore_restrictions_) || + ((pred.restrictions() & (1 << edge->localedgeidx())) && !ignore_turn_restrictions_) || edge->surface() == Surface::kImpassable || IsUserAvoidEdge(edgeid) || (!allow_destination_only_ && !pred.destonly() && edge->destonly()) || (pred.closure_pruning() && IsClosed(edge, tile)) || @@ -446,7 +446,7 @@ bool AutoCost::AllowedReverse(const baldr::DirectedEdge* edge, // Check access, U-turn, and simple turn restriction. // Allow U-turns at dead-end nodes. if (!IsAccessible(opp_edge) || (!pred.deadend() && pred.opp_local_idx() == edge->localedgeidx()) || - ((opp_edge->restrictions() & (1 << pred.opp_local_idx())) && !ignore_restrictions_) || + ((opp_edge->restrictions() & (1 << pred.opp_local_idx())) && !ignore_turn_restrictions_) || opp_edge->surface() == Surface::kImpassable || IsUserAvoidEdge(opp_edgeid) || (!allow_destination_only_ && !pred.destonly() && opp_edge->destonly()) || (pred.closure_pruning() && IsClosed(opp_edge, tile)) || @@ -808,7 +808,7 @@ bool BusCost::AllowedReverse(const baldr::DirectedEdge* edge, // Check access, U-turn, and simple turn restriction. // Allow U-turns at dead-end nodes. if (!IsAccessible(opp_edge) || (!pred.deadend() && pred.opp_local_idx() == edge->localedgeidx()) || - ((opp_edge->restrictions() & (1 << pred.opp_local_idx())) && !ignore_restrictions_) || + ((opp_edge->restrictions() & (1 << pred.opp_local_idx())) && !ignore_turn_restrictions_) || opp_edge->surface() == Surface::kImpassable || IsUserAvoidEdge(opp_edgeid) || (!allow_destination_only_ && !pred.destonly() && opp_edge->destonly()) || (pred.closure_pruning() && IsClosed(opp_edge, tile)) || @@ -987,7 +987,7 @@ bool TaxiCost::AllowedReverse(const baldr::DirectedEdge* edge, // Check access, U-turn, and simple turn restriction. // Allow U-turns at dead-end nodes. if (!IsAccessible(opp_edge) || (!pred.deadend() && pred.opp_local_idx() == edge->localedgeidx()) || - ((opp_edge->restrictions() & (1 << pred.opp_local_idx())) && !ignore_restrictions_) || + ((opp_edge->restrictions() & (1 << pred.opp_local_idx())) && !ignore_turn_restrictions_) || opp_edge->surface() == Surface::kImpassable || IsUserAvoidEdge(opp_edgeid) || (!allow_destination_only_ && !pred.destonly() && opp_edge->destonly()) || (pred.closure_pruning() && IsClosed(opp_edge, tile)) || diff --git a/src/sif/dynamiccost.cc b/src/sif/dynamiccost.cc index 9826dd0082..be25c8d82a 100644 --- a/src/sif/dynamiccost.cc +++ b/src/sif/dynamiccost.cc @@ -122,8 +122,8 @@ BaseCostingOptionsConfig::BaseCostingOptionsConfig() : dest_only_penalty_{0.f, kDefaultDestinationOnlyPenalty, kMaxPenalty}, maneuver_penalty_{0.f, kDefaultManeuverPenalty, kMaxPenalty}, alley_penalty_{0.f, kDefaultAlleyPenalty, kMaxPenalty}, - gate_cost_{0.f, kDefaultGateCost, kMaxPenalty}, gate_penalty_{0.f, kDefaultGatePenalty, - kMaxPenalty}, + gate_cost_{0.f, kDefaultGateCost, kMaxPenalty}, + gate_penalty_{0.f, kDefaultGatePenalty, kMaxPenalty}, private_access_penalty_{0.f, kDefaultPrivateAccessPenalty, kMaxPenalty}, country_crossing_cost_{0.f, kDefaultCountryCrossingCost, kMaxPenalty}, country_crossing_penalty_{0.f, kDefaultCountryCrossingPenalty, kMaxPenalty}, @@ -131,15 +131,13 @@ BaseCostingOptionsConfig::BaseCostingOptionsConfig() toll_booth_penalty_{0.f, kDefaultTollBoothPenalty, kMaxPenalty}, ferry_cost_{0.f, kDefaultFerryCost, kMaxPenalty}, use_ferry_{0.f, kDefaultUseFerry, 1.f}, rail_ferry_cost_{0.f, kDefaultRailFerryCost, kMaxPenalty}, - use_rail_ferry_{0.f, kDefaultUseRailFerry, 1.f}, service_penalty_{0.f, kDefaultServicePenalty, - kMaxPenalty}, - service_factor_{kMinFactor, kDefaultServiceFactor, kMaxFactor}, use_tracks_{0.f, - kDefaultUseTracks, - 1.f}, + use_rail_ferry_{0.f, kDefaultUseRailFerry, 1.f}, + service_penalty_{0.f, kDefaultServicePenalty, kMaxPenalty}, + service_factor_{kMinFactor, kDefaultServiceFactor, kMaxFactor}, + use_tracks_{0.f, kDefaultUseTracks, 1.f}, use_living_streets_{0.f, kDefaultUseLivingStreets, 1.f}, use_lit_{0.f, kDefaultUseLit, 1.f}, - closure_factor_{kClosureFactorRange}, exclude_unpaved_(false), - exclude_cash_only_tolls_(false), include_hot_{false}, include_hov2_{false}, include_hov3_{ - false} { + closure_factor_{kClosureFactorRange}, exclude_unpaved_(false), exclude_cash_only_tolls_(false), + include_hot_{false}, include_hov2_{false}, include_hov3_{false} { } DynamicCost::DynamicCost(const Costing& costing, @@ -152,6 +150,8 @@ DynamicCost::DynamicCost(const Costing& costing, shortest_(costing.options().shortest()), ignore_restrictions_(costing.options().ignore_restrictions()), ignore_common_restrictions_(costing.options().ignore_common_restrictions()), + ignore_turn_restrictions_(costing.options().ignore_restrictions() || + costing.options().ignore_common_restrictions()), ignore_oneways_(costing.options().ignore_oneways()), ignore_access_(costing.options().ignore_access()), ignore_closures_(costing.options().ignore_closures()), diff --git a/src/sif/motorcyclecost.cc b/src/sif/motorcyclecost.cc index 43388c91d3..1f716bd499 100644 --- a/src/sif/motorcyclecost.cc +++ b/src/sif/motorcyclecost.cc @@ -355,7 +355,7 @@ bool MotorcycleCost::Allowed(const baldr::DirectedEdge* edge, // Check access, U-turn, and simple turn restriction. // Allow U-turns at dead-end nodes. if (!IsAccessible(edge) || (!pred.deadend() && pred.opp_local_idx() == edge->localedgeidx()) || - ((pred.restrictions() & (1 << edge->localedgeidx())) && !ignore_restrictions_) || + ((pred.restrictions() & (1 << edge->localedgeidx())) && !ignore_turn_restrictions_) || (edge->surface() > kMinimumMotorcycleSurface) || IsUserAvoidEdge(edgeid) || (!allow_destination_only_ && !pred.destonly() && edge->destonly()) || (pred.closure_pruning() && IsClosed(edge, tile))) { @@ -379,7 +379,7 @@ bool MotorcycleCost::AllowedReverse(const baldr::DirectedEdge* edge, // Check access, U-turn, and simple turn restriction. // Allow U-turns at dead-end nodes. if (!IsAccessible(opp_edge) || (!pred.deadend() && pred.opp_local_idx() == edge->localedgeidx()) || - ((opp_edge->restrictions() & (1 << pred.opp_local_idx())) && !ignore_restrictions_) || + ((opp_edge->restrictions() & (1 << pred.opp_local_idx())) && !ignore_turn_restrictions_) || (opp_edge->surface() > kMinimumMotorcycleSurface) || IsUserAvoidEdge(opp_edgeid) || (!allow_destination_only_ && !pred.destonly() && opp_edge->destonly()) || (pred.closure_pruning() && IsClosed(opp_edge, tile))) { diff --git a/src/sif/truckcost.cc b/src/sif/truckcost.cc index a86dc845c8..3cd54f8769 100644 --- a/src/sif/truckcost.cc +++ b/src/sif/truckcost.cc @@ -439,8 +439,7 @@ inline bool TruckCost::Allowed(const baldr::DirectedEdge* edge, uint8_t& restriction_idx) const { // Check access, U-turn, and simple turn restriction. if (!IsAccessible(edge) || (!pred.deadend() && pred.opp_local_idx() == edge->localedgeidx()) || - ((pred.restrictions() & (1 << edge->localedgeidx())) && - (!ignore_restrictions_ && !ignore_common_restrictions_)) || + ((pred.restrictions() & (1 << edge->localedgeidx())) && (!ignore_turn_restrictions_)) || edge->surface() == Surface::kImpassable || IsUserAvoidEdge(edgeid) || (!allow_destination_only_ && !pred.destonly() && edge->destonly_hgv()) || (pred.closure_pruning() && IsClosed(edge, tile)) || @@ -464,7 +463,7 @@ bool TruckCost::AllowedReverse(const baldr::DirectedEdge* edge, uint8_t& restriction_idx) const { // Check access, U-turn, and simple turn restriction. if (!IsAccessible(opp_edge) || (!pred.deadend() && pred.opp_local_idx() == edge->localedgeidx()) || - ((opp_edge->restrictions() & (1 << pred.opp_local_idx())) && !ignore_restrictions_) || + ((opp_edge->restrictions() & (1 << pred.opp_local_idx())) && !ignore_turn_restrictions_) || opp_edge->surface() == Surface::kImpassable || IsUserAvoidEdge(opp_edgeid) || (!allow_destination_only_ && !pred.destonly() && opp_edge->destonly_hgv()) || (pred.closure_pruning() && IsClosed(opp_edge, tile)) || diff --git a/test/gurka/test_ignore_restrictions.cc b/test/gurka/test_ignore_restrictions.cc new file mode 100644 index 0000000000..ec12f606ea --- /dev/null +++ b/test/gurka/test_ignore_restrictions.cc @@ -0,0 +1,104 @@ + +#include "gurka.h" +#include "test.h" +#include + +using namespace valhalla; + +// we should parameterize this test +// - costing mode (auto, truck, motorcycle, taxi, bus?) +// - restriction type (turn, conditional, dimension) + +std::string get_access_mode(const std::string& costing_mode) { + if (costing_mode == "auto") { + return "motorcar"; + } else if (costing_mode == "truck") { + return "hgv"; + } else if (costing_mode == "motorcycle") { + return "motorcycle"; + } else if (costing_mode == "taxi") { + return "taxi"; + } else if (costing_mode == "bus") { + return "bus"; + } + + throw std::runtime_error("unexpected costing mode " + costing_mode + "."); +} + +class CommonRestrictionTest : public ::testing::TestWithParam { +protected: + static gurka::nodelayout layout; + + static void SetUpTestSuite() { + constexpr double gridsize = 500; + + const std::string map = R"( + A----------B-----C----D + | + E + | + | + F + )"; + + layout = gurka::detail::map_to_coordinates(map, gridsize); + } +}; +gurka::nodelayout CommonRestrictionTest::layout = {}; + +TEST_P(CommonRestrictionTest, IgnoreCommonRestrictions) { + const std::string& costing = GetParam(); + const gurka::ways ways = { + {"AB", {{"highway", "secondary"}}}, + {"BC", {{"highway", "secondary"}}}, + {"CD", {{"highway", "secondary"}}}, + {"AE", {{"highway", "secondary"}}}, + {"EF", + {{"highway", "secondary"}, {get_access_mode(costing) + ":conditional", "no @ (09:00-18:00)"}}}, + }; + const gurka::relations relations = {{{ + {gurka::way_member, "AB", "from"}, + {gurka::way_member, "BC", "to"}, + {gurka::node_member, "B", "via"}, + }, + {{"type", "restriction"}, {"restriction", "no_straight_on"}}}}; + gurka::map map = + gurka::buildtiles(layout, ways, {}, relations, "test/data/ignore_common_restrictions", + {{"mjolnir.timezone", {VALHALLA_BUILD_DIR "test/data/tz.sqlite"}}}); + // first, route through turn restriction, should fail... + try { + valhalla::Api route = gurka::do_action(valhalla::Options::route, map, {"A", "D"}, costing, {}); + FAIL() << "Expected valhalla_exception_t."; + } catch (const valhalla_exception_t& err) { EXPECT_EQ(err.code, 442); } catch (...) { + FAIL() << "Expected valhalla_exception_t."; + } + + // ...but succeed with ignore_common_restrictions + valhalla::Api route = + gurka::do_action(valhalla::Options::route, map, {"A", "D"}, costing, + {{"/costing_options/" + costing + "/ignore_common_restrictions", "1"}}); + gurka::assert::raw::expect_path(route, {"AB", "BC", "CD"}); + + // second, route through time based access restrictions, should fail... + try { + valhalla::Api route = + gurka::do_action(valhalla::Options::route, map, {"A", "F"}, costing, + {{"/date_time/type", "1"}, {"/date_time/value", "2020-10-10T13:00"}}); + FAIL() << "Expected route to fail."; + } catch (const valhalla_exception_t& err) { EXPECT_EQ(err.code, 442); } catch (...) { + FAIL() << "Expected different error code."; + } + + //...but succeed with ignore_common_restrictions + valhalla::Api route_succeed = + gurka::do_action(valhalla::Options::route, map, {"A", "F"}, costing, + {{"/costing_options/" + costing + "/ignore_common_restrictions", "1"}, + {"/date_time/type", "1"}, + {"/date_time/value", "2020-10-10T13:00"}}); + gurka::assert::raw::expect_path(route_succeed, {"AE", "EF"}); +} + +INSTANTIATE_TEST_SUITE_P(CommonRestrictionTest, + CommonRestrictionTest, + ::testing::Values("auto", "truck", "motorcycle", "taxi", "bus") // no lit tag +); diff --git a/test/gurka/test_truck_restrictions.cc b/test/gurka/test_truck_restrictions.cc index 9c0a84a0e9..7368bb8dbf 100644 --- a/test/gurka/test_truck_restrictions.cc +++ b/test/gurka/test_truck_restrictions.cc @@ -165,67 +165,3 @@ TEST(TruckSpeed, MaxTruckSpeed) { // expect lower traffic speeds (< kMaxAssumedTruckSpeed ) to lead to a lower duration ASSERT_LT(traffic_time, traffic_low_speed_time); } - -TEST(TruckSpeed, IgnoreCommonRestrictions) { - constexpr double gridsize = 500; - - const std::string ascii_map = R"( - A----------B-----C----D - | - E - | - | - F - )"; - - const gurka::ways ways = { - {"AB", {{"highway", "secondary"}}}, - {"BC", {{"highway", "secondary"}}}, - {"CD", {{"highway", "secondary"}}}, - {"AE", {{"highway", "secondary"}}}, - {"EF", {{"highway", "secondary"}, {"hgv:conditional", "no @ (09:00-18:00)"}}}, - }; - const gurka::relations relations = {{{ - {gurka::way_member, "AB", "from"}, - {gurka::way_member, "BC", "to"}, - {gurka::node_member, "B", "via"}, - }, - {{"type", "restriction"}, {"restriction", "no_straight_on"}}}}; - - const auto layout = gurka::detail::map_to_coordinates(ascii_map, gridsize); - gurka::map map = - gurka::buildtiles(layout, ways, {}, relations, "test/data/truck_ignore_common_restrictions", - {{"mjolnir.timezone", {VALHALLA_BUILD_DIR "test/data/tz.sqlite"}}}); - - // first, route through turn restriction, should fail... - try { - valhalla::Api route = gurka::do_action(valhalla::Options::route, map, {"A", "D"}, "truck", {}); - FAIL() << "Expected valhalla_exception_t."; - } catch (const valhalla_exception_t& err) { EXPECT_EQ(err.code, 442); } catch (...) { - FAIL() << "Expected valhalla_exception_t."; - } - - // ...but succeed with ignore_common_restrictions - valhalla::Api route = - gurka::do_action(valhalla::Options::route, map, {"A", "D"}, "truck", - {{"/costing_options/truck/ignore_common_restrictions", "1"}}); - gurka::assert::raw::expect_path(route, {"AB", "BC", "CD"}); - - // second, route through time based access restrictions, should fail... - try { - valhalla::Api route = - gurka::do_action(valhalla::Options::route, map, {"A", "F"}, "truck", - {{"/date_time/type", "1"}, {"/date_time/value", "2020-10-10T13:00"}}); - FAIL() << "Expected route to fail."; - } catch (const valhalla_exception_t& err) { EXPECT_EQ(err.code, 442); } catch (...) { - FAIL() << "Expected different error code."; - } - - //...but succeed with ignore_common_restrictions - valhalla::Api route_succeed = - gurka::do_action(valhalla::Options::route, map, {"A", "F"}, "truck", - {{"/costing_options/truck/ignore_common_restrictions", "1"}, - {"/date_time/type", "1"}, - {"/date_time/value", "2020-10-10T13:00"}}); - gurka::assert::raw::expect_path(route_succeed, {"AE", "EF"}); -} diff --git a/valhalla/sif/dynamiccost.h b/valhalla/sif/dynamiccost.h index f07de2cbb4..32f497ddb5 100644 --- a/valhalla/sif/dynamiccost.h +++ b/valhalla/sif/dynamiccost.h @@ -427,6 +427,9 @@ class DynamicCost { thor::EdgeStatus* edgestatus = nullptr, const uint64_t current_time = 0, const uint32_t tz_index = 0) const { + if (ignore_turn_restrictions_) + return false; + // Lambda to get the next predecessor EdgeLabel (that is not a transition) auto next_predecessor = [&edge_labels](const EdgeLabel* label) { // Get the next predecessor - make sure it is valid. Continue to get @@ -998,6 +1001,7 @@ class DynamicCost { bool ignore_restrictions_{false}; bool ignore_common_restrictions_{false}; + bool ignore_turn_restrictions_{false}; bool ignore_oneways_{false}; bool ignore_access_{false}; bool ignore_closures_{false}; From 0c71b9012f30b1714f9e21bc53f1c4f7ff93645d Mon Sep 17 00:00:00 2001 From: Christian Beiwinkel Date: Tue, 27 Feb 2024 15:09:20 +0100 Subject: [PATCH 07/14] more rigorous testing --- src/sif/dynamiccost.cc | 18 +++--- test/gurka/test_ignore_restrictions.cc | 80 +++++++++++++++++++++++++- test/gurka/test_truck_restrictions.cc | 3 +- 3 files changed, 89 insertions(+), 12 deletions(-) diff --git a/src/sif/dynamiccost.cc b/src/sif/dynamiccost.cc index be25c8d82a..b78fa645f4 100644 --- a/src/sif/dynamiccost.cc +++ b/src/sif/dynamiccost.cc @@ -122,8 +122,8 @@ BaseCostingOptionsConfig::BaseCostingOptionsConfig() : dest_only_penalty_{0.f, kDefaultDestinationOnlyPenalty, kMaxPenalty}, maneuver_penalty_{0.f, kDefaultManeuverPenalty, kMaxPenalty}, alley_penalty_{0.f, kDefaultAlleyPenalty, kMaxPenalty}, - gate_cost_{0.f, kDefaultGateCost, kMaxPenalty}, - gate_penalty_{0.f, kDefaultGatePenalty, kMaxPenalty}, + gate_cost_{0.f, kDefaultGateCost, kMaxPenalty}, gate_penalty_{0.f, kDefaultGatePenalty, + kMaxPenalty}, private_access_penalty_{0.f, kDefaultPrivateAccessPenalty, kMaxPenalty}, country_crossing_cost_{0.f, kDefaultCountryCrossingCost, kMaxPenalty}, country_crossing_penalty_{0.f, kDefaultCountryCrossingPenalty, kMaxPenalty}, @@ -131,13 +131,15 @@ BaseCostingOptionsConfig::BaseCostingOptionsConfig() toll_booth_penalty_{0.f, kDefaultTollBoothPenalty, kMaxPenalty}, ferry_cost_{0.f, kDefaultFerryCost, kMaxPenalty}, use_ferry_{0.f, kDefaultUseFerry, 1.f}, rail_ferry_cost_{0.f, kDefaultRailFerryCost, kMaxPenalty}, - use_rail_ferry_{0.f, kDefaultUseRailFerry, 1.f}, - service_penalty_{0.f, kDefaultServicePenalty, kMaxPenalty}, - service_factor_{kMinFactor, kDefaultServiceFactor, kMaxFactor}, - use_tracks_{0.f, kDefaultUseTracks, 1.f}, + use_rail_ferry_{0.f, kDefaultUseRailFerry, 1.f}, service_penalty_{0.f, kDefaultServicePenalty, + kMaxPenalty}, + service_factor_{kMinFactor, kDefaultServiceFactor, kMaxFactor}, use_tracks_{0.f, + kDefaultUseTracks, + 1.f}, use_living_streets_{0.f, kDefaultUseLivingStreets, 1.f}, use_lit_{0.f, kDefaultUseLit, 1.f}, - closure_factor_{kClosureFactorRange}, exclude_unpaved_(false), exclude_cash_only_tolls_(false), - include_hot_{false}, include_hov2_{false}, include_hov3_{false} { + closure_factor_{kClosureFactorRange}, exclude_unpaved_(false), + exclude_cash_only_tolls_(false), include_hot_{false}, include_hov2_{false}, include_hov3_{ + false} { } DynamicCost::DynamicCost(const Costing& costing, diff --git a/test/gurka/test_ignore_restrictions.cc b/test/gurka/test_ignore_restrictions.cc index ec12f606ea..01a2f032b8 100644 --- a/test/gurka/test_ignore_restrictions.cc +++ b/test/gurka/test_ignore_restrictions.cc @@ -98,7 +98,81 @@ TEST_P(CommonRestrictionTest, IgnoreCommonRestrictions) { gurka::assert::raw::expect_path(route_succeed, {"AE", "EF"}); } -INSTANTIATE_TEST_SUITE_P(CommonRestrictionTest, +// check that dimensional restrictions are not affected +TEST_P(CommonRestrictionTest, IgnoreCommonRestrictionsFail) { + const std::string& costing = GetParam(); + + if (costing == "motorcycle") + return; // no height for motorcycle + const gurka::ways ways = { + {"AB", {{"highway", "secondary"}}}, {"BC", {{"highway", "secondary"}, {"maxheight", "2.5"}}}, + {"CD", {{"highway", "secondary"}}}, {"AE", {{"highway", "secondary"}}}, + {"EF", {{"highway", "secondary"}}}, + }; + gurka::map map = + gurka::buildtiles(layout, ways, {}, {}, "test/data/ignore_common_restrictions", + {{"mjolnir.timezone", {VALHALLA_BUILD_DIR "test/data/tz.sqlite"}}}); + // should fail, too low + try { + valhalla::Api route = gurka::do_action(valhalla::Options::route, map, {"A", "D"}, costing, + {{"/costing_options/" + costing + "/height", "3"}}); + FAIL() << "Expected valhalla_exception_t."; + } catch (const valhalla_exception_t& err) { EXPECT_EQ(err.code, 442); } catch (...) { + FAIL() << "Expected valhalla_exception_t."; + } + + // still too low + try { + valhalla::Api route = + gurka::do_action(valhalla::Options::route, map, {"A", "D"}, costing, + {{"/costing_options/" + costing + "/ignore_common_restrictions", "1"}, + {"/costing_options/" + costing + "/height", "3"}}); + FAIL() << "Expected valhalla_exception_t."; + } catch (const valhalla_exception_t& err) { EXPECT_EQ(err.code, 442); } catch (...) { + FAIL() << "Expected valhalla_exception_t."; + } +} +INSTANTIATE_TEST_SUITE_P(CommonRestrictionsTest, CommonRestrictionTest, - ::testing::Values("auto", "truck", "motorcycle", "taxi", "bus") // no lit tag -); + ::testing::Values("auto", "truck", "motorcycle", "taxi", "bus")); + +// make sure truck weight restrictions are not affected by the request parameter +TEST(CommonRestrictionsFail, Truck) { + + constexpr double gridsize = 500; + + const std::string ascii_map = R"( + A----------B-----C----D + )"; + + const auto layout = gurka::detail::map_to_coordinates(ascii_map, gridsize); + const gurka::ways ways = {{"AB", {{"highway", "residential"}}}, + {"BC", {{"highway", "residential"}, {"maxheight", "2.5"}}}, + {"CD", {{"highway", "residential"}}}}; + + gurka::map map = + gurka::buildtiles(layout, ways, {}, {}, "test/data/ignore_common_restrictions_truck", + {{"mjolnir.timezone", {VALHALLA_BUILD_DIR "test/data/tz.sqlite"}}}); + + // too long + try { + valhalla::Api route = gurka::do_action(valhalla::Options::route, map, {"A", "D"}, "truck", + {{"/costing_options/truck/height", "3"}}); + + FAIL() << "Expected valhalla_exception_t."; + } catch (const valhalla_exception_t& err) { EXPECT_EQ(err.code, 442); } catch (...) { + FAIL() << "Expected to fail with a different error code."; + } + + // ...still too long + try { + valhalla::Api route = + gurka::do_action(valhalla::Options::route, map, {"A", "D"}, "truck", + {{"/costing_options/truck/ignore_common_restrictions", "1"}, + {"/costing_options/truck/height", "3"}}); + FAIL() << "Expected no route to be found."; + + } catch (const valhalla_exception_t& err) { EXPECT_EQ(err.code, 442); } catch (...) { + FAIL() << "Expected to fail with a different error code."; + } +} \ No newline at end of file diff --git a/test/gurka/test_truck_restrictions.cc b/test/gurka/test_truck_restrictions.cc index 7368bb8dbf..9c11d099f4 100644 --- a/test/gurka/test_truck_restrictions.cc +++ b/test/gurka/test_truck_restrictions.cc @@ -49,6 +49,7 @@ TEST_P(TruckRestrictionTest, NotAllowed) { try { gurka::do_action(Options::route, map, {"A", "D"}, "truck", {{"/costing_options/truck/" + option, v}}); + FAIL() << "Expected no path to be found"; } catch (const valhalla_exception_t& err) { EXPECT_EQ(err.code, 442); } catch (...) { FAIL() << "Expected valhalla_exception_t."; }; @@ -65,7 +66,7 @@ INSTANTIATE_TEST_SUITE_P(TruckRestrictions, ::testing::Values(std::pair{"height", "6"}, std::pair{"width", "4"}, std::pair{"length", "30"}, - std::pair{"hazmat", "true"}, + std::pair{"hazmat", "1"}, std::pair{"axle_load", "11"}, std::pair{"axle_count", "10"})); From d80085f54a35a2a122faf106c5123679dfaa9fc9 Mon Sep 17 00:00:00 2001 From: Christian Beiwinkel Date: Wed, 28 Feb 2024 14:06:41 +0100 Subject: [PATCH 08/14] added remaining costing modes, adapted test --- src/sif/bicyclecost.cc | 4 ++-- src/sif/motorscootercost.cc | 4 ++-- test/gurka/test_ignore_restrictions.cc | 20 +++++++++++--------- 3 files changed, 15 insertions(+), 13 deletions(-) diff --git a/src/sif/bicyclecost.cc b/src/sif/bicyclecost.cc index 887b312d87..46a07b54d6 100644 --- a/src/sif/bicyclecost.cc +++ b/src/sif/bicyclecost.cc @@ -545,7 +545,7 @@ bool BicycleCost::Allowed(const baldr::DirectedEdge* edge, if (!IsAccessible(edge) || edge->is_shortcut() || (!pred.deadend() && pred.opp_local_idx() == edge->localedgeidx() && pred.mode() == TravelMode::kBicycle) || - (!ignore_restrictions_ && (pred.restrictions() & (1 << edge->localedgeidx()))) || + (!ignore_turn_restrictions_ && (pred.restrictions() & (1 << edge->localedgeidx()))) || IsUserAvoidEdge(edgeid)) { return false; } @@ -582,7 +582,7 @@ bool BicycleCost::AllowedReverse(const baldr::DirectedEdge* edge, opp_edge->use() == Use::kPlatformConnection || (!pred.deadend() && pred.opp_local_idx() == edge->localedgeidx() && pred.mode() == TravelMode::kBicycle) || - (!ignore_restrictions_ && (opp_edge->restrictions() & (1 << pred.opp_local_idx()))) || + (!ignore_turn_restrictions_ && (opp_edge->restrictions() & (1 << pred.opp_local_idx()))) || IsUserAvoidEdge(opp_edgeid)) { return false; } diff --git a/src/sif/motorscootercost.cc b/src/sif/motorscootercost.cc index 2ac26e8c13..7fe8270f05 100644 --- a/src/sif/motorscootercost.cc +++ b/src/sif/motorscootercost.cc @@ -376,7 +376,7 @@ bool MotorScooterCost::Allowed(const baldr::DirectedEdge* edge, // Check access, U-turn, and simple turn restriction. // Allow U-turns at dead-end nodes. if (!IsAccessible(edge) || (!pred.deadend() && pred.opp_local_idx() == edge->localedgeidx()) || - ((pred.restrictions() & (1 << edge->localedgeidx())) && !ignore_restrictions_) || + ((pred.restrictions() & (1 << edge->localedgeidx())) && !ignore_turn_restrictions_) || (edge->surface() > kMinimumScooterSurface) || IsUserAvoidEdge(edgeid) || (!allow_destination_only_ && !pred.destonly() && edge->destonly()) || (pred.closure_pruning() && IsClosed(edge, tile))) { @@ -400,7 +400,7 @@ bool MotorScooterCost::AllowedReverse(const baldr::DirectedEdge* edge, // Check access, U-turn, and simple turn restriction. // Allow U-turns at dead-end nodes. if (!IsAccessible(opp_edge) || (!pred.deadend() && pred.opp_local_idx() == edge->localedgeidx()) || - ((opp_edge->restrictions() & (1 << pred.opp_local_idx())) && !ignore_restrictions_) || + ((opp_edge->restrictions() & (1 << pred.opp_local_idx())) && !ignore_turn_restrictions_) || (opp_edge->surface() > kMinimumScooterSurface) || IsUserAvoidEdge(opp_edgeid) || (!allow_destination_only_ && !pred.destonly() && opp_edge->destonly()) || (pred.closure_pruning() && IsClosed(opp_edge, tile))) { diff --git a/test/gurka/test_ignore_restrictions.cc b/test/gurka/test_ignore_restrictions.cc index 01a2f032b8..471e17f033 100644 --- a/test/gurka/test_ignore_restrictions.cc +++ b/test/gurka/test_ignore_restrictions.cc @@ -5,10 +5,6 @@ using namespace valhalla; -// we should parameterize this test -// - costing mode (auto, truck, motorcycle, taxi, bus?) -// - restriction type (turn, conditional, dimension) - std::string get_access_mode(const std::string& costing_mode) { if (costing_mode == "auto") { return "motorcar"; @@ -20,6 +16,10 @@ std::string get_access_mode(const std::string& costing_mode) { return "taxi"; } else if (costing_mode == "bus") { return "bus"; + } else if (costing_mode == "motor_scooter") { + return "moped"; + } else if (costing_mode == "bicycle") { + return "bicycle"; } throw std::runtime_error("unexpected costing mode " + costing_mode + "."); @@ -102,8 +102,9 @@ TEST_P(CommonRestrictionTest, IgnoreCommonRestrictions) { TEST_P(CommonRestrictionTest, IgnoreCommonRestrictionsFail) { const std::string& costing = GetParam(); - if (costing == "motorcycle") - return; // no height for motorcycle + if (costing == "motorcycle" || costing == "bicycle" || costing == "motor_scooter") + return; // no height restrictions for these + const gurka::ways ways = { {"AB", {{"highway", "secondary"}}}, {"BC", {{"highway", "secondary"}, {"maxheight", "2.5"}}}, {"CD", {{"highway", "secondary"}}}, {"AE", {{"highway", "secondary"}}}, @@ -132,9 +133,10 @@ TEST_P(CommonRestrictionTest, IgnoreCommonRestrictionsFail) { FAIL() << "Expected valhalla_exception_t."; } } -INSTANTIATE_TEST_SUITE_P(CommonRestrictionsTest, - CommonRestrictionTest, - ::testing::Values("auto", "truck", "motorcycle", "taxi", "bus")); +INSTANTIATE_TEST_SUITE_P( + CommonRestrictionsTest, + CommonRestrictionTest, + ::testing::Values("auto", "truck", "motorcycle", "taxi", "bus", "bicycle", "motor_scooter")); // make sure truck weight restrictions are not affected by the request parameter TEST(CommonRestrictionsFail, Truck) { From b6cb707098a8c73d21545dae2602dc456b956aa1 Mon Sep 17 00:00:00 2001 From: Christian Beiwinkel Date: Thu, 29 Feb 2024 15:08:27 +0100 Subject: [PATCH 09/14] renaming --- docs/docs/api/turn-by-turn/api-reference.md | 2 +- proto/options.proto | 4 ++-- src/sif/dynamiccost.cc | 25 ++++++++++----------- src/sif/truckcost.cc | 2 +- test/gurka/test_ignore_restrictions.cc | 18 +++++++-------- valhalla/sif/dynamiccost.h | 6 ++--- 6 files changed, 28 insertions(+), 29 deletions(-) diff --git a/docs/docs/api/turn-by-turn/api-reference.md b/docs/docs/api/turn-by-turn/api-reference.md index f31e7326e4..6ba1685917 100644 --- a/docs/docs/api/turn-by-turn/api-reference.md +++ b/docs/docs/api/turn-by-turn/api-reference.md @@ -122,7 +122,7 @@ These options are available for `auto`, `bus`, and `truck` costing methods. | `ignore_closures` | If set to `true`, ignores all closures, marked due to live traffic closures, during routing. **Note:** This option cannot be set if `location.search_filter.exclude_closures` is also specified in the request and will return an error if it is | | `closure_factor` | A factor that penalizes the cost when traversing a closed edge (eg: if `search_filter.exclude_closures` is `false` for origin and/or destination location and the route starts/ends on closed edges). Its value can range from `1.0` - don't penalize closed edges, to `10.0` - apply high cost penalty to closed edges. Default value is `9.0`. **Note:** This factor is applicable only for motorized modes of transport, i.e `auto`, `motorcycle`, `motor_scooter`, `bus`, `truck` & `taxi`. | | `ignore_restrictions` | If set to `true`, ignores any restrictions (e.g. turn/dimensional/conditional restrictions). Especially useful for matching GPS traces to the road network regardless of restrictions. Default is `false`. | -| `ignore_turn_conditional_restrictions` | Similar to `ignore_restrictions`, but will respect restrictions that impact vehicle safety, such as weight and size restrictions. | +| `ignore_non_vehicular_restrictions` | Similar to `ignore_restrictions`, but will respect restrictions that impact vehicle safety, such as weight and size restrictions. | ###### Other costing options The following options are available for `auto`, `bus`, `taxi`, and `truck` costing methods. diff --git a/proto/options.proto b/proto/options.proto index 571ada7730..4f973b74ab 100644 --- a/proto/options.proto +++ b/proto/options.proto @@ -312,8 +312,8 @@ message Costing { uint32 axle_count = 81; float use_lit = 82; bool disable_hierarchy_pruning = 83; - oneof has_ignore_common_restrictions { - bool ignore_common_restrictions = 84; + oneof has_ignore_non_vehicular_restrictions { + bool ignore_non_vehicular_restrictions = 84; } } diff --git a/src/sif/dynamiccost.cc b/src/sif/dynamiccost.cc index b78fa645f4..1a697911b3 100644 --- a/src/sif/dynamiccost.cc +++ b/src/sif/dynamiccost.cc @@ -122,8 +122,8 @@ BaseCostingOptionsConfig::BaseCostingOptionsConfig() : dest_only_penalty_{0.f, kDefaultDestinationOnlyPenalty, kMaxPenalty}, maneuver_penalty_{0.f, kDefaultManeuverPenalty, kMaxPenalty}, alley_penalty_{0.f, kDefaultAlleyPenalty, kMaxPenalty}, - gate_cost_{0.f, kDefaultGateCost, kMaxPenalty}, gate_penalty_{0.f, kDefaultGatePenalty, - kMaxPenalty}, + gate_cost_{0.f, kDefaultGateCost, kMaxPenalty}, + gate_penalty_{0.f, kDefaultGatePenalty, kMaxPenalty}, private_access_penalty_{0.f, kDefaultPrivateAccessPenalty, kMaxPenalty}, country_crossing_cost_{0.f, kDefaultCountryCrossingCost, kMaxPenalty}, country_crossing_penalty_{0.f, kDefaultCountryCrossingPenalty, kMaxPenalty}, @@ -131,15 +131,13 @@ BaseCostingOptionsConfig::BaseCostingOptionsConfig() toll_booth_penalty_{0.f, kDefaultTollBoothPenalty, kMaxPenalty}, ferry_cost_{0.f, kDefaultFerryCost, kMaxPenalty}, use_ferry_{0.f, kDefaultUseFerry, 1.f}, rail_ferry_cost_{0.f, kDefaultRailFerryCost, kMaxPenalty}, - use_rail_ferry_{0.f, kDefaultUseRailFerry, 1.f}, service_penalty_{0.f, kDefaultServicePenalty, - kMaxPenalty}, - service_factor_{kMinFactor, kDefaultServiceFactor, kMaxFactor}, use_tracks_{0.f, - kDefaultUseTracks, - 1.f}, + use_rail_ferry_{0.f, kDefaultUseRailFerry, 1.f}, + service_penalty_{0.f, kDefaultServicePenalty, kMaxPenalty}, + service_factor_{kMinFactor, kDefaultServiceFactor, kMaxFactor}, + use_tracks_{0.f, kDefaultUseTracks, 1.f}, use_living_streets_{0.f, kDefaultUseLivingStreets, 1.f}, use_lit_{0.f, kDefaultUseLit, 1.f}, - closure_factor_{kClosureFactorRange}, exclude_unpaved_(false), - exclude_cash_only_tolls_(false), include_hot_{false}, include_hov2_{false}, include_hov3_{ - false} { + closure_factor_{kClosureFactorRange}, exclude_unpaved_(false), exclude_cash_only_tolls_(false), + include_hot_{false}, include_hov2_{false}, include_hov3_{false} { } DynamicCost::DynamicCost(const Costing& costing, @@ -151,9 +149,9 @@ DynamicCost::DynamicCost(const Costing& costing, closure_factor_(kDefaultClosureFactor), flow_mask_(kDefaultFlowMask), shortest_(costing.options().shortest()), ignore_restrictions_(costing.options().ignore_restrictions()), - ignore_common_restrictions_(costing.options().ignore_common_restrictions()), + ignore_non_vehicular_restrictions_(costing.options().ignore_non_vehicular_restrictions()), ignore_turn_restrictions_(costing.options().ignore_restrictions() || - costing.options().ignore_common_restrictions()), + costing.options().ignore_non_vehicular_restrictions()), ignore_oneways_(costing.options().ignore_oneways()), ignore_access_(costing.options().ignore_access()), ignore_closures_(costing.options().ignore_closures()), @@ -392,7 +390,8 @@ void ParseBaseCostOptions(const rapidjson::Value& json, JSON_PBF_DEFAULT(co, false, json, "/ignore_oneways", ignore_oneways); JSON_PBF_DEFAULT(co, false, json, "/ignore_access", ignore_access); JSON_PBF_DEFAULT(co, false, json, "/ignore_closures", ignore_closures); - JSON_PBF_DEFAULT(co, false, json, "/ignore_common_restrictions", ignore_common_restrictions); + JSON_PBF_DEFAULT(co, false, json, "/ignore_non_vehicular_restrictions", + ignore_non_vehicular_restrictions); // shortest JSON_PBF_DEFAULT(co, false, json, "/shortest", shortest); diff --git a/src/sif/truckcost.cc b/src/sif/truckcost.cc index 3cd54f8769..94b8b4ba42 100644 --- a/src/sif/truckcost.cc +++ b/src/sif/truckcost.cc @@ -398,7 +398,7 @@ bool TruckCost::ModeSpecificAllowed(const baldr::AccessRestriction& restriction) } break; case AccessType::kMaxAxles: - if (!ignore_common_restrictions_ && (axle_count_ > static_cast(restriction.value()))) { + if (axle_count_ > static_cast(restriction.value())) { return false; } break; diff --git a/test/gurka/test_ignore_restrictions.cc b/test/gurka/test_ignore_restrictions.cc index 471e17f033..6ccf458cba 100644 --- a/test/gurka/test_ignore_restrictions.cc +++ b/test/gurka/test_ignore_restrictions.cc @@ -63,7 +63,7 @@ TEST_P(CommonRestrictionTest, IgnoreCommonRestrictions) { }, {{"type", "restriction"}, {"restriction", "no_straight_on"}}}}; gurka::map map = - gurka::buildtiles(layout, ways, {}, relations, "test/data/ignore_common_restrictions", + gurka::buildtiles(layout, ways, {}, relations, "test/data/ignore_non_vehicular_restrictions", {{"mjolnir.timezone", {VALHALLA_BUILD_DIR "test/data/tz.sqlite"}}}); // first, route through turn restriction, should fail... try { @@ -73,10 +73,10 @@ TEST_P(CommonRestrictionTest, IgnoreCommonRestrictions) { FAIL() << "Expected valhalla_exception_t."; } - // ...but succeed with ignore_common_restrictions + // ...but succeed with ignore_non_vehicular_restrictions valhalla::Api route = gurka::do_action(valhalla::Options::route, map, {"A", "D"}, costing, - {{"/costing_options/" + costing + "/ignore_common_restrictions", "1"}}); + {{"/costing_options/" + costing + "/ignore_non_vehicular_restrictions", "1"}}); gurka::assert::raw::expect_path(route, {"AB", "BC", "CD"}); // second, route through time based access restrictions, should fail... @@ -89,10 +89,10 @@ TEST_P(CommonRestrictionTest, IgnoreCommonRestrictions) { FAIL() << "Expected different error code."; } - //...but succeed with ignore_common_restrictions + //...but succeed with ignore_non_vehicular_restrictions valhalla::Api route_succeed = gurka::do_action(valhalla::Options::route, map, {"A", "F"}, costing, - {{"/costing_options/" + costing + "/ignore_common_restrictions", "1"}, + {{"/costing_options/" + costing + "/ignore_non_vehicular_restrictions", "1"}, {"/date_time/type", "1"}, {"/date_time/value", "2020-10-10T13:00"}}); gurka::assert::raw::expect_path(route_succeed, {"AE", "EF"}); @@ -111,7 +111,7 @@ TEST_P(CommonRestrictionTest, IgnoreCommonRestrictionsFail) { {"EF", {{"highway", "secondary"}}}, }; gurka::map map = - gurka::buildtiles(layout, ways, {}, {}, "test/data/ignore_common_restrictions", + gurka::buildtiles(layout, ways, {}, {}, "test/data/ignore_non_vehicular_restrictions", {{"mjolnir.timezone", {VALHALLA_BUILD_DIR "test/data/tz.sqlite"}}}); // should fail, too low try { @@ -126,7 +126,7 @@ TEST_P(CommonRestrictionTest, IgnoreCommonRestrictionsFail) { try { valhalla::Api route = gurka::do_action(valhalla::Options::route, map, {"A", "D"}, costing, - {{"/costing_options/" + costing + "/ignore_common_restrictions", "1"}, + {{"/costing_options/" + costing + "/ignore_non_vehicular_restrictions", "1"}, {"/costing_options/" + costing + "/height", "3"}}); FAIL() << "Expected valhalla_exception_t."; } catch (const valhalla_exception_t& err) { EXPECT_EQ(err.code, 442); } catch (...) { @@ -153,7 +153,7 @@ TEST(CommonRestrictionsFail, Truck) { {"CD", {{"highway", "residential"}}}}; gurka::map map = - gurka::buildtiles(layout, ways, {}, {}, "test/data/ignore_common_restrictions_truck", + gurka::buildtiles(layout, ways, {}, {}, "test/data/ignore_non_vehicular_restrictions_truck", {{"mjolnir.timezone", {VALHALLA_BUILD_DIR "test/data/tz.sqlite"}}}); // too long @@ -170,7 +170,7 @@ TEST(CommonRestrictionsFail, Truck) { try { valhalla::Api route = gurka::do_action(valhalla::Options::route, map, {"A", "D"}, "truck", - {{"/costing_options/truck/ignore_common_restrictions", "1"}, + {{"/costing_options/truck/ignore_non_vehicular_restrictions", "1"}, {"/costing_options/truck/height", "3"}}); FAIL() << "Expected no route to be found."; diff --git a/valhalla/sif/dynamiccost.h b/valhalla/sif/dynamiccost.h index 32f497ddb5..f16aa0ee47 100644 --- a/valhalla/sif/dynamiccost.h +++ b/valhalla/sif/dynamiccost.h @@ -608,7 +608,7 @@ class DynamicCost { if (access_type == baldr::AccessType::kTimedAllowed) time_allowed = true; - if (current_time == 0 || ignore_common_restrictions_) { + if (current_time == 0 || ignore_non_vehicular_restrictions_) { // No time supplied so ignore time-based restrictions // (but mark the edge (`has_time_restrictions`) continue; @@ -638,7 +638,7 @@ class DynamicCost { // if we have time allowed restrictions then these restrictions are // the only time we can route here. Meaning all other time is restricted. // We looped over all the time allowed restrictions and we were never in range. - return (!time_allowed || (current_time == 0)) || ignore_common_restrictions_; + return (!time_allowed || (current_time == 0)) || ignore_non_vehicular_restrictions_; } /** @@ -1000,7 +1000,7 @@ class DynamicCost { bool shortest_; bool ignore_restrictions_{false}; - bool ignore_common_restrictions_{false}; + bool ignore_non_vehicular_restrictions_{false}; bool ignore_turn_restrictions_{false}; bool ignore_oneways_{false}; bool ignore_access_{false}; From b32d9c0dbd5b6047d40433254a005b93147f452b Mon Sep 17 00:00:00 2001 From: Christian Beiwinkel Date: Thu, 29 Feb 2024 15:14:34 +0100 Subject: [PATCH 10/14] changelog --- CHANGELOG.md | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index ac47a13a07..e283ae7510 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -93,7 +93,7 @@ * ADDED: Improved instructions for blind users [#3694](https://github.com/valhalla/valhalla/pull/3694) * FIXED: Fixed roundoff issue in Tiles Row and Col methods [#4585](https://github.com/valhalla/valhalla/pull/4585) * ADDED: isochrone proper polygon support & pbf output for isochrone [#4575](https://github.com/valhalla/valhalla/pull/4575) - * ADDED: `ignore_common_restrictions` parameter for truck costing [#4606](https://github.com/valhalla/valhalla/pull/4606) + * ADDED: `ignore_non_vehicular_restrictions` parameter for truck costing [#4606](https://github.com/valhalla/valhalla/pull/4606) ## Release Date: 2023-05-11 Valhalla 3.4.0 * **Removed** From 7dbcdf4f86c5a497a831b288e14cdf9da92a14bd Mon Sep 17 00:00:00 2001 From: Nils Date: Mon, 4 Mar 2024 14:39:27 +0100 Subject: [PATCH 11/14] Update api-reference.md --- docs/docs/api/turn-by-turn/api-reference.md | 2 ++ 1 file changed, 2 insertions(+) diff --git a/docs/docs/api/turn-by-turn/api-reference.md b/docs/docs/api/turn-by-turn/api-reference.md index 6ba1685917..1b7a3143d3 100644 --- a/docs/docs/api/turn-by-turn/api-reference.md +++ b/docs/docs/api/turn-by-turn/api-reference.md @@ -123,6 +123,8 @@ These options are available for `auto`, `bus`, and `truck` costing methods. | `closure_factor` | A factor that penalizes the cost when traversing a closed edge (eg: if `search_filter.exclude_closures` is `false` for origin and/or destination location and the route starts/ends on closed edges). Its value can range from `1.0` - don't penalize closed edges, to `10.0` - apply high cost penalty to closed edges. Default value is `9.0`. **Note:** This factor is applicable only for motorized modes of transport, i.e `auto`, `motorcycle`, `motor_scooter`, `bus`, `truck` & `taxi`. | | `ignore_restrictions` | If set to `true`, ignores any restrictions (e.g. turn/dimensional/conditional restrictions). Especially useful for matching GPS traces to the road network regardless of restrictions. Default is `false`. | | `ignore_non_vehicular_restrictions` | Similar to `ignore_restrictions`, but will respect restrictions that impact vehicle safety, such as weight and size restrictions. | +| `ignore_access` | Will ignore mode-specific access tags. Especially useful for matching GPS traces to the road network regardless of restrictions. Default is `false`. | +| `ignore_closures` | Will ignore traffic closures. Default is `false`. | ###### Other costing options The following options are available for `auto`, `bus`, `taxi`, and `truck` costing methods. From 21b727d600320ba590a68d8cd3b6535e1d688be7 Mon Sep 17 00:00:00 2001 From: nilsnolde Date: Mon, 4 Mar 2024 16:39:26 +0100 Subject: [PATCH 12/14] do the last bits --- proto/options.proto | 4 +--- src/sif/dynamiccost.cc | 18 ++++++++++-------- valhalla/sif/dynamiccost.h | 13 ++++++++----- 3 files changed, 19 insertions(+), 16 deletions(-) diff --git a/proto/options.proto b/proto/options.proto index 4f973b74ab..68c92554bb 100644 --- a/proto/options.proto +++ b/proto/options.proto @@ -312,9 +312,7 @@ message Costing { uint32 axle_count = 81; float use_lit = 82; bool disable_hierarchy_pruning = 83; - oneof has_ignore_non_vehicular_restrictions { - bool ignore_non_vehicular_restrictions = 84; - } + bool ignore_non_vehicular_restrictions = 84; } oneof has_options { diff --git a/src/sif/dynamiccost.cc b/src/sif/dynamiccost.cc index 1a697911b3..bf644f5e57 100644 --- a/src/sif/dynamiccost.cc +++ b/src/sif/dynamiccost.cc @@ -122,8 +122,8 @@ BaseCostingOptionsConfig::BaseCostingOptionsConfig() : dest_only_penalty_{0.f, kDefaultDestinationOnlyPenalty, kMaxPenalty}, maneuver_penalty_{0.f, kDefaultManeuverPenalty, kMaxPenalty}, alley_penalty_{0.f, kDefaultAlleyPenalty, kMaxPenalty}, - gate_cost_{0.f, kDefaultGateCost, kMaxPenalty}, - gate_penalty_{0.f, kDefaultGatePenalty, kMaxPenalty}, + gate_cost_{0.f, kDefaultGateCost, kMaxPenalty}, gate_penalty_{0.f, kDefaultGatePenalty, + kMaxPenalty}, private_access_penalty_{0.f, kDefaultPrivateAccessPenalty, kMaxPenalty}, country_crossing_cost_{0.f, kDefaultCountryCrossingCost, kMaxPenalty}, country_crossing_penalty_{0.f, kDefaultCountryCrossingPenalty, kMaxPenalty}, @@ -131,13 +131,15 @@ BaseCostingOptionsConfig::BaseCostingOptionsConfig() toll_booth_penalty_{0.f, kDefaultTollBoothPenalty, kMaxPenalty}, ferry_cost_{0.f, kDefaultFerryCost, kMaxPenalty}, use_ferry_{0.f, kDefaultUseFerry, 1.f}, rail_ferry_cost_{0.f, kDefaultRailFerryCost, kMaxPenalty}, - use_rail_ferry_{0.f, kDefaultUseRailFerry, 1.f}, - service_penalty_{0.f, kDefaultServicePenalty, kMaxPenalty}, - service_factor_{kMinFactor, kDefaultServiceFactor, kMaxFactor}, - use_tracks_{0.f, kDefaultUseTracks, 1.f}, + use_rail_ferry_{0.f, kDefaultUseRailFerry, 1.f}, service_penalty_{0.f, kDefaultServicePenalty, + kMaxPenalty}, + service_factor_{kMinFactor, kDefaultServiceFactor, kMaxFactor}, use_tracks_{0.f, + kDefaultUseTracks, + 1.f}, use_living_streets_{0.f, kDefaultUseLivingStreets, 1.f}, use_lit_{0.f, kDefaultUseLit, 1.f}, - closure_factor_{kClosureFactorRange}, exclude_unpaved_(false), exclude_cash_only_tolls_(false), - include_hot_{false}, include_hov2_{false}, include_hov3_{false} { + closure_factor_{kClosureFactorRange}, exclude_unpaved_(false), + exclude_cash_only_tolls_(false), include_hot_{false}, include_hov2_{false}, include_hov3_{ + false} { } DynamicCost::DynamicCost(const Costing& costing, diff --git a/valhalla/sif/dynamiccost.h b/valhalla/sif/dynamiccost.h index f16aa0ee47..f95dbbc7ee 100644 --- a/valhalla/sif/dynamiccost.h +++ b/valhalla/sif/dynamiccost.h @@ -599,16 +599,17 @@ class DynamicCost { const auto& restriction = restrictions[i]; // Compare the time to the time-based restrictions baldr::AccessType access_type = restriction.type(); - if (access_type == baldr::AccessType::kTimedAllowed || - access_type == baldr::AccessType::kTimedDenied || - access_type == baldr::AccessType::kDestinationAllowed) { + if (!ignore_non_vehicular_restrictions_ && + (access_type == baldr::AccessType::kTimedAllowed || + access_type == baldr::AccessType::kTimedDenied || + access_type == baldr::AccessType::kDestinationAllowed)) { // TODO: if(i > baldr::kInvalidRestriction) LOG_ERROR("restriction index overflow"); restriction_idx = static_cast(i); if (access_type == baldr::AccessType::kTimedAllowed) time_allowed = true; - if (current_time == 0 || ignore_non_vehicular_restrictions_) { + if (current_time == 0) { // No time supplied so ignore time-based restrictions // (but mark the edge (`has_time_restrictions`) continue; @@ -638,7 +639,7 @@ class DynamicCost { // if we have time allowed restrictions then these restrictions are // the only time we can route here. Meaning all other time is restricted. // We looped over all the time allowed restrictions and we were never in range. - return (!time_allowed || (current_time == 0)) || ignore_non_vehicular_restrictions_; + return !time_allowed || (current_time == 0); } /** @@ -1001,6 +1002,8 @@ class DynamicCost { bool ignore_restrictions_{false}; bool ignore_non_vehicular_restrictions_{false}; + // not a requestion parameter, it's true if either ignore_restrictions_ or + // ignore_non_vehicular_restrictions_ is true bool ignore_turn_restrictions_{false}; bool ignore_oneways_{false}; bool ignore_access_{false}; From 2ce6b8b4d689ddf8b41e4f85c7b56ce33ad5e53f Mon Sep 17 00:00:00 2001 From: nilsnolde Date: Mon, 4 Mar 2024 16:58:04 +0100 Subject: [PATCH 13/14] also have a JSON_PBF_DEFAULT_V2 for non-oneof options (actually lots more/most of them, should refactor some), similar to the ranged equivalent --- src/sif/dynamiccost.cc | 4 ++-- valhalla/sif/dynamiccost.h | 20 ++++++++++++++++++++ 2 files changed, 22 insertions(+), 2 deletions(-) diff --git a/src/sif/dynamiccost.cc b/src/sif/dynamiccost.cc index bf644f5e57..c92ec6e33a 100644 --- a/src/sif/dynamiccost.cc +++ b/src/sif/dynamiccost.cc @@ -392,8 +392,8 @@ void ParseBaseCostOptions(const rapidjson::Value& json, JSON_PBF_DEFAULT(co, false, json, "/ignore_oneways", ignore_oneways); JSON_PBF_DEFAULT(co, false, json, "/ignore_access", ignore_access); JSON_PBF_DEFAULT(co, false, json, "/ignore_closures", ignore_closures); - JSON_PBF_DEFAULT(co, false, json, "/ignore_non_vehicular_restrictions", - ignore_non_vehicular_restrictions); + JSON_PBF_DEFAULT_V2(co, false, json, "/ignore_non_vehicular_restrictions", + ignore_non_vehicular_restrictions); // shortest JSON_PBF_DEFAULT(co, false, json, "/shortest", shortest); diff --git a/valhalla/sif/dynamiccost.h b/valhalla/sif/dynamiccost.h index f95dbbc7ee..441a64c1c5 100644 --- a/valhalla/sif/dynamiccost.h +++ b/valhalla/sif/dynamiccost.h @@ -87,6 +87,26 @@ : def)); \ } +/** + * same as above, but for costing options without pbf's awful oneof + * + * @param costing_options pointer to protobuf costing options object + * @param def the default value which is used when neither json nor pbf is provided + * @param json rapidjson value object which should contain user provided costing options + * @param json_key the json key to use to pull a user provided value out of the json + * @param option_name the name of the option will be set on the costing options object + */ + +#define JSON_PBF_DEFAULT_V2(costing_options, def, json, json_key, option_name) \ + { \ + costing_options->set_##option_name( \ + rapidjson::get::type>::type>(json, json_key, \ + costing_options->option_name() \ + ? costing_options->option_name() \ + : def)); \ + } + using namespace valhalla::midgard; namespace valhalla { From 25c441fcddcdbd290288afccda1ba188574f44dd Mon Sep 17 00:00:00 2001 From: nilsnolde Date: Wed, 6 Mar 2024 18:01:58 +0100 Subject: [PATCH 14/14] add boost format.hpp header to a test --- test/gurka/test_closure_penalty.cc | 2 ++ 1 file changed, 2 insertions(+) diff --git a/test/gurka/test_closure_penalty.cc b/test/gurka/test_closure_penalty.cc index a98a78e082..437a84b4f9 100644 --- a/test/gurka/test_closure_penalty.cc +++ b/test/gurka/test_closure_penalty.cc @@ -1,3 +1,5 @@ +#include + #include "gurka.h" #include "test.h"