navigation specific route options builder #5427
There are no files selected for viewing
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,80 @@ | ||
| package com.mapbox.navigation.instrumentation_tests.core | ||
|
|
||
| import android.location.Location | ||
| import androidx.test.platform.app.InstrumentationRegistry | ||
| import com.mapbox.api.directions.v5.DirectionsCriteria | ||
| import com.mapbox.geojson.Point | ||
| import com.mapbox.navigation.base.ExperimentalPreviewMapboxNavigationAPI | ||
| import com.mapbox.navigation.base.options.NavigationOptions | ||
| import com.mapbox.navigation.core.MapboxNavigationProvider | ||
| import com.mapbox.navigation.core.RequestRoutesResult | ||
| import com.mapbox.navigation.instrumentation_tests.R | ||
| import com.mapbox.navigation.instrumentation_tests.activity.EmptyTestActivity | ||
| import com.mapbox.navigation.instrumentation_tests.utils.MapboxNavigationRule | ||
| import com.mapbox.navigation.instrumentation_tests.utils.http.MockDirectionsRequestHandler | ||
| import com.mapbox.navigation.instrumentation_tests.utils.readRawFileText | ||
| import com.mapbox.navigation.instrumentation_tests.utils.runOnMainSync | ||
| import com.mapbox.navigation.testing.ui.BaseTest | ||
| import com.mapbox.navigation.testing.ui.utils.getMapboxAccessTokenFromResources | ||
| import junit.framework.Assert.assertEquals | ||
| import kotlinx.coroutines.Dispatchers | ||
| import kotlinx.coroutines.runBlocking | ||
| import org.junit.Rule | ||
| import org.junit.Test | ||
|
|
||
| @OptIn(ExperimentalPreviewMapboxNavigationAPI::class) | ||
| class RouteOptionsBuilderTest : BaseTest<EmptyTestActivity>(EmptyTestActivity::class.java) { | ||
|
|
||
| @get:Rule | ||
| val mapboxNavigationRule = MapboxNavigationRule() | ||
|
|
||
| @Test | ||
| fun navigateFromCurrentLocation() { | ||
| mockWebServerRule.requestHandlers.add( | ||
| MockDirectionsRequestHandler( | ||
| profile = DirectionsCriteria.PROFILE_DRIVING, | ||
| jsonResponse = readRawFileText(activity, R.raw.reroute_response_dc_very_short), | ||
| expectedCoordinates = null, | ||
| relaxedExpectedCoordinates = true | ||
| ) | ||
| ) | ||
|
|
||
| val mapboxNavigation = runOnMainSync { | ||
| val context = InstrumentationRegistry.getInstrumentation().getTargetContext() | ||
| MapboxNavigationProvider.create( | ||
| NavigationOptions.Builder(context) | ||
| .accessToken(getMapboxAccessTokenFromResources(context)) | ||
| .build() | ||
| ) | ||
| } | ||
|
|
||
| val routeRequest = runBlocking(Dispatchers.Main) { | ||
| mapboxNavigation.startTripSession() | ||
| mapboxNavigation.requestRoutes { builder -> | ||
| builder | ||
| .fromCurrentLocation() | ||
| .toDestination( | ||
| coordinate = Point.fromLngLat(2.0, 2.0) | ||
| ) | ||
| .profileDriving() | ||
| .baseUrl(mockWebServerRule.baseUrl) | ||
| } as RequestRoutesResult.Successful | ||
| } | ||
|
|
||
| val routeOptions = routeRequest.routes.first().routeOptions | ||
| assertEquals( | ||
| listOf( | ||
| Point.fromLngLat(1.0, 1.0), | ||
| Point.fromLngLat(2.0, 2.0), | ||
| ), | ||
| routeOptions.coordinatesList() | ||
| ) | ||
| } | ||
|
|
||
| override fun setupMockLocation(): Location { | ||
| return mockLocationUpdatesRule.generateLocationUpdate { | ||
| longitude = 1.0 | ||
| latitude = 1.0 | ||
| } | ||
| } | ||
| } |
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,22 @@ | ||
| package com.mapbox.navigation.core | ||
|
|
||
| import com.mapbox.api.directions.v5.models.RouteOptions | ||
| import com.mapbox.navigation.base.ExperimentalPreviewMapboxNavigationAPI | ||
| import com.mapbox.navigation.base.route.NavigationRoute | ||
| import com.mapbox.navigation.base.route.RouterFailure | ||
| import com.mapbox.navigation.base.route.RouterOrigin | ||
|
|
||
| @ExperimentalPreviewMapboxNavigationAPI | ||
| sealed class RequestRoutesResult { | ||
| @ExperimentalPreviewMapboxNavigationAPI | ||
| data class Successful( | ||
| val routes: List<NavigationRoute>, | ||
| val routerOrigin: RouterOrigin, | ||
| ) : RequestRoutesResult() | ||
|
|
||
| @ExperimentalPreviewMapboxNavigationAPI | ||
| data class Failed( | ||
| val reasons: List<RouterFailure>, | ||
| val routeOptions: RouteOptions | ||
| ) : RequestRoutesResult() | ||
| } |
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,74 @@ | ||
| package com.mapbox.navigation.core.routeoptions.builder | ||
|
|
||
| import android.location.Location | ||
| import com.mapbox.geojson.Point | ||
| import com.mapbox.navigation.core.trip.session.LocationMatcherResult | ||
| import com.mapbox.navigation.core.trip.session.LocationObserver | ||
| import com.mapbox.navigation.core.trip.session.TripSessionLocationProvider | ||
| import com.mapbox.navigation.utils.internal.toPoint | ||
| import kotlinx.coroutines.suspendCancellableCoroutine | ||
| import kotlinx.coroutines.withTimeout | ||
| import kotlin.coroutines.resume | ||
|
|
||
| internal interface LocationProvider { | ||
| suspend fun getCurrentLocation(): CurrentLocation | ||
| } | ||
|
|
||
| internal data class CurrentLocation( | ||
| val point: Point, | ||
| val bearing: Double?, | ||
| val zLevel: Int? | ||
| ) | ||
|
|
||
| internal class LocationFromTripSessionProvider( | ||
|
There was a problem hiding this comment. This class and internal suspend fun TripSessionLocationProvider.currentLocation() = suspendCancellableCoroutine<CurrentLocation> { continuation ->
// return immediately if locationMatcherResult has already been set
locationMatcherResult?.also {
continuation.resume(it.toCurrentLocation())
return@suspendCancellableCoroutine
}
// subscribe for the next locationMatcherResult
val observer = object : LocationObserver {
override fun onNewRawLocation(rawLocation: Location) = Unit
override fun onNewLocationMatcherResult(locationMatcherResult: LocationMatcherResult) {
unregisterLocationObserver(this)
continuation.resume(locationMatcherResult.toCurrentLocation())
}
}
registerLocationObserver(observer)
continuation.invokeOnCancellation {
unregisterLocationObserver(observer)
}
}
private fun LocationMatcherResult.toCurrentLocation() = CurrentLocation(
point = enhancedLocation.toPoint(),
bearing = enhancedLocation.bearing.toDouble(),
zLevel = zLevel
)There was a problem hiding this comment. Thank you. I agree it can. I'm not sure I understand why an extension function is better than a wrapper class. Please share your thoughts about exception function vs wrapper class . Not sure if the provided code is expected to be fully functional but it will crash on There was a problem hiding this comment.
There is nothing wrong with wrapper classes. They are extremely useful in languages that doesn't provide any other mechanism to add new functionality to an existing class or when we need to store some state. In this specific case, we can extend TripSessionLocationProvider functionality by utilizing Kotlin extensions.
I am assuming the crash is caused by ConcurrentModificationException. We should create new ticket for patching There was a problem hiding this comment. If I replace wrapper by an extension function then |
||
| private val tripSessionLocationProvider: TripSessionLocationProvider | ||
| ) : LocationProvider { | ||
| override suspend fun getCurrentLocation(): CurrentLocation { | ||
| val currentLocation = tripSessionLocationProvider.locationMatcherResult | ||
| return currentLocation?.toCurrentLocation() ?: waitForTheFirstLocationEventWithTimeout() | ||
| } | ||
|
|
||
| private suspend fun waitForTheFirstLocationEventWithTimeout() = | ||
| withTimeout(GETTING_LOCATION_TIMEOUT_MILLISECONDS) { | ||
| waitForTheFirstLocationEvent() | ||
| } | ||
|
|
||
| private suspend fun waitForTheFirstLocationEvent(): CurrentLocation { | ||
| val (result, cleanup) = suspendCancellableCoroutine<Pair<CurrentLocation, () -> Unit>> | ||
| { continuation -> | ||
| val observer = object : LocationObserver { | ||
| override fun onNewRawLocation(rawLocation: Location) { | ||
| } | ||
|
|
||
| override fun onNewLocationMatcherResult( | ||
| locationMatcherResult: LocationMatcherResult | ||
| ) { | ||
| continuation.resume( | ||
| Pair( | ||
| locationMatcherResult.toCurrentLocation(), | ||
| { | ||
| tripSessionLocationProvider.unregisterLocationObserver(this) | ||
| } | ||
| ) | ||
| ) | ||
| } | ||
| } | ||
| tripSessionLocationProvider.registerLocationObserver(observer) | ||
| continuation.invokeOnCancellation { | ||
| tripSessionLocationProvider.unregisterLocationObserver(observer) | ||
| } | ||
| } | ||
| cleanup() | ||
| return result | ||
| } | ||
|
|
||
| private fun LocationMatcherResult.toCurrentLocation() = CurrentLocation( | ||
| point = enhancedLocation.toPoint(), | ||
| bearing = enhancedLocation.bearing.toDouble(), | ||
| zLevel = zLevel | ||
| ) | ||
|
|
||
| private companion object { | ||
| private const val GETTING_LOCATION_TIMEOUT_MILLISECONDS = 30_000L | ||
| } | ||
| } | ||
Oops, something went wrong.
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Let's keep it consistent with other APIs that are in the Nav SDK.