WifiWatchdogService refactor
Minor refactoring of WatchdogService ahead of logic changes.
Lightly tested on a stingray.
Change-Id: I051603a598fe3162f170fb0a09e461fcb9b0038e
diff --git a/services/java/com/android/server/WifiWatchdogService.java b/services/java/com/android/server/WifiWatchdogService.java
index 6c7895e..56bfbe0 100644
--- a/services/java/com/android/server/WifiWatchdogService.java
+++ b/services/java/com/android/server/WifiWatchdogService.java
@@ -78,9 +78,9 @@
*/
public class WifiWatchdogService {
private static final String TAG = "WifiWatchdogService";
- private static final boolean V = false || false;
- private static final boolean D = true || false;
-
+ private static final boolean V = false;
+ private static final boolean D = true;
+
private Context mContext;
private ContentResolver mContentResolver;
private WifiManager mWifiManager;
@@ -108,17 +108,16 @@
private String mSsid;
/**
* The number of access points in the current network ({@link #mSsid}) that
- * have been checked. Only touched in the main thread!
+ * have been checked. Only touched in the main thread, using getter/setter methods.
*/
- private int mNumApsChecked;
+ private int mBssidCheckCount;
/** Whether the current AP check should be canceled. */
private boolean mShouldCancel;
-
+
WifiWatchdogService(Context context) {
mContext = context;
mContentResolver = context.getContentResolver();
mWifiManager = (WifiManager) context.getSystemService(Context.WIFI_SERVICE);
-
createThread();
// The content observer to listen needs a handler, which createThread creates
@@ -410,6 +409,9 @@
* Successful "ignored" pings are *not* ignored (they count in the total number
* of pings), but failures are really ignored.
*/
+
+ // TODO: This is confusing logic and should be rewitten
+ // Here, successful 'ignored' pings are interpreted as a success in the below loop
pingCounter++;
successCounter++;
}
@@ -451,6 +453,7 @@
}
}
+ //TODO: Integer division might cause problems down the road...
int packetLossPercentage = 100 * (numPings - successCounter) / numPings;
if (D) {
Slog.d(TAG, packetLossPercentage
@@ -470,7 +473,7 @@
return false;
}
- if (false && V) {
+ if (V) {
myLogV("backgroundCheckDnsConnectivity: Background checking " +
dns.getHostAddress() + " for connectivity");
}
@@ -731,8 +734,8 @@
* Checks to make sure we haven't exceeded the max number of checks
* we're allowed per network
*/
- mNumApsChecked++;
- if (mNumApsChecked > getMaxApChecks()) {
+ incrementBssidCheckCount();
+ if (getBssidCheckCount() > getMaxApChecks()) {
if (V) {
Slog.v(TAG, " Passed the max attempts (" + getMaxApChecks()
+ "), going to sleep for " + mSsid);
@@ -823,7 +826,7 @@
// Reset the cancel state since this is the entry point of this action
mShouldCancel = false;
- if (false && V) {
+ if (V) {
myLogV("handleBackgroundCheckAp: AccessPoint: " + ap);
}
@@ -962,7 +965,7 @@
if (forceIdleState || (mState != WatchdogState.SLEEP)) {
mState = WatchdogState.IDLE;
}
- mNumApsChecked = 0;
+ resetBssidCheckCount();
}
/**
@@ -990,6 +993,18 @@
SWITCHING_AP
}
+ private int getBssidCheckCount() {
+ return mBssidCheckCount;
+ }
+
+ private void incrementBssidCheckCount() {
+ mBssidCheckCount++;
+ }
+
+ private void resetBssidCheckCount() {
+ this.mBssidCheckCount = 0;
+ }
+
/**
* The main thread for the watchdog monitoring. This will be turned into a
* {@link Looper} thread.
@@ -1120,6 +1135,9 @@
@Override
public void handleMessage(Message msg) {
+ if (V) {
+ myLogV("handleMessage: " + msg.what);
+ }
switch (msg.what) {
case MESSAGE_NETWORK_CHANGED:
handleNetworkChanged((String) msg.obj);
@@ -1263,34 +1281,36 @@
/**
* Describes an access point by its SSID and BSSID.
+ *
*/
private static class AccessPoint {
String ssid;
String bssid;
+ /**
+ * @param ssid cannot be null
+ * @param bssid cannot be null
+ */
AccessPoint(String ssid, String bssid) {
+ if (ssid == null || bssid == null) {
+ Slog.e(TAG, String.format("(%s) INVALID ACCESSPOINT: (%s, %s)",
+ Thread.currentThread().getName(),ssid,bssid));
+ }
this.ssid = ssid;
this.bssid = bssid;
}
-
- private boolean hasNull() {
- return ssid == null || bssid == null;
- }
@Override
public boolean equals(Object o) {
if (!(o instanceof AccessPoint)) return false;
AccessPoint otherAp = (AccessPoint) o;
- boolean iHaveNull = hasNull();
+
// Either we both have a null, or our SSIDs and BSSIDs are equal
- return (iHaveNull && otherAp.hasNull()) ||
- (otherAp.bssid != null && ssid.equals(otherAp.ssid)
- && bssid.equals(otherAp.bssid));
+ return ssid.equals(otherAp.ssid) && bssid.equals(otherAp.bssid);
}
@Override
public int hashCode() {
- if (ssid == null || bssid == null) return 0;
return ssid.hashCode() + bssid.hashCode();
}
@@ -1365,7 +1385,7 @@
return false;
} catch (Exception e) {
- if (V || false) {
+ if (V) {
Slog.d(TAG, "DnsPinger.isReachable got an unknown exception", e);
}
return false;