From 440dccb81bc6e02ffda076f0977989a4bb0958cf Mon Sep 17 00:00:00 2001 From: Patrick Niklaus Date: Thu, 13 Jul 2017 23:19:20 +0000 Subject: [PATCH] Move classes to intersection object and don't emit notifications --- docs/http.md | 4 +- features/car/classes.feature | 64 +++++++++++++------ features/car/destination.feature | 12 ++-- features/support/route.js | 2 +- include/engine/guidance/assemble_steps.hpp | 22 +++---- include/engine/guidance/route_step.hpp | 4 +- src/engine/api/json_factory.cpp | 24 +++---- src/engine/guidance/post_processing.cpp | 1 - .../guidance/intersection_handler.cpp | 5 +- unit_tests/engine/guidance_assembly.cpp | 8 +-- 10 files changed, 84 insertions(+), 62 deletions(-) diff --git a/docs/http.md b/docs/http.md index 772a1ea8d..936a624aa 100644 --- a/docs/http.md +++ b/docs/http.md @@ -583,7 +583,6 @@ step. - `destinations`: The destinations of the way. Will be `undefined` if there are no destinations. - `exits`: The exit numbers or names of the way. Will be `undefined` if there are no exit numbers or names. - `mode`: A string signifying the mode of transportation. -- `classes`: An array of strings signifying the classes of the road as specified in the profile. - `maneuver`: A `StepManeuver` object representing the maneuver. - `intersections`: A list of `Intersection` objects that are passed along the segment, the very first belonging to the StepManeuver - `rotary_name`: The name for the rotary. Optionally included, if the step is a rotary and a rotary name is available. @@ -597,7 +596,6 @@ step. "mode" : "driving", "duration" : 15.6, "weight" : 15.6, - "classes": ["toll", "restricted"], "intersections" : [ { "bearings" : [ 10, 92, 184, 270 ], "lanes" : [ @@ -735,6 +733,7 @@ location of the StepManeuver. Further intersections are listed for every cross-w - `location`: A `[longitude, latitude]` pair describing the location of the turn. - `bearings`: A list of bearing values (e.g. [0,90,180,270]) that are available at the intersection. The bearings describe all available roads at the intersection. Values are between 0-359 (0=true north) +- `classes`: An array of strings signifying the classes (as specified in the profile) of the road exiting the intersection. - `entry`: A list of entry flags, corresponding in a 1:1 relationship to the bearings. A value of `true` indicates that the respective road could be entered on a valid route. `false` indicates that the turn onto the respective road would violate a restriction. - `in`: index into bearings/entry array. Used to calculate the bearing just before the turn. Namely, the clockwise angle from true north to the @@ -753,6 +752,7 @@ location of the StepManeuver. Further intersections are listed for every cross-w "out":2, "bearings":[60,150,240,330], "entry":["false","true","true","true"], + "classes": ["toll", "restricted"], "lanes":{ "indications": ["left", "straight"], "valid": "false" diff --git a/features/car/classes.feature b/features/car/classes.feature index 5211f4528..343b556f1 100644 --- a/features/car/classes.feature +++ b/features/car/classes.feature @@ -17,13 +17,13 @@ Feature: Car - Mode flag | cd | primary | | When I route I should get - | from | to | route | turns | classes | - | a | d | ab,bc,cd,cd | depart,notification right,notification left,arrive | ,ferry,, | - | d | a | cd,bc,ab,ab | depart,notification right,notification left,arrive | ,ferry,, | - | c | a | bc,ab,ab | depart,notification left,arrive | ferry,, | - | d | b | cd,bc,bc | depart,notification right,arrive | ,ferry,ferry | - | a | c | ab,bc,bc | depart,notification right,arrive | ,ferry,ferry | - | b | d | bc,cd,cd | depart,notification left,arrive | ferry,, | + | from | to | route | turns | classes | + | a | d | ab,bc,cd,cd | depart,notification right,notification left,arrive | [()],[(ferry)],[()],[()] | + | d | a | cd,bc,ab,ab | depart,notification right,notification left,arrive | [()],[(ferry)],[()],[()] | + | c | a | bc,ab,ab | depart,notification left,arrive | [(ferry)],[()],[()] | + | d | b | cd,bc,bc | depart,notification right,arrive | [()],[(ferry)],[()] | + | a | c | ab,bc,bc | depart,notification right,arrive | [()],[(ferry)],[()] | + | b | d | bc,cd,cd | depart,notification left,arrive | [(ferry)],[()],[()] | Scenario: Car - We tag motorways with a class @@ -40,10 +40,10 @@ Feature: Car - Mode flag | cd | primary | When I route I should get - | from | to | route | turns | classes | # | - | a | d | ab,bc,cd | depart,notification right,arrive | ,motorway, | | - | a | c | ab,bc,bc | depart,notification right,arrive | ,motorway,motorway | | - | b | d | bc,cd | depart,arrive | motorway, | we don't announce when we leave the highway | + | from | to | route | turns | classes | + | a | d | ab,cd | depart,arrive | [(),(motorway),()],[()] | + | a | c | ab,bc | depart,arrive | [(),(motorway)],[()] | + | b | d | bc,cd | depart,arrive | [(motorway),()],[()] | Scenario: Car - We tag motorway_link with a class Given the node map @@ -59,10 +59,10 @@ Feature: Car - Mode flag | cd | primary | When I route I should get - | from | to | route | turns | classes | # | - | a | d | ab,bc,cd | depart,on ramp right,arrive | ,motorway, | notification replaced by on-ramp | - | a | c | ab,bc,bc | depart,on ramp right,arrive | ,motorway,motorway | " " | - | b | d | bc,cd | depart,arrive | motorway, | no announcement | + | from | to | route | turns | classes | # | + | a | d | ab,bc,cd | depart,on ramp right,arrive | [()],[(motorway),()],[()] | on-ramp at class change | + | a | c | ab,bc,bc | depart,on ramp right,arrive | [()],[(motorway)],[()] | " " | + | b | d | bc,cd | depart,arrive | [(motorway),()],[()] | no announcement | Scenario: Car - We tag restricted with a class @@ -79,8 +79,8 @@ Feature: Car - Mode flag | cd | primary | | When I route I should get - | from | to | route | turns | classes | - | a | d | ab,bc,cd | depart,notification right,arrive| restricted,motorway;restricted, | + | from | to | route | turns | classes | + | a | d | ab,cd | depart,arrive| [(restricted),(motorway,restricted),()],[()] | Scenario: Car - We toll restricted with a class Given the node map @@ -96,6 +96,32 @@ Feature: Car - Mode flag | cd | primary | | When I route I should get - | from | to | route | turns | classes | - | a | d | ab,bc,cd | depart,notification right,arrive | toll,motorway;toll, | + | from | to | route | turns | classes | + | a | d | ab,cd | depart,arrive | [(toll),(motorway,toll),()],[()] | + + Scenario: Car - From roundabout on toll road + Given the node map + """ + c + / \ + a---b d---f + \ / + e + | + g + """ + + And the ways + | nodes | oneway | highway | junction | toll | + | ab | yes | primary | | | + | cb | yes | primary | roundabout | | + | dc | yes | primary | roundabout | | + | be | yes | primary | roundabout | | + | ed | yes | motorway| roundabout | | + | eg | yes | primary | | | + | df | yes | motorway| | yes | + + When I route I should get + | from | to | route | turns | classes | + | a | f | ab,df,df | depart,roundabout-exit-2,arrive | [()],[(),(motorway),(toll,motorway)],[()] | diff --git a/features/car/destination.feature b/features/car/destination.feature index f9eb79fee..aee592a11 100644 --- a/features/car/destination.feature +++ b/features/car/destination.feature @@ -23,11 +23,11 @@ Feature: Car - Destination only, no passing through When I route I should get | from | to | route | | a | b | ab,ab | - | a | c | ab,bcd,bcd | + | a | c | ab,bcd | | a | d | ab,bcd,bcd | | a | e | axye,axye | | e | d | de,de | - | e | c | de,bcd,bcd | + | e | c | de,bcd | | e | b | de,bcd,bcd | | e | a | axye,axye | @@ -51,12 +51,12 @@ Feature: Car - Destination only, no passing through When I route I should get | from | to | route | | a | b | ab,ab | - | a | c | ab,bc,bc | - | a | d | ab,bc,cd | + | a | c | ab,bc | + | a | d | ab,cd | | a | e | axye,axye | | e | d | de,de | - | e | c | de,cd,cd | - | e | b | de,cd,bc | + | e | c | de,cd | + | e | b | de,bc | | e | a | axye,axye | Scenario: Car - Routing inside a destination only area diff --git a/features/support/route.js b/features/support/route.js index 10178a31a..ce86772b5 100644 --- a/features/support/route.js +++ b/features/support/route.js @@ -268,7 +268,7 @@ module.exports = function () { }; this.classesList = (instructions) => { - return this.extractInstructionList(instructions, s => s.classes ? s.classes.join(';') : ''); + return this.extractInstructionList(instructions, s => '[' + s.intersections.map(i => '(' + (i.classes ? i.classes.join(',') : '') + ')').join(',') + ']'); }; this.timeList = (instructions) => { diff --git a/include/engine/guidance/assemble_steps.hpp b/include/engine/guidance/assemble_steps.hpp index f1fc69296..054fc798e 100644 --- a/include/engine/guidance/assemble_steps.hpp +++ b/include/engine/guidance/assemble_steps.hpp @@ -63,7 +63,6 @@ inline std::vector assembleSteps(const datafacade::BaseDataFacade &fa : target_node.forward_segment_id.id; const auto target_name_id = facade.GetNameIndex(target_node_id); const auto target_mode = facade.GetTravelMode(target_node_id); - auto target_classes = facade.GetClasses(facade.GetClassData(target_node_id)); const auto number_of_segments = leg_geometry.GetNumberOfSegments(); @@ -88,7 +87,8 @@ inline std::vector assembleSteps(const datafacade::BaseDataFacade &fa IntermediateIntersection::NO_INDEX, 0, util::guidance::LaneTuple(), - {}}; + {}, + source_classes}; if (leg_data.size() > 0) { @@ -118,7 +118,8 @@ inline std::vector assembleSteps(const datafacade::BaseDataFacade &fa const auto destinations = facade.GetDestinationsForID(step_name_id); const auto exits = facade.GetExitsForID(step_name_id); const auto distance = leg_geometry.segment_distances[segment_index]; - auto classes = facade.GetClasses(path_point.classes); + // intersections contain the classes of exiting road + intersection.classes = facade.GetClasses(path_point.classes); steps.push_back(RouteStep{step_name_id, name.to_string(), @@ -135,8 +136,7 @@ inline std::vector assembleSteps(const datafacade::BaseDataFacade &fa maneuver, leg_geometry.FrontIndex(segment_index), leg_geometry.BackIndex(segment_index) + 1, - {intersection}, - std::move(classes)}); + {intersection}}); if (leg_data_index + 1 < leg_data.size()) { @@ -196,6 +196,8 @@ inline std::vector assembleSteps(const datafacade::BaseDataFacade &fa const auto distance = leg_geometry.segment_distances[segment_index]; const EdgeWeight duration = segment_duration + target_duration; const EdgeWeight weight = segment_weight + target_weight; + // intersections contain the classes of exiting road + intersection.classes = facade.GetClasses(facade.GetClassData(target_node_id)); BOOST_ASSERT(duration >= 0); steps.push_back(RouteStep{step_name_id, facade.GetNameForID(step_name_id).to_string(), @@ -212,8 +214,7 @@ inline std::vector assembleSteps(const datafacade::BaseDataFacade &fa maneuver, leg_geometry.FrontIndex(segment_index), leg_geometry.BackIndex(segment_index) + 1, - {intersection}, - std::move(target_classes)}); + {intersection}}); } // In this case the source + target are on the same edge segment else @@ -255,8 +256,7 @@ inline std::vector assembleSteps(const datafacade::BaseDataFacade &fa std::move(maneuver), leg_geometry.FrontIndex(segment_index), leg_geometry.BackIndex(segment_index) + 1, - {intersection}, - std::move(source_classes)}); + {intersection}}); } BOOST_ASSERT(segment_index == number_of_segments - 1); @@ -269,6 +269,7 @@ inline std::vector assembleSteps(const datafacade::BaseDataFacade &fa 0, IntermediateIntersection::NO_INDEX, util::guidance::LaneTuple(), + {}, {}}; // This step has length zero, the only reason we need it is the target location @@ -295,8 +296,7 @@ inline std::vector assembleSteps(const datafacade::BaseDataFacade &fa std::move(maneuver), leg_geometry.locations.size() - 1, leg_geometry.locations.size(), - {intersection}, - std::move(target_classes)}); + {intersection}}); BOOST_ASSERT(steps.front().intersections.size() == 1); BOOST_ASSERT(steps.front().intersections.front().bearings.size() == 1); diff --git a/include/engine/guidance/route_step.hpp b/include/engine/guidance/route_step.hpp index 32984421c..a404ab6a6 100644 --- a/include/engine/guidance/route_step.hpp +++ b/include/engine/guidance/route_step.hpp @@ -42,6 +42,7 @@ struct IntermediateIntersection // turn lane information util::guidance::LaneTuple lanes; extractor::guidance::TurnLaneDescription lane_description; + std::vector classes; }; inline IntermediateIntersection getInvalidIntersection() @@ -52,6 +53,7 @@ inline IntermediateIntersection getInvalidIntersection() IntermediateIntersection::NO_INDEX, IntermediateIntersection::NO_INDEX, util::guidance::LaneTuple(), + {}, {}}; } @@ -74,7 +76,6 @@ struct RouteStep std::size_t geometry_begin; std::size_t geometry_end; std::vector intersections; - std::vector classes; // remove all information from the route step, marking it as invalid (used to indicate empty // steps to be removed). @@ -128,7 +129,6 @@ inline void RouteStep::Invalidate() geometry_end = 0; intersections.clear(); intersections.push_back(getInvalidIntersection()); - classes.clear(); } // Elongate by another step in front diff --git a/src/engine/api/json_factory.cpp b/src/engine/api/json_factory.cpp index 62702e368..f6b7ad538 100644 --- a/src/engine/api/json_factory.cpp +++ b/src/engine/api/json_factory.cpp @@ -234,6 +234,18 @@ util::json::Object makeIntersection(const guidance::IntermediateIntersection &in if (detail::hasValidLanes(intersection)) result.values["lanes"] = detail::lanesFromIntersection(intersection); + if (!intersection.classes.empty()) + { + util::json::Array classes; + classes.values.reserve(intersection.classes.size()); + std::transform( + intersection.classes.begin(), + intersection.classes.end(), + std::back_inserter(classes.values), + [](const std::string &class_name) { return util::json::String{class_name}; }); + result.values["classes"] = std::move(classes); + } + return result; } @@ -265,18 +277,6 @@ util::json::Object makeRouteStep(guidance::RouteStep step, util::json::Value geo route_step.values["maneuver"] = makeStepManeuver(std::move(step.maneuver)); route_step.values["geometry"] = std::move(geometry); - if (!step.classes.empty()) - { - util::json::Array classes; - classes.values.reserve(step.classes.size()); - std::transform( - step.classes.begin(), - step.classes.end(), - std::back_inserter(classes.values), - [](const std::string &class_name) { return util::json::String{class_name}; }); - route_step.values["classes"] = std::move(classes); - } - util::json::Array intersections; intersections.values.reserve(step.intersections.size()); std::transform(step.intersections.begin(), diff --git a/src/engine/guidance/post_processing.cpp b/src/engine/guidance/post_processing.cpp index 38cc02031..0aaed5c3d 100644 --- a/src/engine/guidance/post_processing.cpp +++ b/src/engine/guidance/post_processing.cpp @@ -485,7 +485,6 @@ void trimShortSegments(std::vector &steps, LegGeometry &geometry) auto &new_next_to_last = *(steps.end() - 2); next_to_last_step.AdaptStepSignage(new_next_to_last); next_to_last_step.mode = new_next_to_last.mode; - next_to_last_step.classes = new_next_to_last.classes; // the geometry indices of the last step are already correct; } else if (util::coordinate_calculation::haversineDistance( diff --git a/src/extractor/guidance/intersection_handler.cpp b/src/extractor/guidance/intersection_handler.cpp index d089d9a94..3b760709b 100644 --- a/src/extractor/guidance/intersection_handler.cpp +++ b/src/extractor/guidance/intersection_handler.cpp @@ -88,10 +88,7 @@ TurnInstruction IntersectionHandler::getInstructionForObvious(const std::size_t // handle travel modes: const auto in_mode = node_based_graph.GetEdgeData(via_edge).travel_mode; const auto out_mode = node_based_graph.GetEdgeData(road.eid).travel_mode; - const auto in_classes = node_based_graph.GetEdgeData(via_edge).classes; - const auto out_classes = node_based_graph.GetEdgeData(road.eid).classes; - // if we just lose class flags we don't want to notify - const auto needs_notification = in_mode != out_mode || !isSubset(out_classes, in_classes); + const auto needs_notification = in_mode != out_mode; if (type == TurnType::Turn) { diff --git a/unit_tests/engine/guidance_assembly.cpp b/unit_tests/engine/guidance_assembly.cpp index e3662c376..012be4e00 100644 --- a/unit_tests/engine/guidance_assembly.cpp +++ b/unit_tests/engine/guidance_assembly.cpp @@ -23,6 +23,7 @@ BOOST_AUTO_TEST_CASE(trim_short_segments) IntermediateIntersection::NO_INDEX, 0, {0, 255}, + {}, {}}; IntermediateIntersection intersection2{{FloatLongitude{-73.981495}, FloatLatitude{40.768275}}, {180}, @@ -30,6 +31,7 @@ BOOST_AUTO_TEST_CASE(trim_short_segments) 0, IntermediateIntersection::NO_INDEX, {0, 255}, + {}, {}}; // Check that duplicated coordinate in the end is removed @@ -53,8 +55,7 @@ BOOST_AUTO_TEST_CASE(trim_short_segments) 0}, 0, 3, - {intersection1}, - {}}, + {intersection1}}, {324, "Central Park West", "", @@ -75,8 +76,7 @@ BOOST_AUTO_TEST_CASE(trim_short_segments) 0}, 2, 3, - {intersection2}, - {}}}; + {intersection2}}}; LegGeometry geometry; geometry.locations = {{FloatLongitude{-73.981492}, FloatLatitude{40.768258}},