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

Set type of trivial intersections where classes change to Suppressed #4816

Merged
merged 1 commit into from
Jan 25, 2018

Conversation

oxidase
Copy link
Contributor

@oxidase oxidase commented Jan 22, 2018

Issue

In some cases like tunnels or bridges way classes may change at trivial intersections of degree two.
Maneuvers at such intersections have turn types NoTurn and intersections will be removed from response intersections arrays.

Tasklist

  • add regression / cucumber cases (see docs/testing.md)
  • review
  • adjust for comments

Requirements / Relations

#4812

@@ -119,6 +119,25 @@ Feature: Car - Mode flag
| from | to | route | turns | classes |
| a | d | ab,bc,cd,cd | depart,new name right,new name left,arrive | [()],[(tunnel)],[()],[()] |

Scenario: Car - We tag tunnel with a class
Background:
Copy link
Member

Choose a reason for hiding this comment

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

Should we change the name of the scenario above? Does it matter if we keep the same name for both scenarios?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

👍 change to Car - We tag classes without intersections

@oxidase oxidase force-pushed the fix/changed_classes branch 2 times, most recently from 019dbce to 21a079f Compare January 22, 2018 15:43
@ghoshkaj
Copy link
Member

Looks good to me! 👍

Copy link
Member

@TheMarex TheMarex left a comment

Choose a reason for hiding this comment

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

Looks good to me. The original reason why we did not announce there, was that some people were worried about the impact on guidance. To be safe I would compar the instruction distribution to master and look how many new suppressed instruction we introduce. Maybe go as far and compare the number of Continue instructions for a number of random routes.

@oxidase
Copy link
Contributor Author

oxidase commented Jan 22, 2018

@TheMarex in SF-area there are 147 new suppressed instructions https://gist.github.com/oxidase/9a78e927b78af1d796639e8dbdaa9acd mainly tunnels and restricted roads.

@oxidase oxidase merged commit 77f8a4f into master Jan 25, 2018
@oxidase oxidase deleted the fix/changed_classes branch January 25, 2018 16:43
@oxidase oxidase added this to the 5.15.1 milestone Jan 29, 2018
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