Note: The naming of this capability and the behavior it controls differ substantially. + * This is the result of a historic implementation bug, b/73893222. */ public boolean hasOnDemandTime() { return (mTopFlags & TOP_HAL_CAPABILITY_ON_DEMAND_TIME) != 0; diff --git a/services/core/java/com/android/server/location/gnss/GnssLocationProvider.java b/services/core/java/com/android/server/location/gnss/GnssLocationProvider.java index 282ad574a0ed8..262013ec8d961 100644 --- a/services/core/java/com/android/server/location/gnss/GnssLocationProvider.java +++ b/services/core/java/com/android/server/location/gnss/GnssLocationProvider.java @@ -113,9 +113,8 @@ import com.android.internal.util.FrameworkStatsLog; import com.android.internal.util.HexDump; import com.android.server.FgThread; import com.android.server.location.gnss.GnssSatelliteBlocklistHelper.GnssSatelliteBlocklistCallback; -import com.android.server.location.gnss.NtpTimeHelper.InjectNtpTimeCallback; +import com.android.server.location.gnss.NetworkTimeHelper.InjectTimeCallback; import com.android.server.location.gnss.hal.GnssNative; -import com.android.server.location.injector.Injector; import com.android.server.location.provider.AbstractLocationProvider; import java.io.FileDescriptor; @@ -138,7 +137,7 @@ import java.util.concurrent.TimeUnit; * {@hide} */ public class GnssLocationProvider extends AbstractLocationProvider implements - InjectNtpTimeCallback, GnssSatelliteBlocklistCallback, GnssNative.BaseCallbacks, + InjectTimeCallback, GnssSatelliteBlocklistCallback, GnssNative.BaseCallbacks, GnssNative.LocationCallbacks, GnssNative.SvStatusCallbacks, GnssNative.AGpsCallbacks, GnssNative.PsdsCallbacks, GnssNative.NotificationCallbacks, GnssNative.LocationRequestCallbacks, GnssNative.TimeCallbacks { @@ -307,7 +306,7 @@ public class GnssLocationProvider extends AbstractLocationProvider implements private boolean mSuplEsEnabled = false; private final LocationExtras mLocationExtras = new LocationExtras(); - private final NtpTimeHelper mNtpTimeHelper; + private final NetworkTimeHelper mNetworkTimeHelper; private final GnssSatelliteBlocklistHelper mGnssSatelliteBlocklistHelper; // Available only on GNSS HAL 2.0 implementations and later. @@ -398,7 +397,7 @@ public class GnssLocationProvider extends AbstractLocationProvider implements } } - public GnssLocationProvider(Context context, Injector injector, GnssNative gnssNative, + public GnssLocationProvider(Context context, GnssNative gnssNative, GnssMetrics gnssMetrics) { super(FgThread.getExecutor(), CallerIdentity.fromContext(context), PROPERTIES, Collections.emptySet()); @@ -470,7 +469,7 @@ public class GnssLocationProvider extends AbstractLocationProvider implements GnssLocationProvider.this::onNetworkAvailable, mHandler.getLooper(), mNIHandler); - mNtpTimeHelper = new NtpTimeHelper(mContext, mHandler.getLooper(), this); + mNetworkTimeHelper = NetworkTimeHelper.create(mContext, mHandler.getLooper(), this); mGnssSatelliteBlocklistHelper = new GnssSatelliteBlocklistHelper(mContext, mHandler.getLooper(), this); @@ -647,18 +646,19 @@ public class GnssLocationProvider extends AbstractLocationProvider implements } /** - * Implements {@link InjectNtpTimeCallback#injectTime} + * Implements {@link InjectTimeCallback#injectTime} */ @Override - public void injectTime(long time, long timeReference, int uncertainty) { - mGnssNative.injectTime(time, timeReference, uncertainty); + public void injectTime(long unixEpochTimeMillis, long elapsedRealtimeMillis, + int uncertaintyMillis) { + mGnssNative.injectTime(unixEpochTimeMillis, elapsedRealtimeMillis, uncertaintyMillis); } /** * Implements {@link GnssNetworkConnectivityHandler.GnssNetworkListener#onNetworkAvailable()} */ private void onNetworkAvailable() { - mNtpTimeHelper.onNetworkAvailable(); + mNetworkTimeHelper.onNetworkAvailable(); // Download only if supported, (prevents an unnecessary on-boot download) if (mSupportsPsds) { synchronized (mLock) { @@ -1145,7 +1145,7 @@ public class GnssLocationProvider extends AbstractLocationProvider implements if ("delete_aiding_data".equals(command)) { deleteAidingData(extras); } else if ("force_time_injection".equals(command)) { - requestUtcTime(); + demandUtcTimeInjection(); } else if ("force_psds_injection".equals(command)) { if (mSupportsPsds) { postWithWakeLockHeld(() -> handleDownloadPsdsData( @@ -1514,9 +1514,9 @@ public class GnssLocationProvider extends AbstractLocationProvider implements /* userResponse= */ 0); } - private void requestUtcTime() { - if (DEBUG) Log.d(TAG, "utcTimeRequest"); - postWithWakeLockHeld(mNtpTimeHelper::retrieveAndInjectNtpTime); + private void demandUtcTimeInjection() { + if (DEBUG) Log.d(TAG, "demandUtcTimeInjection"); + postWithWakeLockHeld(mNetworkTimeHelper::demandUtcTimeInjection); } @@ -1721,9 +1721,16 @@ public class GnssLocationProvider extends AbstractLocationProvider implements public void onCapabilitiesChanged(GnssCapabilities oldCapabilities, GnssCapabilities newCapabilities) { mHandler.post(() -> { - if (mGnssNative.getCapabilities().hasOnDemandTime()) { - mNtpTimeHelper.enablePeriodicTimeInjection(); - requestUtcTime(); + boolean useOnDemandTimeInjection = mGnssNative.getCapabilities().hasOnDemandTime(); + + // b/73893222: There is a historic bug on Android, which means that the capability + // "on demand time" is interpreted as "enable periodic injection" elsewhere but an + // on-demand injection is done here. GNSS developers may have come to rely on the + // periodic behavior, so it has been kept and all methods named to reflect what is + // actually done. "On demand" requests are supported regardless of the capability. + mNetworkTimeHelper.setPeriodicTimeInjectionMode(useOnDemandTimeInjection); + if (useOnDemandTimeInjection) { + demandUtcTimeInjection(); } restartLocationRequest(); @@ -1857,7 +1864,7 @@ public class GnssLocationProvider extends AbstractLocationProvider implements @Override public void onRequestUtcTime() { - requestUtcTime(); + demandUtcTimeInjection(); } @Override diff --git a/services/core/java/com/android/server/location/gnss/GnssManagerService.java b/services/core/java/com/android/server/location/gnss/GnssManagerService.java index 69385a92cab15..2174f4044ffd3 100644 --- a/services/core/java/com/android/server/location/gnss/GnssManagerService.java +++ b/services/core/java/com/android/server/location/gnss/GnssManagerService.java @@ -83,8 +83,7 @@ public class GnssManagerService { mGnssMetrics = new GnssMetrics(mContext, IBatteryStats.Stub.asInterface( ServiceManager.getService(BatteryStats.SERVICE_NAME)), mGnssNative); - mGnssLocationProvider = new GnssLocationProvider(mContext, injector, mGnssNative, - mGnssMetrics); + mGnssLocationProvider = new GnssLocationProvider(mContext, mGnssNative, mGnssMetrics); mGnssStatusProvider = new GnssStatusProvider(injector, mGnssNative); mGnssNmeaProvider = new GnssNmeaProvider(injector, mGnssNative); mGnssMeasurementsProvider = new GnssMeasurementsProvider(injector, mGnssNative); diff --git a/services/core/java/com/android/server/location/gnss/NetworkTimeHelper.java b/services/core/java/com/android/server/location/gnss/NetworkTimeHelper.java new file mode 100644 index 0000000000000..72d6f70145153 --- /dev/null +++ b/services/core/java/com/android/server/location/gnss/NetworkTimeHelper.java @@ -0,0 +1,75 @@ +/* + * Copyright (C) 2022 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 android.content.Context; +import android.os.Looper; + +/** + * An abstraction for use by {@link GnssLocationProvider}. This class allows switching between + * implementations with a compile-time constant change, which is less risky than rolling back a + * whole class. When there is a single implementation again this class can be replaced by that + * implementation. + */ +abstract class NetworkTimeHelper { + + /** + * The callback interface used by {@link NetworkTimeHelper} to report the time to {@link + * GnssLocationProvider}. The callback can happen at any time using the thread associated with + * the looper passed to {@link #create(Context, Looper, InjectTimeCallback)}. + */ + interface InjectTimeCallback { + void injectTime(long unixEpochTimeMillis, long elapsedRealtimeMillis, + int uncertaintyMillis); + } + + /** + * Creates the {@link NetworkTimeHelper} instance for use by {@link GnssLocationProvider}. + */ + static NetworkTimeHelper create( + Context context, Looper looper, InjectTimeCallback injectTimeCallback) { + return new NtpNetworkTimeHelper(context, looper, injectTimeCallback); + } + + /** + * Sets the "on demand time injection" mode. + * + *
Called by {@link GnssLocationProvider} to set the expected time injection behavior. + * When {@code enablePeriodicTimeInjection == true}, the time helper should periodically send + * the time on an undefined schedule. The time can be injected at other times for other reasons + * as well as be requested via {@link #demandUtcTimeInjection()}. + * + * @param periodicTimeInjectionEnabled {@code true} if the GNSS implementation requires periodic + * time signals + */ + abstract void setPeriodicTimeInjectionMode(boolean periodicTimeInjectionEnabled); + + /** + * Requests an asynchronous time injection via {@link InjectTimeCallback#injectTime}, if a + * network time is available. {@link InjectTimeCallback#injectTime} may not be called if a + * network time is not available. + */ + abstract void demandUtcTimeInjection(); + + /** + * Notifies that network connectivity has been established. + * + *
Called by {@link GnssLocationProvider} when the device establishes a data network + * connection. + */ + abstract void onNetworkAvailable(); + +} diff --git a/services/core/java/com/android/server/location/gnss/NtpTimeHelper.java b/services/core/java/com/android/server/location/gnss/NtpNetworkTimeHelper.java similarity index 83% rename from services/core/java/com/android/server/location/gnss/NtpTimeHelper.java rename to services/core/java/com/android/server/location/gnss/NtpNetworkTimeHelper.java index e4bc78827269f..479dbdab8bd58 100644 --- a/services/core/java/com/android/server/location/gnss/NtpTimeHelper.java +++ b/services/core/java/com/android/server/location/gnss/NtpNetworkTimeHelper.java @@ -30,14 +30,11 @@ import com.android.internal.annotations.GuardedBy; import com.android.internal.annotations.VisibleForTesting; /** - * Handles inject NTP time to GNSS. - * - *
The client is responsible to call {@link #onNetworkAvailable()} when network is available - * for retrieving NTP Time. + * Handles injecting network time to GNSS by explicitly making NTP requests when needed. */ -class NtpTimeHelper { +class NtpNetworkTimeHelper extends NetworkTimeHelper { - private static final String TAG = "NtpTimeHelper"; + private static final String TAG = "NtpNetworkTimeHelper"; private static final boolean DEBUG = Log.isLoggable(TAG, Log.DEBUG); // states for injecting ntp @@ -71,23 +68,19 @@ class NtpTimeHelper { private final WakeLock mWakeLock; private final Handler mHandler; - private final InjectNtpTimeCallback mCallback; + private final InjectTimeCallback mCallback; // flags to trigger NTP when network becomes available // initialized to STATE_PENDING_NETWORK so we do NTP when the network comes up after booting @GuardedBy("this") private int mInjectNtpTimeState = STATE_PENDING_NETWORK; - // set to true if the GPS engine requested on-demand NTP time requests + // Enables periodic time injection in addition to injection for other reasons. @GuardedBy("this") - private boolean mOnDemandTimeInjection; - - interface InjectNtpTimeCallback { - void injectTime(long time, long timeReference, int uncertainty); - } + private boolean mPeriodicTimeInjection; @VisibleForTesting - NtpTimeHelper(Context context, Looper looper, InjectNtpTimeCallback callback, + NtpNetworkTimeHelper(Context context, Looper looper, InjectTimeCallback callback, NtpTrustedTime ntpTime) { mConnMgr = (ConnectivityManager) context.getSystemService(Context.CONNECTIVITY_SERVICE); mCallback = callback; @@ -97,14 +90,23 @@ class NtpTimeHelper { mWakeLock = powerManager.newWakeLock(PowerManager.PARTIAL_WAKE_LOCK, WAKELOCK_KEY); } - NtpTimeHelper(Context context, Looper looper, InjectNtpTimeCallback callback) { + NtpNetworkTimeHelper(Context context, Looper looper, InjectTimeCallback callback) { this(context, looper, callback, NtpTrustedTime.getInstance(context)); } - synchronized void enablePeriodicTimeInjection() { - mOnDemandTimeInjection = true; + @Override + synchronized void setPeriodicTimeInjectionMode(boolean periodicTimeInjectionEnabled) { + if (periodicTimeInjectionEnabled) { + mPeriodicTimeInjection = true; + } } + @Override + void demandUtcTimeInjection() { + retrieveAndInjectNtpTime(); + } + + @Override synchronized void onNetworkAvailable() { if (mInjectNtpTimeState == STATE_PENDING_NETWORK) { retrieveAndInjectNtpTime(); @@ -120,7 +122,7 @@ class NtpTimeHelper { return activeNetworkInfo != null && activeNetworkInfo.isConnected(); } - synchronized void retrieveAndInjectNtpTime() { + private synchronized void retrieveAndInjectNtpTime() { if (mInjectNtpTimeState == STATE_RETRIEVING_AND_INJECTING) { // already downloading data return; @@ -166,18 +168,15 @@ class NtpTimeHelper { if (DEBUG) { Log.d(TAG, String.format( - "onDemandTimeInjection=%s, refreshSuccess=%s, delay=%s", - mOnDemandTimeInjection, + "mPeriodicTimeInjection=%s, refreshSuccess=%s, delay=%s", + mPeriodicTimeInjection, refreshSuccess, delay)); } - // TODO(b/73893222): reconcile Capabilities bit 'on demand' name vs. de facto periodic - // injection. - if (mOnDemandTimeInjection || !refreshSuccess) { - /* Schedule next NTP injection. - * Since this is delayed, the wake lock is released right away, and will be held - * again when the delayed task runs. - */ + if (mPeriodicTimeInjection || !refreshSuccess) { + // Schedule next NTP injection. + // Since this is delayed, the wake lock is released right away, and will be held + // again when the delayed task runs. mHandler.postDelayed(this::retrieveAndInjectNtpTime, delay); } } diff --git a/services/robotests/src/com/android/server/location/gnss/NtpTimeHelperTest.java b/services/robotests/src/com/android/server/location/gnss/NtpNetworkTimeHelperTest.java similarity index 77% rename from services/robotests/src/com/android/server/location/gnss/NtpTimeHelperTest.java rename to services/robotests/src/com/android/server/location/gnss/NtpNetworkTimeHelperTest.java index e5a1cfeb55c21..4949091646ddd 100644 --- a/services/robotests/src/com/android/server/location/gnss/NtpTimeHelperTest.java +++ b/services/robotests/src/com/android/server/location/gnss/NtpNetworkTimeHelperTest.java @@ -26,7 +26,7 @@ import android.os.SystemClock; import android.platform.test.annotations.Presubmit; import android.util.NtpTrustedTime; -import com.android.server.location.gnss.NtpTimeHelper.InjectNtpTimeCallback; +import com.android.server.location.gnss.NetworkTimeHelper.InjectTimeCallback; import org.junit.Before; import org.junit.Test; @@ -41,16 +41,16 @@ import java.util.concurrent.CountDownLatch; import java.util.concurrent.TimeUnit; /** - * Unit tests for {@link NtpTimeHelper}. + * Unit tests for {@link NtpNetworkTimeHelper}. */ @RunWith(RobolectricTestRunner.class) @Presubmit -public class NtpTimeHelperTest { +public class NtpNetworkTimeHelperTest { private static final long MOCK_NTP_TIME = 1519930775453L; @Mock private NtpTrustedTime mMockNtpTrustedTime; - private NtpTimeHelper mNtpTimeHelper; + private NtpNetworkTimeHelper mNtpNetworkTimeHelper; private CountDownLatch mCountDownLatch; /** @@ -60,12 +60,12 @@ public class NtpTimeHelperTest { public void setUp() throws Exception { MockitoAnnotations.initMocks(this); mCountDownLatch = new CountDownLatch(1); - InjectNtpTimeCallback callback = + InjectTimeCallback callback = (time, timeReference, uncertainty) -> { assertThat(time).isEqualTo(MOCK_NTP_TIME); mCountDownLatch.countDown(); }; - mNtpTimeHelper = new NtpTimeHelper(RuntimeEnvironment.application, + mNtpNetworkTimeHelper = new NtpNetworkTimeHelper(RuntimeEnvironment.application, Looper.myLooper(), callback, mMockNtpTrustedTime); } @@ -74,13 +74,13 @@ public class NtpTimeHelperTest { * Verify that cached time is returned if cached age is low. */ @Test - public void handleInjectNtpTime_cachedAgeLow_injectTime() throws InterruptedException { + public void demandUtcTimeInjection_cachedAgeLow_injectTime() throws InterruptedException { NtpTrustedTime.TimeResult result = mock(NtpTrustedTime.TimeResult.class); - doReturn(NtpTimeHelper.NTP_INTERVAL - 1).when(result).getAgeMillis(); + doReturn(NtpNetworkTimeHelper.NTP_INTERVAL - 1).when(result).getAgeMillis(); doReturn(MOCK_NTP_TIME).when(result).getTimeMillis(); doReturn(result).when(mMockNtpTrustedTime).getCachedTimeResult(); - mNtpTimeHelper.retrieveAndInjectNtpTime(); + mNtpNetworkTimeHelper.demandUtcTimeInjection(); waitForTasksToBePostedOnHandlerAndRunThem(); assertThat(mCountDownLatch.await(2, TimeUnit.SECONDS)).isTrue(); @@ -90,14 +90,14 @@ public class NtpTimeHelperTest { * Verify that failed inject time and delayed inject time are handled properly. */ @Test - public void handleInjectNtpTime_injectTimeFailed_injectTimeDelayed() + public void demandUtcTimeInjection_injectTimeFailed_injectTimeDelayed() throws InterruptedException { NtpTrustedTime.TimeResult result1 = mock(NtpTrustedTime.TimeResult.class); - doReturn(NtpTimeHelper.NTP_INTERVAL + 1).when(result1).getAgeMillis(); + doReturn(NtpNetworkTimeHelper.NTP_INTERVAL + 1).when(result1).getAgeMillis(); doReturn(result1).when(mMockNtpTrustedTime).getCachedTimeResult(); doReturn(false).when(mMockNtpTrustedTime).forceRefresh(); - mNtpTimeHelper.retrieveAndInjectNtpTime(); + mNtpNetworkTimeHelper.demandUtcTimeInjection(); waitForTasksToBePostedOnHandlerAndRunThem(); assertThat(mCountDownLatch.await(2, TimeUnit.SECONDS)).isFalse(); @@ -106,15 +106,15 @@ public class NtpTimeHelperTest { doReturn(1L).when(result2).getAgeMillis(); doReturn(MOCK_NTP_TIME).when(result2).getTimeMillis(); doReturn(result2).when(mMockNtpTrustedTime).getCachedTimeResult(); - SystemClock.sleep(NtpTimeHelper.RETRY_INTERVAL); + SystemClock.sleep(NtpNetworkTimeHelper.RETRY_INTERVAL); waitForTasksToBePostedOnHandlerAndRunThem(); assertThat(mCountDownLatch.await(2, TimeUnit.SECONDS)).isTrue(); } /** - * Since a thread is created in {@link NtpTimeHelper#retrieveAndInjectNtpTime} and the task to - * be verified is posted in the thread, we have to wait for the task to be posted and then it + * Since a thread is created in {@link NtpNetworkTimeHelper#demandUtcTimeInjection} and the task + * to be verified is posted in the thread, we have to wait for the task to be posted and then it * can be run. */ private void waitForTasksToBePostedOnHandlerAndRunThem() throws InterruptedException {