From 89346d479390e7bf68f77e0be122da2eaec637be Mon Sep 17 00:00:00 2001 From: Xinyi Zhou Date: Thu, 5 Jan 2023 18:15:30 -0800 Subject: [PATCH] Return UNKNOWN code when CHRE is not ready Test: Unit test Ignore-AOSP-First: nearby_not_in_aosp_yet Fix: 263890209 Change-Id: I45b3eec6c83848e129704ed9ec5f2c4a49f982fb --- .../java/android/nearby/NearbyManager.java | 2 +- .../android/server/nearby/NearbyService.java | 5 +- .../nearby/provider/ChreCommunication.java | 14 ++++- .../provider/ChreDiscoveryProvider.java | 9 ++- .../provider/DiscoveryProviderManager.java | 42 ++++++++++---- .../DiscoveryProviderManagerTest.java | 56 ++++++++++++++----- 6 files changed, 94 insertions(+), 34 deletions(-) diff --git a/nearby/framework/java/android/nearby/NearbyManager.java b/nearby/framework/java/android/nearby/NearbyManager.java index 595f62e132..6d6b69a096 100644 --- a/nearby/framework/java/android/nearby/NearbyManager.java +++ b/nearby/framework/java/android/nearby/NearbyManager.java @@ -62,7 +62,7 @@ public class NearbyManager { ScanStatus.ERROR, }) public @interface ScanStatus { - // Default, invalid state. + // The undetermined status, some modules may be initializing. Retry is suggested. int UNKNOWN = 0; // The successful state. int SUCCESS = 1; diff --git a/nearby/service/java/com/android/server/nearby/NearbyService.java b/nearby/service/java/com/android/server/nearby/NearbyService.java index be3b84e519..a1bca1977c 100644 --- a/nearby/service/java/com/android/server/nearby/NearbyService.java +++ b/nearby/service/java/com/android/server/nearby/NearbyService.java @@ -108,10 +108,7 @@ public class NearbyService extends INearbyManager.Stub { CallerIdentity identity = CallerIdentity.fromBinder(mContext, packageName, attributionTag); DiscoveryPermissions.enforceDiscoveryPermission(mContext, identity); - if (mProviderManager.registerScanListener(scanRequest, listener, identity)) { - return NearbyManager.ScanStatus.SUCCESS; - } - return NearbyManager.ScanStatus.ERROR; + return mProviderManager.registerScanListener(scanRequest, listener, identity); } @Override diff --git a/nearby/service/java/com/android/server/nearby/provider/ChreCommunication.java b/nearby/service/java/com/android/server/nearby/provider/ChreCommunication.java index 00e1cb6a3f..9d93843161 100644 --- a/nearby/service/java/com/android/server/nearby/provider/ChreCommunication.java +++ b/nearby/service/java/com/android/server/nearby/provider/ChreCommunication.java @@ -69,6 +69,8 @@ public class ChreCommunication extends ContextHubClientCallback { private final Executor mExecutor; private boolean mStarted = false; + // null when CHRE availability result has not been returned + @Nullable private Boolean mChreSupport = null; @Nullable private ContextHubCommsCallback mCallback; @Nullable private ContextHubClient mContextHubClient; @@ -78,8 +80,13 @@ public class ChreCommunication extends ContextHubClientCallback { mExecutor = executor; } - public boolean available() { - return mContextHubClient != null; + /** + * @return {@code true} if NanoApp is available and {@code null} when CHRE availability result + * has not been returned + */ + @Nullable + public Boolean available() { + return mChreSupport; } /** @@ -138,6 +145,7 @@ public class ChreCommunication extends ContextHubClientCallback { if (mContextHubClient != null) { mContextHubClient.close(); mContextHubClient = null; + mChreSupport = null; } } @@ -245,6 +253,7 @@ public class ChreCommunication extends ContextHubClientCallback { "Found valid contexthub: %s", mQueriedContextHub.getId())); mContextHubClient = mManager.createClient(mContext, mQueriedContextHub, mExecutor, ChreCommunication.this); + mChreSupport = true; mCallback.started(true); return; } @@ -267,6 +276,7 @@ public class ChreCommunication extends ContextHubClientCallback { // there isn't a valid context available on this device. if (mContextHubs.isEmpty()) { mCallback.started(false); + mChreSupport = false; } } } diff --git a/nearby/service/java/com/android/server/nearby/provider/ChreDiscoveryProvider.java b/nearby/service/java/com/android/server/nearby/provider/ChreDiscoveryProvider.java index 105051af48..d0d0de0371 100644 --- a/nearby/service/java/com/android/server/nearby/provider/ChreDiscoveryProvider.java +++ b/nearby/service/java/com/android/server/nearby/provider/ChreDiscoveryProvider.java @@ -24,6 +24,7 @@ import static service.proto.Blefilter.DataElement.ElementType.DE_BATTERY_STATUS; import static service.proto.Blefilter.DataElement.ElementType.DE_CONNECTION_STATUS; import static service.proto.Blefilter.DataElement.ElementType.DE_FAST_PAIR_ACCOUNT_KEY; +import android.annotation.Nullable; import android.content.BroadcastReceiver; import android.content.Context; import android.content.Intent; @@ -59,7 +60,6 @@ public class ChreDiscoveryProvider extends AbstractDiscoveryProvider { /** @hide */ @VisibleForTesting public static final int NANOAPP_MESSAGE_TYPE_CONFIG = 5; - private static final int PRESENCE_UUID = 0xFCF1; private static final int FP_ACCOUNT_KEY_LENGTH = 16; private final ChreCommunication mChreCommunication; @@ -114,7 +114,12 @@ public class ChreDiscoveryProvider extends AbstractDiscoveryProvider { onStart(); } - public boolean available() { + /** + * @return {@code true} if CHRE is available and {@code null} when CHRE availability result + * has not been returned + */ + @Nullable + public Boolean available() { return mChreCommunication.available(); } diff --git a/nearby/service/java/com/android/server/nearby/provider/DiscoveryProviderManager.java b/nearby/service/java/com/android/server/nearby/provider/DiscoveryProviderManager.java index e4c56e4d86..b7574c949f 100644 --- a/nearby/service/java/com/android/server/nearby/provider/DiscoveryProviderManager.java +++ b/nearby/service/java/com/android/server/nearby/provider/DiscoveryProviderManager.java @@ -26,6 +26,7 @@ import android.content.Context; import android.nearby.DataElement; import android.nearby.IScanListener; import android.nearby.NearbyDeviceParcelable; +import android.nearby.NearbyManager; import android.nearby.PresenceScanFilter; import android.nearby.ScanFilter; import android.nearby.ScanRequest; @@ -148,7 +149,8 @@ public class DiscoveryProviderManager implements AbstractDiscoveryProvider.Liste /** * Registers the listener in the manager and starts scan according to the requested scan mode. */ - public boolean registerScanListener(ScanRequest scanRequest, IScanListener listener, + @NearbyManager.ScanStatus + public int registerScanListener(ScanRequest scanRequest, IScanListener listener, CallerIdentity callerIdentity) { synchronized (mLock) { ScanListenerDeathRecipient deathRecipient = (listener != null) @@ -166,23 +168,26 @@ public class DiscoveryProviderManager implements AbstractDiscoveryProvider.Liste mScanTypeScanListenerRecordMap.get(listenerBinder).getScanRequest(); if (scanRequest.equals(savedScanRequest)) { Log.d(TAG, "Already registered the scanRequest: " + scanRequest); - return true; + return NearbyManager.ScanStatus.SUCCESS; } } ScanListenerRecord scanListenerRecord = new ScanListenerRecord(scanRequest, listener, callerIdentity, deathRecipient); mScanTypeScanListenerRecordMap.put(listenerBinder, scanListenerRecord); - if (!startProviders(scanRequest)) { - return false; + Boolean started = startProviders(scanRequest); + if (started == null) { + return NearbyManager.ScanStatus.UNKNOWN; + } + if (!started) { + return NearbyManager.ScanStatus.ERROR; } - NearbyMetrics.logScanStarted(scanListenerRecord.hashCode(), scanRequest); if (mScanMode < scanRequest.getScanMode()) { mScanMode = scanRequest.getScanMode(); invalidateProviderScanMode(); } - return true; + return NearbyManager.ScanStatus.SUCCESS; } } @@ -237,18 +242,33 @@ public class DiscoveryProviderManager implements AbstractDiscoveryProvider.Liste } } - // Returns false when fail to start all the providers. Returns true if any one of the provider - // starts successfully. + /** + * @return {@code null} when all providers are initializing + * {@code false} when fail to start all the providers + * {@code true} when any one of the provider starts successfully + */ @VisibleForTesting - boolean startProviders(ScanRequest scanRequest) { + @Nullable + Boolean startProviders(ScanRequest scanRequest) { if (!scanRequest.isBleEnabled()) { Log.w(TAG, "failed to start any provider because client disabled BLE"); return false; } List scanFilters = getPresenceScanFilters(); + boolean chreOnly = isChreOnly(scanFilters); + Boolean chreAvailable = mChreDiscoveryProvider.available(); + if (chreAvailable == null) { + if (chreOnly) { + Log.w(TAG, "client wants CHRE only and Nearby service is still querying CHRE" + + " status"); + return null; + } + startBleProvider(scanFilters); + return true; + } - if (!mChreDiscoveryProvider.available()) { - if (scanRequest.getScanType() == SCAN_TYPE_NEARBY_PRESENCE && isChreOnly(scanFilters)) { + if (!chreAvailable) { + if (chreOnly) { Log.w(TAG, "failed to start any provider because client wants CHRE only and CHRE" + " is not available"); return false; diff --git a/nearby/tests/unit/src/com/android/server/nearby/provider/DiscoveryProviderManagerTest.java b/nearby/tests/unit/src/com/android/server/nearby/provider/DiscoveryProviderManagerTest.java index b1737e9131..8591b60bd5 100644 --- a/nearby/tests/unit/src/com/android/server/nearby/provider/DiscoveryProviderManagerTest.java +++ b/nearby/tests/unit/src/com/android/server/nearby/provider/DiscoveryProviderManagerTest.java @@ -45,7 +45,6 @@ import org.mockito.Mock; import org.mockito.MockitoAnnotations; import java.util.HashMap; -import java.util.List; import java.util.Map; public class DiscoveryProviderManagerTest { @@ -76,6 +75,7 @@ public class DiscoveryProviderManagerTest { when(mInjector.getAppOpsManager()).thenReturn(mAppOpsManager); when(mBleDiscoveryProvider.getController()).thenReturn(mBluetoothController); when(mChreDiscoveryProvider.getController()).thenReturn(mChreController); + mScanTypeScanListenerRecordMap = new HashMap<>(); mDiscoveryProviderManager = new DiscoveryProviderManager(mContext, mInjector, mBleDiscoveryProvider, @@ -108,10 +108,9 @@ public class DiscoveryProviderManagerTest { mCallerIdentity, mScanListenerDeathRecipient); mScanTypeScanListenerRecordMap.put(mIBinder, record); - - boolean startProviders = mDiscoveryProviderManager.startProviders(scanRequest); + Boolean start = mDiscoveryProviderManager.startProviders(scanRequest); verify(mBluetoothController, never()).start(); - assertThat(startProviders).isTrue(); + assertThat(start).isTrue(); } @Test @@ -127,9 +126,9 @@ public class DiscoveryProviderManagerTest { mCallerIdentity, mScanListenerDeathRecipient); mScanTypeScanListenerRecordMap.put(mIBinder, record); - boolean startProviders = mDiscoveryProviderManager.startProviders(scanRequest); + Boolean start = mDiscoveryProviderManager.startProviders(scanRequest); verify(mBluetoothController, never()).start(); - assertThat(startProviders).isTrue(); + assertThat(start).isTrue(); } @Test @@ -144,9 +143,9 @@ public class DiscoveryProviderManagerTest { mCallerIdentity, mScanListenerDeathRecipient); mScanTypeScanListenerRecordMap.put(mIBinder, record); - boolean startProviders = mDiscoveryProviderManager.startProviders(scanRequest); + Boolean start = mDiscoveryProviderManager.startProviders(scanRequest); verify(mBluetoothController, never()).start(); - assertThat(startProviders).isFalse(); + assertThat(start).isFalse(); } @Test @@ -161,9 +160,9 @@ public class DiscoveryProviderManagerTest { mCallerIdentity, mScanListenerDeathRecipient); mScanTypeScanListenerRecordMap.put(mIBinder, record); - boolean startProviders = mDiscoveryProviderManager.startProviders(scanRequest); + Boolean start = mDiscoveryProviderManager.startProviders(scanRequest); verify(mBluetoothController, never()).start(); - assertThat(startProviders).isTrue(); + assertThat(start).isTrue(); } @Test @@ -178,14 +177,43 @@ public class DiscoveryProviderManagerTest { mCallerIdentity, mScanListenerDeathRecipient); mScanTypeScanListenerRecordMap.put(mIBinder, record); - boolean startProviders = mDiscoveryProviderManager.startProviders(scanRequest); + Boolean start = mDiscoveryProviderManager.startProviders(scanRequest); verify(mBluetoothController, atLeastOnce()).start(); - assertThat(startProviders).isTrue(); + assertThat(start).isTrue(); } @Test - public void testStartChreProvider() { - mDiscoveryProviderManager.startChreProvider(List.of(getPresenceScanFilter())); + public void testStartProviders_chreOnlyChreUndetermined_bleProviderNotStarted() { + when(mChreDiscoveryProvider.available()).thenReturn(null); + + ScanRequest scanRequest = new ScanRequest.Builder() + .setScanType(SCAN_TYPE_NEARBY_PRESENCE) + .addScanFilter(getChreOnlyPresenceScanFilter()).build(); + DiscoveryProviderManager.ScanListenerRecord record = + new DiscoveryProviderManager.ScanListenerRecord(scanRequest, mScanListener, + mCallerIdentity, mScanListenerDeathRecipient); + mScanTypeScanListenerRecordMap.put(mIBinder, record); + + Boolean start = mDiscoveryProviderManager.startProviders(scanRequest); + verify(mBluetoothController, never()).start(); + assertThat(start).isNull(); + } + + @Test + public void testStartProviders_notChreOnlyChreUndetermined_bleProviderStarted() { + when(mChreDiscoveryProvider.available()).thenReturn(null); + + ScanRequest scanRequest = new ScanRequest.Builder() + .setScanType(SCAN_TYPE_NEARBY_PRESENCE) + .addScanFilter(getPresenceScanFilter()).build(); + DiscoveryProviderManager.ScanListenerRecord record = + new DiscoveryProviderManager.ScanListenerRecord(scanRequest, mScanListener, + mCallerIdentity, mScanListenerDeathRecipient); + mScanTypeScanListenerRecordMap.put(mIBinder, record); + + Boolean start = mDiscoveryProviderManager.startProviders(scanRequest); + verify(mBluetoothController, atLeastOnce()).start(); + assertThat(start).isTrue(); } private static PresenceScanFilter getPresenceScanFilter() {