NAVAND-940: fix buildings highlighting #6749
Conversation
dzinad
force-pushed
the
NAVAND-940-dd
branch
from
9194db9 to
4873ca2
Compare
Codecov Report
@@ Coverage Diff @@
## main #6749 +/- ##
=========================================
Coverage 72.54% 72.54%
Complexity 5521 5521
=========================================
Files 772 772
Lines 29888 29888
Branches 3531 3531
=========================================
Hits 21682 21682
Misses 6793 6793
Partials 1413 1413 |
dzinad
force-pushed
the
NAVAND-940-dd
branch
2 times, most recently
from
5c5f22d to
0af4bcb
Compare
dzinad
force-pushed
the
NAVAND-940-dd
branch
from
0af4bcb to
1509cd8
Compare
| val waypointIndex = action.progress.currentLegProgress?.legIndex!! + 1 | ||
| val waypoint = waypoints.filter { it.isLegWaypoint() }.getOrNull(waypointIndex) | ||
| return BuildingResult.GetDestination(waypoint?.target ?: waypoint?.location) | ||
| val routeOptions = action.progress.route.routeOptions() |
There was a problem hiding this comment.
the intention of doing this changed is EV waypoints, that added implicitly and this logic is not working.
There was a problem hiding this comment.
What exactly is not working? I don't see anything here that would be affected by adding EV waypoints.
There was a problem hiding this comment.
@RingerJK could you demonstrate the issue with a unit test?
It seems we're trying to get this into the 2.10.0-rc.1, and it'd be nice to release that soon
There was a problem hiding this comment.
let me provide the sample:
- the SDK requested an EV route with 2 waypoints (origin and destination);
- received the route with an additional waypoint in the middle (EV waypoint);
- so we have the route with 3 waypoints, 2 legs, but originally we requested 1 leg route (2 waypoints)
- as soon as we start to work with requested waypoints (RouteOptions) we get inconsistency because
routeProgerss#legIndex == 1is actually not the destination, it's the EV waypoint.
There was a problem hiding this comment.
We have 2 APIs: queryBuildingOnWaypoint and queryBuildingOnFinalDestination. In the example provided above, I would guess that queryBuildingOnWaypoint would be triggered when the user reaches the 1st waypoint and queryBuildingOnFinalDestination will be triggered when they reach the final destination. What am I missing?
cc @RingerJK
There was a problem hiding this comment.
Oh, legIndex is also affected.
Then we can't merge it. Thanks, I'll continue investigating tomorrow.
There was a problem hiding this comment.
@abhishek1508 at my example when you arrive at the first waypoint and try to take waypoint data via queryBuildingOnWaypoint you will receive data for destination because the index of destination in RouteOptions is actually 1. Does it make sense?
|
Closing in favour of #6754. |
I basically reverted building related changed from #6005. We don't really deal with waypoints there, only waypoint targets (that have the same size as
RoutesOptions#coordinatesList). And for these things we use leg index. So nothing waypoint related. All the indices and lists stay valid even after introducing EV waypoints.The question is whether we want to highlight EV chargers. If yes, I think we can do it separately. For now let's fix old behaviour.