Merge pull request #1344 from garvankeeley/1338-sensortests-followup

Follow up patch to #1338 PR

Author: crankycoder

android/src/main/java/org/mozilla/mozstumbler/service/stumblerthread/motiondetection/LocationChangeSensor.java

@@ -33,31 +33,23 @@ public class LocationChangeSensor extends BroadcastReceiver {
     private final Context mContext;
     private final Handler mHandler = new Handler();
     private ISystemClock sysClock;
-
     private int mPrefMotionChangeDistanceMeters;
     private long mPrefMotionChangeTimeWindowMs;
     private long mStartTimeMs;
     private boolean mDoSingleLocationCheck;
     public static String ACTION_LOCATION_NOT_CHANGING = AppGlobals.ACTION_NAMESPACE + ".LOCATION_UNCHANGING";
-
-    // attributes used to track the state of the LocationChangeSensor
-    boolean checkTimeScheduled = false;
-    Location mLastLocation;
+    private Location mLastLocation;
 
     private final Runnable mCheckTimeout = new Runnable() {
         public void run() {
-            try {
-                if (isTimeWindowForMovementExceeded()) {
-                    AppGlobals.guiLogInfo("No GPS in time window.");
-                    Log.d(LOG_TAG, "No GPS in time window.");
-                    LocalBroadcastManager.getInstance(mContext).sendBroadcastSync(new Intent(ACTION_LOCATION_NOT_CHANGING));
-                    return;
-                }
-                Log.d(LOG_TAG, "We're getting GPS readings in a timely manner. Nothing to see here.");
-                scheduleTimeoutCheck();
-            } finally {
-                checkTimeScheduled = false;
+            if (isTimeWindowForMovementExceeded()) {
+                AppGlobals.guiLogInfo("No GPS in time window.");
+                Log.d(LOG_TAG, "No GPS in time window.");
+                LocalBroadcastManager.getInstance(mContext).sendBroadcastSync(new Intent(ACTION_LOCATION_NOT_CHANGING));
+                return;
             }
+            Log.d(LOG_TAG, "We're getting GPS readings in a timely manner. Nothing to see here.");
+            scheduleTimeoutCheck(mPrefMotionChangeTimeWindowMs);
         }
     };
 
@@ -111,11 +103,12 @@ public void start() {
         IntentFilter intentFilter = new IntentFilter();
         intentFilter.addAction(GPSScanner.ACTION_GPS_UPDATED);
         LocalBroadcastManager.getInstance(mContext).registerReceiver(this, intentFilter);
-        mHandler.postDelayed(mCheckTimeout, mPrefMotionChangeTimeWindowMs);
+
+        scheduleTimeoutCheck(mPrefMotionChangeTimeWindowMs);
     }
 
     public void stop() {
-        mHandler.removeCallbacks(mCheckTimeout);
+        removeTimeoutCheck();
         try {
             LocalBroadcastManager.getInstance(mContext).unregisterReceiver(this);
         } catch (Exception e) {}
@@ -162,20 +155,28 @@ public void onReceive(Context context, Intent intent) {
         }
 
         mDoSingleLocationCheck = false;
-        scheduleTimeoutCheck();
+        scheduleTimeoutCheck(mPrefMotionChangeTimeWindowMs);
     }
 
-    private void scheduleTimeoutCheck() {
+    private void scheduleTimeoutCheck(long delay) {
+        removeTimeoutCheck();
+
+        // Don't schedule it for an exact delay, we want it slightly after this timeout, as the OS can
+        // trigger this earlier than requested (by a fraction of a second).
+        final long addedDelay = 2 * 1000;
+        mHandler.postDelayed(mCheckTimeout, delay + addedDelay);
+
+        Log.d(LOG_TAG, "Scheduled timeout check for " + (delay / 1000) + " seconds");
+    }
+
+    boolean removeTimeoutCheck() {
+        boolean wasScheduled = false;
         try {
             mHandler.removeCallbacks(mCheckTimeout);
+            wasScheduled = true;
         } catch (Exception e) {}
 
-        // Don't schedule it for exactly mPrefMotionChangeTimeWindowMs, we want it slightly after this timeout
-        final long addedDelay = 2 * 1000;
-        Log.d(LOG_TAG, "Scheduled timeout check for " + (addedDelay/1000) + " seconds");
-
-        checkTimeScheduled = true;
-        mHandler.postDelayed(mCheckTimeout, mPrefMotionChangeTimeWindowMs + addedDelay);
+        return wasScheduled;
     }
 
     public void quickCheckForFalsePositiveAfterMotionSensorMovement() {
@@ -187,6 +188,11 @@ public void quickCheckForFalsePositiveAfterMotionSensorMovement() {
         mDoSingleLocationCheck = true;
         Log.d(LOG_TAG, "Scheduled timeout check for " + (kWaitTimeMs/1000) + " seconds");
 
-        mHandler.postDelayed(mCheckTimeout, kWaitTimeMs);
+        scheduleTimeoutCheck(kWaitTimeMs);
     }
+
+    Location testing_getLastLocation() {
+        return mLastLocation;
+    }
+
 }

android/src/main/java/org/mozilla/mozstumbler/service/stumblerthread/scanners/cellscanner/CellInfo.java

@@ -269,7 +269,6 @@ void setNetworkOperator(String mccMnc) {
     }
 
     static String getCellRadioTypeName(int networkType) {
-        Log.d(LOG_TAG, "getCellRadioTypeName("+networkType+")");
         switch (networkType) {
             // If the network is either GSM or any high-data-rate variant of it, the radio
             // field should be specified as `gsm`. This includes `GSM`, `EDGE` and `GPRS`.

android/src/test/java/org/mozilla/mozstumbler/service/stumblerthread/motiondetection/LocationChangeSensorTest.java

@@ -37,7 +37,6 @@ public class LocationChangeSensorTest {
 
     private static final String LOG_TAG = AppGlobals.makeLogTag(LocationChangeSensorTest.class);
     private LocationChangeSensor locationChangeSensor;
-
     private LinkedList<Intent> receivedIntent = new LinkedList<Intent>();
 
     // After DetectUnchangingLocation reports the user is not moving, and the scanning pauses,
@@ -92,11 +91,11 @@ public void testStartMotionDetectionAfterFirstGPSLock() {
         // we are waiting for mPrefMotionChangeTimeWindowMs for the next GPS signal or else
         // we go to sleep.
 
-        assertFalse(locationChangeSensor.checkTimeScheduled);
+        locationChangeSensor.removeTimeoutCheck();
 
         Intent intent = getLocationIntent(0, 0);
         locationChangeSensor.onReceive(ctx, intent);
-        assertTrue(locationChangeSensor.checkTimeScheduled);
+        assertTrue(locationChangeSensor.removeTimeoutCheck());
     }
 
     @Test
@@ -110,19 +109,21 @@ public void testNotFirstGPSFix_MovedDistance() {
         // but the person hasn't actually moved geographically.  Keep the scanners on
         // and schedule the next timeout check to see if the user just stops moving around.
 
-        assertFalse(locationChangeSensor.checkTimeScheduled);
-
         intent = getLocationIntent(20, 30);
         expectedPosition = intent.getParcelableExtra(GPSScanner.NEW_LOCATION_ARG_LOCATION);
         locationChangeSensor.onReceive(ctx, intent);
-        assertEquals(expectedPosition, locationChangeSensor.mLastLocation);
+        assertEquals(expectedPosition, locationChangeSensor.testing_getLastLocation());
 
         intent = getLocationIntent(21, 30);
         locationChangeSensor.onReceive(ctx, intent);
-
         // The new recorded position should be 21, 30
         expectedPosition = intent.getParcelableExtra(GPSScanner.NEW_LOCATION_ARG_LOCATION);
-        assertEquals(expectedPosition, locationChangeSensor.mLastLocation);
+        assertEquals(expectedPosition, locationChangeSensor.testing_getLastLocation());
+
+        intent = getLocationIntent(21.000001, 30);
+        locationChangeSensor.onReceive(ctx, intent);
+        // The new recorded position should be unchanged, movement too small
+        assertEquals(expectedPosition, locationChangeSensor.testing_getLastLocation());
     }
 
     @Test
@@ -136,17 +137,14 @@ public void testNotFirstGPSFix_LongWaitForSecondFix() {
         Intent intent;
         Location expectedPosition;
 
-
         Robolectric.runUiThreadTasksIncludingDelayedTasks();
 
-        assertFalse(locationChangeSensor.checkTimeScheduled);
-
         intent = getLocationIntent(20, 30);
         expectedPosition = intent.getParcelableExtra(GPSScanner.NEW_LOCATION_ARG_LOCATION);
         locationChangeSensor.onReceive(ctx, intent);
         Robolectric.runUiThreadTasksIncludingDelayedTasks();
 
-        assertEquals(expectedPosition, locationChangeSensor.mLastLocation);
+        assertEquals(expectedPosition, locationChangeSensor.testing_getLastLocation());
 
         intent = getLocationIntent(20.01, 30.01);
 
@@ -156,7 +154,7 @@ public void testNotFirstGPSFix_LongWaitForSecondFix() {
         Robolectric.runUiThreadTasksIncludingDelayedTasks();
 
         // The new recorded position should not be changed!
-        assertEquals(expectedPosition, locationChangeSensor.mLastLocation);
+        assertEquals(expectedPosition, locationChangeSensor.testing_getLastLocation());
 
         boolean foundIntent = false;