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

Add roadSide to RouteStep #219

Merged
merged 5 commits into from
Dec 4, 2017
Merged
Show file tree
Hide file tree
Changes from 3 commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
60 changes: 56 additions & 4 deletions MapboxDirections/MBRouteStep.swift
Original file line number Diff line number Diff line change
Expand Up @@ -421,6 +421,45 @@ public enum ManeuverDirection: Int, CustomStringConvertible {
}
}

/**
A `RoadSide` indicates which side of the road cars and traffic flow.
*/
@objc(MBRoadSide)
public enum RoadSide: Int, CustomStringConvertible {
/**
Indicates driving occurs on the `left` side.
*/
case left

/**
Indicates driving occurs on the `right` side.
*/
case right
Copy link

Choose a reason for hiding this comment

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

Although this is pretty self explanatory, can you document this class it's properties?


public init?(description: String) {
var side: RoadSide
switch description {
case "left":
side = .left
case "right":
side = .right
default:
return nil
}

self.init(rawValue: side.rawValue)
}

public var description: String {
switch self {
case .left:
return "left"
case .right:
return "right"
}
}
}

extension String {
internal func tagValues(separatedBy separator: String) -> [String] {
return components(separatedBy: separator).map { $0.trimmingCharacters(in: .whitespaces) }.filter { !$0.isEmpty }
Expand Down Expand Up @@ -488,7 +527,7 @@ struct Road {
open class RouteStep: NSObject, NSSecureCoding {
// MARK: Creating a Step

internal init(finalHeading: CLLocationDirection?, maneuverType: ManeuverType?, maneuverDirection: ManeuverDirection?, maneuverLocation: CLLocationCoordinate2D, name: String, coordinates: [CLLocationCoordinate2D]?, json: JSONDictionary) {
internal init(finalHeading: CLLocationDirection?, maneuverType: ManeuverType?, maneuverDirection: ManeuverDirection?, roadSide: RoadSide, maneuverLocation: CLLocationCoordinate2D, name: String, coordinates: [CLLocationCoordinate2D]?, json: JSONDictionary) {
transportType = TransportType(description: json["mode"] as! String)

let road = Road(name: name, ref: json["ref"] as? String, exits: json["exits"] as? String, destination: json["destinations"] as? String, rotaryName: json["rotary_name"] as? String)
Expand Down Expand Up @@ -542,6 +581,7 @@ open class RouteStep: NSObject, NSSecureCoding {

self.maneuverLocation = maneuverLocation
self.coordinates = coordinates
self.roadSide = roadSide
}

/**
Expand All @@ -557,6 +597,7 @@ open class RouteStep: NSObject, NSSecureCoding {
let maneuverType = ManeuverType(description: maneuver["type"] as! String)
let maneuverDirection = ManeuverDirection(description: maneuver["modifier"] as? String ?? "")
let maneuverLocation = CLLocationCoordinate2D(geoJSON: maneuver["location"] as! [Double])
let roadSide = RoadSide(description: json["driving_side"] as! String) ?? .right
Copy link

Choose a reason for hiding this comment

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

Unsure how to handle a nil here.

Copy link
Contributor

Choose a reason for hiding this comment

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

Is it possible for any production profile to not provide a driving_side property? If so, we would add an unknown enumeration value. If not, we would assert or implicitly unwrap the optional.


let name = json["name"] as! String

Expand All @@ -570,7 +611,7 @@ open class RouteStep: NSObject, NSSecureCoding {
coordinates = nil
}

self.init(finalHeading: finalHeading, maneuverType: maneuverType, maneuverDirection: maneuverDirection, maneuverLocation: maneuverLocation, name: name, coordinates: coordinates, json: json)
self.init(finalHeading: finalHeading, maneuverType: maneuverType, maneuverDirection: maneuverDirection, roadSide: roadSide, maneuverLocation: maneuverLocation, name: name, coordinates: coordinates, json: json)
}

public required init?(coder decoder: NSCoder) {
Expand Down Expand Up @@ -602,6 +643,11 @@ open class RouteStep: NSObject, NSSecureCoding {
maneuverDirection = nil
}

guard let roadSideDescription = decoder.decodeObject(of: NSString.self, forKey: "roadSide") as String?, let roadSide = RoadSide(description: roadSideDescription) else {
return nil
}
self.roadSide = roadSide
Copy link

Choose a reason for hiding this comment

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

Will this be an issue for old routes that are encoded without a roadSide?

Copy link
Contributor

Choose a reason for hiding this comment

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

Yes, let’s assume it’s right if unspecified. That would match OSRM’s behavior prior to Project-OSRM/osrm-backend#4415.

/cc @danpat


if let maneuverLocationDictionary = decoder.decodeObject(of: [NSDictionary.self, NSString.self, NSNumber.self], forKey: "maneuverLocation") as? [String: CLLocationDegrees],
let latitude = maneuverLocationDictionary["latitude"],
let longitude = maneuverLocationDictionary["longitude"] {
Expand Down Expand Up @@ -655,6 +701,7 @@ open class RouteStep: NSObject, NSSecureCoding {

coder.encode(maneuverType?.description, forKey: "maneuverType")
coder.encode(maneuverDirection?.description, forKey: "maneuverDirection")
coder.encode(roadSide.description, forKey: "roadSide")

coder.encode(intersections, forKey: "intersections")

Expand Down Expand Up @@ -785,6 +832,11 @@ open class RouteStep: NSObject, NSSecureCoding {
*/
open let maneuverDirection: ManeuverDirection?

/**
Indicates what side of a bidirectional road the driver must be driving on. Also referred to as the rule of the road.
*/
open let roadSide: RoadSide

/**
The location of the maneuver at the beginning of this step.
*/
Expand Down Expand Up @@ -952,10 +1004,10 @@ internal class RouteStepV4: RouteStep {
let maneuverType = ManeuverType(v4Description: maneuver["type"] as! String)
let maneuverDirection = ManeuverDirection(v4TypeDescription: maneuver["type"] as! String)
let maneuverLocation = CLLocationCoordinate2D(geoJSON: maneuver["location"] as! JSONDictionary)

let roadSide = RoadSide(description: json["driving_side"] as! String) ?? .right
let name = json["way_name"] as! String

self.init(finalHeading: heading, maneuverType: maneuverType, maneuverDirection: maneuverDirection, maneuverLocation: maneuverLocation, name: name, coordinates: nil, json: json)
self.init(finalHeading: heading, maneuverType: maneuverType, maneuverDirection: maneuverDirection, roadSide: roadSide, maneuverLocation: maneuverLocation, name: name, coordinates: nil, json: json)
}
}

Expand Down
2 changes: 1 addition & 1 deletion MapboxDirectionsTests/Fixtures/instructions.json

Large diffs are not rendered by default.

Large diffs are not rendered by default.

Large diffs are not rendered by default.

Large diffs are not rendered by default.

Large diffs are not rendered by default.

3 changes: 2 additions & 1 deletion MapboxDirectionsTests/RouteStepTests.swift
Original file line number Diff line number Diff line change
Expand Up @@ -121,7 +121,7 @@ class RouteStepTests: XCTestCase {
"pronunciation": "ˈaɪˌfoʊ̯n ˈtɛn",
] as [String: Any]

let step = RouteStep(finalHeading: 59, maneuverType: .reachFork, maneuverDirection: .left, maneuverLocation: CLLocationCoordinate2D(latitude: 37.853913, longitude: -122.220694), name: "", coordinates: coordinates, json: json)
let step = RouteStep(finalHeading: 59, maneuverType: .reachFork, maneuverDirection: .left, roadSide: .left, maneuverLocation: CLLocationCoordinate2D(latitude: 37.853913, longitude: -122.220694), name: "", coordinates: coordinates, json: json)

// Encode and decode the route step securely
// This may raise an Obj-C exception if an error is encountered which will fail the tests
Expand Down Expand Up @@ -158,5 +158,6 @@ class RouteStepTests: XCTestCase {
XCTAssertEqual(unarchivedStep.destinations ?? [], step.destinations ?? [])
XCTAssertEqual(unarchivedStep.instructionsSpokenAlongStep ?? [], step.instructionsSpokenAlongStep ?? [])
XCTAssertEqual(unarchivedStep.instructionsDisplayedAlongStep ?? [], step.instructionsDisplayedAlongStep ?? [])
XCTAssertEqual(unarchivedStep.roadSide, RoadSide.left)
}
}