From d5c1e16a1f4bc1ca89e1db25c5c1114a3de35898 Mon Sep 17 00:00:00 2001 From: Cameron Mace Date: Tue, 24 Oct 2017 15:21:20 -0400 Subject: [PATCH 1/5] Changed up NavigationRoute (#413) * Changed up NavigationRoute * remove TODO * added test to ensure bearing were in correct order * Added test to make sure bearing are correct --- .../v5/navigation/NavigationRoute.java | 305 +++++++++++++++++- .../v5/navigation/NavigationRouteTest.java | 50 ++- 2 files changed, 338 insertions(+), 17 deletions(-) diff --git a/libandroid-navigation/src/main/java/com/mapbox/services/android/navigation/v5/navigation/NavigationRoute.java b/libandroid-navigation/src/main/java/com/mapbox/services/android/navigation/v5/navigation/NavigationRoute.java index 0597766147c..7dd6df80fcc 100644 --- a/libandroid-navigation/src/main/java/com/mapbox/services/android/navigation/v5/navigation/NavigationRoute.java +++ b/libandroid-navigation/src/main/java/com/mapbox/services/android/navigation/v5/navigation/NavigationRoute.java @@ -9,28 +9,92 @@ import com.mapbox.directions.v5.DirectionsCriteria.ProfileCriteria; import com.mapbox.directions.v5.MapboxDirections; import com.mapbox.directions.v5.models.DirectionsResponse; +import com.mapbox.directions.v5.models.DirectionsRoute; import com.mapbox.geojson.Point; +import com.mapbox.services.exceptions.ServicesException; import java.util.Locale; +import retrofit2.Call; import retrofit2.Callback; + +/** + * The NavigationRoute class wraps the {@link MapboxDirections} class with parameters which + * must be set inorder for a navigation session to successfully begin. While it is possible + * to pass in any {@link com.mapbox.directions.v5.models.DirectionsRoute} into + * {@link MapboxNavigation#startNavigation(DirectionsRoute)}, using this class will ensure your + * request includes all the proper information needed for the navigation session to begin. + *

+ *

+ * Developer Note: MapboxDirections cannot be directly extended since it id an AutoValue class. + *

+ * 0.5.0 + */ public final class NavigationRoute { private final MapboxDirections mapboxDirections; + /** + * Build a new {@link NavigationRoute} object with the proper navigation parameters already setup. + * + * @return a {@link Builder} object for creating this object + * @since 0.5.0 + */ public static Builder builder() { - return new Builder(); + return new Builder() + .annotations(DirectionsCriteria.ANNOTATION_CONGESTION) + .profile(DirectionsCriteria.PROFILE_DRIVING_TRAFFIC); } + /** + * Private constructor used for the {@link Builder#build()} method. + * + * @param mapboxDirections a new instance of a {@link MapboxDirections} class + * @since 0.5.0 + */ private NavigationRoute(MapboxDirections mapboxDirections) { this.mapboxDirections = mapboxDirections; } + /** + * Call when you have constructed your navigation route with your desired parameters. A + * {@link Callback} must be passed into the method to handle both the response and failure. + * + * @param callback a RetroFit callback which contains an onResponse and onFailure + * @since 0.5.0 + */ public void getRoute(Callback callback) { mapboxDirections.enqueueCall(callback); } + /** + * Wrapper method for Retrofits {@link Call#clone()} call, useful for getting call information + * and allowing you to perform additional functions on this {@link NavigationRoute} class. + * + * @return cloned call + * @since 1.0.0 + */ + public Call getCall() { + return mapboxDirections.cloneCall(); + } + + public void cancelCall() { + getCall().cancel(); + } + + /** + * This builder is used to create a new request to the Mapbox Directions API and removes options + * which would cause this navigation SDK to not behave properly. At a bare minimum, your request + * must include an access token, an origin, and a destination. All other fields can be left alone + * inorder to use the default behaviour of the API. + *

+ * By default, the directions profile is set to driving with traffic but can be changed to + * reflect your users use-case. + *

+ * + * @since 0.5.0 + */ public static final class Builder { private final MapboxDirections.Builder directionsBuilder; @@ -48,86 +112,301 @@ private Builder() { * * @param user a non-null string which will replace the default user used in the directions * request - * @return this directionsBuilder for chaining options together - * @since 1.0.0 + * @return this builder for chaining options together + * @since 0.5.0 */ public Builder user(@NonNull String user) { directionsBuilder.user(user); return this; } + /** + * This selects which mode of transportation the user will be using while navigating from the + * origin to the final destination. The options include driving, driving considering traffic, + * walking, and cycling. Using each of these profiles will result in different routing biases. + * + * @param profile required to be one of the String values found in the {@link ProfileCriteria} + * @return this builder for chaining options together + * @since 0.5.0 + */ public Builder profile(@NonNull @ProfileCriteria String profile) { directionsBuilder.profile(profile); return this; } + /** + * This sets the starting point on the map where the route will begin. It is one of the + * required parameters which must be set for a successful directions response. + * + * @param origin a GeoJson {@link Point} object representing the starting location for the route + * @return this builder for chaining options together + * @since 0.5.0 + */ public Builder origin(@NonNull Point origin) { + origin(origin, null, null); + return this; + } + + /** + * This sets the starting point on the map where the route will begin. It is one of the + * required parameters which must be set for a successful directions response. + * + * @param origin a GeoJson {@link Point} object representing the starting location for the + * route + * @param angle double value used for setting the corresponding coordinate's angle of travel + * when determining the route + * @param tolerance the deviation the bearing angle can vary while determining the route, + * recommended to be either 45 or 90 degree tolerance + * @return this builder for chaining options together + * @since 0.5.0 + */ + public Builder origin(@NonNull Point origin, @Nullable Double angle, + @Nullable Double tolerance) { directionsBuilder.origin(origin); + directionsBuilder.addBearing(angle, tolerance); return this; } + /** + * This sets the ending point on the map where the route will end. It is one of the required + * parameters which must be set for a successful directions response. + * + * @param destination a GeoJson {@link Point} object representing the starting location for the + * route + * @return this builder for chaining options together + * @since 0.50 + */ public Builder destination(@NonNull Point destination) { + destination(destination, null, null); + return this; + } + + /** + * This sets the ending point on the map where the route will end. It is one of the required + * parameters which must be set for a successful directions response. + * + * @param destination a GeoJson {@link Point} object representing the starting location for the + * route + * @param angle double value used for setting the corresponding coordinate's angle of travel + * when determining the route + * @param tolerance the deviation the bearing angle can vary while determining the route, + * recommended to be either 45 or 90 degree tolerance + * @return this builder for chaining options together + * @since 0.5.0 + */ + public Builder destination(@NonNull Point destination, @Nullable Double angle, + @Nullable Double tolerance) { directionsBuilder.destination(destination); + directionsBuilder.addBearing(angle, tolerance); return this; } + /** + * This can be used to set up to 23 additional in-between points which will act as pit-stops + * along the users route. Note that if you are using the + * {@link DirectionsCriteria#PROFILE_DRIVING_TRAFFIC} that the max number of waypoints allowed + * in the request is currently limited to 1. + * + * @param waypoint a {@link Point} which represents the pit-stop or waypoint where you'd like + * one of the {@link com.mapbox.directions.v5.models.RouteLeg} to + * navigate the user to + * @return this builder for chaining options together + * @since 0.5.0 + */ public Builder addWaypoint(@NonNull Point waypoint) { directionsBuilder.addWaypoint(waypoint); return this; } + /** + * This can be used to set up to 23 additional in-between points which will act as pit-stops + * along the users route. Note that if you are using the + * {@link DirectionsCriteria#PROFILE_DRIVING_TRAFFIC} that the max number of waypoints allowed + * in the request is currently limited to 1. + * + * @param waypoint a {@link Point} which represents the pit-stop or waypoint where you'd like + * one of the {@link com.mapbox.directions.v5.models.RouteLeg} to + * navigate the user to + * @param angle double value used for setting the corresponding coordinate's angle of travel + * when determining the route + * @param tolerance the deviation the bearing angle can vary while determining the route, + * recommended to be either 45 or 90 degree tolerance + * @return this builder for chaining options together + * @since 0.5.0 + */ + public Builder addWaypoint(@NonNull Point waypoint, @Nullable Double angle, + @Nullable Double tolerance) { + directionsBuilder.addWaypoint(waypoint); + directionsBuilder.addBearing(angle, tolerance); + return this; + } + + /** + * Optionally set whether to try to return alternative routes. An alternative is classified as a + * route that is significantly different then the fastest route, but also still reasonably fast. + * Not in all circumstances such a route exists. At the moment at most one alternative can be + * returned. + * + * @param alternatives true if you'd like to receive an alternative route, otherwise false or + * null to use the APIs default value + * @return this builder for chaining options together + * @since 0.5.0 + */ public Builder alternatives(@Nullable Boolean alternatives) { directionsBuilder.alternatives(alternatives); return this; } + /** + * Set the instruction language for the directions request, the default is english. Only a + * select number of languages are currently supported, reference the table provided in the see + * link below. + * + * @param language a Locale value representing the language you'd like the instructions to be + * written in when returned + * @return this builder for chaining options together + * @see Supported + * Languages + * @since 0.5.0 + */ public Builder language(@Nullable Locale language) { directionsBuilder.language(language); return this; } + /** + * Whether or not to return additional metadata along the route. Possible values are: + * {@link DirectionsCriteria#ANNOTATION_DISTANCE}, + * {@link DirectionsCriteria#ANNOTATION_DURATION}, + * {@link DirectionsCriteria#ANNOTATION_DURATION} and + * {@link DirectionsCriteria#ANNOTATION_CONGESTION}. Several annotation can be used by + * separating them with {@code ,}. + *

+ * If left alone, this will automatically set Congestion to enabled + *

+ * + * @param annotations string referencing one of the annotation direction criteria's. The strings + * restricted to one or multiple values inside the {@link AnnotationCriteria} + * or null which will result in no annotations being used + * @return this builder for chaining options together + * @see RouteLeg object + * documentation + * @since 0.5.0 + */ public Builder annotations(@Nullable @AnnotationCriteria String... annotations) { directionsBuilder.annotations(annotations); return this; } + /** + * Optionally, Use to filter the road segment the waypoint will be placed on by direction and + * dictates the angle of approach. This option should always be used in conjunction with the + * {@link #radiuses(double...)} parameter. + *

+ * The parameter takes two values per waypoint: the first is an angle clockwise from true north + * between 0 and 360. The second is the range of degrees the angle can deviate by. We recommend + * a value of 45 degrees or 90 degrees for the range, as bearing measurements tend to be + * inaccurate. This is useful for making sure we reroute vehicles on new routes that continue + * traveling in their current direction. A request that does this would provide bearing and + * radius values for the first waypoint and leave the remaining values empty. If provided, the + * list of bearings must be the same length as the list of waypoints, but you can skip a + * coordinate and show its position by passing in null value for both the angle and tolerance + * values. + *

+ * Each bearing value gets associated with the same order which coordinates are arranged in this + * builder. For example, the first bearing added in this builder will be associated with the + * origin {@code Point}, the nth bearing being associated with the nth waypoint added (if added) + * and the last bearing being added will be associated with the destination. + *

+ * If given the chance, you should pass in the bearing information at the same time the point is + * passed in as a waypoint, this way it is ensured the value is matched up correctly with the + * coordinate. + * + * @param angle double value used for setting the corresponding coordinate's angle of travel + * when determining the route + * @param tolerance the deviation the bearing angle can vary while determining the route, + * recommended to be either 45 or 90 degree tolerance + * @return this builder for chaining options together + * @since 0.5.0 + */ public Builder addBearing(@Nullable @FloatRange(from = 0, to = 360) Double angle, @Nullable @FloatRange(from = 0, to = 360) Double tolerance) { directionsBuilder.addBearing(angle, tolerance); return this; } + /** + * Optionally, set the maximum distance in meters that each coordinate is allowed to move when + * snapped to a nearby road segment. There must be as many radiuses as there are coordinates in + * the request. Values can be any number greater than 0 or they can be unlimited simply by + * passing {@link Double#POSITIVE_INFINITY}. + *

+ * If no routable road is found within the radius, a {@code NoSegment} error is returned. + *

+ * + * @param radiuses double array containing the radiuses defined in unit meters. + * @return this builder for chaining options together + * @since 0.5.0 + */ public Builder radiuses(@FloatRange(from = 0) double... radiuses) { directionsBuilder.radiuses(radiuses); return this; } + /** + * Base package name or other simple string identifier. Used inside the calls user agent header. + * + * @param clientAppName base package name or other simple string identifier + * @return this builder for chaining options together + * @since 0.5.0 + */ public Builder clientAppName(@NonNull String clientAppName) { directionsBuilder.clientAppName(clientAppName); return this; } + /** + * Required to call when this is being built. If no access token provided, + * {@link ServicesException} will be thrown. + * + * @param accessToken Mapbox access token, You must have a Mapbox account inorder to use + * the Optimization API + * @return this builder for chaining options together + * @since 0.5.0 + */ public Builder accessToken(@NonNull String accessToken) { directionsBuilder.accessToken(accessToken); return this; } + /** + * Optionally change the APIs base URL to something other then the default Mapbox one. + * + * @param baseUrl base url used as end point + * @return this builder for chaining options together + * @since 0.5.0 + */ public Builder baseUrl(String baseUrl) { directionsBuilder.baseUrl(baseUrl); return this; } + /** + * This uses the provided parameters set using the {@link Builder} and adds the required + * settings for navigation to work correctly. + * + * @return a new instance of Navigation Route + * @since 0.5.0 + */ public NavigationRoute build() { - // These are the default values required to have a directions - // route worthy of navigating along. - directionsBuilder.steps(true); - directionsBuilder.continueStraight(true); - directionsBuilder.annotations(DirectionsCriteria.ANNOTATION_CONGESTION); - directionsBuilder.profile(DirectionsCriteria.PROFILE_DRIVING_TRAFFIC); - directionsBuilder.geometries(DirectionsCriteria.GEOMETRY_POLYLINE6); - directionsBuilder.overview(DirectionsCriteria.OVERVIEW_FULL); - directionsBuilder.voiceInstructions(true); - directionsBuilder.roundaboutExits(true); + // Set the default values which the user cannot alter. + directionsBuilder + .steps(true) + .continueStraight(true) + .geometries(DirectionsCriteria.GEOMETRY_POLYLINE6) + .overview(DirectionsCriteria.OVERVIEW_FULL) + .voiceInstructions(true) + .roundaboutExits(true); return new NavigationRoute(directionsBuilder.build()); } } diff --git a/libandroid-navigation/src/test/java/com/mapbox/services/android/navigation/v5/navigation/NavigationRouteTest.java b/libandroid-navigation/src/test/java/com/mapbox/services/android/navigation/v5/navigation/NavigationRouteTest.java index 05267e0f9d2..a3e0ef1852f 100644 --- a/libandroid-navigation/src/test/java/com/mapbox/services/android/navigation/v5/navigation/NavigationRouteTest.java +++ b/libandroid-navigation/src/test/java/com/mapbox/services/android/navigation/v5/navigation/NavigationRouteTest.java @@ -1,14 +1,17 @@ package com.mapbox.services.android.navigation.v5.navigation; +import com.mapbox.directions.v5.DirectionsCriteria; import com.mapbox.geojson.Point; import com.mapbox.services.android.navigation.v5.BaseTest; +import org.hamcrest.CoreMatchers; +import org.junit.Ignore; import org.junit.Test; -import static junit.framework.Assert.assertEquals; -import static junit.framework.Assert.assertFalse; import static junit.framework.Assert.assertNotNull; -import static junit.framework.Assert.assertTrue; + +import static org.hamcrest.CoreMatchers.containsString; +import static org.junit.Assert.assertThat; public class NavigationRouteTest extends BaseTest { @@ -21,4 +24,43 @@ public void sanityTest() throws Exception { .build(); assertNotNull(navigationRoute); } -} \ No newline at end of file + + @Test + public void changingDefaultValueToCustomWorksProperly() throws Exception { + NavigationRoute navigationRoute = NavigationRoute.builder() + .accessToken(ACCESS_TOKEN) + .origin(Point.fromLngLat(1.0, 2.0)) + .destination(Point.fromLngLat(1.0, 5.0)) + .profile(DirectionsCriteria.PROFILE_CYCLING) + .build(); + + assertThat(navigationRoute.getCall().request().url().toString(), + containsString("/cycling/")); + } + + @Test + public void addingPointAndBearingKeepsCorrectOrder() throws Exception { + NavigationRoute navigationRoute = NavigationRoute.builder() + .accessToken(ACCESS_TOKEN) + .origin(Point.fromLngLat(1.0, 2.0), 90d, 90d) + .addBearing(2.0, 3.0) + .destination(Point.fromLngLat(1.0, 5.0)) + .build(); + + assertThat(navigationRoute.getCall().request().url().toString(), + containsString("bearings=90,90;2,3;")); + } + + @Test + @Ignore + public void reverseOriginDestinationDoesntMessUpBearings() throws Exception { + NavigationRoute navigationRoute = NavigationRoute.builder() + .accessToken(ACCESS_TOKEN) + .destination(Point.fromLngLat(1.0, 5.0),1d,5d) + .origin(Point.fromLngLat(1.0, 2.0), 90d, 90d) + .build(); + + assertThat(navigationRoute.getCall().request().url().toString(), + containsString("bearings=90,90;1,5")); + } +} From 7d1375fbc1c2e51392f90f67a07ae4fe99575394 Mon Sep 17 00:00:00 2001 From: Dan Nesfeder Date: Wed, 25 Oct 2017 14:53:09 -0700 Subject: [PATCH 2/5] Wait for map style to load before initializing run time styling (#423) --- .../navigation/ui/v5/NavigationView.java | 31 ++++++++----------- .../navigation/ui/v5/ThemeSwitcher.java | 7 +++-- 2 files changed, 17 insertions(+), 21 deletions(-) diff --git a/libandroid-navigation-ui/src/main/java/com/mapbox/services/android/navigation/ui/v5/NavigationView.java b/libandroid-navigation-ui/src/main/java/com/mapbox/services/android/navigation/ui/v5/NavigationView.java index d6c9de839f5..183be46368d 100644 --- a/libandroid-navigation-ui/src/main/java/com/mapbox/services/android/navigation/ui/v5/NavigationView.java +++ b/libandroid-navigation-ui/src/main/java/com/mapbox/services/android/navigation/ui/v5/NavigationView.java @@ -102,7 +102,7 @@ public NavigationView(Context context, @Nullable AttributeSet attrs, int defStyl public void onCreate(@Nullable Bundle savedInstanceState) { resumeState = savedInstanceState != null; - initMap(savedInstanceState); + mapView.onCreate(savedInstanceState); } public void onStart() { @@ -158,13 +158,18 @@ public void onMapReady(MapboxMap mapboxMap) { map = mapboxMap; map.setOnScrollListener(this); setMapPadding(0, 0, 0, summaryBehavior.getPeekHeight()); - initRoute(); - initCamera(); - initLocationLayer(); - initLifecycleObservers(); - initNavigationPresenter(); - subscribeViews(); - navigationListener.onNavigationReady(); + ThemeSwitcher.setMapStyle(getContext(), map, new MapboxMap.OnStyleLoadedListener() { + @Override + public void onStyleLoaded(String style) { + initRoute(); + initCamera(); + initLocationLayer(); + initLifecycleObservers(); + initNavigationPresenter(); + subscribeViews(); + navigationListener.onNavigationReady(); + } + }); } /** @@ -414,16 +419,6 @@ public void onClick(View view) { }); } - /** - * Sets up the {@link MapboxMap}. - * - * @param savedInstanceState from onCreate() - */ - private void initMap(Bundle savedInstanceState) { - mapView.onCreate(savedInstanceState); - ThemeSwitcher.setMapStyle(getContext(), mapView); - } - /** * Initializes the {@link BottomSheetBehavior} for {@link SummaryBottomSheet}. */ diff --git a/libandroid-navigation-ui/src/main/java/com/mapbox/services/android/navigation/ui/v5/ThemeSwitcher.java b/libandroid-navigation-ui/src/main/java/com/mapbox/services/android/navigation/ui/v5/ThemeSwitcher.java index e1abda4f931..6ca9f70d949 100644 --- a/libandroid-navigation-ui/src/main/java/com/mapbox/services/android/navigation/ui/v5/ThemeSwitcher.java +++ b/libandroid-navigation-ui/src/main/java/com/mapbox/services/android/navigation/ui/v5/ThemeSwitcher.java @@ -11,6 +11,7 @@ import com.mapbox.mapboxsdk.annotations.Icon; import com.mapbox.mapboxsdk.annotations.IconFactory; import com.mapbox.mapboxsdk.maps.MapView; +import com.mapbox.mapboxsdk.maps.MapboxMap; /** * This class is used to switch theme colors in {@link NavigationView}. @@ -52,14 +53,14 @@ static void toggleTheme(Activity activity) { * Sets the {@link MapView} style based on the current theme setting. * * @param context to retrieve {@link SharedPreferences} - * @param mapView the style will be set on + * @param map the style will be set on */ - static void setMapStyle(Context context, MapView mapView) { + static void setMapStyle(Context context, MapboxMap map, MapboxMap.OnStyleLoadedListener listener) { SharedPreferences preferences = PreferenceManager.getDefaultSharedPreferences(context); boolean darkThemeEnabled = preferences.getBoolean(context.getString(R.string.dark_theme_enabled), false); String nightThemeUrl = context.getString(R.string.navigation_guidance_night_v2); String dayThemeUrl = context.getString(R.string.navigation_guidance_day_v2); - mapView.setStyleUrl(darkThemeEnabled ? nightThemeUrl : dayThemeUrl); + map.setStyleUrl(darkThemeEnabled ? nightThemeUrl : dayThemeUrl, listener); } /** From 4f872e2b1982589b824199e9d1de8b2dc6b9f17a Mon Sep 17 00:00:00 2001 From: Cameron Mace Date: Wed, 25 Oct 2017 18:01:04 -0400 Subject: [PATCH 3/5] adds validation utils class (#424) --- .../v5/navigation/MapboxNavigation.java | 2 ++ .../navigation/v5/utils/ValidationUtils.java | 20 +++++++++++++++++++ 2 files changed, 22 insertions(+) create mode 100644 libandroid-navigation/src/main/java/com/mapbox/services/android/navigation/v5/utils/ValidationUtils.java diff --git a/libandroid-navigation/src/main/java/com/mapbox/services/android/navigation/v5/navigation/MapboxNavigation.java b/libandroid-navigation/src/main/java/com/mapbox/services/android/navigation/v5/navigation/MapboxNavigation.java index a8555fe8fa4..27c76701e68 100644 --- a/libandroid-navigation/src/main/java/com/mapbox/services/android/navigation/v5/navigation/MapboxNavigation.java +++ b/libandroid-navigation/src/main/java/com/mapbox/services/android/navigation/v5/navigation/MapboxNavigation.java @@ -25,6 +25,7 @@ import com.mapbox.services.android.navigation.v5.routeprogress.RouteProgress; import com.mapbox.services.android.navigation.v5.snap.Snap; import com.mapbox.services.android.navigation.v5.snap.SnapToRoute; +import com.mapbox.services.android.navigation.v5.utils.ValidationUtils; import com.mapbox.services.android.telemetry.MapboxEvent; import com.mapbox.services.android.telemetry.MapboxTelemetry; import com.mapbox.services.android.telemetry.location.LocationEngine; @@ -353,6 +354,7 @@ public LocationEngine getLocationEngine() { * @since 0.1.0 */ public void startNavigation(@NonNull DirectionsRoute directionsRoute) { + ValidationUtils.validDirectionsRoute(directionsRoute, options.defaultMilestonesEnabled()); this.directionsRoute = directionsRoute; Timber.d("MapboxNavigation startNavigation called."); if (!isBound) { diff --git a/libandroid-navigation/src/main/java/com/mapbox/services/android/navigation/v5/utils/ValidationUtils.java b/libandroid-navigation/src/main/java/com/mapbox/services/android/navigation/v5/utils/ValidationUtils.java new file mode 100644 index 00000000000..bb63f11afe7 --- /dev/null +++ b/libandroid-navigation/src/main/java/com/mapbox/services/android/navigation/v5/utils/ValidationUtils.java @@ -0,0 +1,20 @@ +package com.mapbox.services.android.navigation.v5.utils; + +import com.mapbox.directions.v5.models.DirectionsRoute; + +import java.util.MissingFormatArgumentException; + +public final class ValidationUtils { + + private ValidationUtils() { + // Class should not be initialized. + } + + public static void validDirectionsRoute(DirectionsRoute directionsRoute, + boolean defaultMilestonesEnabled) { + if (!directionsRoute.routeOptions().voiceInstructions() && defaultMilestonesEnabled) { + throw new MissingFormatArgumentException("Using the default milestone requires the " + + "directions route to include the voice instructions object."); + } + } +} From c71c724b8e6bcf87fda5ebd89d657fb0ef44bb5d Mon Sep 17 00:00:00 2001 From: Dan Nesfeder Date: Wed, 25 Oct 2017 16:35:07 -0700 Subject: [PATCH 4/5] Cancel notification when the service is destroyed, not just when notification button is clicked (#409) --- .../navigation/v5/navigation/NavigationNotification.java | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/libandroid-navigation/src/main/java/com/mapbox/services/android/navigation/v5/navigation/NavigationNotification.java b/libandroid-navigation/src/main/java/com/mapbox/services/android/navigation/v5/navigation/NavigationNotification.java index c1ab7e81d7b..cb60b216d58 100644 --- a/libandroid-navigation/src/main/java/com/mapbox/services/android/navigation/v5/navigation/NavigationNotification.java +++ b/libandroid-navigation/src/main/java/com/mapbox/services/android/navigation/v5/navigation/NavigationNotification.java @@ -111,6 +111,9 @@ void unregisterReceiver() { if (context != null) { context.unregisterReceiver(endNavigationBtnReceiver); } + if (notificationManager != null) { + notificationManager.cancel(NAVIGATION_NOTIFICATION_ID); + } } private boolean newStepName(RouteProgress routeProgress) { @@ -215,9 +218,6 @@ public void onReceive(final Context context, final Intent intent) { }; private void onEndNavigationBtnClick() { - if (notificationManager != null) { - notificationManager.cancel(NAVIGATION_NOTIFICATION_ID); - } if (mapboxNavigation != null) { mapboxNavigation.endNavigation(); } From c8b387769ee0cf2c2b6b7213bd2d608d271e149a Mon Sep 17 00:00:00 2001 From: Dan Nesfeder Date: Thu, 26 Oct 2017 17:29:44 -0700 Subject: [PATCH 5/5] Adjust API Milestone to handle new routes (#425) * Adjust API Milestone to handle new routes * Adjust voice instruction milestone to update on new route provided * Added missing doc --- .../testapp/activity/RerouteActivity.java | 21 +-- .../navigation/v5/milestone/ApiMilestone.java | 64 --------- .../milestone/VoiceInstructionMilestone.java | 123 ++++++++++++++++++ .../v5/navigation/MapboxNavigation.java | 6 +- .../v5/navigation/NavigationConstants.java | 2 +- .../v5/navigation/NavigationRoute.java | 21 +-- .../v5/navigation/NavigationService.java | 8 +- 7 files changed, 155 insertions(+), 90 deletions(-) delete mode 100644 libandroid-navigation/src/main/java/com/mapbox/services/android/navigation/v5/milestone/ApiMilestone.java create mode 100644 libandroid-navigation/src/main/java/com/mapbox/services/android/navigation/v5/milestone/VoiceInstructionMilestone.java diff --git a/app/src/main/java/com/mapbox/services/android/navigation/testapp/activity/RerouteActivity.java b/app/src/main/java/com/mapbox/services/android/navigation/testapp/activity/RerouteActivity.java index 56d51dd9e27..02b9880fb6b 100644 --- a/app/src/main/java/com/mapbox/services/android/navigation/testapp/activity/RerouteActivity.java +++ b/app/src/main/java/com/mapbox/services/android/navigation/testapp/activity/RerouteActivity.java @@ -23,6 +23,7 @@ import com.mapbox.services.Constants; import com.mapbox.services.android.navigation.testapp.R; import com.mapbox.services.android.navigation.v5.location.MockLocationEngine; +import com.mapbox.services.android.navigation.v5.milestone.MilestoneEventListener; import com.mapbox.services.android.navigation.v5.navigation.MapboxNavigation; import com.mapbox.services.android.navigation.v5.navigation.MapboxNavigationOptions; import com.mapbox.services.android.navigation.v5.navigation.NavigationEventListener; @@ -45,11 +46,12 @@ public class RerouteActivity extends AppCompatActivity implements OnMapReadyCallback, LocationEngineListener, Callback, MapboxMap.OnMapClickListener, NavigationEventListener, OffRouteListener, - ProgressChangeListener { + ProgressChangeListener, MilestoneEventListener { @BindView(R.id.mapView) MapView mapView; - + Point origin = Point.fromLngLat(-87.6900, 41.8529); + Point destination = Point.fromLngLat(-87.8921, 41.9794); private LocationLayerPlugin locationLayerPlugin; private LocationEngine locationEngine; private MapboxNavigation navigation; @@ -57,9 +59,6 @@ public class RerouteActivity extends AppCompatActivity implements OnMapReadyCall private boolean running; private Polyline polyline; - Point origin = Point.fromLngLat(-87.6900, 41.8529); - Point destination = Point.fromLngLat(-87.8921, 41.9794); - @Override protected void onCreate(Bundle savedInstanceState) { super.onCreate(savedInstanceState); @@ -89,6 +88,7 @@ public void onMapReady(MapboxMap mapboxMap) { mapboxMap.setLocationSource(locationEngine); mapboxMap.setMyLocationEnabled(true); + navigation.addMilestoneEventListener(this); navigation.setLocationEngine(locationEngine); // Acquire the navigation's route @@ -132,6 +132,11 @@ public void onProgressChange(Location location, RouteProgress routeProgress) { Timber.d("onRouteProgressChange: %s", routeProgress.currentLegProgress().stepIndex()); } + @Override + public void onMilestoneEvent(RouteProgress routeProgress, String instruction, int identifier) { + Timber.d("onMilestoneEvent - Current Instruction: " + instruction); + } + @Override public void onResponse(Call call, Response response) { if (response.body() != null) { @@ -151,13 +156,13 @@ public void onResponse(Call call, Response stepVoiceInstructions; - - public ApiMilestone(Builder builder) { - super(builder); - } - - @Override - public boolean isOccurring(RouteProgress previousRouteProgress, RouteProgress routeProgress) { - if (previousRouteProgress.currentLegProgress().stepIndex() - < routeProgress.currentLegProgress().stepIndex() || stepVoiceInstructions == null) { - stepVoiceInstructions = routeProgress.currentLegProgress().currentStep().voiceInstructions(); - } - - - for (VoiceInstructions voice : routeProgress.currentLegProgress().currentStep().voiceInstructions()) { - if (voice.distanceAlongGeometry() - >= routeProgress.currentLegProgress().currentStepProgress().distanceRemaining()) { - announcement = routeProgress.currentLegProgress().currentStep().voiceInstructions().get(0).announcement(); - stepVoiceInstructions.remove(voice); - return true; - } - } - return false; - } - - public String announcement() { - return announcement; - } - - public static final class Builder extends Milestone.Builder { - - private Trigger.Statement trigger; - - public Builder() { - super(); - } - - @Override - public Builder setTrigger(Trigger.Statement trigger) { - this.trigger = trigger; - return this; - } - - @Override - Trigger.Statement getTrigger() { - return trigger; - } - - @Override - public ApiMilestone build() { - return new ApiMilestone(this); - } - } -} \ No newline at end of file diff --git a/libandroid-navigation/src/main/java/com/mapbox/services/android/navigation/v5/milestone/VoiceInstructionMilestone.java b/libandroid-navigation/src/main/java/com/mapbox/services/android/navigation/v5/milestone/VoiceInstructionMilestone.java new file mode 100644 index 00000000000..0c83d9e9059 --- /dev/null +++ b/libandroid-navigation/src/main/java/com/mapbox/services/android/navigation/v5/milestone/VoiceInstructionMilestone.java @@ -0,0 +1,123 @@ +package com.mapbox.services.android.navigation.v5.milestone; + +import android.text.TextUtils; + +import com.mapbox.directions.v5.models.VoiceInstructions; +import com.mapbox.services.android.navigation.v5.routeprogress.RouteProgress; + +import java.util.List; + +public class VoiceInstructionMilestone extends Milestone { + + private String announcement; + private List stepVoiceInstructions; + + public VoiceInstructionMilestone(Builder builder) { + super(builder); + } + + @Override + public boolean isOccurring(RouteProgress previousRouteProgress, RouteProgress routeProgress) { + if (shouldAddInstructions(previousRouteProgress, routeProgress)) { + clearInstructionList(); + stepVoiceInstructions = routeProgress.currentLegProgress().currentStep().voiceInstructions(); + } + for (VoiceInstructions voice : stepVoiceInstructions) { + if (shouldBeVoiced(routeProgress, voice)) { + announcement = voice.announcement(); + stepVoiceInstructions.remove(voice); + return true; + } + } + return false; + } + + public String announcement() { + return announcement; + } + + /** + * Check if a new set of step instructions should be set. + * + * @param previousRouteProgress most recent progress before the current progress + * @param routeProgress the current route progress + * @return true if new instructions should be added to the list, false if not + */ + private boolean shouldAddInstructions(RouteProgress previousRouteProgress, RouteProgress routeProgress) { + return newStep(previousRouteProgress, routeProgress) + || newRoute(previousRouteProgress, routeProgress) + || stepVoiceInstructions == null; + } + + /** + * Called when adding new instructions to the list. + *

+ * Make sure old announcements are not called (can happen in reroute scenarios). + */ + private void clearInstructionList() { + if (stepVoiceInstructions != null && !stepVoiceInstructions.isEmpty()) { + stepVoiceInstructions.clear(); + } + } + + /** + * Used to check for a new route. Route geometries will be the same only on the first + * update. This is because the previousRouteProgress is reset and the current routeProgress is generated + * from the previous. + * + * @param previousRouteProgress most recent progress before the current progress + * @param routeProgress the current route progress + * @return true if there's a new route, false if not + */ + private boolean newRoute(RouteProgress previousRouteProgress, RouteProgress routeProgress) { + return TextUtils.equals(previousRouteProgress.directionsRoute().geometry(), + routeProgress.directionsRoute().geometry()); + } + + /** + * @param previousRouteProgress most recent progress before the current progress + * @param routeProgress the current route progress + * @return true if on a new step, false if not + */ + private boolean newStep(RouteProgress previousRouteProgress, RouteProgress routeProgress) { + return previousRouteProgress.currentLegProgress().stepIndex() + < routeProgress.currentLegProgress().stepIndex(); + } + + /** + * Uses the current step distance remaining to check against voice instruction distance. + * + * @param routeProgress the current route progress + * @param voice a given voice instruction from the list of step instructions + * @return true if time to voice the announcement, false if not + */ + private boolean shouldBeVoiced(RouteProgress routeProgress, VoiceInstructions voice) { + return voice.distanceAlongGeometry() + >= routeProgress.currentLegProgress().currentStepProgress().distanceRemaining(); + } + + public static final class Builder extends Milestone.Builder { + + private Trigger.Statement trigger; + + public Builder() { + super(); + } + + @Override + Trigger.Statement getTrigger() { + return trigger; + } + + @Override + public Builder setTrigger(Trigger.Statement trigger) { + this.trigger = trigger; + return this; + } + + @Override + public VoiceInstructionMilestone build() { + return new VoiceInstructionMilestone(this); + } + } +} \ No newline at end of file diff --git a/libandroid-navigation/src/main/java/com/mapbox/services/android/navigation/v5/navigation/MapboxNavigation.java b/libandroid-navigation/src/main/java/com/mapbox/services/android/navigation/v5/navigation/MapboxNavigation.java index 27c76701e68..f177558e9bd 100644 --- a/libandroid-navigation/src/main/java/com/mapbox/services/android/navigation/v5/navigation/MapboxNavigation.java +++ b/libandroid-navigation/src/main/java/com/mapbox/services/android/navigation/v5/navigation/MapboxNavigation.java @@ -14,7 +14,7 @@ import com.mapbox.services.android.location.LostLocationEngine; import com.mapbox.services.android.navigation.BuildConfig; import com.mapbox.services.android.navigation.v5.exception.NavigationException; -import com.mapbox.services.android.navigation.v5.milestone.ApiMilestone; +import com.mapbox.services.android.navigation.v5.milestone.VoiceInstructionMilestone; import com.mapbox.services.android.navigation.v5.milestone.Milestone; import com.mapbox.services.android.navigation.v5.milestone.MilestoneEventListener; import com.mapbox.services.android.navigation.v5.location.MockLocationEngine; @@ -42,7 +42,7 @@ import retrofit2.Callback; import timber.log.Timber; -import static com.mapbox.services.android.navigation.v5.navigation.NavigationConstants.DEFAULT_MILESTONE_IDENTIFIER; +import static com.mapbox.services.android.navigation.v5.navigation.NavigationConstants.VOICE_INSTRUCTION_MILESTONE_ID; /** * A MapboxNavigation class for interacting with and customizing a navigation session. @@ -133,7 +133,7 @@ private void initialize(boolean debugLoggingEnabled) { // Create and add default milestones if enabled. milestones = new ArrayList<>(); if (options.defaultMilestonesEnabled()) { - addMilestone(new ApiMilestone.Builder().setIdentifier(DEFAULT_MILESTONE_IDENTIFIER).build()); + addMilestone(new VoiceInstructionMilestone.Builder().setIdentifier(VOICE_INSTRUCTION_MILESTONE_ID).build()); } initializeDefaultLocationEngine(); diff --git a/libandroid-navigation/src/main/java/com/mapbox/services/android/navigation/v5/navigation/NavigationConstants.java b/libandroid-navigation/src/main/java/com/mapbox/services/android/navigation/v5/navigation/NavigationConstants.java index bb4b6e2164f..54472ccc17c 100644 --- a/libandroid-navigation/src/main/java/com/mapbox/services/android/navigation/v5/navigation/NavigationConstants.java +++ b/libandroid-navigation/src/main/java/com/mapbox/services/android/navigation/v5/navigation/NavigationConstants.java @@ -20,7 +20,7 @@ private NavigationConstants() { * * @since 0.7.0 */ - public static final int DEFAULT_MILESTONE_IDENTIFIER = 1; + public static final int VOICE_INSTRUCTION_MILESTONE_ID = 1; /** * Random integer value used for identifying the navigation notification. diff --git a/libandroid-navigation/src/main/java/com/mapbox/services/android/navigation/v5/navigation/NavigationRoute.java b/libandroid-navigation/src/main/java/com/mapbox/services/android/navigation/v5/navigation/NavigationRoute.java index 7dd6df80fcc..292451a61a2 100644 --- a/libandroid-navigation/src/main/java/com/mapbox/services/android/navigation/v5/navigation/NavigationRoute.java +++ b/libandroid-navigation/src/main/java/com/mapbox/services/android/navigation/v5/navigation/NavigationRoute.java @@ -36,25 +36,25 @@ public final class NavigationRoute { private final MapboxDirections mapboxDirections; /** - * Build a new {@link NavigationRoute} object with the proper navigation parameters already setup. + * Private constructor used for the {@link Builder#build()} method. * - * @return a {@link Builder} object for creating this object + * @param mapboxDirections a new instance of a {@link MapboxDirections} class * @since 0.5.0 */ - public static Builder builder() { - return new Builder() - .annotations(DirectionsCriteria.ANNOTATION_CONGESTION) - .profile(DirectionsCriteria.PROFILE_DRIVING_TRAFFIC); + private NavigationRoute(MapboxDirections mapboxDirections) { + this.mapboxDirections = mapboxDirections; } /** - * Private constructor used for the {@link Builder#build()} method. + * Build a new {@link NavigationRoute} object with the proper navigation parameters already setup. * - * @param mapboxDirections a new instance of a {@link MapboxDirections} class + * @return a {@link Builder} object for creating this object * @since 0.5.0 */ - private NavigationRoute(MapboxDirections mapboxDirections) { - this.mapboxDirections = mapboxDirections; + public static Builder builder() { + return new Builder() + .annotations(DirectionsCriteria.ANNOTATION_CONGESTION) + .profile(DirectionsCriteria.PROFILE_DRIVING_TRAFFIC); } /** @@ -215,6 +215,7 @@ public Builder destination(@NonNull Point destination, @Nullable Double angle, */ public Builder addWaypoint(@NonNull Point waypoint) { directionsBuilder.addWaypoint(waypoint); + directionsBuilder.addBearing(null, null); return this; } diff --git a/libandroid-navigation/src/main/java/com/mapbox/services/android/navigation/v5/navigation/NavigationService.java b/libandroid-navigation/src/main/java/com/mapbox/services/android/navigation/v5/navigation/NavigationService.java index 7b4bd570d6d..428187a0002 100644 --- a/libandroid-navigation/src/main/java/com/mapbox/services/android/navigation/v5/navigation/NavigationService.java +++ b/libandroid-navigation/src/main/java/com/mapbox/services/android/navigation/v5/navigation/NavigationService.java @@ -13,7 +13,7 @@ import com.mapbox.directions.v5.models.DirectionsRoute; import com.mapbox.geojson.Point; import com.mapbox.services.android.navigation.R; -import com.mapbox.services.android.navigation.v5.milestone.ApiMilestone; +import com.mapbox.services.android.navigation.v5.milestone.VoiceInstructionMilestone; import com.mapbox.services.android.navigation.v5.milestone.Milestone; import com.mapbox.services.android.navigation.v5.routeprogress.RouteProgress; import com.mapbox.services.android.navigation.v5.utils.RingBuffer; @@ -32,7 +32,7 @@ import timber.log.Timber; -import static com.mapbox.services.android.navigation.v5.navigation.NavigationConstants.DEFAULT_MILESTONE_IDENTIFIER; +import static com.mapbox.services.android.navigation.v5.navigation.NavigationConstants.VOICE_INSTRUCTION_MILESTONE_ID; import static com.mapbox.services.android.navigation.v5.navigation.NavigationConstants.NAVIGATION_NOTIFICATION_ID; import static com.mapbox.services.android.navigation.v5.navigation.NavigationHelper.buildInstructionString; @@ -250,8 +250,8 @@ public void onNewRouteProgress(Location location, RouteProgress routeProgress) { public void onMilestoneTrigger(List triggeredMilestones, RouteProgress routeProgress) { for (Milestone milestone : triggeredMilestones) { String instruction = buildInstructionString(routeProgress, milestone); - if (milestone.getIdentifier() == DEFAULT_MILESTONE_IDENTIFIER) { - instruction = ((ApiMilestone) milestone).announcement(); + if (milestone.getIdentifier() == VOICE_INSTRUCTION_MILESTONE_ID) { + instruction = ((VoiceInstructionMilestone) milestone).announcement(); } mapboxNavigation.getEventDispatcher().onMilestoneEvent( routeProgress, instruction, milestone.getIdentifier());