Improve style and usability of ImportAllTracks

Fix style issues pointed out in code review. Move acquireWakeLock method into
MyTracksUtils and call that from ImportAllTracks and ExportAllTracks. Replace
important Toast messages with dialogs.

MyTracks/res/layout/mytracks_list.xml

@@ -46,6 +46,16 @@ limitations under the License.
     android:background="@drawable/list_bar"
     android:layout_weight="0"
     android:padding="3dip">
+    <TextView
+      android:layout_width="wrap_content"
+      android:layout_height="wrap_content"
+      android:layout_weight="1"/>
+    <Button android:id="@+id/tracklist_btn_import_all"
+      android:layout_width="wrap_content"
+      android:layout_height="48dip"
+      android:layout_gravity="left|top"
+      android:text="@string/tracklist_btn_import_all"
+      android:textSize="14dip"/>
     <Button android:id="@+id/tracklist_btn_export_all"
       android:layout_width="wrap_content"
       android:layout_height="48dip"
@@ -58,15 +68,5 @@ limitations under the License.
       android:layout_gravity="left|top"
       android:text="@string/tracklist_btn_delete_all"
       android:textSize="14dip"/>
-    <TextView
-      android:layout_width="wrap_content"
-      android:layout_height="wrap_content"
-      android:layout_weight="1"/>
-    <Button android:id="@+id/tracklist_btn_import_all"
-      android:layout_width="wrap_content"
-      android:layout_height="48dip"
-      android:layout_gravity="left|top"
-      android:text="@string/tracklist_btn_import_all"
-      android:textSize="14dip"/>
    </LinearLayout>
 </LinearLayout>

MyTracks/res/values/strings.xml

@@ -140,9 +140,19 @@ limitations under the License.
   <string name="tracklist_share_tcx_file">A TCX file</string>
   <string name="tracklist_clear_map">Clear map</string>
   <!-- Import All -->
-  <string name="tracklist_btn_import_all">Import from SD</string>
+  <string name="tracklist_btn_import_all">Import all</string>
-  <string name="import_done">Import finished</string>
+  <string name="import_done">Import finished, imported</string>
-  <string name="import_empty">No files found in /sdcard/MyTracks/gpx/</string>
+  <!-- Format:
+  $1 : Successful imports
+  $2 : Attempted imports
+  $3 : MyTracks directory
+  -->
+  <string name="import_done">Imported %1$d of %2$d GPX files found in /sdcard/%3$s/gpx/</string>
+  <!-- Format:
+  $1 : MyTracks directory
+  -->
+  <string name="import_empty">No files found in /sdcard/%1$s/gpx/</string>
+  <string name="import_error">Import error for file</string>
 
   <!-- Waypoints list -->
   <string name="waypointslist_btn_delete_all">Delete all markers</string>

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

@@ -20,18 +20,17 @@ import com.google.android.apps.mytracks.content.TracksColumns;
 import com.google.android.apps.mytracks.io.TrackWriter;
 import com.google.android.apps.mytracks.io.TrackWriterFactory;
 import com.google.android.apps.mytracks.io.TrackWriterFactory.TrackFileFormat;
+import com.google.android.apps.mytracks.util.MyTracksUtils;
 import com.google.android.maps.mytracks.R;
 
 import android.app.Activity;
 import android.app.AlertDialog;
 import android.app.ProgressDialog;
-import android.content.Context;
 import android.content.DialogInterface;
 import android.content.SharedPreferences;
 import android.database.Cursor;
 import android.os.Handler;
 import android.os.HandlerThread;
-import android.os.PowerManager;
 import android.os.PowerManager.WakeLock;
 import android.util.Log;
 import android.widget.Toast;
@@ -121,7 +120,7 @@ public class ExportAllTracks {
     	  prefs.getLong(activity.getString(R.string.recording_track_key), -1);
     }
     if (recordingTrackId != -1) {
-      acquireWakeLock();
+      wakeLock = MyTracksUtils.acquireWakeLock(activity, wakeLock);
     }
 
     // Now we can safely export everything.
@@ -211,40 +210,4 @@ public class ExportAllTracks {
       }
     }
   }
-
-  /**
-   * Tries to acquire a partial wake lock if not already acquired. Logs errors
-   * and gives up trying in case the wake lock cannot be acquired.
-   */
-  private void acquireWakeLock() {
-    Log.i(MyTracksConstants.TAG, "ExportAllTracks: Aquiring wake lock.");
-    try {
-      PowerManager pm =
-          (PowerManager) activity.getSystemService(Context.POWER_SERVICE);
-      if (pm == null) {
-        Log.e(MyTracksConstants.TAG,
-            "ExportAllTracks: Power manager not found!");
-        return;
-      }
-      if (wakeLock == null) {
-        wakeLock = pm.newWakeLock(PowerManager.PARTIAL_WAKE_LOCK,
-            MyTracksConstants.TAG);
-        if (wakeLock == null) {
-          Log.e(MyTracksConstants.TAG,
-              "ExportAllTracks: Could not create wake lock (null).");
-          return;
-        }
-      }
-      if (!wakeLock.isHeld()) {
-        wakeLock.acquire();
-        if (!wakeLock.isHeld()) {
-          Log.e(MyTracksConstants.TAG,
-              "ExportAllTracks: Could not acquire wake lock.");
-        }
-      }
-    } catch (RuntimeException e) {
-      Log.e(MyTracksConstants.TAG,
-          "ExportAllTracks: Caught unexpected exception: " + e.getMessage(), e);
-    }
-  }
 }

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

@@ -15,32 +15,26 @@
  */
 package com.google.android.apps.mytracks;
 
+import com.google.android.apps.mytracks.content.MyTracksProviderUtils;
+import com.google.android.apps.mytracks.io.GpxImporter;
+import com.google.android.apps.mytracks.util.MyTracksUtils;
+import com.google.android.maps.mytracks.R;
+
 import java.io.File;
 import java.io.FileInputStream;
-import java.io.FileNotFoundException;
-import java.io.IOException;
 import java.util.Iterator;
 import java.util.LinkedList;
 
-import javax.xml.parsers.ParserConfigurationException;
-
-import org.xml.sax.SAXException;
-
 import android.app.Activity;
+import android.app.AlertDialog;
 import android.app.ProgressDialog;
-import android.content.Context;
 import android.content.SharedPreferences;
 import android.os.Handler;
 import android.os.HandlerThread;
-import android.os.PowerManager;
 import android.os.PowerManager.WakeLock;
 import android.util.Log;
 import android.widget.Toast;
 
-import com.google.android.apps.mytracks.content.MyTracksProviderUtils;
-import com.google.android.apps.mytracks.io.GpxImporter;
-import com.google.android.maps.mytracks.R;
-
 /**
  * A class that will import all GPX tracks in /sdcard/MyTracks/gpx/
  *
@@ -51,6 +45,8 @@ public class ImportAllTracks {
   private final Activity activity;
   private WakeLock wakeLock;
   private ProgressDialog progress;
+  private int gpxFileCandidateCount;
+  private int importSuccessCount;
   
   public ImportAllTracks(Activity activity) {
     this.activity = activity;
@@ -83,21 +79,32 @@ public class ImportAllTracks {
     	  prefs.getLong(activity.getString(R.string.recording_track_key), -1);
     }
     if (recordingTrackId != -1) {
-      acquireWakeLock();
+      wakeLock = MyTracksUtils.acquireWakeLock(activity, wakeLock);
     }
 
     // Now we can safely import everything.
     importAll();
 
-    // Release the wake lock if we recorded one.
+    // Release the wake lock if we acquired one.
     // TODO check what happens if we started recording after getting this lock.
     if (wakeLock != null && wakeLock.isHeld()) {
       wakeLock.release();
       Log.i(MyTracksConstants.TAG, "ImportAllTracks: Releasing wake lock.");
     }
-    Log.i(MyTracksConstants.TAG, "ImportAllTracks: Done");
+    
-    Toast.makeText(activity, R.string.import_done, Toast.LENGTH_SHORT).show();
+		Log.i(MyTracksConstants.TAG, "ImportAllTracks: Done");
-  }
+		AlertDialog.Builder builder = new AlertDialog.Builder(activity);
+		if (gpxFileCandidateCount == 0) {
+			builder.setMessage(activity.getString(R.string.import_empty,
+					MyTracksConstants.SDCARD_TOP_DIR));
+		} else {
+			builder.setMessage(activity.getString(R.string.import_done,
+					importSuccessCount, gpxFileCandidateCount,
+					MyTracksConstants.SDCARD_TOP_DIR));
+		}
+		builder.setPositiveButton(R.string.ok, null);
+		builder.show();
+	}
 
   private void makeProgressDialog(final int trackCount) {
     String importMsg = activity.getString(R.string.tracklist_btn_import_all);
@@ -114,112 +121,72 @@ public class ImportAllTracks {
    * Actually import the tracks.
    * This should be called after the wake locks have been acquired.
    */
-  private void importAll() {
+	private void importAll() {
-    try {
+		MyTracksProviderUtils providerUtils = MyTracksProviderUtils.Factory
-      MyTracksProviderUtils providerUtils =
+				.get(activity);
-          MyTracksProviderUtils.Factory.get(activity);
+		LinkedList<File> gpxFiles = new LinkedList<File>();
-
+		File[] gpxFileCandidates = new File("/sdcard/"
-      LinkedList<File> gpxFiles = new LinkedList<File>();
+				+ MyTracksConstants.SDCARD_TOP_DIR + "/gpx").listFiles();
-      File[] gpxFileCandidates = new File("/sdcard/" + MyTracksConstants.SDCARD_TOP_DIR + "/gpx").listFiles(); 
+		if (gpxFileCandidates != null) {
-      if(gpxFileCandidates == null || gpxFileCandidates.length == 0) {
+			for (File file : gpxFileCandidates) {
-    	  Toast.makeText(activity, activity.getString(R.string.import_empty), Toast.LENGTH_LONG).show();
+				if (!file.isDirectory() && file.getName().endsWith(".gpx")) {
-    	  return;
+					gpxFiles.add(file);
-      }
+				}
-      for (File file : gpxFileCandidates) {
+			}
-          if (!file.isDirectory() && file.getName().endsWith(".gpx")) {
-              gpxFiles.add(file);
-          }
-      }
-      
-      final int trackCount = gpxFiles.size();
-      
-      Log.i(MyTracksConstants.TAG,
-          "ImportAllTracks: Importing: " + trackCount + " tracks.");
-      activity.runOnUiThread(new Runnable() {
-        public void run() {
-          makeProgressDialog(trackCount);
-        }
-      });
-
-      Iterator<File> gpxFilesIterator = gpxFiles.iterator();
-      File currentFile;
-      int currentFileNumber = 0;
-      while(gpxFilesIterator.hasNext()) {
-    	currentFile = gpxFilesIterator.next();
-    	final int status = currentFileNumber;
-        activity.runOnUiThread(new Runnable() {
-          public void run() {
-            synchronized (this) {
-              if (progress == null) {
-                return;
-              }
-              progress.setProgress(status);
-            }
-          }
-        });
-
-        Log.i(MyTracksConstants.TAG, "ImportAllTracks: importing: " + currentFile.getName());
-        try {
-			GpxImporter.importGPXFile(new FileInputStream(currentFile), providerUtils);
-		} catch (FileNotFoundException e) {
-			e.printStackTrace();
-            Toast.makeText(activity, "FileNotFoundException", Toast.LENGTH_LONG).show();
-		} catch (ParserConfigurationException e) {
-			e.printStackTrace();
-            Toast.makeText(activity, "ParserConfigurationException", Toast.LENGTH_LONG).show();
-		} catch (SAXException e) {
-			e.printStackTrace();
-            Toast.makeText(activity, "SAXException", Toast.LENGTH_LONG).show();
-		} catch (IOException e) {
-			e.printStackTrace();
-            Toast.makeText(activity, "IOException", Toast.LENGTH_LONG).show();
 		}
+		gpxFileCandidateCount = gpxFiles.size();
 
-		currentFileNumber++;
+		if (gpxFileCandidateCount != 0) {
-      }
+			Log.i(MyTracksConstants.TAG, "ImportAllTracks: Importing: "
-    } finally {
+					+ gpxFileCandidateCount + " tracks.");
-      if (progress != null) {
+			activity.runOnUiThread(new Runnable() {
-        synchronized (this) {
+				public void run() {
-          progress.dismiss();
+					makeProgressDialog(gpxFileCandidateCount);
-          progress = null;
+				}
-        }
+			});
-      }
-    }
-  }
 
-  /**
+			Iterator<File> gpxFilesIterator = gpxFiles.iterator();
-   * Tries to acquire a partial wake lock if not already acquired. Logs errors
+			File currentFile;
-   * and gives up trying in case the wake lock cannot be acquired.
+			int currentFileNumber = 0;
-   */
+			while (gpxFilesIterator.hasNext()) {
-  private void acquireWakeLock() {
+				currentFile = gpxFilesIterator.next();
-    Log.i(MyTracksConstants.TAG, "ImportAllTracks: Acquiring wake lock.");
+				final int status = currentFileNumber;
-    try {
+				activity.runOnUiThread(new Runnable() {
-      PowerManager pm =
+					public void run() {
-          (PowerManager) activity.getSystemService(Context.POWER_SERVICE);
+						synchronized (this) {
-      if (pm == null) {
+							if (progress == null) {
-        Log.e(MyTracksConstants.TAG,
+								return;
-            "ImportAllTracks: Power manager not found!");
+							}
-        return;
+							progress.setProgress(status);
-      }
+						}
-      if (wakeLock == null) {
+					}
-        wakeLock = pm.newWakeLock(PowerManager.PARTIAL_WAKE_LOCK,
+				});
-            MyTracksConstants.TAG);
+				if (importFile(currentFile, providerUtils)) {
-        if (wakeLock == null) {
+					importSuccessCount++;
-          Log.e(MyTracksConstants.TAG,
+				}
-              "ImportAllTracks: Could not create wake lock (null).");
+				currentFileNumber++;
-          return;
+			}
-        }
+			if (progress != null) {
-      }
+				synchronized (this) {
-      if (!wakeLock.isHeld()) {
+					progress.dismiss();
-        wakeLock.acquire();
+					progress = null;
-        if (!wakeLock.isHeld()) {
+				}
-          Log.e(MyTracksConstants.TAG,
+			}
-              "ImportAllTracks: Could not acquire wake lock.");
+		}
-        }
+	}
-      }
+  
-    } catch (RuntimeException e) {
+	private boolean importFile(File gpxFile, MyTracksProviderUtils providerUtils) {
-      Log.e(MyTracksConstants.TAG,
+		Log.i(MyTracksConstants.TAG, "ImportAllTracks: importing: "
-          "ImportAllTracks: Caught unexpected exception: " + e.getMessage(), e);
+				+ gpxFile.getName());
-    }
+		try {
-  }
+			GpxImporter.importGPXFile(new FileInputStream(gpxFile), providerUtils);
+			return true;
+		} catch (Exception e) {
+			Log.w("", e);
+			Toast.makeText(activity,
+					activity.getString(R.string.import_error) + " " + gpxFile.getName(),
+					Toast.LENGTH_LONG).show();
+			return false;
+		}
+	}
 }

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

@@ -15,6 +15,11 @@
  */
 package com.google.android.apps.mytracks;
 
+import com.google.android.apps.mytracks.content.TracksColumns;
+import com.google.android.apps.mytracks.util.StringUtils;
+import com.google.android.apps.mytracks.util.UnitConversions;
+import com.google.android.maps.mytracks.R;
+
 import android.app.ListActivity;
 import android.content.Intent;
 import android.content.SharedPreferences;
@@ -22,22 +27,17 @@ import android.database.Cursor;
 import android.os.Bundle;
 import android.os.Handler;
 import android.view.ContextMenu;
+import android.view.ContextMenu.ContextMenuInfo;
 import android.view.MenuItem;
 import android.view.SubMenu;
 import android.view.View;
-import android.view.Window;
-import android.view.ContextMenu.ContextMenuInfo;
 import android.view.View.OnCreateContextMenuListener;
+import android.view.Window;
 import android.widget.AdapterView;
 import android.widget.ListView;
 import android.widget.SimpleCursorAdapter;
 import android.widget.TextView;
 
-import com.google.android.apps.mytracks.content.TracksColumns;
-import com.google.android.apps.mytracks.util.StringUtils;
-import com.google.android.apps.mytracks.util.UnitConversions;
-import com.google.android.maps.mytracks.R;
-
 /**
  * A list activity displaying all the recorded tracks. There's a context
  * menu (via long press) displaying various options such as showing, editing,

MyTracks/src/com/google/android/apps/mytracks/util/MyTracksUtils.java

@@ -21,12 +21,15 @@ import com.google.android.apps.mytracks.stats.TripStatistics;
 
 import com.google.android.maps.GeoPoint;
 
+import android.app.Activity;
 import android.content.Context;
 import android.content.pm.PackageInfo;
 import android.content.pm.PackageManager;
 import android.content.pm.PackageManager.NameNotFoundException;
 import android.content.pm.Signature;
 import android.location.Location;
+import android.os.PowerManager;
+import android.os.PowerManager.WakeLock;
 import android.util.Log;
 
 import java.util.ArrayList;
@@ -287,6 +290,42 @@ public class MyTracksUtils {
     }
   }
   
+  /**
+   * Tries to acquire a partial wake lock if not already acquired. Logs errors
+   * and gives up trying in case the wake lock cannot be acquired.
+   */
+	public static WakeLock acquireWakeLock(Activity activity, WakeLock wakeLock) {
+		Log.i(MyTracksConstants.TAG, "MyTracksUtils: Acquiring wake lock.");
+		try {
+			PowerManager pm = (PowerManager) activity
+					.getSystemService(Context.POWER_SERVICE);
+			if (pm == null) {
+				Log.e(MyTracksConstants.TAG, "MyTracksUtils: Power manager not found!");
+				return wakeLock;
+			}
+			if (wakeLock == null) {
+				wakeLock = pm.newWakeLock(PowerManager.PARTIAL_WAKE_LOCK,
+						MyTracksConstants.TAG);
+				if (wakeLock == null) {
+					Log.e(MyTracksConstants.TAG,
+							"MyTracksUtils: Could not create wake lock (null).");
+				}
+				return wakeLock;
+			}
+			if (!wakeLock.isHeld()) {
+				wakeLock.acquire();
+				if (!wakeLock.isHeld()) {
+					Log.e(MyTracksConstants.TAG,
+							"MyTracksUtils: Could not acquire wake lock.");
+				}
+			}
+		} catch (RuntimeException e) {
+			Log.e(MyTracksConstants.TAG,
+					"MyTracksUtils: Caught unexpected exception: " + e.getMessage(), e);
+		}
+		return wakeLock;
+	}
+
   /**
    * This is a utility class w/ only static members.
    */