diff options
4 files changed, 215 insertions, 11 deletions
diff --git a/services/core/java/com/android/server/location/gnss/GnssConfiguration.java b/services/core/java/com/android/server/location/gnss/GnssConfiguration.java index 77cd67304729..a081dff9e62d 100644 --- a/services/core/java/com/android/server/location/gnss/GnssConfiguration.java +++ b/services/core/java/com/android/server/location/gnss/GnssConfiguration.java @@ -92,6 +92,8 @@ public class GnssConfiguration { // Represents an HAL interface version. Instances of this class are created in the JNI layer // and returned through native methods. static class HalInterfaceVersion { + // mMajor being this value denotes AIDL HAL. In this case, mMinor denotes the AIDL version. + static final int AIDL_INTERFACE = 3; final int mMajor; final int mMinor; diff --git a/services/core/java/com/android/server/location/gnss/GnssMeasurementsProvider.java b/services/core/java/com/android/server/location/gnss/GnssMeasurementsProvider.java index 6c4c829b051d..041f11d972fe 100644 --- a/services/core/java/com/android/server/location/gnss/GnssMeasurementsProvider.java +++ b/services/core/java/com/android/server/location/gnss/GnssMeasurementsProvider.java @@ -21,6 +21,7 @@ import static android.app.AppOpsManager.OP_MONITOR_HIGH_POWER_LOCATION; import static com.android.server.location.gnss.GnssManagerService.D; import static com.android.server.location.gnss.GnssManagerService.TAG; +import android.annotation.NonNull; import android.annotation.Nullable; import android.app.AppOpsManager; import android.location.GnssMeasurementRequest; @@ -31,6 +32,7 @@ import android.os.IBinder; import android.stats.location.LocationStatsEnums; import android.util.Log; +import com.android.server.location.gnss.GnssConfiguration.HalInterfaceVersion; import com.android.server.location.gnss.hal.GnssNative; import com.android.server.location.injector.AppOpsHelper; import com.android.server.location.injector.Injector; @@ -115,16 +117,6 @@ public final class GnssMeasurementsProvider extends if (request.getIntervalMillis() == GnssMeasurementRequest.PASSIVE_INTERVAL) { return true; } - // The HAL doc does not specify if consecutive start() calls will be allowed. - // Some vendors may ignore the 2nd start() call if stop() is not called. - // Thus, here we always call stop() before calling start() to avoid being ignored. - if (mGnssNative.stopMeasurementCollection()) { - if (D) { - Log.d(TAG, "stopping gnss measurements"); - } - } else { - Log.e(TAG, "error stopping gnss measurements"); - } if (mGnssNative.startMeasurementCollection(request.isFullTracking(), request.isCorrelationVectorOutputsEnabled(), request.getIntervalMillis())) { @@ -139,6 +131,28 @@ public final class GnssMeasurementsProvider extends } @Override + protected boolean reregisterWithService(GnssMeasurementRequest old, + GnssMeasurementRequest request, + @NonNull Collection<GnssListenerRegistration> registrations) { + if (request.getIntervalMillis() == GnssMeasurementRequest.PASSIVE_INTERVAL) { + unregisterWithService(); + return true; + } + HalInterfaceVersion halInterfaceVersion = + mGnssNative.getConfiguration().getHalInterfaceVersion(); + boolean aidlV3Plus = halInterfaceVersion.mMajor == HalInterfaceVersion.AIDL_INTERFACE + && halInterfaceVersion.mMinor >= 3; + if (!aidlV3Plus) { + // The HAL doc does not specify if consecutive start() calls will be allowed. + // Some vendors may ignore the 2nd start() call if stop() is not called. + // Thus, here we always call stop() before calling start() to avoid being ignored. + // AIDL v3+ is free from this issue. + unregisterWithService(); + } + return registerWithService(request, registrations); + } + + @Override protected void unregisterWithService() { if (mGnssNative.stopMeasurementCollection()) { if (D) { diff --git a/services/core/jni/gnss/GnssConfiguration.cpp b/services/core/jni/gnss/GnssConfiguration.cpp index 3677641127f3..b57e451264a4 100644 --- a/services/core/jni/gnss/GnssConfiguration.cpp +++ b/services/core/jni/gnss/GnssConfiguration.cpp @@ -67,7 +67,7 @@ GnssConfiguration::GnssConfiguration(const sp<IGnssConfiguration>& iGnssConfigur : mIGnssConfiguration(iGnssConfiguration) {} jobject GnssConfiguration::getVersion(JNIEnv* env) { - return createHalInterfaceVersionJavaObject(env, 3, 0); + return createHalInterfaceVersionJavaObject(env, 3, mIGnssConfiguration->getInterfaceVersion()); } jboolean GnssConfiguration::setEmergencySuplPdn(jint enable) { diff --git a/services/tests/mockingservicestests/src/com/android/server/location/gnss/GnssMeasurementsProviderTest.java b/services/tests/mockingservicestests/src/com/android/server/location/gnss/GnssMeasurementsProviderTest.java new file mode 100644 index 000000000000..fd9dfe869d52 --- /dev/null +++ b/services/tests/mockingservicestests/src/com/android/server/location/gnss/GnssMeasurementsProviderTest.java @@ -0,0 +1,188 @@ +/* + * Copyright (C) 2023 The Android Open Source Project + * + * Licensed under the Apache License, Version 2.0 (the "License"); + * you may not use this file except in compliance with the License. + * You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, software + * distributed under the License is distributed on an "AS IS" BASIS, + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + * See the License for the specific language governing permissions and + * limitations under the License. + */ + +package com.android.server.location.gnss; + +import static org.mockito.ArgumentMatchers.anyBoolean; +import static org.mockito.ArgumentMatchers.anyInt; +import static org.mockito.Mockito.doReturn; +import static org.mockito.Mockito.eq; +import static org.mockito.Mockito.never; +import static org.mockito.Mockito.spy; +import static org.mockito.Mockito.times; +import static org.mockito.Mockito.verify; + +import android.content.Context; +import android.location.GnssMeasurementRequest; +import android.location.IGnssMeasurementsListener; +import android.location.LocationManager; +import android.location.LocationManagerInternal; +import android.location.util.identity.CallerIdentity; +import android.os.IBinder; +import android.platform.test.annotations.Presubmit; + +import androidx.test.filters.SmallTest; +import androidx.test.runner.AndroidJUnit4; + +import com.android.server.LocalServices; +import com.android.server.location.gnss.hal.FakeGnssHal; +import com.android.server.location.gnss.hal.GnssNative; +import com.android.server.location.injector.FakeUserInfoHelper; +import com.android.server.location.injector.Injector; +import com.android.server.location.injector.TestInjector; + +import org.junit.After; +import org.junit.Before; +import org.junit.Test; +import org.junit.runner.RunWith; +import org.mockito.Mock; +import org.mockito.MockitoAnnotations; + +import java.util.Objects; + +@Presubmit +@SmallTest +@RunWith(AndroidJUnit4.class) +public class GnssMeasurementsProviderTest { + private static final int CURRENT_USER = FakeUserInfoHelper.DEFAULT_USERID; + private static final CallerIdentity IDENTITY = CallerIdentity.forTest(CURRENT_USER, 1000, + "mypackage", "attribution", "listener"); + private static final GnssConfiguration.HalInterfaceVersion HIDL_V2_1 = + new GnssConfiguration.HalInterfaceVersion( + 2, 1); + private static final GnssConfiguration.HalInterfaceVersion AIDL_V3 = + new GnssConfiguration.HalInterfaceVersion( + GnssConfiguration.HalInterfaceVersion.AIDL_INTERFACE, 3); + private static final GnssMeasurementRequest ACTIVE_REQUEST = + new GnssMeasurementRequest.Builder().build(); + private static final GnssMeasurementRequest PASSIVE_REQUEST = + new GnssMeasurementRequest.Builder().setIntervalMillis( + GnssMeasurementRequest.PASSIVE_INTERVAL).build(); + private @Mock Context mContext; + private @Mock LocationManagerInternal mInternal; + private @Mock GnssConfiguration mMockConfiguration; + private @Mock GnssNative.GeofenceCallbacks mGeofenceCallbacks; + private @Mock IGnssMeasurementsListener mListener1; + private @Mock IGnssMeasurementsListener mListener2; + private @Mock IBinder mBinder1; + private @Mock IBinder mBinder2; + + private GnssNative mGnssNative; + + private GnssMeasurementsProvider mTestProvider; + + @Before + public void setUp() { + MockitoAnnotations.initMocks(this); + + doReturn(mBinder1).when(mListener1).asBinder(); + doReturn(mBinder2).when(mListener2).asBinder(); + doReturn(true).when(mInternal).isProviderEnabledForUser(eq(LocationManager.GPS_PROVIDER), + anyInt()); + LocalServices.addService(LocationManagerInternal.class, mInternal); + FakeGnssHal fakeGnssHal = new FakeGnssHal(); + GnssNative.setGnssHalForTest(fakeGnssHal); + Injector injector = new TestInjector(mContext); + mGnssNative = spy(Objects.requireNonNull( + GnssNative.create(injector, mMockConfiguration))); + mGnssNative.setGeofenceCallbacks(mGeofenceCallbacks); + mTestProvider = new GnssMeasurementsProvider(injector, mGnssNative); + mGnssNative.register(); + } + + @After + public void tearDown() { + LocalServices.removeServiceForTest(LocationManagerInternal.class); + } + + @Test + public void testAddListener_active() { + // add the active request + mTestProvider.addListener(ACTIVE_REQUEST, IDENTITY, mListener1); + verify(mGnssNative, times(1)).startMeasurementCollection( + eq(ACTIVE_REQUEST.isFullTracking()), + eq(ACTIVE_REQUEST.isCorrelationVectorOutputsEnabled()), + eq(ACTIVE_REQUEST.getIntervalMillis())); + + // remove the active request + mTestProvider.removeListener(mListener1); + verify(mGnssNative, times(1)).stopMeasurementCollection(); + } + + @Test + public void testAddListener_passive() { + // add the passive request + mTestProvider.addListener(PASSIVE_REQUEST, IDENTITY, mListener1); + verify(mGnssNative, never()).startMeasurementCollection(anyBoolean(), anyBoolean(), + anyInt()); + + // remove the passive request + mTestProvider.removeListener(mListener1); + verify(mGnssNative, times(1)).stopMeasurementCollection(); + } + + @Test + public void testReregister_aidlV3Plus() { + doReturn(AIDL_V3).when(mMockConfiguration).getHalInterfaceVersion(); + + // add the passive request + mTestProvider.addListener(PASSIVE_REQUEST, IDENTITY, mListener1); + verify(mGnssNative, never()).startMeasurementCollection(anyBoolean(), anyBoolean(), + anyInt()); + + // add the active request, reregister with the active request + mTestProvider.addListener(ACTIVE_REQUEST, IDENTITY, mListener2); + verify(mGnssNative, never()).stopMeasurementCollection(); + verify(mGnssNative, times(1)).startMeasurementCollection( + eq(ACTIVE_REQUEST.isFullTracking()), + eq(ACTIVE_REQUEST.isCorrelationVectorOutputsEnabled()), + eq(ACTIVE_REQUEST.getIntervalMillis())); + + // remove the active request, reregister with the passive request + mTestProvider.removeListener(mListener2); + verify(mGnssNative, times(1)).stopMeasurementCollection(); + + // remove the passive request + mTestProvider.removeListener(mListener1); + verify(mGnssNative, times(2)).stopMeasurementCollection(); + } + + @Test + public void testReregister_preAidlV3() { + doReturn(HIDL_V2_1).when(mMockConfiguration).getHalInterfaceVersion(); + + // add the passive request + mTestProvider.addListener(PASSIVE_REQUEST, IDENTITY, mListener1); + verify(mGnssNative, never()).startMeasurementCollection(anyBoolean(), anyBoolean(), + anyInt()); + + // add the active request, reregister with the active request + mTestProvider.addListener(ACTIVE_REQUEST, IDENTITY, mListener2); + verify(mGnssNative, times(1)).stopMeasurementCollection(); + verify(mGnssNative, times(1)).startMeasurementCollection( + eq(ACTIVE_REQUEST.isFullTracking()), + eq(ACTIVE_REQUEST.isCorrelationVectorOutputsEnabled()), + eq(ACTIVE_REQUEST.getIntervalMillis())); + + // remove the active request, reregister with the passive request + mTestProvider.removeListener(mListener2); + verify(mGnssNative, times(2)).stopMeasurementCollection(); + + // remove the passive request + mTestProvider.removeListener(mListener1); + verify(mGnssNative, times(3)).stopMeasurementCollection(); + } +} |