Add ability to override congestion levels for specific road classes. #2741

MaximAlien · 2020-11-13T20:15:20Z

Description

This PR implements the ability to override congestion levels for specific road classes.

Implementation

Since implementation is using Mapbox Directions property RouteStep.segmentIndicesByIntersection it depends on branch vk/478-intersection-geometry. Dependency will be updated after dependent branch is merged to main.

Since Android implementation is using road classes from mapbox_streets_v8 there is slight difference between congestion segments. After iOS is done with implementing same functionality current implementation should re-tested and revised once more.

Update: Most recent version was updated and is currently using MapboxStreetsRoadClass which matches Android implementation.

Screenshots or Gifs

Screenshots show congestion consistency comparison between iOS and Android. In this example .unknown congestion level for .motorway road class is replaced to .low (blue color). Currently there is slight shift of color because of gradient which is used on iOS.

1ec5 · 2020-11-20T18:37:21Z

OfflineRoutingTests is failing because of a decoding error that would probably be fixed by mapbox/mapbox-directions-swift#485:

/Users/distiller/project/MapboxCoreNavigationTests/OfflineRoutingTests.swift:33: error: -[MapboxCoreNavigationTests.OfflineRoutingTests testOfflineDirections] : failed - Unexpected Failure: unknown(underlying: Swift.DecodingError.keyNotFound(CodingKeys(stringValue: "iso_3166_1_alpha3", intValue: nil), Swift.DecodingError.Context(codingPath: [CodingKeys(stringValue: "routes", intValue: nil), _JSONKey(stringValue: "Index 0", intValue: 0), CodingKeys(stringValue: "legs", intValue: nil), _JSONKey(stringValue: "Index 0", intValue: 0), CodingKeys(stringValue: "admins", intValue: nil), _JSONKey(stringValue: "Index 0", intValue: 0)], debugDescription: "No value associated with key CodingKeys(stringValue: \"iso_3166_1_alpha3\", intValue: nil) (\"iso_3166_1_alpha3\").", underlyingError: nil)))
/Users/distiller/project/MapboxCoreNavigationTests/OfflineRoutingTests.swift:43: error: -[MapboxCoreNavigationTests.OfflineRoutingTests testOfflineDirections] : Asynchronous wait failed: Exceeded timeout of 2 seconds, with unfulfilled expectations: "Calculate route offline".
/Users/distiller/project/MapboxCoreNavigationTests/OfflineRoutingTests.swift:46: error: -[MapboxCoreNavigationTests.OfflineRoutingTests testOfflineDirections] : failed - No route returned

CHANGELOG.md

Cartfile

MapboxNavigation/NavigationMapView.swift

scripts/.gitignore

MaximAlien · 2020-11-20T19:54:29Z

OfflineRoutingTests is failing because of a decoding error that would probably be fixed by mapbox/mapbox-directions-swift#485:

/Users/distiller/project/MapboxCoreNavigationTests/OfflineRoutingTests.swift:33: error: -[MapboxCoreNavigationTests.OfflineRoutingTests testOfflineDirections] : failed - Unexpected Failure: unknown(underlying: Swift.DecodingError.keyNotFound(CodingKeys(stringValue: "iso_3166_1_alpha3", intValue: nil), Swift.DecodingError.Context(codingPath: [CodingKeys(stringValue: "routes", intValue: nil), _JSONKey(stringValue: "Index 0", intValue: 0), CodingKeys(stringValue: "legs", intValue: nil), _JSONKey(stringValue: "Index 0", intValue: 0), CodingKeys(stringValue: "admins", intValue: nil), _JSONKey(stringValue: "Index 0", intValue: 0)], debugDescription: "No value associated with key CodingKeys(stringValue: \"iso_3166_1_alpha3\", intValue: nil) (\"iso_3166_1_alpha3\").", underlyingError: nil)))
/Users/distiller/project/MapboxCoreNavigationTests/OfflineRoutingTests.swift:43: error: -[MapboxCoreNavigationTests.OfflineRoutingTests testOfflineDirections] : Asynchronous wait failed: Exceeded timeout of 2 seconds, with unfulfilled expectations: "Calculate route offline".
/Users/distiller/project/MapboxCoreNavigationTests/OfflineRoutingTests.swift:46: error: -[MapboxCoreNavigationTests.OfflineRoutingTests testOfflineDirections] : failed - No route returned

Yep, I've seen that. Since current PR already depends on vk/478-intersection-geometry we'll have to wait till both mapbox/mapbox-directions-swift#490 and mapbox/mapbox-directions-swift#485 land in Mapbox Directions.

MapboxNavigation/DayStyle.swift

MaximAlien · 2020-12-14T23:18:07Z

@1ec5, can you please take a look at this PR once more? RoadClasses option set was replaced with MapboxStreetsRoadClass, so iOS implementation is now in par with Android.

CHANGELOG.md

Example/ViewController.swift

MapboxNavigation/NavigationMapView.swift

1ec5 · 2020-12-16T19:44:57Z

MapboxNavigation/NavigationMapView.swift

+    func streetsRoadClasses(_ leg: RouteLeg) -> [MapboxStreetsRoadClass?] {
+        // Pick only valid segment indices for specific `Intersection` in `RouteStep`.
+        // Array of segment indexes can look like this: [0, 3, 24, 28, 48, 50, 51, 53].
+        let segmentIndices = leg.steps.compactMap({ $0.segmentIndicesByIntersection?.compactMap({ $0 }) }).reduce([], +)


I think RouteLeg.segmentRangesByStep may simplify this computation.

Seems that segmentRangesByStep is not detailed enough, as it stores only lower and upper bounds. segmentIndicesByIntersection is more precise in this case, as it stores segment indices for Intersection.

MapboxCoreNavigation/RouteLeg.swift

1ec5 · 2020-12-17T16:58:44Z

MapboxCoreNavigation/RouteLeg.swift

+        let streetsRoadClasses = segmentIndices.enumerated().map({ segmentIndices.indices.contains($0.offset + 1) && streetsRoadClassesInLeg.indices.contains($0.offset) ?
+                                                                    Array(repeating: streetsRoadClassesInLeg[$0.offset], count: segmentIndices[$0.offset + 1] - segmentIndices[$0.offset]) : [] }).reduce([], +)


At a glance, it seems like this calculation would be simplified by zipping segmentIndices, but it’s hard to be sure. I’m a bit wary of the manual index arithmetic, but I trust you’ve already tested the multi-leg route edge case. I can’t think of any other edge cases at the moment.

Just tried several approaches with zipping and striding, but seems that it doesn't help to make this code more elegant and simple. I propose to move on with landing this PR so that we can start testing and I can follow-up with any better solutions in future PRs.

…emove unused `trafficAlternateLow` property. Remove unused `.gitignore` file.

…lass`.

…denCongestionLevels`.

MaximAlien added UI Work related to visual components, Android Auto, Camera, 3D, voice, etc. iOS labels Nov 13, 2020

MaximAlien added this to the v1.2.0 milestone Nov 13, 2020

MaximAlien self-assigned this Nov 13, 2020

MaximAlien requested a review from a team November 14, 2020 00:29

MaximAlien changed the title ~~Route congestion consistency~~ Add ability to override congestion levels for specific road classes. Nov 14, 2020

MaximAlien force-pushed the maxim/656-route-congestion-consistency branch 2 times, most recently from 11eb7e3 to ceb649e Compare November 19, 2020 20:59

MaximAlien mentioned this pull request Nov 20, 2020

Route Duration Annotations #2734

Merged

1ec5 reviewed Nov 20, 2020

View reviewed changes

MaximAlien added the blocked Blocked by dependency or unclarity. label Dec 4, 2020

MaximAlien requested a review from a team December 14, 2020 19:20

MaximAlien force-pushed the maxim/656-route-congestion-consistency branch 2 times, most recently from c512422 to cbc2f49 Compare December 14, 2020 20:11

MaximAlien removed the blocked Blocked by dependency or unclarity. label Dec 14, 2020

MaximAlien commented Dec 14, 2020

View reviewed changes

MapboxNavigation/DayStyle.swift Show resolved Hide resolved

MaximAlien requested a review from 1ec5 December 14, 2020 23:16

1ec5 reviewed Dec 16, 2020

View reviewed changes

MaximAlien requested review from 1ec5 and a team December 16, 2020 20:38

MaximAlien force-pushed the maxim/656-route-congestion-consistency branch from 91a5c64 to 02aaeb0 Compare December 17, 2020 00:43

1ec5 approved these changes Dec 17, 2020

View reviewed changes

MaximAlien added 5 commits December 17, 2020 10:43

Add initial implementation of congestion consistency.

635694c

Improve algorithm which calculates road classes for specific route. R…

8f5b139

…emove unused `trafficAlternateLow` property. Remove unused `.gitignore` file.

Add unit tests for congestion consistency functionality.

adb35d0

Improve documentation.

58fe518

Update changelog.

48952f9

MaximAlien added 13 commits December 17, 2020 10:44

Use higher order functions for road classes creation.

67986cf

Use RoadClasses option set instead of array.

21582ea

Update to most recent version of Mapbox Directions.

61adbca

Update to Mapbox Directions v1.2.0-alpha.3.

de67a7d

Replace RoadClasses with newly added MapboxStreetsRoadClass.

d0beb48

Further renaming after moving to `MapboxDirections.MapboxStreetsRoadC…

2707c2c

…lass`.

Minor improvement in changelog.

cbe4628

Move test fixtures to TestHelper.

af8671f

Use set of MapboxStreetsRoadClass instead of array.

2883ecb

Simplify condition which overrides congestion level.

4aa04a5

Rename trafficOverrideStreetsRoadClasses to `roadClassesWithOverrid…

cb585ff

…denCongestionLevels`.

Move streetsRoadClasses to be part of RouteLeg.

ce2df5b

Update changelog.

bf436db

MaximAlien force-pushed the maxim/656-route-congestion-consistency branch from b87b6df to bf436db Compare December 17, 2020 18:49

MaximAlien merged commit 6ef0a48 into main Dec 17, 2020

MaximAlien deleted the maxim/656-route-congestion-consistency branch December 17, 2020 19:04

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Add ability to override congestion levels for specific road classes. #2741

Add ability to override congestion levels for specific road classes. #2741

MaximAlien commented Nov 13, 2020 •

edited

Loading

1ec5 commented Nov 20, 2020

MaximAlien commented Nov 20, 2020

MaximAlien commented Dec 14, 2020

1ec5 Dec 16, 2020

MaximAlien Dec 16, 2020 •

edited

Loading

1ec5 Dec 17, 2020

MaximAlien Dec 17, 2020

		let streetsRoadClasses = segmentIndices.enumerated().map({ segmentIndices.indices.contains($0.offset + 1) && streetsRoadClassesInLeg.indices.contains($0.offset) ?
		Array(repeating: streetsRoadClassesInLeg[$0.offset], count: segmentIndices[$0.offset + 1] - segmentIndices[$0.offset]) : [] }).reduce([], +)

Add ability to override congestion levels for specific road classes. #2741

Add ability to override congestion levels for specific road classes. #2741

Conversation

MaximAlien commented Nov 13, 2020 • edited Loading

Description

Implementation

Screenshots or Gifs

1ec5 commented Nov 20, 2020

MaximAlien commented Nov 20, 2020

MaximAlien commented Dec 14, 2020

1ec5 Dec 16, 2020

Choose a reason for hiding this comment

MaximAlien Dec 16, 2020 • edited Loading

Choose a reason for hiding this comment

1ec5 Dec 17, 2020

Choose a reason for hiding this comment

MaximAlien Dec 17, 2020

Choose a reason for hiding this comment

MaximAlien commented Nov 13, 2020 •

edited

Loading

MaximAlien Dec 16, 2020 •

edited

Loading