Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

enable snapping_include_static_closure by default for reroute #6164

Merged
merged 1 commit into from
Aug 16, 2022
Merged

enable snapping_include_static_closure by default for reroute #6164

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
1 change: 1 addition & 0 deletions CHANGELOG.md
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -13,6 +13,7 @@ Mapbox welcomes participation and contributions from everyone.
- Fixed an issue where some callbacks for completed operations would be invoked again if the `TileStore` service process was terminated. [#6168](https://github.com/mapbox/mapbox-navigation-android/pull/6168)
- Fixed calculation of `AlternativeRouteMetadata` (duration and distance). [#6168](https://github.com/mapbox/mapbox-navigation-android/pull/6168)
- Updated `NavigationView` to render upcoming maneuvers. [#6175](https://github.com/mapbox/mapbox-navigation-android/pull/6175)
- Made the SDK use `snapping_include_static_closures=true` for an origin of each re-route request. Similar to `snapping_include_closures=true`, but it includes statically closed roads, that is long-term. [#6164](https://github.com/mapbox/mapbox-navigation-android/pull/6164)

## Mapbox Navigation SDK 2.8.0-alpha.2 - 12 August, 2022
### Changelog
Expand Down
49 changes: 27 additions & 22 deletions ...igation-core/src/main/java/com/mapbox/navigation/core/routeoptions/RouteOptionsUpdater.kt
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -28,7 +28,7 @@ class RouteOptionsUpdater {
* This carries over or adapts all of the request parameters to best fit the existing situation and remaining portion of the route.
*
* Notable adjustments:
* - `snapping_include_closures=true` is set for the origin of the request to aid with potential need to navigate out of a closed section of the road
* - `snapping_include_closures=true` and `snapping_include_static_closures=true` are set for the origin of the request to aid with potential need to navigate out of a closed section of the road
* - `depart_at`/`arrive_by` parameters are cleared as they are not applicable in update/re-route scenario
*
* @return `RouteOptionsResult.Error` if a new [RouteOptions] instance cannot be combined based on the input given.
Expand Down Expand Up @@ -100,29 +100,16 @@ class RouteOptionsUpdater {
it.addAll(approachesList.subList(index, coordinatesList.size))
}
}
).apply {
)
.apply {
if (routeOptions.profile() == DirectionsCriteria.PROFILE_DRIVING_TRAFFIC) {
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Was this removed on purpose? Look at #6147

cc @VysotskiVadim

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Rebase issues. Moved it back.

snappingIncludeClosuresList(
let snappingClosures@{
val snappingClosures =
routeOptions.snappingIncludeClosuresList()
mutableListOf<Boolean?>().apply {
// append true for the origin of the re-route request
add(true)
if (snappingClosures.isNullOrEmpty()) {
// create `null` value for each upcoming waypoint
addAll(arrayOfNulls<Boolean>(remainingWaypoints))
} else {
// get existing values for each upcoming waypoint
addAll(
snappingClosures.subList(
index + 1,
coordinatesList.size
)
)
}
}
}
routeOptions.snappingIncludeClosuresList()
.withFirstTrue(remainingWaypoints, index, coordinatesList.size)
)
snappingIncludeStaticClosuresList(
routeOptions.snappingIncludeStaticClosuresList()
.withFirstTrue(remainingWaypoints, index, coordinatesList.size)
)
}
}
Expand Down Expand Up @@ -261,6 +248,24 @@ class RouteOptionsUpdater {
return updatedStartWaypointIndicesIndex
}

private fun List<Boolean>?.withFirstTrue(
remainingWaypoints: Int,
index: Int,
coordinatesListSize: Int,
): List<Boolean?> {
return mutableListOf<Boolean?>().also { newList ->
// append true for the origin of the re-route request
newList.add(true)
if (isNullOrEmpty()) {
// create `null` value for each upcoming waypoint
newList.addAll(arrayOfNulls<Boolean>(remainingWaypoints))
} else {
// get existing values for each upcoming waypoint
newList.addAll(subList(index + 1, coordinatesListSize))
}
}
}

/**
* Describes a result of generating new options from the original request and the current route progress.
*/
Expand Down
123 changes: 123 additions & 0 deletions ...ion-core/src/test/java/com/mapbox/navigation/core/routeoptions/RouteOptionsUpdaterTest.kt
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -8,6 +8,7 @@ import com.mapbox.core.constants.Constants
import com.mapbox.geojson.Point
import com.mapbox.navigation.base.trip.model.RouteProgress
import com.mapbox.navigation.core.trip.session.LocationMatcherResult
import com.mapbox.navigation.testing.factories.createLocation
import io.mockk.MockKAnnotations
import io.mockk.every
import io.mockk.mockk
Expand Down Expand Up @@ -566,4 +567,126 @@ class RouteOptionsUpdaterTest {
}
}
}

@RunWith(Parameterized::class)
class SnappingStaticClosuresOptionsParameterized(
val routeOptions: RouteOptions,
val remainingWaypointsParameter: Int,
val legIndex: Int,
val expectedSnappingStaticClosures: String?
) {

private lateinit var routeRefreshAdapter: RouteOptionsUpdater
private lateinit var locationMatcherResult: LocationMatcherResult

companion object {
@JvmStatic
@Parameterized.Parameters
fun params() = listOf(
arrayOf(
provideRouteOptionsWithCoordinates()
.toBuilder()
.snappingIncludeStaticClosuresList(
listOf(
true,
false,
true,
false
)
)
.build(),
3,
0,
"true;false;true;false"
),
arrayOf(
provideRouteOptionsWithCoordinates(),
1,
2,
"true;"
),
arrayOf(
provideRouteOptionsWithCoordinates()
.toBuilder()
.snappingIncludeStaticClosuresList(
listOf(
true,
false,
false,
false
)
)
.build(),
2,
1,
"true;false;false"
),
arrayOf(
provideRouteOptionsWithCoordinates().toBuilder()
.snappingIncludeStaticClosuresList(
listOf(
true,
false,
true,
false
)
)
.build(),
1,
2,
"true;false"
),
arrayOf(
provideRouteOptionsWithCoordinates().toBuilder()
.snappingIncludeStaticClosuresList(null)
.profile(DirectionsCriteria.PROFILE_CYCLING)
.build(),
1,
2,
null
)
)
}

@Before
fun setup() {
MockKAnnotations.init(this, relaxUnitFun = true, relaxed = true)
mockLocation()

routeRefreshAdapter = RouteOptionsUpdater()
}

@Test
fun snappingClosuresOptions() {
val routeProgress: RouteProgress = mockk(relaxed = true) {
every { remainingWaypoints } returns remainingWaypointsParameter
every { currentLegProgress?.legIndex } returns legIndex
}

val newRouteOptions =
routeRefreshAdapter.update(routeOptions, routeProgress, locationMatcherResult)
.let {
assertTrue(it is RouteOptionsUpdater.RouteOptionsResult.Success)
return@let it as RouteOptionsUpdater.RouteOptionsResult.Success
}
.routeOptions

val actualSnappingStaticClosures = newRouteOptions.snappingIncludeStaticClosures()

assertEquals(expectedSnappingStaticClosures, actualSnappingStaticClosures)
MapboxRouteOptionsUpdateCommonTest.checkImmutableFields(routeOptions, newRouteOptions)
}

private fun mockLocation() {
val location = createLocation(
-122.4232,
23.54423,
DEFAULT_REROUTE_BEARING_ANGLE,
)
locationMatcherResult = mockk {
every { enhancedLocation } returns location
every { zLevel } returns DEFAULT_Z_LEVEL
}
}
}
}