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 speed limit data returned by MapboxDirections.swift to the RouteProgress object #2112

Closed
wants to merge 6 commits into from
Closed
Show file tree
Hide file tree
Changes from all 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
2 changes: 1 addition & 1 deletion Cartfile
Original file line number Diff line number Diff line change
@@ -1,6 +1,6 @@
binary "https://www.mapbox.com/ios-sdk/Mapbox-iOS-SDK.json" ~> 4.3
binary "https://www.mapbox.com/ios-sdk/MapboxNavigationNative.json" ~> 6.1.1
github "mapbox/MapboxDirections.swift" ~> 0.27.3
github "mapbox/MapboxDirections.swift" "maxspeedAnnotations"
Copy link
Contributor

Choose a reason for hiding this comment

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

This PR depends on mapbox/mapbox-directions-swift#367. Once that PR lands, we’ll need to change this dependency to master. Finally, once we publish a new release of MapboxDirections.swift, we’d change this line and corresponding lines in the podspecs to require the new release.

github "mapbox/turf-swift" ~> 0.3
github "mapbox/mapbox-events-ios" ~> 0.8.1
github "ceeK/Solar" ~> 2.1.0
Expand Down
4 changes: 2 additions & 2 deletions MapboxCoreNavigation/NavigationRouteOptions.swift
Original file line number Diff line number Diff line change
Expand Up @@ -25,7 +25,7 @@ open class NavigationRouteOptions: RouteOptions {
shapeFormat = .polyline6
includesSteps = true
routeShapeResolution = .full
attributeOptions = [.congestionLevel, .expectedTravelTime]
attributeOptions = [.congestionLevel, .expectedTravelTime, .maximumSpeedLimit]
includesSpokenInstructions = true
locale = Locale.nationalizedCurrent
distanceMeasurementSystem = Locale.current.usesMetricSystem ? .metric : .imperial
Expand Down Expand Up @@ -79,7 +79,7 @@ open class NavigationMatchOptions: MatchOptions {
includesSteps = true
routeShapeResolution = .full
shapeFormat = .polyline6
attributeOptions = [.congestionLevel, .expectedTravelTime]
attributeOptions = [.congestionLevel, .expectedTravelTime, .maximumSpeedLimit]
includesSpokenInstructions = true
locale = Locale.nationalizedCurrent
distanceMeasurementSystem = Locale.current.usesMetricSystem ? .metric : .imperial
Expand Down
40 changes: 40 additions & 0 deletions MapboxCoreNavigation/RouteProgress.swift
Original file line number Diff line number Diff line change
Expand Up @@ -171,6 +171,11 @@ open class RouteProgress: NSObject {
*/
public var congestionTimesPerStep: [[[CongestionLevel: TimeInterval]]] = [[[:]]]

/**
An array containing speed limits on the route broken up by leg and then step.
*/
public var maximumSpeedLimitsByLeg: [[[SpeedLimit]]] = []

/**
Intializes a new `RouteProgress`.

Expand All @@ -184,6 +189,7 @@ open class RouteProgress: NSObject {
super.init()

for (legIndex, leg) in route.legs.enumerated() {
var maximumSpeedLimitsByStep: [[SpeedLimit]] = []
var maneuverCoordinateIndex = 0

congestionTimesPerStep.append([])
Expand Down Expand Up @@ -217,9 +223,43 @@ open class RouteProgress: NSObject {
}

congestionTravelTimesSegmentsByStep.append(congestionTravelTimesSegmentsByLeg)

maneuverCoordinateIndex = 0
if let segmentMaximumSpeedLimits = leg.segmentMaximumSpeedLimits {
for step in leg.steps {
Copy link
Contributor

Choose a reason for hiding this comment

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

Consolidate this loop with the loop above that populates the segmentCongestionLevels and expectedSegmentTravelTimes properties for better performance. We’ll need to refactor a bit of the code, so that the absence of traffic congestion data doesn’t prevent us from processing speed limit data and vice versa.

guard let coordinates = step.coordinates else { continue }
let stepCoordinateCount = step.maneuverType == .arrive ? Int(step.coordinateCount) : coordinates.dropLast().count
let nextManeuverCoordinateIndex = maneuverCoordinateIndex + stepCoordinateCount - 1

guard nextManeuverCoordinateIndex < segmentMaximumSpeedLimits.count else { continue }

let stepSegmentMaximumSpeedLimits = Array(segmentMaximumSpeedLimits[maneuverCoordinateIndex..<nextManeuverCoordinateIndex])
maneuverCoordinateIndex = nextManeuverCoordinateIndex
maximumSpeedLimitsByStep.append(stepSegmentMaximumSpeedLimits)
}
}

maximumSpeedLimitsByLeg.append(maximumSpeedLimitsByStep)
}
}

/**
Returns the SpeedLimit for the current position along the route. Returns SpeedLimit.invalid if the speed limit is unknown or missing.
*/
public var currentSpeedLimit: SpeedLimit {
let coordinatesLeftOnStepCount = Int(floor((Double(currentLegProgress.currentStepProgress.step.coordinateCount)) * currentLegProgress.currentStepProgress.fractionTraveled))
Copy link
Contributor

Choose a reason for hiding this comment

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

This is the same approach taken by #1242. Unfortunately, it assumes that each coordinate is evenly spaced along the step, which is almost never the case. For our purposes, we should assume that the segments are of arbitrary length.

With traffic congestion, it made sense to store congestion levels by segment, because we use those segments to create a shape layer all at once. Displaying the right speed limit at the right time is a very similar problem to presenting the right spoken or visual instruction at the right time, as RouteController and LegacyRouteController do. As you loop over the attributes, set each speed limit to the cumulative distance along the step. Then this computed property can become:

public var currentSpeedLimit: SpeedLimit {
    
    let speedLimits = maximumSpeedLimitsByLeg[legIndex][currentLegProgress.stepIndex]
    return speedLimits.last { $0.distance <= distanceTraveled } ?? .invalid
}

Copy link
Contributor Author

Choose a reason for hiding this comment

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

@1ec5 - Are you suggesting that I add a distance property to the SpeedLimit object or to keep a collection of (SpeedLimit, distance) pairs that is private to RouteProgress, built in the init() and referenced by currentSpeedLimit()?

Copy link
Contributor

Choose a reason for hiding this comment

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

The latter, a collection of (SpeedLimit, distance) pairs. If the additional nesting turns out to be problematic, then the tuples could be grouped by leg and the distances could be measured from the start of the leg, but the tradeoff would be worse performance during the trip.


guard coordinatesLeftOnStepCount >= 0, legIndex < maximumSpeedLimitsByLeg.count, currentLegProgress.stepIndex < maximumSpeedLimitsByLeg[legIndex].count, let coordinates = currentLegProgress.currentStepProgress.step.coordinates else { return SpeedLimit.invalid }

let indexedCoordinate = LineString(coordinates).indexedCoordinateFromStart(distance: currentLegProgress.currentStepProgress.distanceTraveled)

guard let index = indexedCoordinate?.index, index < maximumSpeedLimitsByLeg[legIndex][currentLegProgress.stepIndex].count else { return SpeedLimit.invalid }

let speedLimit = maximumSpeedLimitsByLeg[legIndex][currentLegProgress.stepIndex][index]

return speedLimit
}

public var averageCongestionLevelRemainingOnLeg: CongestionLevel? {
let coordinatesLeftOnStepCount = Int(floor((Double(currentLegProgress.currentStepProgress.step.coordinateCount)) * currentLegProgress.currentStepProgress.fractionTraveled))

Expand Down