No more stuttering during navigation
ClosedPublic

Authored by sanjibanb on Jul 14 2016, 12:11 PM.

Details

Reviewers
nienhueser
rahn

Diff Detail

Repository
R34 Marble
Lint
Lint Skipped
Unit
Unit Tests Skipped
sanjibanb updated this revision to Diff 5158.Jul 14 2016, 12:11 PM
sanjibanb retitled this revision from to No more stuttering during navigation.
sanjibanb updated this object.
sanjibanb edited the test plan for this revision. (Show Details)
sanjibanb added reviewers: nienhueser, rahn.
sanjibanb set the repository for this revision to R34 Marble.
sanjibanb added a project: Marble.
sanjibanb added a subscriber: Marble.
nienhueser added inline comments.Jul 14 2016, 6:30 PM
src/lib/marble/routing/Route.h
33 ↗(On Diff #5158)

no benefit of making the return type const, so just
int indexOf( RouteSegment index ) const;
However the parameter should be a const reference, so
int indexOf(const RouteSegment &segment) const;

src/lib/marble/routing/VoiceNavigationModel.cpp
26 ↗(On Diff #5158)

Please add a constructor that sets both announcementDone and turnInstructionDone to false. That simplifies things below.

332 ↗(On Diff #5158)

this loop is not needed when the constructor initializes the members to false (see comment above).

382 ↗(On Diff #5158)

Use a reference like
VoiceNavigationModelPrivate::Announcement & curAnnouncement = d->m_announcementList[index];
and you can avoid the update call
d->m_announcementList[index] = curAnnouncement;
below.

sanjibanb updated this revision to Diff 5178.Jul 14 2016, 7:34 PM
sanjibanb marked 4 inline comments as done.

Incorporated review comments

This comment was removed by sanjibanb.
nienhueser accepted this revision.Jul 14 2016, 8:33 PM
nienhueser edited edge metadata.

I wonder if we should reset the state at some point. Think of doing a wrong turn and therefore coming back to an earlier segment of the route. We can work on that later though.

src/lib/marble/routing/VoiceNavigationModel.cpp
336

is that needed?

This revision is now accepted and ready to land.Jul 14 2016, 8:33 PM
sanjibanb marked an inline comment as done.Jul 14 2016, 8:35 PM
This comment was removed by sanjibanb.
sanjibanb added inline comments.Jul 14 2016, 8:36 PM
src/lib/marble/routing/VoiceNavigationModel.cpp
336

The d->m_lastRoutePath != route.path() check is made to check whether the route has been changed or not. In case it does, then the array already contains some Announcement objects, which unless the size of array is made 0, won't be reset to false, which is why I added that line there.

sanjibanb closed this revision.Jul 14 2016, 8:43 PM
sanjibanb marked an inline comment as done.

Pushed

About resetting the state, I think that with the current code, in case the user takes a wrong turn, say, left turn instead of right, then a new route will be recalculated once he deviates from the route enough, thus states will be reset. So that is fine.

However, this will not be the case if the user takes a direct U-turn backwards, but staying on the route, hence not deviating from the route. In that case, states will not be reset. But they should be, yes. So that he is told the correct instructions again once he turns to start proceeding in the correct direction. So, yeah, that part of the task can be resumed from master.