From e97179d846c3942e3455afb0b413d956093560e7 Mon Sep 17 00:00:00 2001 From: Soonil Nagarkar Date: Tue, 2 Feb 2021 14:41:00 -0800 Subject: Update batching APIs After API council feedback, use List in public APIs rather than LocationResult. Bug: 173712888 Test: atest CtsLocationFineTestCases Change-Id: I02caa1f9c7164b52ef55c71ca14fb2bd7e311a85 --- .../java/android/location/ILocationListener.aidl | 4 +- .../java/android/location/LocationListener.java | 16 ++-- .../java/android/location/LocationManager.java | 23 ++++-- .../java/android/location/LocationRequest.java | 2 +- location/java/android/location/LocationResult.java | 64 ++++++++++------ .../provider/ILocationProviderManager.aidl | 5 +- .../location/provider/LocationProviderBase.java | 88 ++++++++++++++++------ 7 files changed, 134 insertions(+), 68 deletions(-) (limited to 'location/java') diff --git a/location/java/android/location/ILocationListener.aidl b/location/java/android/location/ILocationListener.aidl index ce92661cb87c..40d8615b95f7 100644 --- a/location/java/android/location/ILocationListener.aidl +++ b/location/java/android/location/ILocationListener.aidl @@ -16,7 +16,7 @@ package android.location; -import android.location.LocationResult; +import android.location.Location; import android.os.IRemoteCallback; /** @@ -24,7 +24,7 @@ import android.os.IRemoteCallback; */ oneway interface ILocationListener { - void onLocationChanged(in LocationResult locationResult, in @nullable IRemoteCallback onCompleteCallback); + void onLocationChanged(in List locations, in @nullable IRemoteCallback onCompleteCallback); void onProviderEnabledChanged(String provider, boolean enabled); void onFlushComplete(int requestCode); } diff --git a/location/java/android/location/LocationListener.java b/location/java/android/location/LocationListener.java index 523117b39762..35a40910e373 100644 --- a/location/java/android/location/LocationListener.java +++ b/location/java/android/location/LocationListener.java @@ -19,6 +19,7 @@ package android.location; import android.annotation.NonNull; import android.os.Bundle; +import java.util.List; import java.util.concurrent.Executor; /** @@ -48,15 +49,18 @@ public interface LocationListener { /** * Called when the location has changed and locations are being delivered in batches. The * default implementation calls through to ({@link #onLocationChanged(Location)} with all - * locations in the batch, from earliest to latest. + * locations in the batch. The list of locations is always guaranteed to be non-empty, and is + * always guaranteed to be ordered from earliest location to latest location (so that the + * earliest location in the batch is at index 0 in the list, and the latest location in the + * batch is at index size-1 in the list). * * @see LocationRequest#getMaxUpdateDelayMillis() - * @param locationResult the location result list + * @param locations the location list */ - default void onLocationChanged(@NonNull LocationResult locationResult) { - final int size = locationResult.size(); - for (int i = 0; i < size; ++i) { - onLocationChanged(locationResult.get(i)); + default void onLocationChanged(@NonNull List locations) { + final int size = locations.size(); + for (int i = 0; i < size; i++) { + onLocationChanged(locations.get(i)); } } diff --git a/location/java/android/location/LocationManager.java b/location/java/android/location/LocationManager.java index fdb044d0dcf6..add3d31e9435 100644 --- a/location/java/android/location/LocationManager.java +++ b/location/java/android/location/LocationManager.java @@ -231,19 +231,26 @@ public class LocationManager { /** * Key used for an extra holding a {@link Location} value when a location change is sent using - * a PendingIntent. + * a PendingIntent. If the location change includes a list of batched locations via + * {@link #KEY_LOCATIONS} then this key will still be present, and will hold the last location + * in the batch. Use {@link Intent#getParcelableExtra(String)} to retrieve the location. * * @see #requestLocationUpdates(String, LocationRequest, PendingIntent) */ public static final String KEY_LOCATION_CHANGED = "location"; /** - * Key used for an extra holding a {@link LocationResult} value when a location change is sent - * using a PendingIntent. + * Key used for an extra holding a array of {@link Location}s when a location change is sent + * using a PendingIntent. This key will only be present if the location change includes + * multiple (ie, batched) locations, otherwise only {@link #KEY_LOCATION_CHANGED} will be + * present. Use {@link Intent#getParcelableArrayExtra(String)} to retrieve the locations. + * + *

The array of locations will never be empty, and will ordered from earliest location to + * latest location, the same as with {@link LocationListener#onLocationChanged(List)}. * * @see #requestLocationUpdates(String, LocationRequest, PendingIntent) */ - public static final String KEY_LOCATION_RESULT = "locationResult"; + public static final String KEY_LOCATIONS = "locations"; /** * Key used for an extra holding an integer request code when location flush completion is sent @@ -2969,12 +2976,12 @@ public class LocationManager { } @Override - public void onLocationChanged(LocationResult locationResult, + public void onLocationChanged(List locations, @Nullable IRemoteCallback onCompleteCallback) { executeSafely(mExecutor, () -> mListener, new ListenerOperation() { @Override public void operate(LocationListener listener) { - listener.onLocationChanged(locationResult); + listener.onLocationChanged(locations); } @Override @@ -3321,8 +3328,8 @@ public class LocationManager { } @Override - public void onLocationChanged(@NonNull LocationResult locationResult) { - mCallback.onLocationBatch(locationResult.asList()); + public void onLocationChanged(@NonNull List locations) { + mCallback.onLocationBatch(locations); } } diff --git a/location/java/android/location/LocationRequest.java b/location/java/android/location/LocationRequest.java index 323e740ee8e3..cb56ee5318ee 100644 --- a/location/java/android/location/LocationRequest.java +++ b/location/java/android/location/LocationRequest.java @@ -605,7 +605,7 @@ public final class LocationRequest implements Parcelable { * When available, batching can provide substantial power savings to the device, and clients are * encouraged to take advantage where appropriate for the use case. * - * @see LocationListener#onLocationChanged(LocationResult) + * @see LocationListener#onLocationChanged(java.util.List) * @return the maximum time by which a location update may be delayed */ public @IntRange(from = 0) long getMaxUpdateDelayMillis() { diff --git a/location/java/android/location/LocationResult.java b/location/java/android/location/LocationResult.java index 79a000c23484..8423000b6276 100644 --- a/location/java/android/location/LocationResult.java +++ b/location/java/android/location/LocationResult.java @@ -19,7 +19,6 @@ package android.location; import android.annotation.IntRange; import android.annotation.NonNull; import android.annotation.Nullable; -import android.annotation.SystemApi; import android.os.Parcel; import android.os.Parcelable; @@ -34,25 +33,31 @@ import java.util.function.Predicate; /** * A location result representing a list of locations, ordered from earliest to latest. + * + * @hide */ public final class LocationResult implements Parcelable { /** - * Creates a new LocationResult from the given location. + * Creates a new LocationResult from the given locations, making a copy of each location. + * Locations must be ordered in the same order they were derived (earliest to latest). */ - public static @NonNull LocationResult create(@NonNull Location location) { - ArrayList locations = new ArrayList<>(1); - locations.add(new Location(Objects.requireNonNull(location))); - return new LocationResult(locations); + public static @NonNull LocationResult create(@NonNull List locations) { + Preconditions.checkArgument(!locations.isEmpty()); + ArrayList locationsCopy = new ArrayList<>(locations.size()); + for (Location location : locations) { + locationsCopy.add(new Location(Objects.requireNonNull(location))); + } + return new LocationResult(locationsCopy); } /** - * Creates a new LocationResult from the given locations. Locations must be ordered in the same - * order they were derived (earliest to latest). + * Creates a new LocationResult from the given locations, making a copy of each location. + * Locations must be ordered in the same order they were derived (earliest to latest). */ - public static @NonNull LocationResult create(@NonNull List locations) { - Preconditions.checkArgument(!locations.isEmpty()); - ArrayList locationsCopy = new ArrayList<>(locations.size()); + public static @NonNull LocationResult create(@NonNull Location... locations) { + Preconditions.checkArgument(locations.length > 0); + ArrayList locationsCopy = new ArrayList<>(locations.length); for (Location location : locations) { locationsCopy.add(new Location(Objects.requireNonNull(location))); } @@ -60,16 +65,27 @@ public final class LocationResult implements Parcelable { } /** - * Creates a new LocationResult that takes ownership of the given location without copying it. - * Callers must ensure the given location is never mutated after this method is called. - * - * @hide + * Creates a new LocationResult that takes ownership of the given locations without copying + * them. Callers must ensure the given locations are never mutated after this method is called. + * Locations must be ordered in the same order they were derived (earliest to latest). + */ + public static @NonNull LocationResult wrap(@NonNull List locations) { + Preconditions.checkArgument(!locations.isEmpty()); + return new LocationResult(new ArrayList<>(locations)); + } + + /** + * Creates a new LocationResult that takes ownership of the given locations without copying + * them. Callers must ensure the given locations are never mutated after this method is called. + * Locations must be ordered in the same order they were derived (earliest to latest). */ - @SystemApi - public static @NonNull LocationResult wrap(@NonNull Location location) { - ArrayList locations = new ArrayList<>(1); - locations.add(Objects.requireNonNull(location)); - return new LocationResult(locations); + public static @NonNull LocationResult wrap(@NonNull Location... locations) { + Preconditions.checkArgument(locations.length > 0); + ArrayList newLocations = new ArrayList<>(locations.length); + for (Location location : locations) { + newLocations.add(Objects.requireNonNull(location)); + } + return new LocationResult(newLocations); } private final ArrayList mLocations; @@ -112,7 +128,7 @@ public final class LocationResult implements Parcelable { } /** - * Returns the numer of locations in this location result. + * Returns the number of locations in this location result. */ public @IntRange(from = 1) int size() { return mLocations.size(); @@ -139,9 +155,9 @@ public final class LocationResult implements Parcelable { * @hide */ public @NonNull LocationResult deepCopy() { - ArrayList copy = new ArrayList<>(mLocations.size()); final int size = mLocations.size(); - for (int i = 0; i < size; ++i) { + ArrayList copy = new ArrayList<>(size); + for (int i = 0; i < size; i++) { copy.add(new Location(mLocations.get(i))); } return new LocationResult(copy); @@ -164,7 +180,7 @@ public final class LocationResult implements Parcelable { * Returns a LocationResult with only locations that pass the given predicate. This * implementation will avoid allocations when no locations are filtered out. The predicate is * guaranteed to be invoked once per location, in order from earliest to latest. If all - * locations are filtered out a null value is returned instead of an empty LocationResult. + * locations are filtered out a null value is returned. * * @hide */ diff --git a/location/java/android/location/provider/ILocationProviderManager.aidl b/location/java/android/location/provider/ILocationProviderManager.aidl index e3f51d9a23e1..50ed046f09cd 100644 --- a/location/java/android/location/provider/ILocationProviderManager.aidl +++ b/location/java/android/location/provider/ILocationProviderManager.aidl @@ -16,7 +16,7 @@ package android.location.provider; -import android.location.LocationResult; +import android.location.Location; import android.location.provider.ProviderProperties; /** @@ -28,6 +28,7 @@ interface ILocationProviderManager { void onSetAllowed(boolean allowed); void onSetProperties(in ProviderProperties properties); - void onReportLocation(in LocationResult locationResult); + void onReportLocation(in Location location); + void onReportLocations(in List locations); void onFlushComplete(); } diff --git a/location/java/android/location/provider/LocationProviderBase.java b/location/java/android/location/provider/LocationProviderBase.java index 8f455cd7df07..ae6395d5d12d 100644 --- a/location/java/android/location/provider/LocationProviderBase.java +++ b/location/java/android/location/provider/LocationProviderBase.java @@ -25,12 +25,13 @@ import android.annotation.SystemApi; import android.content.Context; import android.content.Intent; import android.location.Location; -import android.location.LocationResult; import android.os.Bundle; import android.os.IBinder; import android.os.RemoteException; import android.util.Log; +import java.util.ArrayList; +import java.util.List; import java.util.Objects; /** @@ -200,35 +201,29 @@ public abstract class LocationProviderBase { * Reports a new location from this provider. */ public void reportLocation(@NonNull Location location) { - reportLocation(LocationResult.create(location)); + ILocationProviderManager manager = mManager; + if (manager != null) { + try { + manager.onReportLocation(stripExtras(location)); + } catch (RemoteException e) { + throw e.rethrowFromSystemServer(); + } catch (RuntimeException e) { + Log.w(mTag, e); + } + } } /** - * Reports a new location result from this provider. - * - *

May only be used from Android S onwards. + * Reports a new batch of locations from this provider. Locations must be ordered in the list + * from earliest first to latest last. */ - public void reportLocation(@NonNull LocationResult locationResult) { + public void reportLocations(@NonNull List locations) { ILocationProviderManager manager = mManager; if (manager != null) { - locationResult = locationResult.map(location -> { - // remove deprecated extras to save on serialization costs - Bundle extras = location.getExtras(); - if (extras != null && (extras.containsKey(EXTRA_NO_GPS_LOCATION) - || extras.containsKey("coarseLocation"))) { - location = new Location(location); - extras = location.getExtras(); - extras.remove(EXTRA_NO_GPS_LOCATION); - extras.remove("coarseLocation"); - if (extras.isEmpty()) { - location.setExtras(null); - } - } - return location; - }); + try { - manager.onReportLocation(locationResult); + manager.onReportLocations(stripExtras(locations)); } catch (RemoteException e) { throw e.rethrowFromSystemServer(); } catch (RuntimeException e) { @@ -246,9 +241,9 @@ public abstract class LocationProviderBase { /** * Requests a flush of any pending batched locations. The callback must always be invoked once - * per invocation, and should be invoked after {@link #reportLocation(LocationResult)} has been - * invoked with any flushed locations. The callback may be invoked immediately if no locations - * are flushed. + * per invocation, and should be invoked after {@link #reportLocation(Location)} or + * {@link #reportLocations(List)} has been invoked with any flushed locations. The callback may + * be invoked immediately if no locations are flushed. */ public abstract void onFlush(@NonNull OnFlushCompleteCallback callback); @@ -259,6 +254,49 @@ public abstract class LocationProviderBase { @SuppressLint("NullableCollection") @Nullable Bundle extras); + private static Location stripExtras(Location location) { + Bundle extras = location.getExtras(); + if (extras != null && (extras.containsKey(EXTRA_NO_GPS_LOCATION) + || extras.containsKey("indoorProbability") + || extras.containsKey("coarseLocation"))) { + location = new Location(location); + extras = location.getExtras(); + extras.remove(EXTRA_NO_GPS_LOCATION); + extras.remove("indoorProbability"); + extras.remove("coarseLocation"); + if (extras.isEmpty()) { + location.setExtras(null); + } + } + return location; + } + + private static List stripExtras(List locations) { + List mapped = locations; + final int size = locations.size(); + int i = 0; + for (Location location : locations) { + Location newLocation = stripExtras(location); + if (mapped != locations) { + mapped.add(newLocation); + } else if (newLocation != location) { + mapped = new ArrayList<>(size); + int j = 0; + for (Location copiedLocation : locations) { + if (j >= i) { + break; + } + mapped.add(copiedLocation); + j++; + } + mapped.add(newLocation); + } + i++; + } + + return mapped; + } + private final class Service extends ILocationProvider.Stub { Service() {} -- cgit v1.2.3-59-g8ed1b