Merge "Handle GnssMeasurement registration according to HAL versions" into udc-dev
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 77cd673..a081dff 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 @@
// 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 6c4c829..041f11d 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 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.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 @@
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 @@
}
@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 3677641..b57e451 100644
--- a/services/core/jni/gnss/GnssConfiguration.cpp
+++ b/services/core/jni/gnss/GnssConfiguration.cpp
@@ -67,7 +67,7 @@
: 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 0000000..fd9dfe8
--- /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();
+ }
+}