Add turn lane info at maneuver point #1830

dgearhart · 2019-05-28T16:35:53Z

Issue

Updated data processing for turn lane info
Added turn lane info at maneuver point. Returns info when format=osrm - for example:

Tasklist

Test with new data
Review - you must request approval to merge any PR to master
Generally use squash merge to rebase and clean comments before merging
Update the changelog

…rn_lanes

…o turn_lanes

…ecause we need to know what the turns, intesections, etc are so that we can properly aggment the turn lanes.

…ncer.

…rn lanes.

…rn_lanes

…nes at maneuver point

…o turn_lanes

danpaz

Looks good to me! mostly just questions for my understanding

danpaz · 2019-05-30T21:42:34Z

src/mjolnir/graphbuilder.cc

          std::string turnlane_tags;
          if (forward && w.fwd_turn_lanes_index() > 0 && edge.attributes.way_end) {
            turnlane_tags = osmdata.name_offset_map.name(w.fwd_turn_lanes_index());
+            if (!turnlane_tags.empty()) {
+              std::string str = TurnLanes::GetTurnLaneString(turnlane_tags);
+              if (!str.empty()) { // don't add if invalid.


Should we trim the string in case the value is empty space?

Never really worried about this in the past. Always assumed that when the user enters in values in the editors, the value is checked.

danpaz · 2019-05-30T21:59:47Z

src/mjolnir/graphenhancer.cc

+        bUpdated = true;
+        break;
+      } else
+        break;


Think you can omit this else break

No. Don't think so. We want to break out of the loop if any of the conditions are not found.

I misread the code here 👍

danpaz · 2019-05-30T22:06:31Z

src/mjolnir/graphenhancer.cc

+      LOG_ERROR("Mismatch in turn lane count before " + std::to_string(tl_before) +
+                ""
+                " and after " +
+                std::to_string(turn_lanes.size()) + " tileid = " + std::to_string(tile_id.tileid()));


What action should we take if we see this error message?

The tile creation process usually seg faults when things like this happen. This is just an indicator to help us figure it out.

danpaz · 2019-05-30T22:08:54Z

src/mjolnir/graphtilebuilder.cc

+// Add turn lanes
+void GraphTileBuilder::AddTurnLanes(const std::vector<TurnLanes>& turn_lanes) {
+  turnlanes_builder_.clear();
+  // Add restrictions to the list


Stray comment

Thanks. Updated.

danpaz · 2019-05-30T22:19:45Z

src/tyr/route_serializer_osrm.cc

+    // Check if there is an active turn lane
+    // Verify that turn lanes are not non-directional
+    if (prev_edge && (prev_edge->turn_lanes_size() > 0) && prev_edge->HasActiveTurnLane() &&
+        !prev_edge->HasNonDirectionalTurnLane()) {


For my understanding - why did you exclude non-directional turn lanes? Is it to avoid having lanes that are all straight?

@danpaz this is so we do not return turn lanes with none or empty. If possible, our goal is to determine what they should be and mark them accordingly during data processing - mostly they will be marked as through

…rn_lanes

codecov · 2019-05-31T13:16:42Z

Codecov Report

❗ No coverage uploaded for pull request base (master@c1d8b39). Click here to learn what that means.
The diff coverage is 63.82%.

@@            Coverage Diff            @@
##             master    #1830   +/-   ##
=========================================
  Coverage          ?   80.51%           
=========================================
  Files             ?      365           
  Lines             ?    49936           
  Branches          ?        0           
=========================================
  Hits              ?    40207           
  Misses            ?     9729           
  Partials          ?        0

Impacted Files	Coverage Δ
valhalla/baldr/turnlanes.h	`98.27% <ø> (ø)`
valhalla/mjolnir/graphtilebuilder.h	`100% <ø> (ø)`
valhalla/mjolnir/osmdata.h	`100% <ø> (ø)`
src/tyr/route_serializer_osrm.cc	`0.38% <0%> (ø)`
src/mjolnir/osmdata.cc	`10.66% <0%> (ø)`
test/astar.cc	`93.52% <100%> (ø)`
src/mjolnir/graphtilebuilder.cc	`74.95% <100%> (ø)`
valhalla/odin/enhancedtrippath.h	`77.43% <100%> (ø)`
src/thor/triplegbuilder.cc	`67.19% <100%> (ø)`
test/countryaccess.cc	`66.66% <100%> (ø)`
... and 6 more

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update c1d8b39...699b814. Read the comment docs.

kevinkreiser · 2019-05-31T14:24:08Z

lua/graph.lua

@@ -1482,20 +1482,20 @@ function filter_tags_generic(kv)
  end

  lane_count = numeric_prefix(kv["lanes"],false)
-  if lane_count and lane_count > 10 then
-    lane_count = 10
+  if lane_count and lane_count > 15 then


before it was if we have more than 10 we cap at 10, now its if we have more than 15 we have none?

dgearhart · 2019-05-31T14:30:20Z

src/odin/enhancedtrippath.cc

+  return false;
+}
+
+bool EnhancedTripLeg_Edge::ActivateTurnLanes(uint16_t turn_lane_direction) {


activates the turn lanes based on the specified turn lane direction

dgearhart · 2019-05-31T14:32:03Z

src/odin/enhancedtrippath.cc

@@ -619,6 +652,11 @@ std::string EnhancedTripLeg_Edge::ToString() const {
  str += " | truck_route=";
  str += std::to_string(truck_route());

+  if (turn_lanes_size() > 0) {


output for debugging

dgearhart · 2019-05-31T14:35:08Z

src/odin/enhancedtrippath.cc

@@ -891,6 +929,109 @@ std::string EnhancedTripLeg_Edge::SignElementsToParameterString(

  return str;
 }
+
+std::string EnhancedTripLeg_Edge::TurnLanesToString(


debug output to see turn lane info on each edge - it will like: turn_lanes=left|left;through|through;right

dgearhart · 2019-05-31T14:36:13Z

src/odin/maneuversbuilder.cc

@@ -55,6 +56,49 @@ void CountAndSortExitSignList(std::vector<Sign>* prev_signs, std::vector<Sign>*
  SortExitSignList(curr_signs);
 }

+uint16_t GetExpectedTurnLaneDirection(Maneuver& maneuver) {


determines the expected turn lane direction based on the specified maneuver type

dgearhart · 2019-05-31T14:36:46Z

src/odin/maneuversbuilder.cc

@@ -83,6 +127,18 @@ std::list<Maneuver> ManeuversBuilder::Build() {
  // Combine maneuvers
  Combine(maneuvers);

+#ifdef LOGGING_LEVEL_TRACE


moved this block to the proper spot in the code

dgearhart · 2019-05-31T14:38:09Z

src/odin/maneuversbuilder.cc

@@ -2277,5 +2324,46 @@ void ManeuversBuilder::EnhanceSignlessInterchnages(std::list<Maneuver>& maneuver
  }
 }

+void ManeuversBuilder::ProcessTurnLanes(std::list<Maneuver>& maneuvers) {


walks the maneuvers and activates the proper turn lanes at each transition point

dgearhart · 2019-05-31T14:40:09Z

src/thor/triplegbuilder.cc

@@ -1344,7 +1344,10 @@ TripLeg_Edge* TripLegBuilder::AddTripEdge(const AttributesController& controller
  // If turn lanes exist
  if (directededge->turnlanes()) {
    auto turnlanes = graphtile->turnlanes(idx);
-    // TODO - add to TripLeg
+    for (auto tl : turnlanes) {
+      TurnLane* turn_lane = trip_edge->add_turn_lanes();


this could be done in one line with out variable

dgearhart · 2019-05-31T14:41:07Z

src/tyr/route_serializer_osrm.cc

@@ -388,6 +388,7 @@ json::ArrayPtr intersections(const valhalla::DirectionsLeg::Maneuver& maneuver,
    // NOTE: curr_edge does not exist for the arrive maneuver
    auto node = etp->GetEnhancedNode(i);
    auto curr_edge = etp->GetCurrEdge(i);
+    auto prev_edge = etp->GetPrevEdge(i);


the previous edge is needed in multiple spots - so setting it here

dgearhart · 2019-05-31T14:43:19Z

src/tyr/route_serializer_osrm.cc

+        // Process 'valid' flag
+        lane->emplace("valid", turn_lane.is_active());
+
+        // Process 'indications' array - add indications from left to right


serialize the turn lanes from left to right

dgearhart · 2019-05-31T14:43:54Z

valhalla/baldr/turnlanes.h

@@ -56,7 +56,7 @@ const static std::unordered_map<std::string, uint16_t> kTurnLaneMasks =
     {"slight_left", kTurnLaneSlightLeft},
     {"slight_right", kTurnLaneSlightRight},
     {"right", kTurnLaneRight},
-     {"sharp right", kTurnLaneSharpRight},
+     {"sharp_right", kTurnLaneSharpRight},


dgearhart · 2019-05-31T14:44:46Z

valhalla/odin/enhancedtrippath.h

@@ -316,6 +316,18 @@ class EnhancedTripLeg_Edge {
    return mutable_edge_->truck_route();
  }

+  int turn_lanes_size() const {


added access to the turn lane methods

…rn_lanes

gknisely and others added 30 commits May 16, 2019 10:14

no lanes over 15 as they dont exist.

9304c23

Merge branch 'master' of https://github.com/valhalla/valhalla into tu…

b6f21a3

…rn_lanes

Added TurnLanes

7768eb4

Switched from an array of strings to a uint32 bitmask

05f96f8

Merge branch 'master' of https://github.com/valhalla/valhalla into tu…

2e4b2cc

…rn_lanes

Updated naming to TurnLane

874b973

added turn lanes to trip_edge

9ff15f9

added turn lane test routes

80220ee

print turn lane info if using trace

fffe214

Merge branch 'master' of https://github.com/valhalla/valhalla into tu…

11ee3ce

…rn_lanes

update nones for turnlanes.

5ca8e5a

Merge branch 'turn_lanes' of https://github.com/valhalla/valhalla int…

85a8a4a

…o turn_lanes

Updated the turn_lane ToString method

e92d066

Merge branch 'turn_lanes' of https://github.com/valhalla/valhalla int…

4c8c77b

…o turn_lanes

updated.

a29e912

Merge branch 'turn_lanes' of https://github.com/valhalla/valhalla int…

dd509e1

…o turn_lanes

store the name index and not the text offset.

002df43

enhancer now creates and updates turnlanes. we have to do this here b…

34907d6

…ecause we need to know what the turns, intesections, etc are so that we can properly aggment the turn lanes.

added the ability to update the entire turn lane list.

1dec5d8

fixed typo.

4f97d29

added logic to just load the unique_names from temp file for the enha…

7bf7131

…ncer.

need to send unique names via osm_data obj to the enhancer for the tu…

7349dea

…rn lanes.

fixed test.

4949909

updated.

4e10738

rm'd couts

f9235ea

Added a couple more test routes

47ea423

updated debug output

6bfdcc0

Merge branch 'master' of https://github.com/valhalla/valhalla into tu…

c00a66b

…rn_lanes

Added turn lanes to serializer

b1f8db1

added the GetExpectedTurnLaneDirection method to activate the turn la…

3c1678d

…nes at maneuver point

dgearhart added 4 commits May 30, 2019 12:34

Added some more test routes

7c90c12

Merge branch 'turn_lanes' of https://github.com/valhalla/valhalla int…

2367358

…o turn_lanes

Added more test routes

312ed31

Updates to changelog for #1830

e2d03ba

danpaz reviewed May 30, 2019

View reviewed changes

gknisely and others added 2 commits May 30, 2019 21:27

Update graphtilebuilder.cc

72ab3a9

Merge branch 'master' of https://github.com/valhalla/valhalla into tu…

699b814

…rn_lanes

kevinkreiser reviewed May 31, 2019

View reviewed changes

dgearhart commented May 31, 2019

View reviewed changes

dgearhart added 2 commits May 31, 2019 10:57

Merge branch 'master' of https://github.com/valhalla/valhalla into tu…

6d6efd4

…rn_lanes

Updated to include with 3.0.5 since 3.0.4 was tagged

443bc73

kevinkreiser approved these changes May 31, 2019

View reviewed changes

Merge branch 'master' of https://github.com/valhalla/valhalla into tu…

e48f595

…rn_lanes

dgearhart merged commit 583821f into master May 31, 2019

dgearhart deleted the turn_lanes branch June 3, 2019 11:42

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Add turn lane info at maneuver point #1830

Add turn lane info at maneuver point #1830

dgearhart commented May 28, 2019 •

edited

Loading

danpaz left a comment

danpaz May 30, 2019

gknisely May 31, 2019

danpaz May 30, 2019

gknisely May 31, 2019

danpaz May 31, 2019

danpaz May 30, 2019

gknisely May 31, 2019

danpaz May 30, 2019

gknisely May 31, 2019

danpaz May 30, 2019

dgearhart May 30, 2019

codecov bot commented May 31, 2019

kevinkreiser May 31, 2019

gknisely May 31, 2019

dgearhart May 31, 2019

dgearhart May 31, 2019

dgearhart May 31, 2019

dgearhart May 31, 2019

dgearhart May 31, 2019

dgearhart May 31, 2019

dgearhart May 31, 2019

dgearhart May 31, 2019

dgearhart May 31, 2019

dgearhart May 31, 2019

dgearhart May 31, 2019

Add turn lane info at maneuver point #1830

Add turn lane info at maneuver point #1830

Conversation

dgearhart commented May 28, 2019 • edited Loading

Issue

Tasklist

danpaz left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

codecov bot commented May 31, 2019

Codecov Report

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

dgearhart commented May 28, 2019 •

edited

Loading