Skip to content

Fix IndexedRouteResponse handling after rerouting#3344

Merged
1ec5 merged 1 commit into
mainfrom
fix-refresh-identifier
Sep 9, 2021
Merged

Fix IndexedRouteResponse handling after rerouting#3344
1ec5 merged 1 commit into
mainfrom
fix-refresh-identifier

Conversation

@OttyLab
Copy link
Copy Markdown
Contributor

@OttyLab OttyLab commented Sep 9, 2021

Summary

Reroute does not update indexedRouteResponse and refresh uses old route's identifier to update route annotations. This provides unexpected speed limit, congestion information display.

Detail

Reroute does not update indexedRouteResponse. indexedRouteResponse.routeResponse.identifier represents route's identifier and used when refreshing.

Therefore, indexedRouteResponse needs to be updated here.

Screenshot

After the reroute, speed limit shows 90km/h but 1 min later it becomes 70km/h. This road is 90km/h and 70km/h is wrong.

repro_s.mp4

Expected behavior

expected_s.mp4

@OttyLab OttyLab added the bug Something isn’t working label Sep 9, 2021
@1ec5
Copy link
Copy Markdown
Contributor

1ec5 commented Sep 9, 2021

Thanks for contributing this fix! This PR includes some additional debugging code that would need to be cleaned up, but I went ahead and rolled your commit into #3345, which also includes more substantial changes in this area. Please have a look at that PR. Hopefully we can land the whole thing soon, but if not, then we can clean up this PR and merge it instead.

Copy link
Copy Markdown
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.

On second thought, #3345 has amassed so many changes that we’re a bit wary of landing it literally right before v2.0.0-rc.1, so we’ll clean up this more targeted fix, land it for v2.0.0-rc.1, and land #3345 right after to squeeze in additional testing before v2.0.0-rc.2.

Comment thread Example/ViewController.swift Outdated
Comment thread Example/ViewController.swift Outdated
Comment thread MapboxNavigation-SPM.xcodeproj/project.pbxproj Outdated
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

This is correct for now. #3345 will change the behavior of the method that this closure is passed into, so that the closest matching route will no longer be guaranteed to be at index 0; instead, the closure will receive the index of the closest matching route.

@1ec5 1ec5 force-pushed the fix-refresh-identifier branch from aba8a86 to e39b112 Compare September 9, 2021 10:08
@1ec5 1ec5 added this to the v2.0.0 (RC) milestone Sep 9, 2021
@1ec5 1ec5 linked an issue Sep 9, 2021 that may be closed by this pull request
@1ec5 1ec5 merged commit 2a00e12 into main Sep 9, 2021
@1ec5 1ec5 deleted the fix-refresh-identifier branch September 9, 2021 10:18
@1ec5
Copy link
Copy Markdown
Contributor

1ec5 commented Sep 9, 2021

Updated the changelog in e68d748.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

bug Something isn’t working

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Parallel copies of route in Router can get out of sync

2 participants