[NAVAND-1066] increase max distance to route line valid to continue vanishing updates to 10m

[LukasPaczos]

Description

The 3 meters threshold has been introduced in the past to avoid cases where going off-road would continue vanishing route line updates (which is a very poor user experience) or to avoid vanishing route line updates in any other situation where the puck is detached from the route line, like a late detection of an off-route event.

We’ve already increased this threshold from 1-2 meters to 3 in the past but this PR proposes to increase that even further to 10m as through bug reports we noticed more cases where a misalignment between offboard and onboard datasets can cause a difference between drawn and matched location by more than the previous 3m thresholds which can lead to freezes of updates to the route line. Occasional detached updates of the vanishing point should offer a better UX than an extended freeze. The 10 meters threshold should keep us on the safe side for valid scenarios but still be able to ignore major off-road events and pause the updates.

[github-actions]

Changelog

Features

• Added MapboxTripStarter to simplify the solution for managing the trip session and replaying routes. This also makes it possible to share the replay state between drop-in-ui and android-auto. #6794

Bug fixes and improvements

• Removed NavigationRoute#hasUnexpectedClosures and added RouteProgress#hasUnexpectedUpcomingClosures instead that checks whether route has upcoming unexpected closures (the algorithm does not take into account closures that the puck has already been on) #6841
• Improved NavigationView camera behavior to go back into overview state if routes change during route preview state. #6840
• Increased max distance from the user indicator to route line valid to continue vanishing updates from 3m to 10m. #changes

Known issues ⚠️

Other changes

Android Auto Changelog

Features

Bug fixes and improvements

[codecov]

Codecov Report

Merging #6854 (7d49654) into main (ce11135) will not change coverage.
The diff coverage is n/a.

@@            Coverage Diff            @@
##               main    #6854   +/-   ##
=========================================
  Coverage     72.65%   72.65%           
  Complexity     5565     5565           
=========================================
  Files           781      781           
  Lines         30128    30128           
  Branches       3562     3562           
=========================================
  Hits          21890    21890           
  Misses         6815     6815           
  Partials       1423     1423           

Impacted Files	Coverage Δ
...ox/navigation/ui/maps/route/RouteLayerConstants.kt	100.00% <ø> (ø)

[dzinad]

I'm not very familiar with this piece of code but it looks like we are trying to understand whether we are off route or not by calculating the distance from the point to the route line. Why don't we just use the state NN provides us with in RouteProgress?

[LukasPaczos]

Why don't we just use the state NN provides us with in RouteProgress?

We do that already in

mapbox-navigation-android/libnavui-maps/src/main/java/com/mapbox/navigation/ui/maps/route/line/api/VanishingRouteLine.kt

Lines 54 to 60 in 42aaa24

	fun updateVanishingPointState(routeProgressState: RouteProgressState) {
	vanishingPointState = when (routeProgressState) {
	RouteProgressState.TRACKING -> VanishingPointState.ENABLED
	RouteProgressState.COMPLETE -> VanishingPointState.ONLY_INCREASE_PROGRESS
	else -> VanishingPointState.DISABLED
	}
	}

However, the vanishing point updates are based on the position of the actual location indicator on the map, together with its own transitions. So while NN's state is an input value, it's not the only one. To accommodate for weird bugs, or a puck jumping from one place to another on the map (for any reason), we have this additional threshold to abort vanishing point updates when the actual puck on the map is far away from the route.

[LukasPaczos]

There's additional optimization on top of this what we can do, which is tracking the offRoadProba and pausing updates when needed, but we still need to keep the threshold in case the puck is animated by the Maps SDK in and unexpected way.

[dzinad]

So while NN's state is an input value, it's not the only one.

It sounds like it shouldn't be like this. No single source of truth.

To accommodate for weird bugs, or a puck jumping from one place to another on the map (for any reason)

TBH I think that if the user is driving and their location on the map unexpectedly jumped somewhere they wouldn't really care about a vanishing route line... Looks like the whole state would be weird and it wouldn't make sense to fix just something (in this case vanishing route line). It's kinda similar to the situation where I tried to try-catch starting the service. You said the whole state would be inconsistent. I think here it's the same issue.

It's not directly related to this fix but the whole "let's use 10 meters instead of 3" doesn't sound exactly right to me. We've encountered an issue with 9 meters, who says there won't be an issue with 12 meters tomorrow?
So I'm trying to understand whether we need these heuristics at all.

[LukasPaczos]

That's not entirely true. The information about where the route line should be vanished is not coming from Nav SDK or Nav Native, it's only partially based on the navigation state. The primary information comes from where the puck is on the map in relation to the route line. Both of these elements live in the Maps SDK and our calculation make sure to work well with what's rendered (to the degree possible), it's not a stateful information, it's the end result of a presentation layer.

However, the navigation state is important to help resolve ambiguities. For example what happens when route line overlaps (for example due to a u-turn or cross passes), or when we have complex maneuvers like roundabouts. In these cases just running a visual calculation wouldn't be enough, we'd have multiple possible candidates for the vanishing point. This is where the navigation state is used to rule out incorrect candidates based on our progress along the route.

That's why the logic cannot be fully reliant on the navigation state, we need additional helpers to account for the fact that we don't know how the user will animate the puck - will they use keypoints or not, are they trying some custom thing that moves the puck's position around, do they simulate location which makes the puck jump from one street to another, etc. In these cases having a generic distance threshold improves the UX a little bit.

It's not directly related to this fix but the whole "let's use 10 meters instead of 3" doesn't sound exactly right to me. We've encountered an issue with 9 meters, who says there won't be an issue with 12 meters tomorrow?

That's a completely valid concern. The threshold value is entirely subjective based on know cases, it's a perfect example of a magic number that helps in some cases where other systems fail. We wouldn't have to deal with this problem if the entirety of the route line animation was done in the Maps SDK, directly in the rendering engine. However, we couldn't make that change right now because of this missing information of the navigation state that is needed to resolve ambiguities and has to come from Nav SDK. This is definitely a potential improvement area though.