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

Split destinations into name, ref, ref:to, ... #3304

Open
freenerd opened this issue Nov 14, 2016 · 8 comments
Open

Split destinations into name, ref, ref:to, ... #3304

freenerd opened this issue Nov 14, 2016 · 8 comments
Milestone

Comments

@freenerd
Copy link
Member

freenerd commented Nov 14, 2016

We should do the same name-ref splitting as in #2704 for destinations. Right now these are string concatenated but we've seen the need to separate these again (e.g. Project-OSRM/osrm-text-instructions#51) Naming of fields and response is up for debate.

Example route that would be impacted

# OSM Tags
destination: "Chain Bridge;McLean"
destination:ref: "VA 123"

# OSRM Response Now
destinations: "VA 123: Chain Bridge, McLean"

# OSRM Response Fixed
destinations: {
  names: "Chain Bridge, McLean",
  refs: "VA 123"
}
@freenerd freenerd added this to the 6.0 milestone Nov 14, 2016
@freenerd
Copy link
Member Author

Note that MapboxDirections.swift is currently doing this separation by splitting the string.

@TheMarex
Copy link
Member

Generally I think this is a good idea. Can someone give me some context why we do the ; -> , thing?

On the API side I would like to encode it as:

destinations: "Chain Bridge, McLean",
destination_refs: "VA 123"

Just because of the object overhead and it feels more consistent with name, ref and pronunciation.

@daniel-j-h
Copy link
Member

The question still stands if these responses should be arrays instead. Otherwise we're pushing the delimiter-separated list parsing just one level deeper.

E.g. instead of

destination_refs: "VA 123; VA 456"

should this be

destination_refs: [
  "VA 123",
  "VA 45"
]

The object overhead seems to be the only reason for not doing this.

@1ec5
Copy link
Member

1ec5 commented Jun 29, 2017

This issue makes it difficult for a client to determine whether to assign a highway shield to a given step. For example, this way is tagged:

destination:ref = CA 87 South
destination:ref:to = I 280

with no destination tag, because the on-ramp’s signage shows an I-280 shield instead of a textual destination name. OSRM gives a route step that travels along this way a destination of CA 87 South.

A client such as the Mapbox Navigation SDK needs to display plain text if CA 87 South is a road name or a shield if it’s a ref. Unfortunately, destination is used in either case, so the current logic in MapboxDirections.swift is unable to distinguish between name and ref. The navigation SDK, therefore, would have to fall back on fragile pattern matching (like /^([A-Z][A-Z]?)[- ]([\d\w]+)\b/i).

I’m not opposed to the additional structure proposed in #3329 and #3304 (comment), although MapboxDirections.swift already splits destination by semicolon anyways.

/cc @ericrwolfe

@1ec5
Copy link
Member

1ec5 commented Jul 12, 2017

#3542 would in some cases require destination:to and destination:ref:to as context.

@1ec5
Copy link
Member

1ec5 commented Sep 22, 2017

A client such as the Mapbox Navigation SDK needs to display plain text if CA 87 South is a road name or a shield if it’s a ref. Unfortunately, destination is used in either case, so the current logic in MapboxDirections.swift is unable to distinguish between name and ref. The navigation SDK, therefore, would have to fall back on fragile pattern matching (like /^([A-Z][A-Z]?)[- ]([\d\w]+)\b/i).

For mapbox/mapbox-navigation-ios#353, we had to resort to a hack that matches the destination property against a list of known ref prefixes; if there’s a match, the navigation SDK treats the destination property’s value as a ref in order to choose a route shield. This is suboptimal, because it’s entirely possible that the destination should be treated as a road name or place name rather than a ref, but the navigation SDK has no way to distinguish. It also doesn’t have enough context to distinguish one ref prefix from another. (M can be a state route in Michigan or a motorway in the UK or Australia.)

As part of expanding shield coverage beyond the United States (mapbox/mapbox-navigation-ios#334), it’ll become necessary to put the destination and destination:ref values in separate fields, even if they each remain flat strings with semicolons.

Copy link

github-actions bot commented Jul 8, 2024

This issue seems to be stale. It will be closed in 30 days if no further activity occurs.

@github-actions github-actions bot added the Stale label Jul 8, 2024
@1ec5 1ec5 removed the Stale label Jul 9, 2024
Copy link

github-actions bot commented Jan 5, 2025

This issue seems to be stale. It will be closed in 30 days if no further activity occurs.

@github-actions github-actions bot added the Stale label Jan 5, 2025
@1ec5 1ec5 removed the Stale label Jan 5, 2025
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

No branches or pull requests

4 participants