Merging Sandor's changes

MyTracks/src/com/google/android/apps/mytracks/services/PeriodicTaskExecuter.java

@@ -25,7 +25,7 @@ import java.util.Timer;
 import java.util.TimerTask;
 
 /**
- * This class will periodically announce the user's trip statitics.
+ * This class will periodically announce the user's trip statistics.
  *
  * @author Sandor Dornbush
  */
@@ -52,6 +52,11 @@ public class PeriodicTaskExecuter {
    * @param interval The interval in milliseconds
    */
   public void scheduleTask(long interval) {
+    // TODO: Decouple service from this class once and forever.
+    if (!service.isRecording()) {
+      return;
+    }
+    
     timer.cancel();
     timer.purge();
     timer = new Timer();
@@ -61,12 +66,14 @@ public class PeriodicTaskExecuter {
 
     long now = System.currentTimeMillis();
     long next = service.getTripStatistics().getStartTime();
-    while (next < now) next += interval;
+    if (next < now) {
+      next = now + interval - ((now - next) % interval);
+    }
 
     Date start = new Date(next);
     Log.i(MyTracksConstants.TAG,
-          "StatusAnnouncer scheduled to start at " + start + " every "
+        task.getClass().getSimpleName() + " scheduled to start at " + start
-          + interval + " milliseconds.");
+        + " every " + interval + " milliseconds.");
     timer.scheduleAtFixedRate(new PeriodicTimerTask(), start, interval);
   }
 
@@ -74,6 +81,8 @@ public class PeriodicTaskExecuter {
    * Cleans up this object.
    */
   public void shutdown() {
+    Log.i(MyTracksConstants.TAG,
+        task.getClass().getSimpleName() + " shutting down.");
     timer.cancel();
     timer.purge();
     timer = null;

MyTracks/src/com/google/android/apps/mytracks/services/SplitManager.java

@@ -73,6 +73,11 @@ public class SplitManager {
    * Calculates the next distance that a split should be inserted at.
    */
   public void calculateNextSplit() {
+    // TODO: Decouple service from this class once and forever.
+    if (!service.isRecording()) {
+      return;
+    }
+    
     if (splitFrequency >= 0) {
       nextSplitDistance = Double.MAX_VALUE;
       Log.d(MyTracksConstants.TAG,
@@ -121,6 +126,12 @@ public class SplitManager {
    */
   public void setSplitFrequency(int splitFrequency) {
     this.splitFrequency = splitFrequency;
+    
+    // TODO: Decouple service from this class once and forever.
+    if (!service.isRecording()) {
+      return;
+    }
+    
     if (splitFrequency < 1) {
       if (splitExecuter != null) {
         splitExecuter.shutdown();

MyTracks/src/com/google/android/apps/mytracks/services/StatusAnnouncerTask.java

@@ -239,8 +239,10 @@ public class StatusAnnouncerTask implements PeriodicTask {
     // Stop listening to phone state.
     listenToPhoneState(phoneListener, PhoneStateListener.LISTEN_NONE);
 
-    tts.shutdown();
+    if (tts != null) {
-    tts = null;
+      tts.shutdown();
+      tts = null;
+    }
   }
 
   /**

MyTracks/src/com/google/android/apps/mytracks/services/TrackRecordingService.java

@@ -663,9 +663,11 @@ public class TrackRecordingService extends Service implements LocationListener {
       restoreStats(recordingTrack);
       isRecording = true;
     } else {
-      // Make sure we have consistent state in shared preferences.
+      if (recordingTrackId != -1) {
-      Log.w(MyTracksConstants.TAG, "TrackRecordingService.onCreate: Resetting "
+        // Make sure we have consistent state in shared preferences.
-          + "an orphaned recording track: " + recordingTrackId);
+        Log.w(MyTracksConstants.TAG, "TrackRecordingService.onCreate: "
+            + "Resetting an orphaned recording track = " + recordingTrackId);
+      }
       prefManager.setRecordingTrack(recordingTrackId = -1);
     }
     showNotification();
@@ -677,7 +679,9 @@ public class TrackRecordingService extends Service implements LocationListener {
    * the announcements, otherwise this method is no-op.
    */
   private void setUpAnnouncer() {
-    if (announcementFrequency != -1) {
+    Log.d(MyTracksConstants.TAG, "TrackRecordingService.setUpAnnouncer: "
+        + announcementExecuter);
+    if (announcementFrequency != -1 && recordingTrackId != -1) {
       if (announcementExecuter == null) {
         StatusAnnouncerFactory statusAnnouncerFactory =
             new StatusAnnouncerFactory(ApiFeatures.getInstance());
@@ -689,6 +693,18 @@ public class TrackRecordingService extends Service implements LocationListener {
       announcementExecuter.scheduleTask(announcementFrequency * 60000);
     }
   }
+  
+  private void shutdownAnnouncer() {
+    Log.d(MyTracksConstants.TAG, "TrackRecordingService.shutdownAnnouncer: "
+        + announcementExecuter);
+    if (announcementExecuter != null) {
+      try {
+        announcementExecuter.shutdown();
+      } finally {
+        announcementExecuter = null;
+      }
+    }
+  }
 
   @Override
   public void onDestroy() {
@@ -701,9 +717,7 @@ public class TrackRecordingService extends Service implements LocationListener {
     isRecording = false;
     showNotification();
     unregisterLocationListener();
-    if (announcementExecuter != null) {
+    shutdownAnnouncer();
-      announcementExecuter.shutdown();
-    }
     splitManager.shutdown();
     super.onDestroy();
   }
@@ -936,6 +950,7 @@ public class TrackRecordingService extends Service implements LocationListener {
           }
          
           isRecording = false;
+          shutdownAnnouncer();
           Track recordingTrack = providerUtils.getTrack(recordingTrackId);
           if (recordingTrack != null) {
             TripStatistics stats = recordingTrack.getStatistics();
@@ -1042,10 +1057,7 @@ public class TrackRecordingService extends Service implements LocationListener {
   public void setAnnouncementFrequency(int announcementFrequency) {
     this.announcementFrequency = announcementFrequency;
     if (announcementFrequency == -1) {
-      if (announcementExecuter != null) {
+      shutdownAnnouncer();
-        announcementExecuter.shutdown();
-        announcementExecuter = null;
-      }
     } else {
       setUpAnnouncer();
     }

MyTracksTest/src/com/google/android/apps/mytracks/services/TrackRecordingServiceTest.java

@@ -46,6 +46,11 @@ import java.util.List;
  * Tests for the MyTracks track recording service.
  * 
  * @author Bartlomiej Niechwiej
+ * 
+ * TODO: The original class, ServiceTestCase, has a few limitations, e.g.
+ * it's not possible to properly shutdown the service, unless tearDown()
+ * is called, which prevents from testing multiple scenarios in a single
+ * test (see runFunctionTest for more details).
  */
 public class TrackRecordingServiceTest
     extends ServiceTestCase<TrackRecordingService> {
@@ -141,6 +146,9 @@ public class TrackRecordingServiceTest
     
     sharedPreferences = context.getSharedPreferences(
         MyTracksSettings.SETTINGS_NAME, 0);
+    // Let's use default values.
+    sharedPreferences.edit().clear().commit();
+    
     // Disable auto resume by default.
     updateAutoResumePrefs(0, -1);
     // No recording track.
@@ -376,32 +384,7 @@ public class TrackRecordingServiceTest
   public void testIntegration_completeRecordingSession() throws Exception {
     List<Track> tracks = providerUtils.getAllTracks();
     assertTrue(tracks.isEmpty());
-    
+    fullRecordingSession();
-    ITrackRecordingService service = bindAndGetService(createStartIntent());
-    assertFalse(service.isRecording());
-    
-    // Start a track.
-    long id = service.startNewTrack();
-    assertTrue(id >= 0);
-    assertTrue(service.isRecording());
-    Track track = providerUtils.getTrack(id);
-    assertNotNull(track);
-    assertEquals(id, track.getId());
-    assertEquals(id, sharedPreferences.getLong(
-        context.getString(R.string.recording_track_key), -1));
-    assertEquals(id, service.getRecordingTrackId());
-    
-    // Stop the track.  Validate if it has correct data.
-    service.endCurrentTrack();
-    assertFalse(service.isRecording());
-    assertEquals(-1, service.getRecordingTrackId());
-    track = providerUtils.getTrack(id);
-    assertNotNull(track);
-    assertEquals(id, track.getId());
-    TripStatistics tripStatistics = track.getStatistics();
-    assertNotNull(tripStatistics);
-    assertTrue(tripStatistics.getStartTime() > 0);
-    assertTrue(tripStatistics.getStopTime() >= tripStatistics.getStartTime());
   }
   
   @MediumTest
@@ -547,6 +530,80 @@ public class TrackRecordingServiceTest
     assertEquals(1, service.insertWaypointMarker(waypoint));
   }
   
+  @MediumTest
+  public void testWithProperties_noAnnouncementFreq() throws Exception {
+    functionalTest(R.string.announcement_frequency_key, (Object) null);
+  }
+
+  @MediumTest
+  public void testWithProperties_defaultAnnouncementFreq() throws Exception {
+    functionalTest(R.string.announcement_frequency_key, 1);
+  }
+  
+  @MediumTest
+  public void testWithProperties_noMaxRecordingDist() throws Exception {
+    functionalTest(R.string.max_recording_distance_key, (Object) null);
+  }
+
+  @MediumTest
+  public void testWithProperties_defaultMaxRecordingDist() throws Exception {
+    functionalTest(R.string.max_recording_distance_key, 5);
+  }
+
+  @MediumTest
+  public void testWithProperties_noMinRecordingDist() throws Exception {
+    functionalTest(R.string.min_recording_distance_key, (Object) null);
+  }
+
+  @MediumTest
+  public void testWithProperties_defaultMinRecordingDist() throws Exception {
+    functionalTest(R.string.min_recording_distance_key, 2);
+  }
+
+  @MediumTest
+  public void testWithProperties_noSignalSamplingFreq() throws Exception {
+    functionalTest(R.string.signal_sampling_frequency_key, (Object) null);
+  }
+
+  @MediumTest
+  public void testWithProperties_defaultSignalSamplingFreq() throws Exception {
+    functionalTest(R.string.signal_sampling_frequency_key, 1);
+  }
+
+  @MediumTest
+  public void testWithProperties_noSplitFreq() throws Exception {
+    functionalTest(R.string.split_frequency_key, (Object) null);
+  }
+
+  @MediumTest
+  public void testWithProperties_defaultSplitFreqByDist() throws Exception {
+    functionalTest(R.string.split_frequency_key, 5);
+  }
+ 
+  @MediumTest
+  public void testWithProperties_defaultSplitFreqByTime() throws Exception {
+    functionalTest(R.string.split_frequency_key, -2);
+  }
+
+  @MediumTest
+  public void testWithProperties_noMetricUnits() throws Exception {
+    functionalTest(R.string.metric_units_key, (Object) null);
+  }
+
+  @MediumTest
+  public void testWithProperties_metricUnitsEnabled() throws Exception {
+    functionalTest(R.string.metric_units_key, true);
+  }
+
+  @MediumTest
+  public void testWithProperties_metricUnitsDisabled() throws Exception {
+    functionalTest(R.string.metric_units_key, false);
+  }
+  
+  // TODO: Add the following tests:
+  // R.string.min_recording_interval_key
+  // R.string.min_required_accuracy_key
+  
   private ITrackRecordingService bindAndGetService(Intent intent) {
     ITrackRecordingService service = ITrackRecordingService.Stub.asInterface(
         bindService(intent));
@@ -592,4 +649,61 @@ public class TrackRecordingServiceTest
     editor.putLong(context.getString(R.string.recording_track_key), id);
     editor.commit();
   }
+
+  // TODO: We support multiple values for readability, however this test's
+  // base class doesn't properly shutdown the service, so it's not possible
+  // to pass more than 1 value at a time.
+  private void functionalTest(int resourceId, Object ...values)
+      throws Exception {
+    final String key = context.getString(resourceId);
+    for (Object value : values) {
+      // Remove all properties and set the property for the given key. 
+      Editor editor = sharedPreferences.edit();
+      editor.clear();
+      if (value instanceof String) {
+        editor.putString(key, (String) value); 
+      } else if (value instanceof Long) {
+        editor.putLong(key, (Long) value);
+      } else if (value instanceof Integer) {
+        editor.putInt(key, (Integer) value);
+      } else if (value instanceof Boolean) {
+        editor.putBoolean(key, (Boolean) value);
+      } else if (value == null) {
+        // Do nothing, as clear above has already removed this property.
+      }
+      editor.commit();
+      
+      fullRecordingSession();
+    }
+  }
+  
+  private void fullRecordingSession() throws Exception {
+    ITrackRecordingService service = bindAndGetService(createStartIntent());
+    assertFalse(service.isRecording());
+    
+    // Start a track.
+    long id = service.startNewTrack();
+    assertTrue(id >= 0);
+    assertTrue(service.isRecording());
+    Track track = providerUtils.getTrack(id);
+    assertNotNull(track);
+    assertEquals(id, track.getId());
+    assertEquals(id, sharedPreferences.getLong(
+        context.getString(R.string.recording_track_key), -1));
+    assertEquals(id, service.getRecordingTrackId());
+    
+    // TODO: Add a few locations, insert markers, etc.
+    
+    // Stop the track.  Validate if it has correct data.
+    service.endCurrentTrack();
+    assertFalse(service.isRecording());
+    assertEquals(-1, service.getRecordingTrackId());
+    track = providerUtils.getTrack(id);
+    assertNotNull(track);
+    assertEquals(id, track.getId());
+    TripStatistics tripStatistics = track.getStatistics();
+    assertNotNull(tripStatistics);
+    assertTrue(tripStatistics.getStartTime() > 0);
+    assertTrue(tripStatistics.getStopTime() >= tripStatistics.getStartTime());
+  }
 }