Skip to content

Commit 302aa7e

Browse files
dzinaddzinad
committed
NAVAND-1073: meet code review
1 parent cbf6d04 commit 302aa7e

11 files changed

Lines changed: 91 additions & 68 deletions

File tree

‎examples/src/main/AndroidManifest.xml‎

Lines changed: 0 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -95,11 +95,6 @@
9595
android:label="@string/title_multileg_route">
9696
</activity>
9797

98-
<activity
99-
android:name=".RouteRefreshActivity"
100-
android:label="@string/title_route_refresh">
101-
</activity>
102-
10398
<activity
10499
android:name="com.mapbox.navigation.examples.MainActivity"
105100
android:exported="true">

‎examples/src/main/java/com/mapbox/navigation/examples/MainActivity.kt‎

Lines changed: 0 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -25,7 +25,6 @@ import com.mapbox.navigation.examples.core.MapboxVoiceActivity
2525
import com.mapbox.navigation.examples.core.MultiLegRouteExampleActivity
2626
import com.mapbox.navigation.examples.core.R
2727
import com.mapbox.navigation.examples.core.ReplayHistoryActivity
28-
import com.mapbox.navigation.examples.core.RouteRefreshActivity
2928
import com.mapbox.navigation.examples.core.camera.MapboxCameraAnimationsActivity
3029
import com.mapbox.navigation.examples.core.databinding.LayoutActivityMainBinding
3130
import com.mapbox.navigation.examples.util.LocationPermissionsHelper
@@ -141,11 +140,6 @@ class MainActivity : AppCompatActivity(), PermissionsListener {
141140
getString(R.string.description_draw_utility),
142141
RouteDrawingActivity::class.java
143142
),
144-
SampleItem(
145-
getString(R.string.title_route_refresh),
146-
getString(R.string.description_route_refresh),
147-
RouteRefreshActivity::class.java
148-
),
149143
)
150144
}
151145

‎examples/src/main/res/values/strings.xml‎

Lines changed: 0 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -36,11 +36,7 @@
3636
<string name="title_building_highlight" translatable="false">BuildingHighlight Example</string>
3737
<string name="description_building_highlight" translatable="false">Demonstrates how to highlight and extrude buildings on arrival.</string>
3838

39-
<string name="title_route_refresh" translatable="false">Route Refresh Activity</string>
40-
<string name="description_route_refresh" translatable="false">Lets you play with route refreshes and check various scenarios.</string>
41-
4239
<string name="label_start_navigation" translatable="false">Start Navigation</string>
43-
<string name="label_clear_traffic" translatable="false">Clear traffic</string>
4440
<string name="play_history" translatable="false">Play history</string>
4541
<string name="select_history" translatable="false">Select history</string>
4642
<string name="replay_playback_speed_seekbar" translatable="false">Replay speed %d</string>

‎instrumentation-tests/src/androidTest/java/com/mapbox/navigation/instrumentation_tests/core/RouteRefreshTest.kt‎

Lines changed: 18 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -556,19 +556,23 @@ class RouteRefreshTest : BaseTest<EmptyTestActivity>(EmptyTestActivity::class.ja
556556
val alternativeRoutes = mapboxNavigation.requestRoutes(routeOptions)
557557
.getSuccessfulResultOrThrowException()
558558
.routes
559-
// alternative which was requested on the second leg of the original route,
560-
// so the alternative has only one leg while the original route has two
559+
// In this test setup we are considering a case where user was driving along the route,
560+
// started the second leg and received an alternative, and selected it before the fork.
561+
// This means that the primary route is shorter than the alternative route (former primary route).
561562
val primaryRoute = alternativeForMultileg(activity).toNavigationRoutes().first()
562563

563564
// corresponds to currentRouteGeometryIndex = 70 for alternative route and 11 for the primary route
564565
mockLocationUpdatesRule.pushLocationUpdate(
565566
mockLocationUpdatesRule.generateLocationUpdate {
566-
latitude = 38.581798
567-
longitude = -121.476146
567+
latitude = 38.581798
568+
longitude = -121.476146
568569
}
569570
)
570571

571-
mapboxNavigation.setNavigationRoutes(listOf(primaryRoute) + alternativeRoutes, initialLegIndex = 0)
572+
mapboxNavigation.setNavigationRoutes(
573+
listOf(primaryRoute) + alternativeRoutes,
574+
initialLegIndex = 0
575+
)
572576
mapboxNavigation.startTripSession()
573577

574578
mapboxNavigation.routeProgressUpdates()
@@ -590,7 +594,11 @@ class RouteRefreshTest : BaseTest<EmptyTestActivity>(EmptyTestActivity::class.ja
590594
0.0001
591595
)
592596

593-
assertEquals(201.673, alternativeRoutes[0].getSumOfDurationAnnotationsFromLeg(1), 0.0001)
597+
assertEquals(
598+
201.673,
599+
alternativeRoutes[0].getSumOfDurationAnnotationsFromLeg(1),
600+
0.0001
601+
)
594602
assertEquals(202.881, refreshedRoutes[1].getSumOfDurationAnnotationsFromLeg(1), 0.0001)
595603

596604
assertEquals(194.3, primaryRoute.getSumOfDurationAnnotationsFromLeg(0), 0.0001)
@@ -861,7 +869,10 @@ class RouteRefreshTest : BaseTest<EmptyTestActivity>(EmptyTestActivity::class.ja
861869
}
862870

863871
private fun alternativeForMultileg(context: Context): MockRoute {
864-
val jsonResponse = readRawFileText(context, R.raw.route_response_single_route_multileg_alternative)
872+
val jsonResponse = readRawFileText(
873+
context,
874+
R.raw.route_response_single_route_multileg_alternative
875+
)
865876
val coordinates = listOf(
866877
Point.fromLngLat(38.577427, -121.478077),
867878
Point.fromLngLat(38.582195, -121.468458)

‎libnavigation-core/src/main/java/com/mapbox/navigation/core/routealternatives/AlternativeRouteProgressDataProvider.kt‎

Lines changed: 4 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -4,7 +4,6 @@ import com.mapbox.navigation.base.route.NavigationRoute
44
import com.mapbox.navigation.base.utils.DecodeUtils.stepsGeometryToPoints
55
import com.mapbox.navigation.core.RouteProgressData
66
import com.mapbox.navigation.utils.internal.ThreadController
7-
import kotlinx.coroutines.Dispatchers
87
import kotlinx.coroutines.withContext
98

109
internal object AlternativeRouteProgressDataProvider {
@@ -37,7 +36,10 @@ internal object AlternativeRouteProgressDataProvider {
3736
return RouteProgressData(legIndex, routeGeometryIndex, legGeometryIndex)
3837
}
3938

40-
private suspend fun prevLegsGeometryIndicesCount(route: NavigationRoute, currentLegIndex: Int): Int {
39+
private suspend fun prevLegsGeometryIndicesCount(
40+
route: NavigationRoute,
41+
currentLegIndex: Int
42+
): Int {
4143
return withContext(ThreadController.DefaultDispatcher) {
4244
var result = 0
4345
val stepsGeometries by lazy { route.directionsRoute.stepsGeometryToPoints() }

‎libnavigation-core/src/test/java/com/mapbox/navigation/core/routealternatives/AlternativeRouteProgressDataProviderTest.kt‎

Lines changed: 16 additions & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -121,7 +121,9 @@ class AlternativeRouteProgressDataProviderTest {
121121
listOf(List(10) { mockk() }, List(7) { mockk() }, List(15) { mockk() }),
122122
listOf(List(23) { mockk() }),
123123
listOf(List(32) { mockk() }, List(19) { mockk() }),
124-
listOf(List(14) { mockk() }, List(23) { mockk() }), // 135 points for legs #0-#3
124+
listOf(List(14) { mockk() }, List(23) { mockk() }),
125+
// 136 points for legs #0-#3
126+
// (135 if you don't count the current leg's starting point)
125127
listOf(List(8) { mockk() }),
126128
)
127129
)
@@ -145,12 +147,14 @@ class AlternativeRouteProgressDataProviderTest {
145147
primaryForkLegGeometryIndex = 85,
146148
alternativeForkLegIndex = 5,
147149
alternativeForkRouteGeometryIndex = 220,
148-
alternativeForkLegGeometryIndex = 86,
150+
alternativeForkLegGeometryIndex = 85,
149151
listOf(
150152
listOf(List(10) { mockk() }, List(7) { mockk() }, List(15) { mockk() }),
151153
listOf(List(23) { mockk() }),
152154
listOf(List(32) { mockk() }, List(19) { mockk() }),
153-
listOf(List(14) { mockk() }, List(23) { mockk() }), // 135 points for legs #0-#3
155+
listOf(List(14) { mockk() }, List(23) { mockk() }),
156+
// 136 points for legs #0-#3
157+
// (135 if you don't count the current leg's starting point)
154158
listOf(List(8) { mockk() }),
155159
)
156160
)
@@ -177,10 +181,10 @@ class AlternativeRouteProgressDataProviderTest {
177181
alternativeForkLegGeometryIndex = 81,
178182
listOf(
179183
listOf(List(10) { mockk() }, List(7) { mockk() }, List(15) { mockk() }),
180-
listOf(List(23) { mockk() }), // 135 points for legs #0-#1
184+
listOf(List(23) { mockk() }),
185+
// 136 points for legs #0-#1
186+
// (135 if you don't count the current leg's starting point)
181187
listOf(List(32) { mockk() }, List(19) { mockk() }),
182-
listOf(List(14) { mockk() }, List(23) { mockk() }),
183-
listOf(List(8) { mockk() }),
184188
)
185189
)
186190
val expected = RouteProgressData(2, 170, 119)
@@ -196,15 +200,14 @@ class AlternativeRouteProgressDataProviderTest {
196200
@Test
197201
fun `before fork, alternative starts after the primary route on the current leg`() =
198202
coroutineRule.runBlockingTest {
199-
// primary leg and route indices diff is 140
200203
val primaryRouteProgressData = RouteProgressData(4, 200, 60)
201204
val alternativeMetadata = alternativeRouteMetadata(
202205
primaryForkLegIndex = 4,
203206
primaryForkRouteGeometryIndex = 250,
204207
primaryForkLegGeometryIndex = 110,
205208
alternativeForkLegIndex = 0,
206209
alternativeForkRouteGeometryIndex = 90,
207-
alternativeForkLegGeometryIndex = 20,
210+
alternativeForkLegGeometryIndex = 90,
208211
listOf(
209212
listOf(List(10) { mockk() }, List(7) { mockk() }, List(15) { mockk() }),
210213
listOf(List(23) { mockk() }),
@@ -233,14 +236,17 @@ class AlternativeRouteProgressDataProviderTest {
233236
primaryForkLegGeometryIndex = 90,
234237
alternativeForkLegIndex = 4,
235238
alternativeForkRouteGeometryIndex = 250,
236-
alternativeForkLegGeometryIndex = 10,
239+
alternativeForkLegGeometryIndex = 115,
237240
listOf(
238241
listOf(List(10) { mockk() }, List(7) { mockk() }, List(15) { mockk() }),
239242
listOf(List(23) { mockk() }),
240243
listOf(List(32) { mockk() }, List(19) { mockk() }),
241244
listOf(
242245
List(14) { mockk() },
243-
List(23) { mockk() }), // 135 points for legs #0-#3
246+
List(23) { mockk() }
247+
),
248+
// 136 points for legs #0-#3
249+
// (135 if you don't count the current leg's starting point)
244250
listOf(List(8) { mockk() }),
245251
)
246252
)

‎qa-test-app/src/main/AndroidManifest.xml‎

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -77,6 +77,7 @@
7777
<activity android:name=".view.DropInButtonsActivity" />
7878

7979
<activity android:name=".view.RoutesPreviewActivity" />
80+
<activity android:name=".view.RouteRefreshActivity" />
8081
<activity android:name=".view.SpeedInfoActivity"
8182
android:theme="@style/Theme.Fullscreen" />
8283
<activity

‎qa-test-app/src/main/java/com/mapbox/navigation/qa_test_app/domain/TestActivitySuite.kt‎

Lines changed: 7 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -20,6 +20,7 @@ import com.mapbox.navigation.qa_test_app.view.RoadObjectsActivity
2020
import com.mapbox.navigation.qa_test_app.view.RouteLineFeaturesActivity
2121
import com.mapbox.navigation.qa_test_app.view.RouteLineScalingActivity
2222
import com.mapbox.navigation.qa_test_app.view.RouteNumericTrafficUpdateActivity
23+
import com.mapbox.navigation.qa_test_app.view.RouteRefreshActivity
2324
import com.mapbox.navigation.qa_test_app.view.RouteRestrictionsActivity
2425
import com.mapbox.navigation.qa_test_app.view.RouteTrafficUpdateActivity
2526
import com.mapbox.navigation.qa_test_app.view.RoutesPreviewActivity
@@ -242,6 +243,12 @@ object TestActivitySuite {
242243
) { activity ->
243244
activity.startActivity<RestAreaActivity>()
244245
},
246+
TestActivityDescription(
247+
"Route Refresh Example",
248+
R.string.description_route_refresh,
249+
) { activity ->
250+
activity.startActivity<RouteRefreshActivity>()
251+
},
245252
)
246253

247254
fun getTestActivities(category: String): List<TestActivityDescription> {

examples/src/main/java/com/mapbox/navigation/examples/core/RouteRefreshActivity.kt renamed to qa-test-app/src/main/java/com/mapbox/navigation/qa_test_app/view/RouteRefreshActivity.kt

Lines changed: 39 additions & 33 deletions
Original file line numberDiff line numberDiff line change
@@ -1,14 +1,14 @@
1-
package com.mapbox.navigation.examples.core
1+
package com.mapbox.navigation.qa_test_app.view
22

33
import android.annotation.SuppressLint
44
import android.content.res.Configuration
55
import android.content.res.Resources
66
import android.graphics.Color
77
import android.location.Location
8-
import androidx.appcompat.app.AppCompatActivity
98
import android.os.Bundle
109
import android.view.View.INVISIBLE
1110
import android.view.View.VISIBLE
11+
import androidx.appcompat.app.AppCompatActivity
1212
import androidx.core.content.ContextCompat
1313
import com.mapbox.api.directions.v5.models.DirectionsRoute
1414
import com.mapbox.api.directions.v5.models.RouteOptions
@@ -21,7 +21,6 @@ import com.mapbox.maps.plugin.LocationPuck2D
2121
import com.mapbox.maps.plugin.animation.camera
2222
import com.mapbox.maps.plugin.gestures.gestures
2323
import com.mapbox.maps.plugin.locationcomponent.location
24-
import com.mapbox.navigation.base.ExperimentalPreviewMapboxNavigationAPI
2524
import com.mapbox.navigation.base.extensions.applyDefaultNavigationOptions
2625
import com.mapbox.navigation.base.extensions.applyLanguageAndVoiceUnitOptions
2726
import com.mapbox.navigation.base.options.NavigationOptions
@@ -45,8 +44,9 @@ import com.mapbox.navigation.core.routealternatives.RouteAlternativesError
4544
import com.mapbox.navigation.core.trip.session.LocationMatcherResult
4645
import com.mapbox.navigation.core.trip.session.LocationObserver
4746
import com.mapbox.navigation.core.trip.session.RouteProgressObserver
48-
import com.mapbox.navigation.examples.core.databinding.ActivityRouteRefreshBinding
49-
import com.mapbox.navigation.examples.util.Utils
47+
import com.mapbox.navigation.qa_test_app.R
48+
import com.mapbox.navigation.qa_test_app.databinding.LayoutRouteRefreshBinding
49+
import com.mapbox.navigation.qa_test_app.utils.Utils
5050
import com.mapbox.navigation.ui.maps.camera.NavigationCamera
5151
import com.mapbox.navigation.ui.maps.camera.data.MapboxNavigationViewportDataSource
5252
import com.mapbox.navigation.ui.maps.camera.lifecycle.NavigationBasicGesturesHandler
@@ -75,11 +75,10 @@ import kotlinx.coroutines.launch
7575
* 5. Wait for refresh: the traffic should reappear.
7676
* 6. Optionally, remove the traffic and wait for refresh again.
7777
*/
78-
@OptIn(ExperimentalPreviewMapboxNavigationAPI::class)
7978
class RouteRefreshActivity : AppCompatActivity() {
8079

8180
/* ----- Layout binding reference ----- */
82-
private lateinit var binding: ActivityRouteRefreshBinding
81+
private lateinit var binding: LayoutRouteRefreshBinding
8382

8483
/* ----- Mapbox Maps components ----- */
8584
private lateinit var mapboxMap: MapboxMap
@@ -208,7 +207,7 @@ class RouteRefreshActivity : AppCompatActivity() {
208207
@SuppressLint("MissingPermission")
209208
override fun onCreate(savedInstanceState: Bundle?) {
210209
super.onCreate(savedInstanceState)
211-
binding = ActivityRouteRefreshBinding.inflate(layoutInflater)
210+
binding = LayoutRouteRefreshBinding.inflate(layoutInflater)
212211
setContentView(binding.root)
213212
mapboxMap = binding.mapView.getMapboxMap()
214213

@@ -232,20 +231,24 @@ class RouteRefreshActivity : AppCompatActivity() {
232231
.routeRefreshOptions(RouteRefreshOptions.Builder().intervalMillis(30000).build())
233232
.build()
234233
)
235-
mapboxNavigation.registerRouteAlternativesObserver(object : NavigationRouteAlternativesObserver {
236-
override fun onRouteAlternatives(
237-
routeProgress: RouteProgress,
238-
alternatives: List<NavigationRoute>,
239-
routerOrigin: RouterOrigin
240-
) {
241-
mapboxNavigation.setNavigationRoutes(listOf(routeProgress.navigationRoute) + alternatives)
242-
}
234+
mapboxNavigation.registerRouteAlternativesObserver(
235+
object :
236+
NavigationRouteAlternativesObserver {
237+
override fun onRouteAlternatives(
238+
routeProgress: RouteProgress,
239+
alternatives: List<NavigationRoute>,
240+
routerOrigin: RouterOrigin
241+
) {
242+
mapboxNavigation.setNavigationRoutes(
243+
listOf(routeProgress.navigationRoute) + alternatives
244+
)
245+
}
243246

244-
override fun onRouteAlternativesError(error: RouteAlternativesError) {
245-
TODO("Not yet implemented")
247+
override fun onRouteAlternativesError(error: RouteAlternativesError) {
248+
// no-op
249+
}
246250
}
247-
248-
})
251+
)
249252
// move the camera to current location on the first update
250253
mapboxNavigation.registerLocationObserver(object : LocationObserver {
251254
override fun onNewRawLocation(rawLocation: Location) {
@@ -360,18 +363,22 @@ class RouteRefreshActivity : AppCompatActivity() {
360363
val newRoutes = routes.map {
361364
it.directionsRoute
362365
.toBuilder()
363-
.legs(it.directionsRoute.legs()?.map { leg ->
364-
leg.toBuilder()
365-
.annotation(
366-
leg.annotation()?.let { annotation ->
367-
annotation.toBuilder()
368-
.congestion(annotation.congestion()?.map { null })
369-
.congestionNumeric(annotation.congestionNumeric()?.map { null })
370-
.build()
371-
}
372-
)
373-
.build()
374-
})
366+
.legs(
367+
it.directionsRoute.legs()?.map { leg ->
368+
leg.toBuilder()
369+
.annotation(
370+
leg.annotation()?.let { annotation ->
371+
annotation.toBuilder()
372+
.congestion(annotation.congestion()?.map { "unknown" })
373+
.congestionNumeric(
374+
annotation.congestionNumeric()?.map { null }
375+
)
376+
.build()
377+
}
378+
)
379+
.build()
380+
}
381+
)
375382
.build()
376383
.toNavigationRoute(it.origin)
377384
}
@@ -460,7 +467,6 @@ class RouteRefreshActivity : AppCompatActivity() {
460467
viewportDataSource.onRouteChanged(routes.first())
461468
viewportDataSource.evaluate()
462469

463-
464470
// show UI elements
465471
binding.routeOverview.visibility = VISIBLE
466472
binding.routeOverview.showTextAndExtend(2000L)

examples/src/main/res/layout/activity_route_refresh.xml renamed to qa-test-app/src/main/res/layout/layout_route_refresh.xml

Lines changed: 4 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1,8 +1,11 @@
11
<?xml version="1.0" encoding="utf-8"?>
22
<androidx.constraintlayout.widget.ConstraintLayout xmlns:android="http://schemas.android.com/apk/res/android"
33
xmlns:app="http://schemas.android.com/apk/res-auto"
4+
xmlns:tools="http://schemas.android.com/tools"
45
android:layout_width="match_parent"
5-
android:layout_height="match_parent">
6+
android:layout_height="match_parent"
7+
tools:context=".view.RouteRefreshActivity"
8+
>
69

710
<com.mapbox.maps.MapView
811
android:id="@+id/mapView"

0 commit comments

Comments
 (0)