Merge "Add retry logic for wifi Hal starting failure" into oc-dev

2 files changed, 103 insertions, 18 deletions

diff --git a/service/java/com/android/server/wifi/HalDeviceManager.java b/service/java/com/android/server/wifi/HalDeviceManager.java
index 4203c36c7..c54be9b9a 100644
--- a/service/java/com/android/server/wifi/HalDeviceManager.java
+++ b/service/java/com/android/server/wifi/HalDeviceManager.java
@@ -42,6 +42,8 @@ import android.util.MutableBoolean;
 import android.util.MutableInt;
 import android.util.SparseArray;
 
+import com.android.internal.annotations.VisibleForTesting;
+
 import java.io.FileDescriptor;
 import java.io.PrintWriter;
 import java.util.ArrayList;
@@ -60,6 +62,12 @@ public class HalDeviceManager {
     private static final String TAG = "HalDeviceManager";
     private static final boolean DBG = false;
 
+    private static final int START_HAL_RETRY_INTERVAL_MS = 20;
+    // Number of attempts a start() is re-tried. A value of 0 means no retries after a single
+    // attempt.
+    @VisibleForTesting
+    public static final int START_HAL_RETRY_TIMES = 3;
+
     // public API
     public HalDeviceManager() {
         mInterfaceAvailableForRequestListeners.put(IfaceType.STA, new HashSet<>());
@@ -1054,15 +1062,35 @@ public class HalDeviceManager {
                     Log.w(TAG, "startWifi called but mWifi is null!?");
                     return false;
                 } else {
-                    WifiStatus status = mWifi.start();
-                    boolean success = status.code == WifiStatusCode.SUCCESS;
-                    if (success) {
-                        initIWifiChipDebugListeners();
-                        managerStatusListenerDispatch();
-                    } else {
-                        Log.e(TAG, "Cannot start IWifi: " + statusString(status));
+                    int triedCount = 0;
+                    while (triedCount <= START_HAL_RETRY_TIMES) {
+                        WifiStatus status = mWifi.start();
+                        if (status.code == WifiStatusCode.SUCCESS) {
+                            initIWifiChipDebugListeners();
+                            managerStatusListenerDispatch();
+                            if (triedCount != 0) {
+                                Log.d(TAG, "start IWifi succeeded after trying "
+                                         + triedCount + " times");
+                            }
+                            return true;
+                        } else if (status.code == WifiStatusCode.ERROR_NOT_AVAILABLE) {
+                            // Should retry. Hal might still be stopping.
+                            Log.e(TAG, "Cannot start IWifi: " + statusString(status)
+                                    + ", Retrying...");
+                            try {
+                                Thread.sleep(START_HAL_RETRY_INTERVAL_MS);
+                            } catch (InterruptedException ignore) {
+                                // no-op
+                            }
+                            triedCount++;
+                        } else {
+                            // Should not retry on other failures.
+                            Log.e(TAG, "Cannot start IWifi: " + statusString(status));
+                            return false;
+                        }
                     }
-                    return success;
+                    Log.e(TAG, "Cannot start IWifi after trying " + triedCount + " times");
+                    return false;
                 }
             } catch (RemoteException e) {
                 Log.e(TAG, "startWifi exception: " + e);
diff --git a/tests/wifitests/src/com/android/server/wifi/HalDeviceManagerTest.java b/tests/wifitests/src/com/android/server/wifi/HalDeviceManagerTest.java
index 4635f843b..570100bbf 100644
--- a/tests/wifitests/src/com/android/server/wifi/HalDeviceManagerTest.java
+++ b/tests/wifitests/src/com/android/server/wifi/HalDeviceManagerTest.java
@@ -16,6 +16,8 @@
 
 package com.android.server.wifi;
 
+import static com.android.server.wifi.HalDeviceManager.START_HAL_RETRY_TIMES;
+
 import static junit.framework.Assert.assertEquals;
 
 import static org.hamcrest.core.IsEqual.equalTo;
@@ -211,7 +213,7 @@ public class HalDeviceManagerTest {
         mInOrder.verify(mWifiMock).registerEventCallback(mWifiEventCallbackCaptor.capture());
 
         // act: start
-        mDut.start();
+        collector.checkThat(mDut.start(), equalTo(true));
         mWifiEventCallbackCaptor.getValue().onStart();
         mTestLooper.dispatchAll();
 
@@ -239,7 +241,7 @@ public class HalDeviceManagerTest {
         mInOrder.verify(mManagerStatusListenerMock).onStatusChanged();
 
         // act: start again
-        mDut.start();
+        collector.checkThat(mDut.start(), equalTo(true));
         mWifiEventCallbackCaptor.getValue().onStart();
         mTestLooper.dispatchAll();
 
@@ -1000,6 +1002,54 @@ public class HalDeviceManagerTest {
         assertEquals(0, results.size());
     }
 
+    /**
+     * Test start HAL can retry upon failure.
+     */
+    @Test
+    public void testStartHalRetryUponNotAvailableFailure() throws Exception {
+        // Override the stubbing for mWifiMock in before().
+        when(mWifiMock.start())
+            .thenReturn(getStatus(WifiStatusCode.ERROR_NOT_AVAILABLE))
+            .thenReturn(mStatusOk);
+
+        BaselineChip chipMock = new BaselineChip();
+        chipMock.initialize();
+        mInOrder = inOrder(mServiceManagerMock, mWifiMock, chipMock.chip,
+                mManagerStatusListenerMock);
+        executeAndValidateInitializationSequence();
+        executeAndValidateStartupSequence(2, true);
+    }
+
+    /**
+     * Test start HAL fails after multiple retry failures.
+     */
+    @Test
+    public void testStartHalRetryFailUponMultipleNotAvailableFailures() throws Exception {
+        // Override the stubbing for mWifiMock in before().
+        when(mWifiMock.start()).thenReturn(getStatus(WifiStatusCode.ERROR_NOT_AVAILABLE));
+
+        BaselineChip chipMock = new BaselineChip();
+        chipMock.initialize();
+        mInOrder = inOrder(mServiceManagerMock, mWifiMock, chipMock.chip);
+        executeAndValidateInitializationSequence();
+        executeAndValidateStartupSequence(START_HAL_RETRY_TIMES + 1, false);
+    }
+
+    /**
+     * Test start HAL fails after multiple retry failures.
+     */
+    @Test
+    public void testStartHalRetryFailUponTrueFailure() throws Exception {
+        // Override the stubbing for mWifiMock in before().
+        when(mWifiMock.start()).thenReturn(getStatus(WifiStatusCode.ERROR_UNKNOWN));
+
+        BaselineChip chipMock = new BaselineChip();
+        chipMock.initialize();
+        mInOrder = inOrder(mServiceManagerMock, mWifiMock, chipMock.chip);
+        executeAndValidateInitializationSequence();
+        executeAndValidateStartupSequence(1, false);
+    }
+
     // utilities
     private void dumpDut(String prefix) {
         StringWriter sw = new StringWriter();
@@ -1026,20 +1076,27 @@ public class HalDeviceManagerTest {
         collector.checkThat("isReady is true", mDut.isReady(), equalTo(true));
     }
 
-    private void executeAndValidateStartupSequence() throws Exception {
+    private void executeAndValidateStartupSequence()throws Exception {
+        executeAndValidateStartupSequence(1, true);
+    }
+
+    private void executeAndValidateStartupSequence(int numAttempts, boolean success)
+            throws Exception {
         // act: register listener & start Wi-Fi
         mDut.registerStatusListener(mManagerStatusListenerMock, mTestLooper.getLooper());
-        mDut.start();
+        collector.checkThat(mDut.start(), equalTo(success));
 
         // verify
-        mInOrder.verify(mWifiMock).start();
+        mInOrder.verify(mWifiMock, times(numAttempts)).start();
 
-        // act: trigger onStart callback of IWifiEventCallback
-        mWifiEventCallbackCaptor.getValue().onStart();
-        mTestLooper.dispatchAll();
+        if (success) {
+            // act: trigger onStart callback of IWifiEventCallback
+            mWifiEventCallbackCaptor.getValue().onStart();
+            mTestLooper.dispatchAll();
 
-        // verify: onStart called on registered listener
-        mInOrder.verify(mManagerStatusListenerMock).onStatusChanged();
+            // verify: onStart called on registered listener
+            mInOrder.verify(mManagerStatusListenerMock).onStatusChanged();
+        }
     }
 
     private IWifiIface validateInterfaceSequence(ChipMockBase chipMock,