Android/frameworks_base
1
0
Fork 0
Code Issues Pull Requests Actions Packages Projects Releases Wiki Activity

Refactoring: Support alt. network time source

Browse Source 
Refactoring to support an alternative source of network time for passing
to the GNSS component. The new implementation will be submitted in a
follow-up.

NtpTimeHelper has been replaced by NetworkTimeHelper in
GnssLocationProvider. NetworkTimeHelper provides the stable interface
between GnssLocationProvider and the original / alternative impl for
what was NtpTimeHelper.

NtpTimeHelper has been renamed NtpNetworkTimeHelper.

These changes are not intended to change any behavior. There are some
minor changes between the interaction between GnssLocationProvider and
the NetworkTimeHelper class, but these are not expected to alter the
runtime behavior.

The NetworkTimeHelper.setPeriodicTimeInjectionMode() method touches
a pre-existing bug: The method name reflected the effect of the method,
which is the near-opposite of what the capability name would suggest.
This appears to be due to an accidental logic inversion, not by intent.
As can be seen in the changes for GnssLocationProvider: the
enablePeriodicTimeInjection() method was called when
mGnssNative.getCapabilities().hasOnDemandTime() was true. The existence
of bug 73893222 supports the fact that there is a long-standing bug
here. The intent with this commit is not to fix it or alter behavior,
just to make it more obvious, as it is unclear if the current behavior
is relied upon somewhere. Comments and field names have been improved to
try to clarify the actual behavior.

Bug: 73893222
Bug: 222295093
Test: atest services/robotests/src/com/android/server/location/gnss/NtpNetworkTimeHelperTest.java
Change-Id: I0b1ba43a55ff531df343c022650e3f5721dda7f1
This commit is contained in:
Neil Fuller
2022-12-13 14:49:06 +00:00
parent b9080f55ee
commit 8ad07e271f
6 changed files with 147 additions and 63 deletions
Download Patch File Download Diff File Expand all files Collapse all files

6
location/java/android/location/GnssCapabilities.java
View File

@@ -240,7 +240,11 @@ public final class GnssCapabilities implements Parcelable {
}
/**
* Returns {@code true} if GNSS chipset supports on demand time, {@code false} otherwise.
* Returns {@code true} if GNSS chipset requests periodic time signal injection from the
* platform in addition to on-demand and occasional time updates, {@code false} otherwise.
*
* <p><em>Note: The naming of this capability and the behavior it controls differ substantially.
* This is the result of a historic implementation bug, b/73893222.</em>
*/
public boolean hasOnDemandTime() {
return (mTopFlags & TOP_HAL_CAPABILITY_ON_DEMAND_TIME) != 0;

43
services/core/java/com/android/server/location/gnss/GnssLocationProvider.java
View File

@@ -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

3
services/core/java/com/android/server/location/gnss/GnssManagerService.java
View File

@@ -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);

75
services/core/java/com/android/server/location/gnss/NetworkTimeHelper.java Normal file
View File

@@ -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.
*
* <p>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.
*
* <p>Called by {@link GnssLocationProvider} when the device establishes a data network
* connection.
*/
abstract void onNetworkAvailable();
}

53
services/core/java/com/android/server/location/gnss/NtpTimeHelper.java → services/core/java/com/android/server/location/gnss/NtpNetworkTimeHelper.java
View File

@@ -30,14 +30,11 @@ import com.android.internal.annotations.GuardedBy;
import com.android.internal.annotations.VisibleForTesting;
/**
* Handles inject NTP time to GNSS.
*
* <p>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);
}
}

30
services/robotests/src/com/android/server/location/gnss/NtpTimeHelperTest.java → services/robotests/src/com/android/server/location/gnss/NtpNetworkTimeHelperTest.java
View File

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