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

Correct NavigationMatchOptions congestion level in cycling profile … #4564

Conversation

azarovalex
Copy link
Contributor

@azarovalex azarovalex commented Oct 13, 2023

Briding changes from (#4563) to main.

Correct the .numericCongestionLevel attribute to .congestionLevel when using the cycling profile. This is the same as #3495 (#3496) but for NavigationMatchOptions instead of NavigationRouteOptions.

attributeOptions = [.expectedTravelTime, .maximumSpeedLimit]
if profileIdentifier == .cycling {
// https://github.com/mapbox/mapbox-navigation-ios/issues/3495
attributeOptions.update(with: .congestionLevel)
} else {
attributeOptions.update(with: .numericCongestionLevel)
}

…4563)

Correct the `.numericCongestionLevel` attribute to `.congestionLevel` when using the cycling profile. This is the same as #3495 (#3496) but for `NavigationMatchOptions` instead of `NavigationRouteOptions`.

https://github.com/mapbox/mapbox-navigation-ios/blob/ff4873f77bd91505820e6027f1a4a43efc4a7a3d/Sources/MapboxCoreNavigation/NavigationRouteOptions.swift#L28-L34

Signed-off-by: Matt Robinson <[email protected]>
@azarovalex azarovalex self-assigned this Oct 13, 2023
@azarovalex azarovalex requested a review from a team as a code owner October 13, 2023 14:26
@codecov
Copy link

codecov bot commented Oct 13, 2023

Codecov Report

Merging #4564 (535ac15) into main (ff4873f) will not change coverage.
The diff coverage is n/a.

Impacted file tree graph

@@           Coverage Diff           @@
##             main    #4564   +/-   ##
=======================================
  Coverage   59.79%   59.79%           
=======================================
  Files         189      189           
  Lines       21216    21216           
=======================================
  Hits        12687    12687           
  Misses       8529     8529           

@azarovalex
Copy link
Contributor Author

azarovalex commented Oct 13, 2023

I updated initial logic a bit based on Directions API documentation which states that both congestion and congestion_numeric parameters are available only in mapbox/driving-traffic profile.

While this is technically a breaking change for custom profiles, because they'll no longer receive any congestion info, a proper way to receive congestions along the route in this case is to explicitly update attributeOptions on the app side.

@azarovalex
Copy link
Contributor Author

azarovalex commented Oct 13, 2023

@mattrobmattrob Hey Matt, thanks again for your contribution, I'm going the merge the current version, please take a look and let me know if you have any questions.

I assume you don't need congestion levels for cycling profile, right? Officially Directions API don't support them and to be honest I'm not sure that this data even makes sense for cylcing profile.

@mattrobmattrob
Copy link
Contributor

LGTM! Thank you.

@azarovalex azarovalex merged commit 2c637ce into main Oct 13, 2023
20 checks passed
@azarovalex azarovalex deleted the public/mattrobmattrob-mr/correct.match.options.congestion.level.cyling branch October 13, 2023 16:38
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants