From 22015e251b013a6b41d83f5c6e93324a7723dd38 Mon Sep 17 00:00:00 2001 From: Quang Luong Date: Fri, 27 Dec 2019 14:40:23 -0800 Subject: Split WifiEntryCallback into separate callbacks for each action WifiEntryCallback has callback methods for a general WifiEntry update and for returning the results of certain actions such as connect and disconnect. WifiEntryPreference in Settings needs to listen on WifiEntryCallback.onUpdated() to update the entry in the display, but other clients need to listen on the result of the other actions in order to display a toast or prompt the user for a password, etc. Thus, these callback methods must be separated from onUpdated(). Bug: 70983952 Test: atest WifiTrackerLibTests Change-Id: I2580fd9c8939bac32144bb12e6cdc6f2f7779b98 --- .../android/wifitrackerlib/StandardWifiEntry.java | 74 ++++++++---- .../src/com/android/wifitrackerlib/WifiEntry.java | 129 ++++++++++----------- .../wifitrackerlib/StandardWifiEntryTest.java | 11 +- 3 files changed, 123 insertions(+), 91 deletions(-) diff --git a/libs/WifiTrackerLib/src/com/android/wifitrackerlib/StandardWifiEntry.java b/libs/WifiTrackerLib/src/com/android/wifitrackerlib/StandardWifiEntry.java index 5ec8e3712..244ba0224 100644 --- a/libs/WifiTrackerLib/src/com/android/wifitrackerlib/StandardWifiEntry.java +++ b/libs/WifiTrackerLib/src/com/android/wifitrackerlib/StandardWifiEntry.java @@ -57,6 +57,9 @@ class StandardWifiEntry extends WifiEntry { @Nullable private WifiConfiguration mWifiConfig; @Nullable private NetworkInfo mNetworkInfo; @Nullable private WifiInfo mWifiInfo; + @Nullable private ConnectCallback mConnectCallback; + @Nullable private DisconnectCallback mDisconnectCallback; + @Nullable private ForgetCallback mForgetCallback; private boolean mCalledConnect = false; private boolean mCalledDisconnect = false; @@ -216,7 +219,8 @@ class StandardWifiEntry extends WifiEntry { } @Override - public void connect() { + public void connect(@Nullable ConnectCallback callback) { + mConnectCallback = callback; if (mWifiConfig == null) { // Unsaved network if (mSecurity == SECURITY_NONE @@ -235,14 +239,18 @@ class StandardWifiEntry extends WifiEntry { } else { connectConfig.allowedKeyManagement.set(WifiConfiguration.KeyMgmt.NONE); } - mWifiManager.connect(connectConfig, new ConnectListener()); + mWifiManager.connect(connectConfig, new ConnectActionListener()); } else { // Secure network - notifyOnConnectResult(WifiEntryCallback.CONNECT_STATUS_FAILURE_NO_CONFIG); + if (callback != null) { + mCallbackHandler.post(() -> + callback.onConnectResult( + ConnectCallback.CONNECT_STATUS_FAILURE_NO_CONFIG)); + } } } else { // Saved network - mWifiManager.connect(mWifiConfig.networkId, new ConnectListener()); + mWifiManager.connect(mWifiConfig.networkId, new ConnectActionListener()); } } @@ -252,13 +260,14 @@ class StandardWifiEntry extends WifiEntry { } @Override - public void disconnect() { + public void disconnect(@Nullable DisconnectCallback callback) { if (canDisconnect()) { mCalledDisconnect = true; + mDisconnectCallback = callback; mCallbackHandler.postDelayed(() -> { - if (mCalledDisconnect) { - notifyOnDisconnectResult( - WifiEntryCallback.DISCONNECT_STATUS_FAILURE_UNKNOWN); + if (callback != null && mCalledDisconnect) { + callback.onDisconnectResult( + DisconnectCallback.DISCONNECT_STATUS_FAILURE_UNKNOWN); } }, 10_000 /* delayMillis */); mWifiManager.disconnect(); @@ -271,9 +280,10 @@ class StandardWifiEntry extends WifiEntry { } @Override - public void forget() { + public void forget(@Nullable ForgetCallback callback) { if (mWifiConfig != null) { - mWifiManager.forget(mWifiConfig.networkId, new ForgetListener()); + mForgetCallback = callback; + mWifiManager.forget(mWifiConfig.networkId, new ForgetActionListener()); } } @@ -283,7 +293,7 @@ class StandardWifiEntry extends WifiEntry { } @Override - public void signIn() { + public void signIn(@Nullable SignInCallback callback) { // TODO(b/70983952): Fill this method in } @@ -470,14 +480,23 @@ class StandardWifiEntry extends WifiEntry { } if (mCalledConnect && getConnectedState() == CONNECTED_STATE_CONNECTED) { mCalledConnect = false; - notifyOnConnectResult(WifiEntryCallback.CONNECT_STATUS_SUCCESS); + mCallbackHandler.post(() -> { + if (mConnectCallback != null) { + mConnectCallback.onConnectResult(ConnectCallback.CONNECT_STATUS_SUCCESS); + } + }); } } else { mNetworkInfo = null; } if (mCalledDisconnect && getConnectedState() == CONNECTED_STATE_DISCONNECTED) { mCalledDisconnect = false; - notifyOnDisconnectResult(WifiEntryCallback.DISCONNECT_STATUS_SUCCESS); + mCallbackHandler.post(() -> { + if (mDisconnectCallback != null) { + mDisconnectCallback.onDisconnectResult( + DisconnectCallback.DISCONNECT_STATUS_SUCCESS); + } + }); } notifyOnUpdated(); } @@ -496,14 +515,16 @@ class StandardWifiEntry extends WifiEntry { + getSecurityFromWifiConfiguration(config); } - class ConnectListener implements WifiManager.ActionListener { + private class ConnectActionListener implements WifiManager.ActionListener { @Override public void onSuccess() { mCalledConnect = true; // If we aren't connected to the network after 10 seconds, trigger the failure callback mCallbackHandler.postDelayed(() -> { - if (mCalledConnect && getConnectedState() == CONNECTED_STATE_DISCONNECTED) { - notifyOnConnectResult(WifiEntryCallback.CONNECT_STATUS_FAILURE_UNKNOWN); + if (mConnectCallback != null && mCalledConnect + && getConnectedState() == CONNECTED_STATE_DISCONNECTED) { + mConnectCallback.onConnectResult( + ConnectCallback.CONNECT_STATUS_FAILURE_UNKNOWN); mCalledConnect = false; } }, 10_000 /* delayMillis */); @@ -511,19 +532,32 @@ class StandardWifiEntry extends WifiEntry { @Override public void onFailure(int i) { - notifyOnConnectResult(WifiEntryCallback.CONNECT_STATUS_FAILURE_UNKNOWN); + mCallbackHandler.post(() -> { + if (mConnectCallback != null) { + mConnectCallback.onConnectResult( + mConnectCallback.CONNECT_STATUS_FAILURE_UNKNOWN); + } + }); } } - class ForgetListener implements WifiManager.ActionListener { + class ForgetActionListener implements WifiManager.ActionListener { @Override public void onSuccess() { - notifyOnForgetResult(WifiEntryCallback.FORGET_STATUS_SUCCESS); + mCallbackHandler.post(() -> { + if (mForgetCallback != null) { + mForgetCallback.onForgetResult(ForgetCallback.FORGET_STATUS_SUCCESS); + } + }); } @Override public void onFailure(int i) { - notifyOnForgetResult(WifiEntryCallback.FORGET_STATUS_FAILURE_UNKNOWN); + mCallbackHandler.post(() -> { + if (mForgetCallback != null) { + mForgetCallback.onForgetResult(ForgetCallback.FORGET_STATUS_FAILURE_UNKNOWN); + } + }); } } } diff --git a/libs/WifiTrackerLib/src/com/android/wifitrackerlib/WifiEntry.java b/libs/WifiTrackerLib/src/com/android/wifitrackerlib/WifiEntry.java index 7bf94b5c5..4752e8449 100644 --- a/libs/WifiTrackerLib/src/com/android/wifitrackerlib/WifiEntry.java +++ b/libs/WifiTrackerLib/src/com/android/wifitrackerlib/WifiEntry.java @@ -26,6 +26,7 @@ import androidx.annotation.AnyThread; import androidx.annotation.IntDef; import androidx.annotation.MainThread; import androidx.annotation.NonNull; +import androidx.annotation.Nullable; import java.lang.annotation.Retention; import java.lang.annotation.RetentionPolicy; @@ -226,22 +227,22 @@ public abstract class WifiEntry implements Comparable { /** Returns whether the entry should show a connect option */ public abstract boolean canConnect(); /** Connects to the network */ - public abstract void connect(); + public abstract void connect(@Nullable ConnectCallback callback); /** Returns whether the entry should show a disconnect option */ public abstract boolean canDisconnect(); /** Disconnects from the network */ - public abstract void disconnect(); + public abstract void disconnect(@Nullable DisconnectCallback callback); /** Returns whether the entry should show a forget option */ public abstract boolean canForget(); /** Forgets the network */ - public abstract void forget(); + public abstract void forget(@Nullable ForgetCallback callback); /** Returns whether the network can be signed-in to */ public abstract boolean canSignIn(); /** Sign-in to the network. For captive portals. */ - public abstract void signIn(); + public abstract void signIn(@Nullable SignInCallback callback); /** Returns whether the network can be shared via QR code */ public abstract boolean canShare(); @@ -295,10 +296,30 @@ public abstract class WifiEntry implements Comparable { } /** - * Listener for changes to the state of the WifiEntry or the result of actions on the WifiEntry. - * These callbacks will be invoked on the main thread. + * Listener for changes to the state of the WifiEntry. + * This callback will be invoked on the main thread. */ public interface WifiEntryCallback { + /** + * Indicates the state of the WifiEntry has changed and clients may retrieve updates through + * the WifiEntry getter methods. + */ + @MainThread + void onUpdated(); + } + + @AnyThread + protected void notifyOnUpdated() { + if (mListener != null) { + mCallbackHandler.post(() -> mListener.onUpdated()); + } + } + + /** + * Listener for changes to the state of the WifiEntry. + * This callback will be invoked on the main thread. + */ + public interface ConnectCallback { @Retention(RetentionPolicy.SOURCE) @IntDef(value = { CONNECT_STATUS_SUCCESS, @@ -312,6 +333,18 @@ public abstract class WifiEntry implements Comparable { int CONNECT_STATUS_FAILURE_NO_CONFIG = 1; int CONNECT_STATUS_FAILURE_UNKNOWN = 2; + /** + * Result of the connect request indicated by the CONNECT_STATUS constants. + */ + @MainThread + void onConnectResult(@ConnectStatus int status); + } + + /** + * Listener for changes to the state of the WifiEntry. + * This callback will be invoked on the main thread. + */ + public interface DisconnectCallback { @Retention(RetentionPolicy.SOURCE) @IntDef(value = { DISCONNECT_STATUS_SUCCESS, @@ -322,7 +355,18 @@ public abstract class WifiEntry implements Comparable { int DISCONNECT_STATUS_SUCCESS = 0; int DISCONNECT_STATUS_FAILURE_UNKNOWN = 1; + /** + * Result of the disconnect request indicated by the DISCONNECT_STATUS constants. + */ + @MainThread + void onDisconnectResult(@DisconnectStatus int status); + } + /** + * Listener for changes to the state of the WifiEntry. + * This callback will be invoked on the main thread. + */ + public interface ForgetCallback { @Retention(RetentionPolicy.SOURCE) @IntDef(value = { FORGET_STATUS_SUCCESS, @@ -334,6 +378,18 @@ public abstract class WifiEntry implements Comparable { int FORGET_STATUS_SUCCESS = 0; int FORGET_STATUS_FAILURE_UNKNOWN = 1; + /** + * Result of the forget request indicated by the FORGET_STATUS constants. + */ + @MainThread + void onForgetResult(@ForgetStatus int status); + } + + /** + * Listener for changes to the state of the WifiEntry. + * This callback will be invoked on the main thread. + */ + public interface SignInCallback { @Retention(RetentionPolicy.SOURCE) @IntDef(value = { SIGNIN_STATUS_SUCCESS, @@ -345,31 +401,6 @@ public abstract class WifiEntry implements Comparable { int SIGNIN_STATUS_SUCCESS = 0; int SIGNIN_STATUS_FAILURE_UNKNOWN = 1; - /** - * Indicates the state of the WifiEntry has changed and clients may retrieve updates through - * the WifiEntry getter methods. - */ - @MainThread - void onUpdated(); - - /** - * Result of the connect request indicated by the CONNECT_STATUS constants. - */ - @MainThread - void onConnectResult(@ConnectStatus int status); - - /** - * Result of the disconnect request indicated by the DISCONNECT_STATUS constants. - */ - @MainThread - void onDisconnectResult(@DisconnectStatus int status); - - /** - * Result of the forget request indicated by the FORGET_STATUS constants. - */ - @MainThread - void onForgetResult(@ForgetStatus int status); - /** * Result of the sign-in request indicated by the SIGNIN_STATUS constants. */ @@ -377,6 +408,7 @@ public abstract class WifiEntry implements Comparable { void onSignInResult(@SignInStatus int status); } + // TODO (b/70983952) Come up with a sorting scheme that does the right thing. @Override public int compareTo(@NonNull WifiEntry other) { @@ -416,39 +448,4 @@ public abstract class WifiEntry implements Comparable { .append(getSecurity()) .toString(); } - - @AnyThread - protected void notifyOnUpdated() { - if (mListener != null) { - mCallbackHandler.post(() -> mListener.onUpdated()); - } - } - - @AnyThread - protected void notifyOnConnectResult(@WifiEntryCallback.ConnectStatus int status) { - if (mListener != null) { - mCallbackHandler.post(() -> mListener.onConnectResult(status)); - } - } - - @AnyThread - protected void notifyOnDisconnectResult(@WifiEntryCallback.DisconnectStatus int status) { - if (mListener != null) { - mCallbackHandler.post(() -> mListener.onDisconnectResult(status)); - } - } - - @AnyThread - protected void notifyOnForgetResult(@WifiEntryCallback.ForgetStatus int status) { - if (mListener != null) { - mCallbackHandler.post(() -> mListener.onForgetResult(status)); - } - } - - @AnyThread - protected void notifyOnSignInResult(@WifiEntryCallback.SignInStatus int status) { - if (mListener != null) { - mCallbackHandler.post(() -> mListener.onSignInResult(status)); - } - } } diff --git a/libs/WifiTrackerLib/tests/src/com/android/wifitrackerlib/StandardWifiEntryTest.java b/libs/WifiTrackerLib/tests/src/com/android/wifitrackerlib/StandardWifiEntryTest.java index e4ddaa364..ccaeebddc 100644 --- a/libs/WifiTrackerLib/tests/src/com/android/wifitrackerlib/StandardWifiEntryTest.java +++ b/libs/WifiTrackerLib/tests/src/com/android/wifitrackerlib/StandardWifiEntryTest.java @@ -57,6 +57,7 @@ public class StandardWifiEntryTest { public static final int BAD_LEVEL = 1; @Mock private WifiEntry.WifiEntryCallback mMockListener; + @Mock private WifiEntry.ConnectCallback mMockConnectCallback; @Mock private WifiManager mMockWifiManager; @Mock private WifiInfo mMockWifiInfo; @Mock private NetworkInfo mMockNetworkInfo; @@ -353,7 +354,7 @@ public class StandardWifiEntryTest { config.networkId = 1; entry.updateConfig(config); - entry.connect(); + entry.connect(null /* ConnectCallback */); verify(mMockWifiManager, times(1)).connect(eq(1), any()); } @@ -364,7 +365,7 @@ public class StandardWifiEntryTest { buildScanResult("ssid", "bssid0", 0, GOOD_RSSI)), mMockWifiManager); - entry.connect(); + entry.connect(null /* ConnectCallback */); verify(mMockWifiManager, times(1)).connect(any(), any()); } @@ -378,11 +379,11 @@ public class StandardWifiEntryTest { mMockWifiManager); entry.setListener(mMockListener); - entry.connect(); + entry.connect(mMockConnectCallback); mTestLooper.dispatchAll(); - verify(mMockListener, times(1)) - .onConnectResult(WifiEntry.WifiEntryCallback.CONNECT_STATUS_FAILURE_NO_CONFIG); + verify(mMockConnectCallback, times(1)) + .onConnectResult(WifiEntry.ConnectCallback.CONNECT_STATUS_FAILURE_NO_CONFIG); } @Test -- cgit v1.2.3