-
Notifications
You must be signed in to change notification settings - Fork 90
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
Attributes #118
Attributes #118
Conversation
MapboxDirections/MBAnnotation.swift
Outdated
// MapboxDirections | ||
// | ||
// Created by Bobby Sudekum on 3/20/17. | ||
// Copyright © 2017 Mapbox. All rights reserved. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Omit this file header comment. This is a permissively licensed project under version control, so the comment adds little value.
MapboxDirections/MBRouteLeg.swift
Outdated
@@ -98,6 +130,9 @@ open class RouteLeg: NSObject, NSSecureCoding { | |||
*/ | |||
open let steps: [RouteStep] | |||
|
|||
|
|||
open let annotation: [AnnotationType:[Any]]? |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Annotation is a poor name for this information for the following reasons:
- It's a collection of data, so a plural word is preferable.
- The Mapbox iOS and macOS SDKs already have a concept of annotations (for consistency with MapKit) that's completely different: markers and overlays on a map. Google Maps calls them markers, but developers are required to use the Mapbox SDKs with this library anyways.
- The term doesn't say anything about the relationship between the route leg and the information indicated by the property.
Some suggestions:
- nodeAttributes (so far only for the nodeIdentifiers key) and segmentAttributes (for the other keys)
- attributes
- userInfo – Although this may sound strange, it's consistent with Cocoa APIs like NSError and NSNotification.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
With a split between "node attributes" and "segment attributes", we'll eventually be able to distribute the attribute values among each step (while still retaining the full, leg-level attribute structure as an option).
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
A little trivia: "segment" used to be a primitive data type in OpenStreetMap back in the day.
MapboxDirections/MBAnnotation.swift
Outdated
|
||
case duration | ||
|
||
case nodes |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
(I'm going to call annotation types "attributes" for the rest of this review; see below.)
Let's call this openStreetMapNodeIdentifier
, similar to how MapboxGeocoder.swift has a wikidataItemIdentifier
. If on the other hand we expect the same underlying string to be used for non-OSM data sources, nodeIdentifier
would be better. (I'd like to avoid using "node" by itself, because perhaps we may associate more attributes with OSM nodes in the future.)
MapboxDirections/MBAnnotation.swift
Outdated
@objc(AnnotationType) | ||
public enum AnnotationType: Int, CustomStringConvertible { | ||
|
||
case congestion |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
"Congestion" is a strange word to use on its own, since it's uncountable. (Imagine talking about "congestions".) Let's call it congestionLevel
.
MapboxDirections/MBRouteLeg.swift
Outdated
|
||
var annotation: Annotation = [:] | ||
if json["annotation"] != nil { | ||
let rawAnnoation = json["annotation"] as! [String: Any] |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
s/annoation/annotation/
MapboxDirections/MBRouteLeg.swift
Outdated
if json["annotation"] != nil { | ||
let rawAnnoation = json["annotation"] as! [String: Any] | ||
if rawAnnoation["congestion"] != nil { | ||
annotation[.congestion] = (rawAnnoation["congestion"] as! [String]).map { CongestionLevel(description: $0) ?? .unknown} |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
To avoid repeating yourself, use an if let … as? …
above`.
@@ -256,6 +258,15 @@ open class RouteOptions: NSObject { | |||
params.append(URLQueryItem(name: "radiuses", value: accuracies)) | |||
} | |||
|
|||
|
|||
if annotation.count > 0 { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Nit: !annotation.isEmpty
@@ -211,6 +211,8 @@ open class RouteOptions: NSObject { | |||
*/ | |||
open var routeShapeResolution = RouteShapeResolution.low | |||
|
|||
open var annotation: [AnnotationType] = [] |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
attributes
MapboxDirections/MBRouteLeg.swift
Outdated
@@ -40,6 +68,9 @@ open class RouteLeg: NSObject, NSSecureCoding { | |||
} | |||
source = decodedSource | |||
|
|||
|
|||
annotation = nil |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The encode method encodes the attribute data, so the decode method should decode it.
MapboxDirections/MBAnnotation.swift
Outdated
|
||
case distance | ||
|
||
case duration |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
"Durations" can indicate lots of things. Let's call this expectedTravelTime
, for consistency with the expectedTravelTime
property of RouteLeg and RouteStep.
@1ec5 updated with feedback |
@@ -15,6 +15,8 @@ | |||
358D48871E2EAB4500F32A65 /* MBRouteOptions.m in Sources */ = {isa = PBXBuildFile; fileRef = 358D48851E2EAB4500F32A65 /* MBRouteOptions.m */; }; | |||
358D48881E2EAB4500F32A65 /* MBRouteOptions.m in Sources */ = {isa = PBXBuildFile; fileRef = 358D48851E2EAB4500F32A65 /* MBRouteOptions.m */; }; | |||
358D48891E2EAB4500F32A65 /* MBRouteOptions.m in Sources */ = {isa = PBXBuildFile; fileRef = 358D48851E2EAB4500F32A65 /* MBRouteOptions.m */; }; | |||
C51538CC1E807FF00093FF3E /* MBAnnotation.swift in Sources */ = {isa = PBXBuildFile; fileRef = C51538CB1E807FF00093FF3E /* MBAnnotation.swift */; }; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This file needs to be added to all four framework targets. Because it’s only in the iOS framework target, macOS, tvOS, and watchOS builds are failing with numerous errors, starting with:
▸ Compiling MBRouteLeg.swift
❌ /Users/vagrant/git/MapboxDirections/MBRouteLeg.swift:11:36: use of undeclared type 'SegmentAttribute'
typealias SegmentAttributes = [SegmentAttribute:[Any]]
^~~~~~~~~~~~~~~~
❌ /Users/vagrant/git/MapboxDirections/MBRouteLeg.swift:129:34: use of undeclared type 'SegmentAttribute'
open let segmentAttributes: [SegmentAttribute:[Any]]?
^~~~~~~~~~~~~~~~
❌ /Users/vagrant/git/MapboxDirections/MBRouteLeg.swift:173:14: cannot invoke 'RouteLegV4.init' with an argument list of type '(steps: [RouteStepV4], json: JSONDictionary, source: Waypoint, destination: Waypoint, profileIdentifier: MBDirectionsProfileIdentifier)'
self.init(steps: steps, json: json, source: source, destination: destination, profileIdentifier: profileIdentifier)
^
@@ -15,6 +15,8 @@ | |||
358D48871E2EAB4500F32A65 /* MBRouteOptions.m in Sources */ = {isa = PBXBuildFile; fileRef = 358D48851E2EAB4500F32A65 /* MBRouteOptions.m */; }; | |||
358D48881E2EAB4500F32A65 /* MBRouteOptions.m in Sources */ = {isa = PBXBuildFile; fileRef = 358D48851E2EAB4500F32A65 /* MBRouteOptions.m */; }; | |||
358D48891E2EAB4500F32A65 /* MBRouteOptions.m in Sources */ = {isa = PBXBuildFile; fileRef = 358D48851E2EAB4500F32A65 /* MBRouteOptions.m */; }; | |||
C51538CC1E807FF00093FF3E /* MBAnnotation.swift in Sources */ = {isa = PBXBuildFile; fileRef = C51538CB1E807FF00093FF3E /* MBAnnotation.swift */; }; | |||
C5247D711E818A24004B6154 /* AnnotationTests.swift in Sources */ = {isa = PBXBuildFile; fileRef = C5247D701E818A24004B6154 /* AnnotationTests.swift */; }; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Likewise, this file needs to be added to all four unit test targets.
MapboxDirections/MBAnnotation.swift
Outdated
@@ -0,0 +1,45 @@ | |||
import Foundation | |||
|
|||
@objc(AnnotationType) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This type needs a documentation comment.
MapboxDirections/MBAnnotation.swift
Outdated
@objc(AnnotationType) | ||
public enum SegmentAttribute: Int, CustomStringConvertible { | ||
|
||
case distance |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Each of these cases needs a documentation comment.
MapboxDirections/MBAnnotation.swift
Outdated
import Foundation | ||
|
||
@objc(AnnotationType) | ||
public enum SegmentAttribute: Int, CustomStringConvertible { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This type should be called simply Attribute
, because openStreetMapNodeIdentifier
is a node attribute, not a segment attribute.
MapboxDirections/MBRouteLeg.swift
Outdated
@@ -40,6 +63,9 @@ open class RouteLeg: NSObject, NSSecureCoding { | |||
} | |||
source = decodedSource | |||
|
|||
|
|||
segmentAttributes = nil |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@@ -256,6 +258,16 @@ open class RouteOptions: NSObject { | |||
params.append(URLQueryItem(name: "radiuses", value: accuracies)) | |||
} | |||
|
|||
|
|||
if segmentAttributes.count > 0 { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
|
||
if segmentAttributes.count > 0 { | ||
let segmentAttributesStrings = segmentAttributes.map { | ||
$0.description |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
String(describing: $0)
XCTAssertNotNil(annotation[.distance]) | ||
XCTAssertNotNil(annotation[.expectedTravelTime]) | ||
XCTAssertNotNil(annotation[.openStreetMapNodeIdentifier]) | ||
XCTAssertNotNil(annotation[.speed]) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
These tests should also check specific attribute values.
XCTAssertNotNil(annotation[.openStreetMapNodeIdentifier]) | ||
XCTAssertNotNil(annotation[.speed]) | ||
|
||
let nodes = annotation[.openStreetMapNodeIdentifier]!.count |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Check that there are the right number of node identifier attributes.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Some bad news about Objective-C bridging, but it turns out that addressing these issues will make the API make more sense even in Swift.
MapboxDirections/MBRouteLeg.swift
Outdated
@@ -59,6 +84,10 @@ open class RouteLeg: NSObject, NSSecureCoding { | |||
return nil | |||
} | |||
profileIdentifier = MBDirectionsProfileIdentifier(rawValue: decodedProfileIdentifier) | |||
|
|||
segmentAttributes = decoder.decodeObject(of: [NSArray.self], forKey: "segmentAttributes") as? [Attribute: [Any]] ?? [:] |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The code below encodes the object as a dictionary. This line tries to decode the object as an NSArray, which will produce nil. Even if it were to somehow produce an array, this line then casts the array to a dictionary, which will produce nil.
The class you pass into decodeObject(of:forKey:)
should match the class of the object you pass into encode(_:forKey:)
below.
@@ -211,6 +211,10 @@ open class RouteOptions: NSObject { | |||
*/ | |||
open var routeShapeResolution = RouteShapeResolution.low | |||
|
|||
open var segmentAttributes: [Attribute] = [] |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
An array of enum values won't bridge to Objective-C. Besides, we want an OptionSet for this particular property, since it doesn't make sense to specify .speeds
twice, for example. I think we're going to need something similar to LaneIndication – an Objective-C NS_OPTION
instead of this Swift enumeration.
MapboxDirections/MBRouteLeg.swift
Outdated
@@ -98,6 +129,11 @@ open class RouteLeg: NSObject, NSSecureCoding { | |||
*/ | |||
open let steps: [RouteStep] | |||
|
|||
|
|||
open let segmentAttributes: [Attribute:[Any]]? |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This property won't bridge to Objective-C, because NSDictionary's keys need to be objects, not enumeration values. (Option sets won't bridge either.) We'll probably want to use an Objective-C string enumeration (NS_STRING_ENUM
) instead of the Swift enumeration. It would be separate from the option set used in RouteOptions.
If that's too much trouble, RouteLeg could have a separate property for each attribute. For example, segmentSpeeds
would be of type [CLLocationSpeed]
. That would be pretty clean, but I don't know what the story will be for any unsupported attributes we get back from the API.
MapboxDirections/MBAttribute.h
Outdated
|
||
MBAttributeSpeed = (1 << 4), | ||
|
||
MBAttributeAll = 0x0ffffUL |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
We probably don’t need an “all” option for attributes.
@1ec5 this is ready for another round of 👀 |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This is coming together nicely. The tests are looking good too. Don’t forget to add documentation for any new types and properties.
Cartfile.resolved
Outdated
binary "https://www.mapbox.com/ios-sdk/Mapbox-iOS-SDK.json" "3.4.2" | ||
github "AliSoftware/OHHTTPStubs" "57feceaabf333e72b2c637dfba6c13a7a5c49619" | ||
binary "https://www.mapbox.com/ios-sdk/Mapbox-iOS-SDK.json" "3.5.0" | ||
github "AliSoftware/OHHTTPStubs" "f90c2bb0fb882e43761ab963ca8869d349d2c6e3" |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This is fine, but in the future, let’s split out any changes that occur due to running carthage update
into separate PRs, so that we can narrow down any regressions they cause. If you just need to get set up, use carthage bootstrap
instead.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I'll revert this
MapboxDirections/MBRouteLeg.swift
Outdated
self.segmentAttributes = segmentAttributes | ||
self.nodeAttributes = nodeAttributes | ||
|
||
var segmentDistances: [CLLocationDistance]? = nil |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Nit: an optional is nil
by default; no need to set it to nil
.
MapboxDirections/MBRouteLeg.swift
Outdated
|
||
open let nodeAttributes: [Attribute:[Any]]? | ||
open let nodeOpenStreetMapNodeIdentifiers: [Int]? |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
To avoid overflow, OSM node IDs must be stored as 64-bit integers. Int
can be either 32-bit or 64-bit according to the device’s architecture. To be on the safe side and to clarify our intent, use Int64
.
MapboxDirections/MBRouteLeg.swift
Outdated
|
||
open let nodeAttributes: [Attribute:[Any]]? | ||
open let nodeOpenStreetMapNodeIdentifiers: [Int]? |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
“Node” is in the name twice, unnecessarily: rename this property to openStreetMapNodeIdentifiers
.
MapboxDirections/MBRouteLeg.swift
Outdated
@@ -129,10 +134,13 @@ open class RouteLeg: NSObject, NSSecureCoding { | |||
*/ | |||
open let steps: [RouteStep] | |||
|
|||
open let segmentDistances: [CLLocationDistance]? | |||
|
|||
open let segmentExpectedTravelTimes: [TimeInterval]? |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The name of this property makes it sound like a verb, “to segment the expected travel times”. Rename this property to expectedSegmentTravelTimes
.
String(describing: $0) | ||
}.joined(separator: ",") | ||
}.joined(separator: ",") |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Attribute.description
is already a comma-separated list, so you can set attributesStrings
to String(describing: segmentAttributes + .openStreetMapNodeIdentifier)
to get the same effect.
@@ -211,6 +211,10 @@ open class RouteOptions: NSObject { | |||
*/ | |||
open var routeShapeResolution = RouteShapeResolution.low | |||
|
|||
open var segmentAttributes: [Attribute] = [] |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Now that Attribute is an OptionSet, we no longer need an array of Attributes; a single Attribute can store multiple options.
@@ -211,6 +211,10 @@ open class RouteOptions: NSObject { | |||
*/ | |||
open var routeShapeResolution = RouteShapeResolution.low | |||
|
|||
open var segmentAttributes: [Attribute] = [] | |||
|
|||
open var nodeAttributes: [Attribute] = [] |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Compared to the situation in RouteLeg before, there’s no need to distinguish between segment and node attributes in RouteOptions – the two behave exactly the same as far as requesting a route is concerned. A single attributeOptions
property will do.
MapboxDirections/MBAttribute.h
Outdated
@@ -0,0 +1,10 @@ | |||
typedef NS_OPTIONS(NSUInteger, MBAttribute) { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
One more rename! Now that this is an option set instead of a single attribute, rename this type to MBAttributeOptions, so it’s easier to talk about individual attributes versus the whole set of attributes.
@1ec5 fixed |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
A little more finessing around options, plus documentation, and I think this will be good to go!
@@ -211,6 +211,10 @@ open class RouteOptions: NSObject { | |||
*/ | |||
open var routeShapeResolution = RouteShapeResolution.low | |||
|
|||
open var segmentAttributes: AttributeOptions = [] | |||
|
|||
open var nodeAttributes: AttributeOptions = [] |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Combine these two properties into a single attributes property.
Started documentation in 5c2c5c0 |
MapboxDirections/MBAttribute.h
Outdated
@@ -1,10 +1,17 @@ | |||
/** | |||
Attributes are metadata information for a given route. The number of attributes returned will be a direct 1-1 relationship with the route's full geometry. Each type will return an ordered list of requested attributes. For `.distance`, `.expectedTrabelTime`, and `.speed` there will be one less value when compared to the route geometry. This is because these values represent the data on segment between geometry points. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The
.distance
,.expectedTrabelTime
, and.speed
attributes have one fewer value than the.openStreetMapNodeIdentifier
attribute.
MapboxDirections/MBAttribute.swift
Outdated
Creates an AttributeOptions from the given description strings. | ||
*/ | ||
public init?(description: String) { | ||
var scope: AttributeOptions = [] |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Nit: funky indentation.
MapboxDirections/MBAttribute.h
Outdated
*/ | ||
typedef NS_OPTIONS(NSUInteger, MBAttributeOptions) { | ||
|
||
/// Segment distance. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Distances are measured in meters.
MapboxDirections/MBAttribute.h
Outdated
// Segment expected travel time in seconds. | ||
MBAttributeExpectedTravelTime = (1 << 2), | ||
|
||
// Segment current speed. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Speeds are measured in meters per second.
MapboxDirections/MBAttribute.h
Outdated
// Segment current speed. | ||
MBAttributeSpeed = (1 << 3), | ||
|
||
// Unique OpenStreetMap node. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This option returns unique node identifiers, not the nodes themselves. Link to this OpenStreetMap Wiki article for more information. Also note that this attribute option is added automatically when any of the other attributes are added.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Ah, I thought we had baked that logic into RouteOptions, but it seems to have been removed. 👍
@@ -211,6 +211,13 @@ open class RouteOptions: NSObject { | |||
*/ | |||
open var routeShapeResolution = RouteShapeResolution.low | |||
|
|||
/** | |||
AttributeOptions for the route. Any combination of `.AttributeOptions` can be specified. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Remove the extra dot: .AttributeOptions
implies that there’s an AttributeOptions.AttributeOptions
.
/** | ||
AttributeOptions for the route. Any combination of `.AttributeOptions` can be specified. | ||
|
||
The default value of this property is `[]`. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
By default, no attribute options are specified.
@@ -211,6 +211,13 @@ open class RouteOptions: NSObject { | |||
*/ | |||
open var routeShapeResolution = RouteShapeResolution.low | |||
|
|||
/** | |||
AttributeOptions for the route. Any combination of `.AttributeOptions` can be specified. | |||
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Note that this option requires the routeShapeResolution
property to be set to .full
. (The default is .low
.)
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Unfortunately, this is not true. This should be ticketed upstream.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Can you elaborate? Do the attributes still work with .low
resolution? Do the values make any sense?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
If it’s the case that .low
resolution yields the same values (which reflect the .full
geometry), let’s recommend that the developer set .full
to make use of these options.
MapboxDirections/MBRouteLeg.swift
Outdated
open let expectedSegmentTravelTimes: [TimeInterval]? | ||
|
||
/** | ||
An array of current speeds along segment. This value is dynamic and does not represent the speed limit but rather the average speed for a given segment |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
An array containing the expected average speed (measured in meters per second) between each coordinate in the route leg geometry.
These values are dynamic; rather than speed limits, they account for the road’s classification and/or any traffic congestion (if the profile identifier is set to
.automobileAvoidingTraffic
).This property is set if the
RouteOptions.attributeOptions
property contains.speed
.
MapboxDirections/MBRouteLeg.swift
Outdated
open let segmentSpeeds: [CLLocationSpeed]? | ||
|
||
/** | ||
An array of OpenStreetMap nodes. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
An array containing OpenStreetMap node identifiers, one for each coordinate along the route geometry.
This property is set if the
RouteOptions.attributeOptions
property contains.openStreetMapNodeIdentifier
.
MapboxDirections/MBAttribute.h
Outdated
@@ -0,0 +1,17 @@ | |||
/** | |||
Attributes are metadata information for a given route. The number of attributes returned will be a direct 1-1 relationship with the route's full geometry. The .distance, .expectedTrabelTime, and .speed attributes have one fewer value than the .openStreetMapNodeIdentifier attribute. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Typo: s/expectedTrabelTime/expectedTravelTime/
MapboxDirections/MBAttribute.h
Outdated
@@ -0,0 +1,17 @@ | |||
/** | |||
Attributes are metadata information for a given route. The number of attributes returned will be a direct 1-1 relationship with the route's full geometry. The .distance, .expectedTrabelTime, and .speed attributes have one fewer value than the .openStreetMapNodeIdentifier attribute. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Surround references like .distance
in backticks so jazzy can autolink them.
MapboxDirections/MBAttribute.h
Outdated
// Segment current speed. Speeds are measured in meters per second. | ||
MBAttributeSpeed = (1 << 3), | ||
|
||
// Unique node identifiers (https://wiki.openstreetmap.org/wiki/Node) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Use Markdown syntax:
[OpenStreetMap node identifier](https://wiki.openstreetmap.org/wiki/Node).
MapboxDirections/MBRouteLeg.swift
Outdated
open let segmentSpeeds: [CLLocationSpeed]? | ||
|
||
/** | ||
An array containing OpenStreetMap node identifiers (https://wiki.openstreetmap.org/wiki/Node), one for each coordinate along the route geometry. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Use Markdown syntax here too.
@1ec5 think this is ready to go |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Whew! Thanks for all the iterations; this looks great.
Adds optional query param
annotation
. This is an array of annotation types, allowed enum values:.distance
,.duration
,.speed
,.node
.Still need to add documentation.
/cc @1ec5