Add rotary and use lane types

[freenerd]

These two types were not present yet.

@1ec5 What other changes are required to bring these types in?

[1ec5]

This fixes #79.

[1ec5]

We’ll need to add support for the rotary_name type as well. Let’s land #91 first; then we can either add a junctionNames property on RouteStep or add a heuristic for rotaries that puts name in the destinations property and rotary_name in the names property, for consistency with ramps.

[freenerd]

we can either add a junctionNames property on RouteStep
add a heuristic for rotaries that puts name in the destinations property and rotary_name in the names property, for consistency with ramps.

For integrating osrm-text-instructions it will be more useful to have direct access to the rotary_name property. I'd rather not have heuristics that mangle with properties, but instead retain the original response structure in the object and add helper functions that return heuristically-derived results

[1ec5]

For integrating osrm-text-instructions it will be more useful to have direct access to the rotary_name property.

Direct equivalence to JSON properties is a non-goal for this library, but #60 would keep the original serialized JSON around for anyone who finds it useful. The tradeoff in that case is higher memory usage.

Once #91 lands, the logic I’m proposing for RouteStep would look something like this:

if maneuverType == .rotary {
    destinations = road.names ?? road.destinations
    names = road.rotaryNames
} else {
    destinations = road.destinations
    names = road.rotaryNames
}

I don’t consider this to be “mangling”, since you can easily reverse this logic:

if maneuverType == .rotary {
    rotaryName = step.names.componentsJoined(by: ";")
    name = step.destinations.componentsJoined(by: ";")
} else {
    destination = step.destinations.componentsJoined(by: ";")
    name = step.names.componentsJoined(by: ";")
}

The only problematic case would be for a rotary tagged with destination, but the destination tag seems ill-suited to rotaries anyways.

You’ll probably have a lot more difficulty reversing the logic in #91 that deals with ref, but again, the point was never to match the Directions API response one-for-one.

[freenerd]

destinations = road.names ?? road.destinations

This will be problematic for instructions, since we loose semantic if destinations is names or destinations. For the difference, see this example:

                   "name": "Enter {rotary_name} and exit onto {way_name}",
                    "destination": "Enter {rotary_name} and exit towards {destination}"

Take this example:

{
  "rotary_name": "Dupont Circle",
  "name":  "6th Avenue",
}

If we only expose destination then the correct instruction Enter Dupont Circle and exit onto 6th Avenue would become Enter Dupont Circle and exit towards 6th Avenue.

[1ec5]

If we only expose destination then the correct instruction Enter Dupont Circle and exit onto 6th Avenue would become Enter Dupont Circle and exit towards 6th Avenue.

As I pointed out in #93 (comment), I think you can safely ignore the destination for rotary instructions and interpret the destinations property as the exit name instead.

[1ec5]

An alternative would be to put rotary_name into the name property and name into a new exitName property (so named for consistency with exitIndex). If we go that route, we should consider putting name into exitName and leaving name empty for roundabout instructions.

[freenerd]

@1ec5 This sounds good to me, since all information is carried through and no semantics are lost. To paraphrase:

if type == .rotary || type == .roundabout {
  step.name = json.rotary_name
  step.destinations = json.destinations
  step.exitName = json.name // empty on .roundabout
}

[1ec5]

Yes, that’s the idea. My only reservation is that there’s already a proliferation of road attribute properties on RouteStep. Ideally we’d shunt them all into a struct, but that won’t bridge to Objective-C; meanwhile, an Objective-C class would be overkill. But if it helps us rationalize our naming logic, then so be it.

I’m assuming we don’t want this behavior for TurnAtRoundabout since it’s supposed to be similar to an ordinary intersection.

[1ec5]

bcca7e0 implements the approach described in #93 (comment). Ready for review.

[freenerd]

🎉 i can't approve my own PR, but let's get this out

[freenerd]

I think this is different now with exitNames.

The step's name is the name of the rotary itself. The exit name is the name of the road to take after exiting the roundabout.