Merge "Handle GnssMeasurement registration according to HAL versions" into udc-dev am: 9a5ff71251

Original change: https://googleplex-android-review.googlesource.com/c/platform/frameworks/base/+/22100046

Change-Id: I5a0d29b765df43595ce9559bd72527367ba32d69
Signed-off-by: Automerger Merge Worker <android-build-automerger-merge-worker@system.gserviceaccount.com>

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;
 

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())) {
@@ -138,6 +130,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()) {

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) {

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();
+    }
+}