Skip to content
New issue

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

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

Already on GitHub? Sign in to your account

Cardinal direction fix #1741

Merged
merged 13 commits into from
Mar 13, 2019
Merged

Cardinal direction fix #1741

merged 13 commits into from
Mar 13, 2019

Conversation

dnesbitt61
Copy link
Member

@dnesbitt61 dnesbitt61 commented Mar 13, 2019

Fix highway directional for singly digitized roads and for some cases on doubly digitized ways where a relation has a direction. This required changes to OSM parsing logic to handle the forward and backward direction and dding reverse way refs to OSMData.

When a singlely digitized edge has a forward and backward direction for the reference/relation the code that builds EdgeInfo (within Graphbuilder as well as Hierarchy and Shortcut builders) has to check if a different name occurs in opposing directions of an edge. If so, then when creating EdgeInfo a new EdgeInfo is forced to be created so that different names can be supported. This creates some extra duplication of shape, but the total increase in tile sizes for North America is modest.

Testing of route differences was performed (specifically using the highway cardinal direction routes).

  • Add tests
  • Review - you must request approval to merge any PR to master
  • Add #fixes with the issue number that this PR addresses
  • Generally use squash merge to rebase and clean comments before merging
  • Update the changelog

Example 1

image

< BEFORE
< 2: Turn left onto Walnut Street/US 22.
> AFTER
> 2: Turn left onto Walnut Street/US 22 East.

Example 2

image

< BEFORE
< 2: Turn left onto Walnut Street/US 22.
> AFTER
> 2: Turn left onto Walnut Street/US 22 West.

gknisely and others added 9 commits March 5, 2019 15:38
…n of the edge

(e.g. forward and reverse ref - directionals). Fix formatting. Had to update the AddEdgeInfo
methods to accept a flag that forces adding a new EdgeInfo (even if the nodes and edge Id/hash match).
…edgeinfo offset

(and end node is the same tile).
@dgearhart dgearhart self-requested a review March 13, 2019 13:24
dgearhart
dgearhart previously approved these changes Mar 13, 2019
Copy link
Member

@dgearhart dgearhart left a comment

Choose a reason for hiding this comment

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

just need to update changelog - otherwise 👍

Copy link
Member

@dgearhart dgearhart left a comment

Choose a reason for hiding this comment

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

:shipit:

@dnesbitt61 dnesbitt61 merged commit 105edc5 into master Mar 13, 2019
@dnesbitt61 dnesbitt61 deleted the cardinal_direction branch March 13, 2019 15:24
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants