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 #2114

Merged
merged 14 commits into from
Dec 27, 2019

Conversation

avi-c
Copy link
Contributor

@avi-c avi-c commented Apr 24, 2019

Shifting from PR #2112

Incorporate the new maxspeed annotations on the RouteLeg object from MapboxDirections.swift. Provide this data to Navigation SDK clients via the maximumSpeedLimitsByLeg array on the RouteProgress object RouteLegProgress.currentSpeedLimit property.

Cartfile Outdated Show resolved Hide resolved
MapboxCoreNavigation/RouteProgress.swift Outdated Show resolved Hide resolved
MapboxCoreNavigation/RouteProgress.swift Outdated Show resolved Hide resolved
MapboxCoreNavigation/RouteProgress.swift Outdated Show resolved Hide resolved
MapboxCoreNavigation/RouteProgress.swift Outdated Show resolved Hide resolved
Copy link
Contributor

@1ec5 1ec5 left a comment

Choose a reason for hiding this comment

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

Thanks, I think this PR is all set as far as the current API is concerned. Some upcoming changes to the API will require minimal additional work here (to determine the currently relevant sign style and unit), and we’ll have to revive the UI in #1242 before we can consider the feature complete, but this is a good foundation for a non-production application to prototype a UI.

All the builds are failing on CI, likely because Cartfile.resolved hasn’t been updated to reflect the changes in Cartfile.

MapboxCoreNavigation/RouteProgress.swift Outdated Show resolved Hide resolved
@@ -25,7 +25,11 @@ open class NavigationRouteOptions: RouteOptions {
shapeFormat = .polyline6
includesSteps = true
routeShapeResolution = .full
attributeOptions = [.congestionLevel, .expectedTravelTime, .maximumSpeedLimit]
if profileIdentifier == .walking {
Copy link
Contributor

Choose a reason for hiding this comment

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

profileIdentifier is mutable, so technically it’s possible for the developer to change the profile identifier to walking after initializing the object. We could address that case by conditionally adding maximumSpeedLimit in an override of the URLQueryItems property instead of this initializer. But maybe it isn’t a big deal.

@1ec5 1ec5 mentioned this pull request Aug 7, 2019
@1ec5 1ec5 added feature New feature request. blocked: upstream topic: traffic UI Work related to visual components, Android Auto, Camera, 3D, voice, etc. labels Aug 7, 2019
@1ec5 1ec5 added this to the v0.38.0 milestone Aug 7, 2019
@1ec5
Copy link
Contributor

1ec5 commented Aug 7, 2019

Once mapbox/mapbox-directions-swift#379 lands, this PR will be blocked by #2206.

@1ec5 1ec5 mentioned this pull request Sep 16, 2019
3 tasks
@1ec5
Copy link
Contributor

1ec5 commented Oct 1, 2019

This PR is no longer blocked by either mapbox/mapbox-directions-swift#379 or #2206. However, mapbox/mapbox-directions-swift#250 does need to land first.

@1ec5 1ec5 modified the milestones: v0.38.0, v0.39.0 Oct 2, 2019
@1ec5 1ec5 modified the milestones: v0.39.0, v1.0.0 Nov 26, 2019
@1ec5
Copy link
Contributor

1ec5 commented Dec 21, 2019

This PR needs to be rebased to resolve conflicts, but mapbox/mapbox-directions-swift#250 depends on mapbox/mapbox-directions-swift#382, which master is incompatible with. #2275 fixes some of the compatibility issues but isn’t ready yet, and it depends on mapbox/mapbox-directions-swift#393, which isn’t ready either. 🤕

@1ec5 1ec5 self-assigned this Dec 22, 2019
@1ec5 1ec5 removed the UI Work related to visual components, Android Auto, Camera, 3D, voice, etc. label Dec 27, 2019
avi-c and others added 10 commits December 26, 2019 16:14
… a segment and not be considered evenly distributed along the segment coordinates.
…edLimit per route segment array in favor of positioned SpeedLimit tuples.
…ofile request since they are invalid for that profile.

Rework the logic for including maximumSpeedLimits annotations to restrict them to auto driving profiles.
Get just the current segment’s speed limit instead of bucketing the speed limits upfront.
@1ec5 1ec5 merged commit 43825d8 into master Dec 27, 2019
@1ec5 1ec5 deleted the maxspeedAnnotations branch December 27, 2019 01:13
@1ec5 1ec5 modified the milestones: v1.0.0, v0.x next Apr 14, 2020
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
feature New feature request. topic: traffic
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants