Don't use ceremonial name when way is interstate #291

bsudekum · 2017-06-20T10:37:50Z

In cases where the user is coming up to a highway, we should not show the ceremonial highway name.

What should the text say? Right now it says US-280 and also shows the shield image.

/cc @1ec5 @frederoni @ericrwolfe

frederoni · 2017-06-20T15:17:35Z

What should the text say?

This is unclear to me as well. We should add a snapshot test for this case when we figured out what the correct way is.

1ec5 · 2017-06-20T18:38:31Z

MapboxNavigation/RouteManeuverViewController.swift

@@ -119,7 +119,11 @@ class RouteManeuverViewController: UIViewController {

    public func updateStreetNameForStep() {
        if let name = step?.names?.first {
-            streetLabel.unabridgedText = name
+            if let step = step, let ref = step.codes?.first, ["i", "us"].contains(ref.lowercased().components(separatedBy: " ").first!) {


I think we’re on solid footing with Interstates, but I’m not so sure about doing this for U.S. highways. Sure, a few like U.S. 1 and U.S. 101 are freeways in some places, but not in others, and the vast majority are surface streets where the name matters more. (Think Van Ness in San Francisco or Constitution Ave. NW in D.C.)

Could we filter and display the US-101 for segments of type `highway` and use the ceremonial name in other cases? `Redwood Highway` in this screenshot is confusing, but to your point the other street names should show their proper names.

Unfortunately, the Directions API doesn’t provide any information about the “type” of a road segment. #297 for example must query the map’s vector tiles for this information, but it can only do so for features currently visible on the map, not roads that may be far out of view.

1ec5 · 2017-06-20T18:39:19Z

MapboxNavigation/RouteManeuverViewController.swift

@@ -119,7 +119,11 @@ class RouteManeuverViewController: UIViewController {

    public func updateStreetNameForStep() {
        if let name = step?.names?.first {
-            streetLabel.unabridgedText = name
+            if let step = step, let ref = step.codes?.first, ["i", "us"].contains(ref.lowercased().components(separatedBy: " ").first!) {


This can be a bit simpler: let ref = step.codes?.first, ref.hasPrefix("I ").

@1ec5 this does not account for ref.hasPrefix("US ") though. contains allows us to add more cases to this array.

We shouldn’t unconditionally remove the name from U.S. Routes. See #291 (comment). But I agree with this approach if we end up needing to remove the name from multiple networks.

1ec5 · 2017-06-20T18:40:38Z

MapboxNavigation/RouteManeuverViewController.swift

@@ -119,7 +119,11 @@ class RouteManeuverViewController: UIViewController {

    public func updateStreetNameForStep() {
        if let name = step?.names?.first {
-            streetLabel.unabridgedText = name
+            if let step = step, let ref = step.codes?.first, ["i", "us"].contains(ref.lowercased().components(separatedBy: " ").first!) {
+                streetLabel.unabridgedText = ref.components(separatedBy: " ").joined(separator: "-")


U.S. Routes are written “US 123” in most states. Only a few states like Oklahoma and Kansas hyphenate them.

1ec5 · 2017-06-20T18:41:22Z

MapboxNavigation/RouteManeuverViewController.swift

@@ -119,7 +119,11 @@ class RouteManeuverViewController: UIViewController {

    public func updateStreetNameForStep() {
        if let name = step?.names?.first {
-            streetLabel.unabridgedText = name
+            if let step = step, let ref = step.codes?.first, ["i", "us"].contains(ref.lowercased().components(separatedBy: " ").first!) {
+                streetLabel.unabridgedText = ref.components(separatedBy: " ").joined(separator: "-")


Can we try to fall back to the step’s destinations, since the shield will already be visible to the right?

ericrwolfe · 2017-06-20T21:51:26Z

Eventually we should show the highway icon + the direction ( East), but until we can scoot the highway icon to the left, let's just show text on the left plus the highway icon on the right as is currently implemented.

Ultimately, we want our turn banner to match the highway sign overhead as closely as possible, so the banner text should take the following order of precedence, when the appropriate data is available:

I-5 S / Los Angeles: always show direction + destination when available
I-5 South: always show direction when available
I-5: Fallback if no other data is available

Voice announcements should take similar levels of precedence:

I 5 South towards Los Angeles
I 5 South
I 5

We should also take care to combine/coalesce maneuvers where the highway ref stays the same but only the ceremonial name changes.

Per #291 (comment), can we check the road class for highway or motorway before displaying the highway ref and fall back to the ceremonial name for smaller streets?

1ec5 · 2017-06-20T23:10:04Z

Given the limited information we get from the Directions API, I think we can only address the Interstate case reliably. If we show both the name and shield for a U.S. Route that’s a freeway, there’s a chance that we’d look pedantic, but at least we wouldn’t be hiding any information that might be required for wayfinding.

1ec5 · 2017-06-20T23:13:22Z

It’s a bit less reliable, but the other approach I mentioned in Project-OSRM/osrm-text-instructions#51 (comment) would also be feasible: the step’s expected distance divided by expected duration yields an average expected speed. If the speed is at least 50 mph (or the equivalent in metric), treat the way as a freeway. The reason it’s less reliable is that traffic-aware directions might (correctly) increase the expected duration along a congested highway, causing the speed to drop below freeway speed.

ericrwolfe · 2017-06-20T23:26:42Z

I'm game for speed as a proxy/hack to road class until we can get more detailed responses back from the directions API or OSRMTextInstructions.

ericrwolfe · 2017-06-21T17:25:31Z

Having highway shields inline like we used to do would be the preferred approach, except with the shields on the left and with a direction suffix and destination <I-5> S / Los Angeles instead of the ceremonial name at highway speeds.

One of the most important points in fixing this issue is adding a direction suffix and destination information when available. It's common to see highways where both directions are served by the same initial offramp, and our current ceremonial name approach does a poor job differentiating those.

Here's some logic we've used in the past to estimate if the current step is a highway:

let speed = currentStep.distance / currentStep.expectedTravelTime
// Convert 47.5mph to m/s 
if speed > 47.5 * 1609.344 / 3600.0 {
}

In the case of the ambiguous us prefix, we could default to the following logic (<> indicates an inline shield):

<US-101> N / Portland: highway speeds: always show direction + destination when available
<US-101> North: highway speeds: always show direction when available
Van Ness Ave <US-101>: local speeds: use name + show the highway shield inlined to the right
<US-101> Van Ness Ave: Fallback: highway shield on the left, ceremonial name on the right

1ec5 · 2017-06-21T19:39:16Z

One of the most important points in fixing this issue is adding a direction suffix and destination information when available.

I think this may rely on one of the more fragile parts of the exit tagging scheme. OSM has specific, structured tags for destination refs, but there's no dedicated tag for the direction. (This information is encoded in route relations, but OSRM lacks the ability to process relations.) So mappers have generally taken to saying "US 101 North" as the ref. Anything that requires string munging on our end will have uneven reliability: consider that the direction is usually given as a suffix in California, whereas it may be given as a prefix in other parts of the U.S. due to differing signage standards.

1ec5 · 2017-06-22T19:42:49Z

MapboxNavigation/RouteManeuverViewController.swift

+
+        if let destinations = step?.destinations {
+            let dest = destinations.prefix(min(streetLabelLines, destinations.count)).joined(separator: "\n")
+            streetLabel.unabridgedText = stepRef != nil ? "\(String(describing: stepRef)) - \(dest)" : dest


This could result in a string like:

Optional(I) - Mobile Biloxi

1ec5 · 2017-06-24T03:45:08Z

MapboxNavigation/RouteManeuverViewController.swift

-            streetLabel.unabridgedText = stepRef != nil ? "\(String(describing: stepRef)) - \(dest)" : dest
+
+            if let stepRef = stepRef {
+                streetLabel.unabridgedText = "\(String(describing: stepRef)) - \(dest)"


stepRef is already a String; no need to create a new string out of it.

1ec5 · 2017-06-24T03:45:44Z

MapboxNavigation/RouteManeuverViewController.swift

-            streetLabel.unabridgedText = stepRef != nil ? "\(String(describing: stepRef)) - \(dest)" : dest
+
+            if let stepRef = stepRef {
+                streetLabel.unabridgedText = "\(String(describing: stepRef)) - \(dest)"


I agree with #291 (comment) that / would be a more aesthetically pleasing delimiter than -.

bsudekum · 2017-07-06T19:58:50Z

@1ec5 I think this ready to go.

1ec5 · 2017-07-07T01:59:42Z

MapboxNavigation/RouteManeuverViewController.swift

-        } else if let destinations = step?.destinations {
-            destinationLabel.unabridgedText = destinations.prefix(min(numberOfDestinationLines, destinations.count)).joined(separator: "\n")
+        var stepRef: String?
+        if let step = step, let ref = step.codes?.first, ["i", "us"].contains(ref.lowercased().components(separatedBy: " ").first!) {


We shouldn't remove the name from all U.S. routes. That would be a severe usability regression, because the vast majority of U.S. routes are surface streets, and in urban areas they're much more likely to be identified by name than by route number.

A proper solution to this problem appears to be coming soon in Project-OSRM/osrm-text-instructions#51, based on Project-OSRM/osrm-backend#4204. If we can't wait until then to solve the relatively few cases of U.S. routes on freeways, then we should only omit the name of a U.S. route if it contains the word "freeway", "expressway", or "highway".

If we can't wait until then to solve the relatively few cases of U.S. routes on freeways, then we should only omit the name of a U.S. route if it contains the word "freeway", "expressway", or "highway".

I'm game for this approach 😃 @bsudekum.

bsudekum · 2017-07-18T00:06:22Z

@1ec5 @ericrwolfe updated this, now:

We prioritize destination over name
If there is no destination but there is a name with one of the keywords, use the ref
If not, use the name.

bsudekum · 2017-07-18T00:58:12Z

I played around with getting the voice instructions to follow this logic as well. It looks like this will require a change to osrm-text-instructions.

bsudekum · 2017-07-18T21:43:22Z

@1ec5 updated this to attempt to change the bottom label to be consistent with the maneuver text.

I'm finding it difficult to to go from 280 to I-280 though.

1ec5 · 2017-07-18T22:41:54Z

MapboxNavigation/Highways.plist

+	<key>us-highway-duplex</key>
+	<string>US</string>
+	<key>us-highway-alternate</key>
+	<string>US</string>


Given:

shield: us-highway-alternate ref: 50

the label will incorrectly say “US 50”, whereas it would be more accurate to say “US 50 Alt” or “Alt US 50”. Same for the other banners below.

1ec5 · 2017-07-18T22:55:04Z

MapboxNavigation/String.swift

+
+    var isFreeway: Bool {
+        return self.components(separatedBy: " ").filter {
+            ["freeway", "expressway", "highway", "fwy", "hwy"].contains($0)


The Mapbox Streets source abbreviates “expressway” as “Expy”.

1ec5 · 2017-07-18T22:57:29Z

MapboxNavigation/String.swift

@@ -28,4 +25,10 @@ extension String {
            ("'", "&apos;")
            ])
    }
+
+    var isFreeway: Bool {
+        return self.components(separatedBy: " ").filter {


Nit: drop self..

1ec5 · 2017-07-18T22:59:22Z

#291 (comment):

A proper solution to this problem appears to be coming soon in Project-OSRM/osrm-text-instructions#51, based on Project-OSRM/osrm-backend#4204.

Underlying support for this feature is happening in mapbox/mapbox-directions-swift#154 and will also require changes to OSRM Text Instructions in Swift.

1ec5 · 2017-07-21T10:04:50Z

MapboxNavigation/RouteMapViewController.swift

                        } else {
                            currentName = nil
                        }
                    }
                }
            }

-            if smallestLabelDistance < 5 && currentName != nil {
+            if smallestLabelDistance < 10 && currentName != nil {


Is this change necessary?

1ec5 · 2017-07-21T18:31:00Z

MapboxNavigation/RouteMapViewController.swift

+                                if let highwayName = HighwayNamesByPrefix[shield] {
+                                    currentName = "\(highwayName) \(ref)"
+                                } else {
+                                    currentName = ref


For any country outside the U.S., this logic would display only the number. Even inside the U.S., something like a county road would get just the number, which is opposite what I would expect. I think it would be preferable in all these cases to either show the name and number or to show nothing at all.

1ec5 · 2017-07-21T22:40:47Z

#410 is an alternative to this PR, at least as far as the turn banner is concerned. We can pick this PR back up later to deal with the current road name label, but I don’t think it’s a huge deal compared to the turn banner, since the base map also displays freeway names.

bsudekum · 2017-07-22T17:44:45Z

Let's close this out. A followup PR will most likely go in another direction than the one started here.

bsudekum added the ⚠️ DO NOT MERGE PR should not be merged! label Jun 20, 2017

1ec5 reviewed Jun 20, 2017

View reviewed changes

1ec5 mentioned this pull request Jun 20, 2017

Store current road details in map view controller #297

Closed

1ec5 reviewed Jun 22, 2017

View reviewed changes

bsudekum removed the ⚠️ DO NOT MERGE PR should not be merged! label Jun 23, 2017

1ec5 reviewed Jun 24, 2017

View reviewed changes

This was referenced Jun 29, 2017

Display shields for destinations in turn banner #320

Closed

Use relations parsed from OSRM from to include cardinal direction in instructions Project-OSRM/osrm-text-instructions#119

Closed

bsudekum force-pushed the ceremonial branch from 23d26e9 to c5ee68a Compare July 6, 2017 19:58

1ec5 suggested changes Jul 7, 2017

View reviewed changes

ericrwolfe mentioned this pull request Jul 12, 2017

Improve current way name label #193

Closed

Bobby Sudekum added 2 commits July 17, 2017 16:45

Don't use ceremonial name when way is interstate

d8ebc10

More ref

7a5da03

bsudekum force-pushed the ceremonial branch from c5ee68a to 7a5da03 Compare July 17, 2017 23:45

check for keywords in name

03f8a3c

Use isFreeway

ca65fe0

Bobby Sudekum added 2 commits July 18, 2017 14:43

remove

9c104c6

Use plist

c175151

1ec5 reviewed Jul 18, 2017

View reviewed changes

1ec5 reviewed Jul 21, 2017

View reviewed changes

1ec5 mentioned this pull request Jul 21, 2017

Suppress freeway names in turn banner #410

Merged

bsudekum closed this Jul 22, 2017

1ec5 mentioned this pull request May 17, 2018

Reimplement current road name label atop navigator status/electronic horizon #1440

Closed

JThramer deleted the ceremonial branch September 7, 2018 20:34

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Don't use ceremonial name when way is interstate #291

Don't use ceremonial name when way is interstate #291

bsudekum commented Jun 20, 2017

frederoni commented Jun 20, 2017

1ec5 Jun 20, 2017

ericrwolfe Jun 20, 2017

1ec5 Jun 20, 2017

1ec5 Jun 20, 2017 •

edited

Loading

bsudekum Jun 22, 2017

1ec5 Jun 22, 2017

1ec5 Jun 20, 2017 •

edited

Loading

1ec5 Jun 20, 2017

ericrwolfe commented Jun 20, 2017

1ec5 commented Jun 20, 2017

1ec5 commented Jun 20, 2017 •

edited

Loading

ericrwolfe commented Jun 20, 2017

ericrwolfe commented Jun 21, 2017

1ec5 commented Jun 21, 2017

1ec5 Jun 22, 2017

1ec5 Jun 24, 2017

1ec5 Jun 24, 2017

bsudekum commented Jul 6, 2017

1ec5 Jul 7, 2017

ericrwolfe Jul 12, 2017

bsudekum commented Jul 18, 2017

bsudekum commented Jul 18, 2017

bsudekum commented Jul 18, 2017

1ec5 Jul 18, 2017

1ec5 Jul 18, 2017

1ec5 Jul 18, 2017

1ec5 commented Jul 18, 2017

1ec5 Jul 21, 2017

bsudekum Jul 21, 2017

1ec5 Jul 21, 2017

1ec5 commented Jul 21, 2017

bsudekum commented Jul 22, 2017

Don't use ceremonial name when way is interstate #291

Don't use ceremonial name when way is interstate #291

Conversation

bsudekum commented Jun 20, 2017

frederoni commented Jun 20, 2017

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

1ec5 Jun 20, 2017 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

1ec5 Jun 20, 2017 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

ericrwolfe commented Jun 20, 2017

1ec5 commented Jun 20, 2017

1ec5 commented Jun 20, 2017 • edited Loading

ericrwolfe commented Jun 20, 2017

ericrwolfe commented Jun 21, 2017

1ec5 commented Jun 21, 2017

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

bsudekum commented Jul 6, 2017

Choose a reason for hiding this comment

Choose a reason for hiding this comment

bsudekum commented Jul 18, 2017

bsudekum commented Jul 18, 2017

bsudekum commented Jul 18, 2017

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

1ec5 commented Jul 18, 2017

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

1ec5 commented Jul 21, 2017

bsudekum commented Jul 22, 2017

1ec5 Jun 20, 2017 •

edited

Loading

1ec5 Jun 20, 2017 •

edited

Loading

1ec5 commented Jun 20, 2017 •

edited

Loading