Address comments. Add periodic task factories.

MyTracks/res/values/arrays_do_not_translate.xml

@@ -116,7 +116,7 @@ limitations under the License.
   </array>
 
   <string-array name="ant_sensor_type_values">
-  	<item>@string/ant_sensor_type</item>
+    <item>@string/ant_sensor_type</item>
-  	<item>@string/srm_ant_bridge_sensor_type</item>
+    <item>@string/srm_ant_bridge_sensor_type</item>
   </string-array>
 </resources>

MyTracks/src/com/google/android/apps/mytracks/SettingsActivity.java

@@ -280,6 +280,9 @@ public class SettingsActivity extends PreferenceActivity {
     setTaskOptions(isMetric, R.string.split_frequency_key);
   }
 
+  /**
+   * Set the user visible options for a periodic task.
+   */
   private void setTaskOptions(boolean isMetric, int listId) {
     final ListPreference taskFrequency =
         (ListPreference) findPreference(getString(listId));
@@ -288,7 +291,6 @@ public class SettingsActivity extends PreferenceActivity {
         : R.array.task_frequency_options_ft);
   }
 
-
   /**
    * Configures preference actions related to bluetooth.
    */

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

@@ -31,8 +31,7 @@ import com.google.android.apps.mytracks.content.WaypointsColumns;
 import com.google.android.apps.mytracks.content.Sensor.SensorDataSet;
 import com.google.android.apps.mytracks.services.sensors.SensorManager;
 import com.google.android.apps.mytracks.services.sensors.SensorManagerFactory;
-import com.google.android.apps.mytracks.services.tasks.PeriodicTask;
+import com.google.android.apps.mytracks.services.tasks.PeriodicTaskExecutor;
-import com.google.android.apps.mytracks.services.tasks.PeriodicTaskExecuter;
 import com.google.android.apps.mytracks.services.tasks.SplitTask;
 import com.google.android.apps.mytracks.services.tasks.StatusAnnouncerFactory;
 import com.google.android.apps.mytracks.stats.TripStatistics;
@@ -115,10 +114,10 @@ public class TrackRecordingService extends Service implements LocationListener {
   private double length;
 
   /**
-   * Status announcer executer.
+   * Status announcer executor.
    */
-  private PeriodicTaskExecuter announcementExecuter;
+  private PeriodicTaskExecutor announcementExecutor;
-  private PeriodicTaskExecuter splitExecuter;
+  private PeriodicTaskExecutor splitExecutor;
 
   private SensorManager sensorManager;
   
@@ -189,9 +188,9 @@ public class TrackRecordingService extends Service implements LocationListener {
   private Location lastValidLocation;
 
   /**
-   * The frequency of status announcements.
+   * A service to run tasks outside of the main thread.
    */
-  private ExecutorService executerServce;
+  private ExecutorService executorService;
 
   /*
    * Utility functions
@@ -275,8 +274,8 @@ public class TrackRecordingService extends Service implements LocationListener {
           "Caught SQLiteException: " + e.getMessage(), e);
       return false;
     }
-    announcementExecuter.update();
+    announcementExecutor.update();
-    splitExecuter.update();
+    splitExecutor.update();
     return true;
   }
 
@@ -428,7 +427,6 @@ public class TrackRecordingService extends Service implements LocationListener {
     TripStatistics stats = track.getStatistics();
     statsBuilder = new TripStatisticsBuilder(stats.getStartTime());
     statsBuilder.setMinRecordingDistance(minRecordingDistance);
-    setUpAnnouncer();
 
     length = 0;
     lastValidLocation = null;
@@ -477,8 +475,8 @@ public class TrackRecordingService extends Service implements LocationListener {
       }
     }
 
-    announcementExecuter.restore();
+    announcementExecutor.restore();
-    splitExecuter.restore();
+    splitExecutor.restore();
   }
 
   /*
@@ -486,7 +484,7 @@ public class TrackRecordingService extends Service implements LocationListener {
    */
   @Override
   public void onLocationChanged(final Location location) {
-    this.executerServce.submit(
+    executorService.submit(
       new Runnable() {
         @Override
         public void run() {
@@ -666,8 +664,7 @@ public class TrackRecordingService extends Service implements LocationListener {
         (NotificationManager) getSystemService(NOTIFICATION_SERVICE);
     locationManager = (LocationManager) getSystemService(LOCATION_SERVICE);
 
-    setUpAnnouncer();
+    setUpTaskExecutors();
-    this.splitExecuter = new PeriodicTaskExecuter(this, new SplitTask());
     prefManager = new PreferenceManager(this);
     registerLocationListener();
 
@@ -692,44 +689,31 @@ public class TrackRecordingService extends Service implements LocationListener {
       prefManager.setRecordingTrack(recordingTrackId = -1);
     }
     showNotification();
-    executerServce = Executors.newSingleThreadExecutor();
+    executorService = Executors.newSingleThreadExecutor();
   }
 
   /**
-   * Creates an {@link Executer} and schedules {@class SafeStatusAnnouncerTask}.
+   * Creates the periodic task executors.
-   * The announcer requires a TTS service and user should have enabled
-   * the announcements, otherwise this method is no-op.
    */
-  private void setUpAnnouncer() {
+  private void setUpTaskExecutors() {
-    Log.d(TAG, "TrackRecordingService.setUpAnnouncer: "
+    Log.d(TAG, "TrackRecordingService.setUpTasks: "
-        + announcementExecuter);
+        + announcementExecutor);
-    StatusAnnouncerFactory statusAnnouncerFactory =
+    announcementExecutor = new PeriodicTaskExecutor(
-        new StatusAnnouncerFactory(ApiFeatures.getInstance());
+        this, new StatusAnnouncerFactory(ApiFeatures.getInstance()));
-    PeriodicTask announcer = statusAnnouncerFactory.create(
+    splitExecutor = new PeriodicTaskExecutor(this, new SplitTask.Factory());
-        TrackRecordingService.this);
-    if (announcer == null) {
-      return;
-    }
-
-    announcementExecuter = new PeriodicTaskExecuter(
-        TrackRecordingService.this, announcer);
   }
   
-  private void shutdownExecuters() {
+  private void shutdownTaskExecutors() {
     Log.d(TAG, "TrackRecordingService.shutdownExecuters");
-    if (announcementExecuter != null) {
+    try {
-      try {
+      announcementExecutor.shutdown();
-        announcementExecuter.shutdown();
+    } finally {
-      } finally {
+      announcementExecutor = null;
-        announcementExecuter = null;
-      }
     }
-    if (splitExecuter != null) {
+    try {
-      try {
+      splitExecutor.shutdown();
-        splitExecuter.shutdown();
+    } finally {
-      } finally {
+      splitExecutor = null;
-        splitExecuter = null;
-      }
     }
   }
 
@@ -746,7 +730,7 @@ public class TrackRecordingService extends Service implements LocationListener {
     timer.cancel();
     timer.purge();
     unregisterLocationListener();
-    shutdownExecuters();
+    shutdownTaskExecutors();
     if (sensorManager != null) {
       sensorManager.shutdown();
       sensorManager = null;
@@ -1118,8 +1102,8 @@ public class TrackRecordingService extends Service implements LocationListener {
     // Notify the world that we're now recording.
     sendTrackBroadcast(
         R.string.track_started_broadcast_action, recordingTrackId);
-    announcementExecuter.restore();
+    announcementExecutor.restore();
-    splitExecuter.restore();
+    splitExecutor.restore();
 
     return recordingTrackId;
   }
@@ -1130,7 +1114,8 @@ public class TrackRecordingService extends Service implements LocationListener {
       throw new IllegalStateException("No recording track in progress!");
     }
 
-    shutdownExecuters();
+    announcementExecutor.shutdown();
+    splitExecutor.shutdown();
     isRecording = false;
     Track recordingTrack = providerUtils.getTrack(recordingTrackId);
     if (recordingTrack != null) {
@@ -1236,23 +1221,15 @@ public class TrackRecordingService extends Service implements LocationListener {
   }
 
   public void setAnnouncementFrequency(int announcementFrequency) {
-    if (announcementExecuter != null) {
+    announcementExecutor.setTaskFrequency(announcementFrequency);
-      announcementExecuter.setTaskFrequency(announcementFrequency);
-    }
   }
 
   public void setSplitFrequency(int frequency) {
-    if (splitExecuter != null) {
+    splitExecutor.setTaskFrequency(frequency);
-      splitExecuter.setTaskFrequency(frequency);
-    }
   }
 
   public void setMetricUnits(boolean metric) {
-    if (announcementExecuter != null) {
+    announcementExecutor.setMetricUnits(metric);
-      announcementExecuter.setMetricUnits(metric);
+    splitExecutor.setMetricUnits(metric);
-    }
-    if (splitExecuter != null) {
-      splitExecuter.setMetricUnits(metric);
-    }
   }
 }

MyTracks/src/com/google/android/apps/mytracks/services/tasks/PeriodicTaskExecutor.java

@@ -27,7 +27,7 @@ import android.util.Log;
  *
  * @author Sandor Dornbush
  */
-public class PeriodicTaskExecuter {
+public class PeriodicTaskExecutor {
 
   /**
    * The frequency of the task.
@@ -42,19 +42,21 @@ public class PeriodicTaskExecuter {
   private double nextTaskDistance = 0;
 
   /**
-   * Time based executer.
+   * Time based executor.
    */
-  private TimerTaskExecuter timerExecuter = null;
+  private TimerTaskExecutor timerExecutor = null;
 
   private boolean metricUnits;
 
   private final TrackRecordingService service;
 
-  private final PeriodicTask task;
+  private final PeriodicTaskFactory factory;
 
-  public PeriodicTaskExecuter(TrackRecordingService service, PeriodicTask task) {
+  private PeriodicTask task;
+
+  public PeriodicTaskExecutor(TrackRecordingService service, PeriodicTaskFactory factory) {
     this.service = service;
-    this.task = task;
+    this.factory = factory;
   }
 
   /**
@@ -65,21 +67,30 @@ public class PeriodicTaskExecuter {
     if (!service.isRecording()) {
       return;
     }
-
+    
     if (!isTimeFrequency()) {
-      if (timerExecuter != null) {
+      if (timerExecutor != null) {
-        timerExecuter.shutdown();
+        timerExecutor.shutdown();
-        timerExecuter = null;
+        timerExecutor = null;
       }
     }
     if (taskFrequency == 0) {
       return;
     }
+    
+    // Try to make the task.
+    task = factory.create(service);
+    // Returning null is ok.
+    if (task == null) {
+      return;
+    }
+    task.start();
+
     if (isTimeFrequency()) {
-      if (timerExecuter == null) {
+      if (timerExecutor == null) {
-        timerExecuter = new TimerTaskExecuter(task, service);
+        timerExecutor = new TimerTaskExecutor(task, service);
       }
-      timerExecuter.scheduleTask(taskFrequency * 60000);
+      timerExecutor.scheduleTask(taskFrequency * 60000);
     } else {
       // For distance based splits.
       calculateNextTaskDistance();
@@ -90,8 +101,13 @@ public class PeriodicTaskExecuter {
    * Shuts down the manager.
    */
   public void shutdown() {
-    if (timerExecuter != null) {
+    if (task != null) {
-      timerExecuter.shutdown();
+      task.shutdown();
+      task = null;
+    }
+    if (timerExecutor != null) {
+      timerExecutor.shutdown();
+      timerExecutor = null;
     }
   }
 
@@ -100,7 +116,7 @@ public class PeriodicTaskExecuter {
    */
   void calculateNextTaskDistance() {
     // TODO: Decouple service from this class once and forever.
-    if (!service.isRecording()) {
+    if (!service.isRecording() || task == null) {
       return;
     }
     
@@ -125,7 +141,7 @@ public class PeriodicTaskExecuter {
    * Updates executer with new trip statistics.
    */
   public void update() {
-    if (!isDistanceFrequency()) {
+    if (!isDistanceFrequency() || task == null) {
       return;
     }
     // Convert the distance in meters to km or mi.

MyTracks/src/com/google/android/apps/mytracks/services/tasks/PeriodicTaskFactory.java

@@ -0,0 +1,34 @@
+/*
+ * Copyright 2010 Google Inc.
+ * 
+ * Licensed under the Apache License, Version 2.0 (the "License"); you may not
+ * use this file except in compliance with the License. You may obtain a copy of
+ * the License at
+ * 
+ * http://www.apache.org/licenses/LICENSE-2.0
+ * 
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS, WITHOUT
+ * WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. See the
+ * License for the specific language governing permissions and limitations under
+ * the License.
+ */
+package com.google.android.apps.mytracks.services.tasks;
+
+import android.content.Context;
+
+/**
+ * An interface for classes that can create periodic tasks.
+ *
+ * @author Sandor Dornbush
+ */
+public interface PeriodicTaskFactory {
+
+  /**
+   * Creates a periodic task which does voice announcements.
+   *
+   * @return the task, or null if task is not supported
+   */
+  abstract PeriodicTask create(Context context);
+
+}

MyTracks/src/com/google/android/apps/mytracks/services/tasks/SplitTask.java

@@ -19,9 +19,11 @@ package com.google.android.apps.mytracks.services.tasks;
 import com.google.android.apps.mytracks.content.WaypointCreationRequest;
 import com.google.android.apps.mytracks.services.TrackRecordingService;
 
+import android.content.Context;
+
 
 /**
- * A simple task to insert statistics markers every n minutes.
+ * A simple task to insert statistics markers periodically.
  * @author Sandor Dornbush
  */
 public class SplitTask implements PeriodicTask {
@@ -38,4 +40,12 @@ public class SplitTask implements PeriodicTask {
   @Override
   public void start() {
   }
+  
+  public static class Factory implements PeriodicTaskFactory {
+
+    @Override
+    public PeriodicTask create(Context context) {
+      return new SplitTask();
+    }
+  }
 }

MyTracks/src/com/google/android/apps/mytracks/services/tasks/StatusAnnouncerFactory.java

@@ -26,7 +26,7 @@ import android.media.AudioManager;
  *
  * @author Rodrigo Damazio
  */
-public class StatusAnnouncerFactory {
+public class StatusAnnouncerFactory implements PeriodicTaskFactory {
 
   private final boolean hasTts;
 
@@ -34,10 +34,8 @@ public class StatusAnnouncerFactory {
     this.hasTts = apiFeatures.hasTextToSpeech();
   }
 
-  /**
+  /* (non-Javadoc)
-   * Creates a periodic task which does voice announcements.
+   * @see com.google.android.apps.mytracks.services.tasks.PeriodicTaskFactory#create(android.content.Context)
-   *
-   * @return the task, or null if announcements are not supported
    */
   public PeriodicTask create(Context context) {
     if (hasTts) {

MyTracks/src/com/google/android/apps/mytracks/services/tasks/TimerTaskExecutor.java

@@ -31,7 +31,7 @@ import com.google.android.apps.mytracks.services.TrackRecordingService;
  *
  * @author Sandor Dornbush
  */
-public class TimerTaskExecuter {
+public class TimerTaskExecutor {
 
   private final PeriodicTask task;
   private final TrackRecordingService service;
@@ -42,7 +42,7 @@ public class TimerTaskExecuter {
    */
   private Timer timer;
 
-  public TimerTaskExecuter(PeriodicTask task,
+  public TimerTaskExecutor(PeriodicTask task,
                            TrackRecordingService service) {
     this.task = task;
     this.service = service;

MyTracksTest/src/com/google/android/apps/mytracks/services/tasks/StatusAnnouncerFactoryTest.java

@@ -58,7 +58,7 @@ public class StatusAnnouncerFactoryTest extends AndroidTestCase {
 
   public void testCreate() {
     apiFeatures.setHasTextToSpeech(true);
-    StatusAnnouncerFactory factory =
+    PeriodicTaskFactory factory =
         new StatusAnnouncerFactory(apiFeatures);
     PeriodicTask task = factory.create(getContext());
     assertTrue(task instanceof StatusAnnouncerTask);
@@ -66,7 +66,7 @@ public class StatusAnnouncerFactoryTest extends AndroidTestCase {
 
   public void testCreate_notAvailable() {
     apiFeatures.setHasTextToSpeech(false);
-    StatusAnnouncerFactory factory =
+    PeriodicTaskFactory factory =
         new StatusAnnouncerFactory(apiFeatures);
     PeriodicTask task = factory.create(getContext());
     assertNull(task);