Add support for congestion numeric attribute #3325
Merged
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
bamx23
approved these changes
Sep 6, 2021
Udumft
reviewed
Sep 6, 2021
Comment on lines
+13
to
+37
| precondition(low.lowerBound >= 0, "Congestion level ranges can't include negative values.") | ||
| precondition(low.upperBound <= moderate.lowerBound, "Values from the moderate congestion level range can't intersect with or be lower than ones from the low congestion level range.") | ||
| precondition(moderate.upperBound <= heavy.lowerBound, "Values from the heavy congestion level range can't intersect with or be lower than ones from the moderate congestion level range.") | ||
| precondition(heavy.upperBound <= severe.lowerBound, "Values from the severe congestion level range can't intersect with or be lower than ones from the heavy congestion level range.") | ||
| precondition(severe.upperBound <= CongestionRangeSevere.upperBound, "Congestion level ranges can't include values greater than 100.") |
There was a problem hiding this comment.
Will it be correct if there's a gap between subsequent congestion levels? Should we instead verify if low.upperBound == moderate.lowerBound? Otherwise it seems that it will treat those gaps as "unknown" congestion?
There was a problem hiding this comment.
Not sure, what would be the use case for the gaps in intervals, but the "unknown" level of congestion for a value that you haven't defined for some reason looks semantically correct.
chezzdev
force-pushed
the
pad-congestion-numeric
branch
2 times, most recently
from
5305c20
to
585d9ec
Compare
chezzdev
force-pushed
the
pad-congestion-numeric
branch
from
585d9ec
to
410924d
Compare
chezzdev
force-pushed
the
pad-congestion-numeric
branch
from
410924d
to
e504f93
Compare
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Fixes mapbox/navigation-sdks#892.
Depends on mapbox/mapbox-directions-swift#575.
Based on discussions in the team, it's decided to make congestion numeric usage compatible with regular congestion levels.
For that reason, numeric values are matched to congestion levels using pre-defined ranges and then used as usual.
numericCongestionLevelattribute over.congestionLevelone inRouteLeg.numericCongestionLevelinstead of.congestionLevelinNavigationRouteOptions