Skip to content

Check for valid index before updating steps in NavigationRouteProcessor#1435

Merged
danesfeder merged 1 commit into
masterfrom
dan-steps-oob
Oct 18, 2018
Merged

Check for valid index before updating steps in NavigationRouteProcessor#1435
danesfeder merged 1 commit into
masterfrom
dan-steps-oob

Conversation

@danesfeder
Copy link
Copy Markdown
Contributor

Fixes #1413

This could be a data edge case with MapMatching given @Danny-James setup, but still something we should guard against.

cc @kevinkreiser something to look into here (could very well be a problem on our side) - NavigationStatus is providing leg / step indices that are out of bounds of the current route leg / step lists.

@danesfeder danesfeder added bug Defect to be fixed. ✓ ready for review labels Oct 18, 2018
@danesfeder danesfeder added this to the 0.22.0 milestone Oct 18, 2018
@danesfeder danesfeder self-assigned this Oct 18, 2018
Copy link
Copy Markdown
Contributor

@Guardiola31337 Guardiola31337 left a comment

Choose a reason for hiding this comment

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

A couple of comments @danesfeder

Also looking forward to hearing from @kevinkreiser on

something to look into here (could very well be a problem on our side) - NavigationStatus is providing leg / step indices that are out of bounds of the current route leg / step lists.

private void updateSteps(DirectionsRoute route, int legIndex, int stepIndex, int upcomingStepIndex) {
currentLeg = route.legs().get(legIndex);
List<RouteLeg> legs = route.legs();
if (legIndex < legs.size()) {
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.

Shouldn't be <=?

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.

Scratch this, it's 👌

}
List<LegStep> steps = currentLeg.steps();
currentStep = steps.get(stepIndex);
if (stepIndex < steps.size()) {
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.

Same here ☝️

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.

Scratch this, it's 👌

Copy link
Copy Markdown
Contributor

@Guardiola31337 Guardiola31337 left a comment

Choose a reason for hiding this comment

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

Sounds good to me, although as I noted I'm curious about how the indices could get out of sync 🤔 cc @kevinkreiser

@danesfeder danesfeder merged commit e663e75 into master Oct 18, 2018
@danesfeder danesfeder deleted the dan-steps-oob branch October 18, 2018 19:38
@kevinkreiser
Copy link
Copy Markdown
Contributor

yeah they should not be out of sync, im going to do some 🔍 on this!

@danesfeder danesfeder mentioned this pull request Oct 24, 2018
11 tasks
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

bug Defect to be fixed.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants