Address comments.

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

@@ -26,7 +26,7 @@ import com.google.android.apps.mymaps.MyMapsList;
 import com.google.android.apps.mytracks.content.MyTracksProviderUtils;
 import com.google.android.apps.mytracks.content.Track;
 import com.google.android.apps.mytracks.content.Waypoint;
-import com.google.android.apps.mytracks.content.WaypointType;
+import com.google.android.apps.mytracks.content.WaypointCreationRequest;
 import com.google.android.apps.mytracks.io.AuthManager;
 import com.google.android.apps.mytracks.io.AuthManagerFactory;
 import com.google.android.apps.mytracks.io.GpxImporter;
@@ -63,6 +63,7 @@ import android.os.Bundle;
 import android.os.Handler;
 import android.os.HandlerThread;
 import android.os.IBinder;
+import android.os.RemoteException;
 import android.util.Log;
 import android.view.Menu;
 import android.view.MenuItem;
@@ -428,8 +429,10 @@ public class MyTracks extends TabActivity implements OnTouchListener,
     if (isRecording()) {
       if (event.getAction() == MotionEvent.ACTION_DOWN) {
         try {
-          insertWaypoint(WaypointType.STATISTICS);
+          insertWaypoint(WaypointCreationRequest.DEFAULT_STATISTICS);
-        } catch (Exception e) {
+        } catch (RemoteException e) {
+          Log.e(MyTracksConstants.TAG, "Cannot insert statistics marker.", e);
+        } catch (IllegalStateException e) {
           Log.e(MyTracksConstants.TAG, "Cannot insert statistics marker.", e);
         }
         return true;
@@ -888,25 +891,20 @@ public class MyTracks extends TabActivity implements OnTouchListener,
    * Inserts a waypoint marker.
    *
    * @return Id of the inserted statistics marker.
-   * @throws Exception If the insertion failed.
+   * @throws RemoteException 
    */
-  public long insertWaypoint(WaypointType type) throws Exception {
+  public long insertWaypoint(WaypointCreationRequest request) throws RemoteException {
     if (trackRecordingService == null) {
       throw new IllegalStateException("The recording service is not bound.");
     }
     try {
-      long waypointId =
+      long waypointId = trackRecordingService.insertWaypoint(request);
-          trackRecordingService.insertWaypoint(type);
       if (waypointId >= 0) {
         Toast.makeText(this, R.string.status_statistics_inserted,
             Toast.LENGTH_LONG).show();
-        return waypointId;
-      } else {
-        Toast.makeText(this, R.string.error_unable_to_insert_marker,
-            Toast.LENGTH_LONG).show();
-        throw new Exception("Insertion failed.");
       }
-    } catch (Exception e) {
+      return waypointId;
+    } catch (RemoteException e) {
       Toast.makeText(this, R.string.error_unable_to_insert_marker,
           Toast.LENGTH_LONG).show();
       throw e;

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

@@ -42,6 +42,8 @@ import android.widget.ImageView;
 public class MyTracksWaypointDetails extends Activity
     implements OnClickListener {
 
+  public static final String WAYPOINT_ID_EXTRA = "waypointid";
+
   /**
    * The id of the way point being edited (taken from bundle, "waypointid")
    */
@@ -78,7 +80,7 @@ public class MyTracksWaypointDetails extends Activity
     }
 
     // Required extra when launching this intent:
-    waypointId = getIntent().getLongExtra("waypointid", -1);
+    waypointId = getIntent().getLongExtra(WAYPOINT_ID_EXTRA, -1);
     if (waypointId < 0) {
       Log.d(MyTracksConstants.TAG,
           "MyTracksWaypointsDetails intent was launched w/o waypoint id.");

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

@@ -17,7 +17,7 @@ package com.google.android.apps.mytracks;
 
 import com.google.android.apps.mytracks.content.MyTracksProviderUtils;
 import com.google.android.apps.mytracks.content.Waypoint;
-import com.google.android.apps.mytracks.content.WaypointType;
+import com.google.android.apps.mytracks.content.WaypointCreationRequest;
 import com.google.android.apps.mytracks.content.WaypointsColumns;
 import com.google.android.apps.mytracks.util.StringUtils;
 import com.google.android.maps.mytracks.R;
@@ -29,6 +29,7 @@ import android.content.Intent;
 import android.content.SharedPreferences;
 import android.database.Cursor;
 import android.os.Bundle;
+import android.os.RemoteException;
 import android.util.Log;
 import android.view.ContextMenu;
 import android.view.ContextMenu.ContextMenuInfo;
@@ -89,7 +90,7 @@ public class MyTracksWaypointsList extends ListActivity
   protected void onListItemClick(ListView l, View v, int position, long id) {
     Intent result = new Intent();
     result.putExtra("trackid", trackId);
-    result.putExtra("waypointid", id);
+    result.putExtra(MyTracksWaypointDetails.WAYPOINT_ID_EXTRA, id);
     setResult(MyTracksConstants.EDIT_WAYPOINT, result);
     finish();
   }
@@ -105,7 +106,7 @@ public class MyTracksWaypointsList extends ListActivity
         case MyTracksConstants.MENU_EDIT: {
           Intent intent = new Intent(this, MyTracksWaypointDetails.class);
           intent.putExtra("trackid", trackId);
-          intent.putExtra("waypointid", waypointId);
+          intent.putExtra(MyTracksWaypointDetails.WAYPOINT_ID_EXTRA, waypointId);
           startActivity(intent);
           return true;
         }
@@ -167,28 +168,30 @@ public class MyTracksWaypointsList extends ListActivity
 
   @Override
   public void onClick(View v) {
-    WaypointType type;
+    WaypointCreationRequest request;
     switch (v.getId()) {
       case R.id.waypointslist_btn_insert_waypoint:
-        type = WaypointType.MARKER;
+        request = WaypointCreationRequest.DEFAULT_MARKER;
         break;
       case R.id.waypointslist_btn_insert_statistics:
-        type = WaypointType.MARKER;
+        request = WaypointCreationRequest.DEFAULT_STATISTICS;
         break;
       default:
         return;
     }
     long id;
     try {
-      id = MyTracks.getInstance().insertWaypoint(type);
+      id = MyTracks.getInstance().insertWaypoint(request);
-    } catch (Exception e) {
+    } catch (RemoteException e) {
-      Log.e(MyTracksConstants.TAG, "Cannot insert statistics marker.", e);
+      Log.e(MyTracksConstants.TAG, "Cannot insert marker.", e);
-      e.printStackTrace();
+      return;
+    } catch (IllegalStateException e) {
+      Log.e(MyTracksConstants.TAG, "Cannot insert marker.", e);
       return;
     }
     if (id >= 0) {
       Intent intent = new Intent(this, MyTracksWaypointDetails.class);
-      intent.putExtra("waypointid", id);
+      intent.putExtra(MyTracksWaypointDetails.WAYPOINT_ID_EXTRA, id);
       startActivity(intent);
     }
   }

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

@@ -16,7 +16,7 @@
 package com.google.android.apps.mytracks.services;
 
 import com.google.android.apps.mytracks.MyTracksConstants;
-import com.google.android.apps.mytracks.content.WaypointType;
+import com.google.android.apps.mytracks.content.WaypointCreationRequest;
 import com.google.android.apps.mytracks.util.UnitConversions;
 
 import android.util.Log;
@@ -112,7 +112,7 @@ public class SplitManager {
     }
 
     if (distance > this.nextSplitDistance) {
-      service.insertWaypoint(WaypointType.STATISTICS);
+      service.insertWaypoint(WaypointCreationRequest.DEFAULT_STATISTICS);
       calculateNextSplit();
     }
   }

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

@@ -16,7 +16,8 @@
 
 package com.google.android.apps.mytracks.services;
 
-import com.google.android.apps.mytracks.content.WaypointType;
+import com.google.android.apps.mytracks.content.WaypointCreationRequest;
+
 
 /**
  * A simple task to insert statistics markers every n minutes.
@@ -26,7 +27,7 @@ public class TimeSplitTask implements PeriodicTask {
 
   @Override
   public void run(TrackRecordingService service) {
-    service.insertWaypoint(WaypointType.STATISTICS);
+    service.insertWaypoint(WaypointCreationRequest.DEFAULT_STATISTICS);
   }
 
   @Override

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

@@ -26,7 +26,7 @@ import com.google.android.apps.mytracks.content.Sensor;
 import com.google.android.apps.mytracks.content.Track;
 import com.google.android.apps.mytracks.content.TracksColumns;
 import com.google.android.apps.mytracks.content.Waypoint;
-import com.google.android.apps.mytracks.content.WaypointType;
+import com.google.android.apps.mytracks.content.WaypointCreationRequest;
 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;
@@ -858,26 +858,23 @@ public class TrackRecordingService extends Service implements LocationListener {
     return isRecording;
   }
 
-  public long insertWaypoint(WaypointType type) {
+  public long insertWaypoint(WaypointCreationRequest request) {
     if (!isRecording()) {
       throw new IllegalStateException(
           "Unable to insert waypoint marker while not recording!");
     }
+    if (request == null) {
+      request = WaypointCreationRequest.DEFAULT_MARKER;
+    }
     Waypoint wpt = new Waypoint();
-    switch (type) {
+    switch (request.getType()) {
       case MARKER:
-        wpt.setIcon(getString(R.string.waypoint_icon_url));
+        buildMarker(wpt, request);
-        wpt.setName(getString(R.string.waypoint));
-        wpt.setType(Waypoint.TYPE_WAYPOINT);
         break;
       case STATISTICS:
         buildStatisticsMarker(wpt);
         break;
     }
-    if (!isRecording()) {
-      throw new IllegalStateException(
-          "Unable to insert waypoint marker while not recording!");
-    }
     wpt.setTrackId(recordingTrackId);
     wpt.setLength(length);
     if (lastValidLocation != null) {
@@ -890,6 +887,23 @@ public class TrackRecordingService extends Service implements LocationListener {
     return Long.parseLong(uri.getLastPathSegment());
   }
 
+  private void buildMarker(Waypoint wpt, WaypointCreationRequest request) {
+    wpt.setType(Waypoint.TYPE_WAYPOINT);
+    if (request.getIcon() == null) {
+      wpt.setIcon(getString(R.string.waypoint_icon_url));
+    } else {
+      wpt.setIcon(request.getIcon());
+    }
+    if (request.getName() == null) {
+      wpt.setName(getString(R.string.waypoint));
+    } else {
+      wpt.setName(request.getName());
+    }
+    if (request.getDescription() != null) {
+      wpt.setDescription(request.getDescription());
+    }
+  }
+
   /**
    * Build a statistics marker.
    * A statistics marker holds the stats for the* last segment up to this marker.
@@ -898,11 +912,6 @@ public class TrackRecordingService extends Service implements LocationListener {
    * @return the unique id of the inserted marker
    */
   private void buildStatisticsMarker(Waypoint waypoint) {
-    if (!isRecording()) {
-      throw new IllegalStateException(
-          "Unable to insert statistics marker while not recording!");
-    }
-    
     StringUtils utils = new StringUtils(TrackRecordingService.this);
 
     // Set stop and total time in the stats data
@@ -989,14 +998,14 @@ public class TrackRecordingService extends Service implements LocationListener {
     /**
      * Inserts a waypoint marker in the track being recorded.
      *
-     * @param type of the waypoint to insert
+     * @param request Details of the waypoint to insert
      * @return the unique ID of the inserted marker
      */
-    public long insertWaypoint(WaypointType type) {
+    public long insertWaypoint(WaypointCreationRequest request) {
       if (service == null) {
         throw new IllegalStateException("The service has been already detached!");
       }
-      return service.insertWaypoint(type);
+      return service.insertWaypoint(request);
     }
 
     @Override
@@ -1073,7 +1082,7 @@ public class TrackRecordingService extends Service implements LocationListener {
     providerUtils.updateTrack(track);
     statsBuilder = new TripStatisticsBuilder(startTime);
     waypointStatsBuilder = new TripStatisticsBuilder(startTime);
-    currentWaypointId = insertWaypoint(WaypointType.STATISTICS);
+    currentWaypointId = insertWaypoint(WaypointCreationRequest.DEFAULT_STATISTICS);
     setUpAnnouncer();
     length = 0;
     showNotification();

MyTracksLib/src/com/google/android/apps/mytracks/content/WaypointCreationRequest.aidl

@@ -1,2 +1,2 @@
 package com.google.android.apps.mytracks.content;
-parcelable WaypointType;
+parcelable WaypointCreationRequest;

MyTracksLib/src/com/google/android/apps/mytracks/content/WaypointCreationRequest.java

@@ -0,0 +1,106 @@
+/*
+ * 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.content;
+
+import android.os.Parcel;
+import android.os.Parcelable;
+
+/**
+ * A request for the service to create a waypoint at the current location.
+ *
+ * @author Sandor Dornbush
+ */
+public class WaypointCreationRequest implements Parcelable {
+
+  public static enum WaypointType {
+    MARKER,
+    STATISTICS;
+  }
+  
+  private WaypointType type;
+  private String name;
+  private String description;
+  private String icon;
+
+  public final static WaypointCreationRequest DEFAULT_MARKER =
+      new WaypointCreationRequest(WaypointType.MARKER);
+  public final static WaypointCreationRequest DEFAULT_STATISTICS =
+      new WaypointCreationRequest(WaypointType.STATISTICS);
+  
+  public WaypointCreationRequest(WaypointType type) {
+    this.type = type;
+  }
+
+  public static class Creator implements Parcelable.Creator<WaypointCreationRequest> {
+
+    public WaypointCreationRequest createFromParcel(Parcel source) {
+      int i = source.readInt();
+      if (i > WaypointType.values().length) {
+        throw new IllegalArgumentException("Could not find waypoint type: " + i);
+      }
+      WaypointCreationRequest request = new WaypointCreationRequest(WaypointType.values()[i]);
+      return request;
+    }
+
+    public WaypointCreationRequest[] newArray(int size) {
+      return new WaypointCreationRequest[size];
+    }
+  }
+
+  public static final Creator CREATOR = new Creator();
+  
+  @Override
+  public int describeContents() {
+    return 0;
+  }
+
+  @Override
+  public void writeToParcel(Parcel parcel, int arg1) {
+    parcel.writeInt(type.ordinal());
+  }
+
+  public WaypointType getType() {
+    return type;
+  }
+
+  public void setType(WaypointType type) {
+    this.type = type;
+  }
+
+  public String getName() {
+    return name;
+  }
+
+  public void setName(String name) {
+    this.name = name;
+  }
+
+  public String getDescription() {
+    return description;
+  }
+
+  public void setDescription(String description) {
+    this.description = description;
+  }
+
+  public String getIcon() {
+    return icon;
+  }
+
+  public void setIcon(String icon) {
+    this.icon = icon;
+  }
+}

MyTracksLib/src/com/google/android/apps/mytracks/content/WaypointType.java

@@ -1,60 +0,0 @@
-/*
- * 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.content;
-
-import android.os.Parcel;
-import android.os.Parcelable;
-
-/**
- * An enumeration of the types of waypoints that can be created.
- *
- * @author Sandor Dornbush
- */
-public enum WaypointType implements Parcelable {
-
-  MARKER,
-  STATISTICS;
-  
-  public static class Creator implements Parcelable.Creator<WaypointType> {
-
-    public WaypointType createFromParcel(Parcel source) {
-      int i = source.readInt();
-      for (WaypointType type : WaypointType.values()) {
-        if (i == type.ordinal()) {
-          return type;
-        }
-      }
-      return null;
-    }
-
-    public WaypointType[] newArray(int size) {
-      return new WaypointType[size];
-    }
-  }
-
-  public static final Creator CREATOR = new Creator();
-  
-  @Override
-  public int describeContents() {
-    return 0;
-  }
-
-  @Override
-  public void writeToParcel(Parcel parcel, int arg1) {
-    parcel.writeInt(ordinal());
-  }
-
-}

MyTracksLib/src/com/google/android/apps/mytracks/services/ITrackRecordingService.aidl

@@ -15,7 +15,7 @@
  */
 package com.google.android.apps.mytracks.services;
 
-import com.google.android.apps.mytracks.content.WaypointType;
+import com.google.android.apps.mytracks.content.WaypointCreationRequest;
 
 /**
  * MyTracks service.
@@ -45,10 +45,10 @@ interface ITrackRecordingService {
   /**
    * Inserts a waypoint marker in the track being recorded.
    *
-   * @param type of the waypoint to insert
+   * @param request Details for the waypoint to be inserted.
    * @return the unique ID of the inserted marker
    */
-  long insertWaypoint(in WaypointType type);
+  long insertWaypoint(in WaypointCreationRequest request);
 
   /**
    * Inserts a location in the track being recorded.

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

@@ -22,7 +22,7 @@ import com.google.android.apps.mytracks.content.MyTracksProvider;
 import com.google.android.apps.mytracks.content.MyTracksProviderUtils;
 import com.google.android.apps.mytracks.content.Track;
 import com.google.android.apps.mytracks.content.Waypoint;
-import com.google.android.apps.mytracks.content.WaypointType;
+import com.google.android.apps.mytracks.content.WaypointCreationRequest;
 import com.google.android.apps.mytracks.stats.TripStatistics;
 import com.google.android.apps.mytracks.util.ApiFeatures;
 import com.google.android.maps.mytracks.R;
@@ -548,7 +548,7 @@ public class TrackRecordingServiceTest
     assertFalse(service.isRecording());
     
     try {
-      service.insertWaypoint(WaypointType.STATISTICS);
+      service.insertWaypoint(WaypointCreationRequest.DEFAULT_STATISTICS);
       fail("Expecting IllegalStateException");
     } catch (IllegalStateException e) {
       // Expected.
@@ -562,8 +562,8 @@ public class TrackRecordingServiceTest
     ITrackRecordingService service = bindAndGetService(createStartIntent());
     assertTrue(service.isRecording());
     
-    assertEquals(1, service.insertWaypoint(WaypointType.STATISTICS));
+    assertEquals(1, service.insertWaypoint(WaypointCreationRequest.DEFAULT_STATISTICS));
-    assertEquals(2, service.insertWaypoint(WaypointType.STATISTICS));
+    assertEquals(2, service.insertWaypoint(WaypointCreationRequest.DEFAULT_STATISTICS));
     
     Waypoint wpt = providerUtils.getWaypoint(1);
     assertEquals(getContext().getString(R.string.stats_icon_url),
@@ -586,7 +586,7 @@ public class TrackRecordingServiceTest
     assertFalse(service.isRecording());
     
     try {
-      service.insertWaypoint(WaypointType.MARKER);
+      service.insertWaypoint(WaypointCreationRequest.DEFAULT_MARKER);
       fail("Expecting IllegalStateException");
     } catch (IllegalStateException e) {
       // Expected.
@@ -600,7 +600,7 @@ public class TrackRecordingServiceTest
     ITrackRecordingService service = bindAndGetService(createStartIntent());
     assertTrue(service.isRecording());
     
-    assertEquals(1, service.insertWaypoint(WaypointType.MARKER));
+    assertEquals(1, service.insertWaypoint(WaypointCreationRequest.DEFAULT_MARKER));
     Waypoint wpt = providerUtils.getWaypoint(1);
     assertEquals(getContext().getString(R.string.waypoint_icon_url),
         wpt.getIcon());
@@ -806,9 +806,9 @@ public class TrackRecordingServiceTest
       service.recordLocation(loc);
       
       if (i % 10 == 0) {
-        service.insertWaypoint(WaypointType.STATISTICS);
+        service.insertWaypoint(WaypointCreationRequest.DEFAULT_STATISTICS);
       } else if (i % 7 == 0) {
-        service.insertWaypoint(WaypointType.MARKER);
+        service.insertWaypoint(WaypointCreationRequest.DEFAULT_MARKER);
       }
     }
     

SignalStrengthSampler/src/com/google/android/apps/mytracks/signalstrength/SignalStrengthService.java

@@ -20,7 +20,7 @@ import static com.google.android.apps.mytracks.signalstrength.SignalStrengthCons
 import com.google.android.apps.mytracks.content.MyTracksProviderUtils;
 import com.google.android.apps.mytracks.content.MyTracksProviderUtilsImpl;
 import com.google.android.apps.mytracks.content.Waypoint;
-import com.google.android.apps.mytracks.content.WaypointType;
+import com.google.android.apps.mytracks.content.WaypointCreationRequest;
 import com.google.android.apps.mytracks.services.ITrackRecordingService;
 import com.google.android.apps.mytracks.signalstrength.SignalStrengthListener.SignalStrengthCallback;
 
@@ -167,18 +167,13 @@ public class SignalStrengthService extends Service
           return;
         }
 
-        // First we create a basic waypoint.
+        // Create a waypoint.
-        waypointId = mytracksService.insertWaypoint(WaypointType.MARKER);
+        WaypointCreationRequest request =
-       
+          new WaypointCreationRequest(WaypointCreationRequest.WaypointType.MARKER);
-        // Then we fetch the actual waypoint from the provider.
+        request.setName("Signal Strength");
-        Waypoint wpt = utils.getWaypoint(waypointId);
+        request.setIcon(icon);
-        wpt.setName("Signal Strength");
+        request.setDescription(description);
-        wpt.setType(Waypoint.TYPE_WAYPOINT);
+        waypointId = mytracksService.insertWaypoint(request);
-        wpt.setIcon(icon);
-        wpt.setDescription(description);
-
-        // Finally write the modified waypoint back to the db.
-        utils.updateWaypoint(wpt);
       }
 
       if (waypointId >= 0) {

SignalStrengthSamplerTest/.classpath

@@ -2,8 +2,8 @@
 <classpath>
 	<classpathentry combineaccessrules="false" exported="true" kind="src" path="/SignalStrengthSampler"/>
 	<classpathentry kind="src" path="src"/>
-	<classpathentry kind="src" path="gen"/>
 	<classpathentry kind="con" path="com.android.ide.eclipse.adt.ANDROID_FRAMEWORK"/>
 	<classpathentry kind="lib" path="libs/AndroidMockRuntime.jar"/>
+	<classpathentry kind="src" path="gen"/>
 	<classpathentry kind="output" path="bin"/>
 </classpath>