diff --git a/MyTracks/src/com/google/android/apps/mytracks/ChartActivity.java b/MyTracks/src/com/google/android/apps/mytracks/ChartActivity.java index 0e3ece007..3c9f8267b 100644 --- a/MyTracks/src/com/google/android/apps/mytracks/ChartActivity.java +++ b/MyTracks/src/com/google/android/apps/mytracks/ChartActivity.java @@ -139,8 +139,8 @@ public class ChartActivity extends Activity implements TrackDataListener { } @Override - protected void onStart() { - super.onStart(); + protected void onResume() { + super.onResume(); dataHub.registerTrackDataListener(this, EnumSet.of( ListenerDataType.SELECTED_TRACK_CHANGED, @@ -151,10 +151,10 @@ public class ChartActivity extends Activity implements TrackDataListener { } @Override - protected void onStop() { + protected void onPause() { dataHub.unregisterTrackDataListener(this); - super.onStop(); + super.onPause(); } private void zoomIn() { @@ -418,6 +418,9 @@ public class ChartActivity extends Activity implements TrackDataListener { @Override public boolean onUnitsChanged(boolean metric) { + boolean changed = metric != this.metricUnits; + if (!changed) return false; + this.metricUnits = metric; chartView.setMetricUnits(metric); @@ -427,6 +430,9 @@ public class ChartActivity extends Activity implements TrackDataListener { @Override public boolean onReportSpeedChanged(boolean reportSpeed) { + boolean changed = reportSpeed != this.reportSpeed; + if (!changed) return false; + this.reportSpeed = reportSpeed; chartView.setReportSpeed(reportSpeed, this); diff --git a/MyTracks/src/com/google/android/apps/mytracks/MapActivity.java b/MyTracks/src/com/google/android/apps/mytracks/MapActivity.java index 7640ac9a6..6eb1934d7 100644 --- a/MyTracks/src/com/google/android/apps/mytracks/MapActivity.java +++ b/MyTracks/src/com/google/android/apps/mytracks/MapActivity.java @@ -178,9 +178,9 @@ public class MapActivity extends com.google.android.maps.MapActivity } @Override - protected void onStart() { + protected void onResume() { Log.d(TAG, "MapActivity.onStart"); - super.onStart(); + super.onResume(); dataHub.registerTrackDataListener(this, EnumSet.of( ListenerDataType.SELECTED_TRACK_CHANGED, @@ -201,12 +201,12 @@ public class MapActivity extends com.google.android.maps.MapActivity } @Override - protected void onStop() { + protected void onPause() { Log.d(TAG, "MapActivity.onStop"); dataHub.unregisterTrackDataListener(this); - super.onStop(); + super.onPause(); } // Utility functions: @@ -508,8 +508,7 @@ public class MapActivity extends com.google.android.maps.MapActivity @Override public void onCurrentLocationChanged(Location location) { if (!location.getProvider().equals(LocationManager.GPS_PROVIDER)) { - Log.d(TAG, - "MapActivity: Network location update received (provider '" + location.getProvider() + "'."); + return; } currentLocation = location; diff --git a/MyTracks/src/com/google/android/apps/mytracks/StatsActivity.java b/MyTracks/src/com/google/android/apps/mytracks/StatsActivity.java index db9ada305..06a3e7c46 100644 --- a/MyTracks/src/com/google/android/apps/mytracks/StatsActivity.java +++ b/MyTracks/src/com/google/android/apps/mytracks/StatsActivity.java @@ -28,6 +28,7 @@ import com.google.android.maps.mytracks.R; import android.app.Activity; import android.location.Location; +import android.location.LocationManager; import android.os.Bundle; import android.util.DisplayMetrics; import android.util.Log; @@ -131,18 +132,18 @@ public class StatsActivity extends Activity implements TrackDataListener { } @Override - protected void onStart() { + protected void onResume() { dataHub.registerTrackDataListener(this, EnumSet.of( ListenerDataType.SELECTED_TRACK_CHANGED, ListenerDataType.TRACK_UPDATES, ListenerDataType.LOCATION_UPDATES, ListenerDataType.DISPLAY_PREFERENCES)); - super.onStart(); + super.onResume(); } @Override - protected void onStop() { + protected void onPause() { dataHub.unregisterTrackDataListener(this); if (thread != null) { @@ -155,6 +156,9 @@ public class StatsActivity extends Activity implements TrackDataListener { @Override public boolean onUnitsChanged(boolean metric) { + // Ignore if unchanged. + if (metric == utils.isMetricUnits()) return false; + utils.setMetricUnits(metric); updateLabels(); @@ -163,6 +167,9 @@ public class StatsActivity extends Activity implements TrackDataListener { @Override public boolean onReportSpeedChanged(boolean displaySpeed) { + // Ignore if unchanged. + if (displaySpeed == utils.isReportSpeed()) return false; + utils.setReportSpeed(displaySpeed); updateLabels(); @@ -254,6 +261,10 @@ public class StatsActivity extends Activity implements TrackDataListener { @Override public void onCurrentLocationChanged(final Location loc) { + if (!loc.getProvider().equals(LocationManager.GPS_PROVIDER)) { + return; + } + if (dataHub.isRecordingSelected()) { runOnUiThread(new Runnable() { @Override diff --git a/MyTracks/src/com/google/android/apps/mytracks/StatsUtilities.java b/MyTracks/src/com/google/android/apps/mytracks/StatsUtilities.java index f6327dace..674122f2c 100644 --- a/MyTracks/src/com/google/android/apps/mytracks/StatsUtilities.java +++ b/MyTracks/src/com/google/android/apps/mytracks/StatsUtilities.java @@ -83,6 +83,8 @@ public class StatsUtilities { public void setText(int id, double d, NumberFormat format) { if (!Double.isNaN(d) && !Double.isInfinite(d)) { setText(id, format.format(d)); + } else { + setUnknown(id); } } @@ -218,7 +220,7 @@ public class StatsUtilities { setGrade(R.id.max_grade_register, maxGrade); } - public void setAllStats(TripStatistics stats) { + public void setAllStats(TripStatistics stats) { setTime(R.id.moving_time_register, stats.getMovingTime()); setDistance(R.id.total_distance_register, stats.getTotalDistance() / 1000); setSpeed(R.id.average_speed_register, stats.getAverageSpeed() * 3.6); diff --git a/MyTracks/src/com/google/android/apps/mytracks/content/TrackDataHub.java b/MyTracks/src/com/google/android/apps/mytracks/content/TrackDataHub.java index 4ed5bcde6..f8205a35a 100644 --- a/MyTracks/src/com/google/android/apps/mytracks/content/TrackDataHub.java +++ b/MyTracks/src/com/google/android/apps/mytracks/content/TrackDataHub.java @@ -103,7 +103,7 @@ public class TrackDataHub { @Override public void notifyPointsUpdated() { - TrackDataHub.this.notifyPointsUpdated(true, + TrackDataHub.this.notifyPointsUpdated(true, 0, getListenersFor(ListenerDataType.POINT_UPDATES), getListenersFor(ListenerDataType.SAMPLED_OUT_POINT_UPDATES)); } @@ -170,16 +170,16 @@ public class TrackDataHub { // Cached GPS readings private Location lastSeenLocation; - private boolean hasProviderEnabled; + private boolean hasProviderEnabled = true; private boolean hasFix; private boolean hasGoodFix; // Transient state about the selected track private long selectedTrackId; - private long recordingTrackId; private long firstSeenLocationId; private long lastSeenLocationId; private int numLoadedPoints; + private int lastSamplingFrequency; /** * Default constructor. @@ -238,7 +238,6 @@ public class TrackDataHub { private void loadSharedPreferences() { selectedTrackId = preferences.getLong(SELECTED_TRACK_KEY, -1); - recordingTrackId = preferences.getLong(RECORDING_TRACK_KEY, -1); useMetricUnits = preferences.getBoolean(METRIC_UNITS_KEY, true); reportSpeed = preferences.getBoolean(SPEED_REPORTING_KEY, true); minRequiredAccuracy = preferences.getInt(MIN_REQUIRED_ACCURACY_KEY, @@ -339,7 +338,7 @@ public class TrackDataHub { if (!started) { loadSharedPreferences(); } - return recordingTrackId > 0; + return preferences.getLong(RECORDING_TRACK_KEY, -1) > 0; } /** Returns whether the selected track is still being recorded. */ @@ -347,7 +346,8 @@ public class TrackDataHub { if (!started) { loadSharedPreferences(); } - return recordingTrackId > 0 && recordingTrackId == selectedTrackId; + long recordingTrackId = preferences.getLong(RECORDING_TRACK_KEY, -1); + return recordingTrackId > 0 && recordingTrackId == selectedTrackId; } /** @@ -414,7 +414,11 @@ public class TrackDataHub { * Reloads all track data received so far into the specified listeners. */ public void reloadDataForListener(TrackDataListener listener) { - reloadDataForListener(listeners.getRegistration(listener)); + ListenerRegistration registration; + synchronized (listeners) { + registration = listeners.getRegistration(listener); + } + reloadDataForListener(registration); } /** @@ -425,21 +429,29 @@ public class TrackDataHub { Log.w(TAG, "Not started, not reloading"); return; } + if (registration == null) { + return; + } runInListenerThread(new Runnable() { @SuppressWarnings("unchecked") @Override public void run() { + // Reload everything if either it's a different track, or the track has been resampled + // (this also covers the case of a new registration). + boolean reloadAll = registration.lastTrackId != selectedTrackId || + registration.lastSamplingFrequency != lastSamplingFrequency; + Log.d(TAG, "Doing a " + (reloadAll ? "full" : "partial") + " reload for " + registration); + TrackDataListener listener = registration.listener; Set listenerSet = Collections.singleton(listener); if (registration.isInterestedIn(ListenerDataType.DISPLAY_PREFERENCES)) { - // Ignore the return values here, we're already sending the full data set anyway - listener.onUnitsChanged(useMetricUnits); - listener.onReportSpeedChanged(reportSpeed); + reloadAll |= listener.onUnitsChanged(useMetricUnits); + reloadAll |= listener.onReportSpeedChanged(reportSpeed); } - if (registration.isInterestedIn(ListenerDataType.SELECTED_TRACK_CHANGED)) { + if (reloadAll && registration.isInterestedIn(ListenerDataType.SELECTED_TRACK_CHANGED)) { notifySelectedTrackChanged(selectedTrackId, listenerSet); } @@ -452,8 +464,10 @@ public class TrackDataHub { boolean interestedInSampledOutPoints = registration.isInterestedIn(ListenerDataType.SAMPLED_OUT_POINT_UPDATES); if (interestedInPoints || interestedInSampledOutPoints) { - notifyPointsCleared(listenerSet); + if (reloadAll) notifyPointsCleared(listenerSet); + notifyPointsUpdated(false, + reloadAll ? 0 : registration.lastPointId + 1, listenerSet, interestedInSampledOutPoints ? listenerSet : Collections.EMPTY_SET); } @@ -481,14 +495,16 @@ public class TrackDataHub { * Reloads all track data received so far into the specified listeners. */ private void loadDataForAllListeners() { - if (!listeners.hasListeners()) { - Log.d(TAG, "No listeners, not reloading"); - return; - } if (!started) { Log.w(TAG, "Not started, not reloading"); return; } + synchronized (listeners) { + if (!listeners.hasListeners()) { + Log.d(TAG, "No listeners, not reloading"); + return; + } + } runInListenerThread(new Runnable() { @Override @@ -510,7 +526,7 @@ public class TrackDataHub { Set sampledOutPointListeners = getListenersFor(ListenerDataType.SAMPLED_OUT_POINT_UPDATES); notifyPointsCleared(pointListeners); - notifyPointsUpdated(true, pointListeners, sampledOutPointListeners); + notifyPointsUpdated(true, 0, pointListeners, sampledOutPointListeners); notifyWaypointUpdated(getListenersFor(ListenerDataType.WAYPOINT_UPDATES)); @@ -532,9 +548,7 @@ public class TrackDataHub { * @param key the key to the preference that changed */ private void notifyPreferenceChanged(String key) { - if (RECORDING_TRACK_KEY.equals(key)) { - recordingTrackId = preferences.getLong(RECORDING_TRACK_KEY, -1); - } else if (MIN_REQUIRED_ACCURACY_KEY.equals(key)) { + if (MIN_REQUIRED_ACCURACY_KEY.equals(key)) { minRequiredAccuracy = preferences.getInt(MIN_REQUIRED_ACCURACY_KEY, Constants.DEFAULT_MIN_REQUIRED_ACCURACY); } else if (METRIC_UNITS_KEY.equals(key)) { @@ -559,7 +573,9 @@ public class TrackDataHub { for (TrackDataListener listener : displayListeners) { // TODO: Do the reloading just once for all interested listeners if (listener.onReportSpeedChanged(reportSpeed)) { - reloadDataForListener(listeners.getRegistration(listener)); + synchronized (listeners) { + reloadDataForListener(listeners.getRegistration(listener)); + } } } } @@ -577,7 +593,9 @@ public class TrackDataHub { for (TrackDataListener listener : displayListeners) { if (listener.onUnitsChanged(useMetricUnits)) { - reloadDataForListener(listeners.getRegistration(listener)); + synchronized (listeners) { + reloadDataForListener(listeners.getRegistration(listener)); + } } } } @@ -796,23 +814,20 @@ public class TrackDataHub { /** * Notifies the given listeners about track points in the given ID range. * - * @param minPointId the first point ID to notify, inclusive - * @param maxPointId the last poind ID to notify, inclusive * @param keepState whether to load and save state about the already-notified points. * If true, only new points are reported. * If false, then the whole track will be loaded, without affecting the store. - * @param listeners the listeners to notify - * @param trackDataListeners + * @param minPointId the first point ID to notify, inclusive */ private void notifyPointsUpdated(final boolean keepState, - final Set sampledListeners, + final long minPointId, final Set sampledListeners, final Set sampledOutListeners) { if (sampledListeners.isEmpty() && sampledOutListeners.isEmpty()) return; runInListenerThread(new Runnable() { @Override public void run() { - notifyPointsUpdatedSync(keepState, sampledListeners, sampledOutListeners); + notifyPointsUpdatedSync(keepState, minPointId, sampledListeners, sampledOutListeners); } }); } @@ -821,12 +836,14 @@ public class TrackDataHub { * Asynchronous version of the above method. */ private void notifyPointsUpdatedSync(boolean keepState, - Set sampledListeners, + long minPointId, Set sampledListeners, Set sampledOutListeners) { // If we're loading state, start from after the last seen point up to the last recorded one // (all new points) // If we're not loading state, then notify about all the previously-seen points. - long minPointId = keepState ? lastSeenLocationId + 1 : 0; + if (minPointId <= 0) { + minPointId = keepState ? lastSeenLocationId + 1 : 0; + } long maxPointId = keepState ? -1 : lastSeenLocationId; // TODO: Move (re)sampling to a separate class. @@ -914,11 +931,36 @@ public class TrackDataHub { lastSeenLocationId = localLastSeenLocationId; } + // Always keep the sampling frequency - if it changes we'll do a full reload above anyway. + lastSamplingFrequency = pointSamplingFrequency; + + // Update the listener state + // TODO: Optimize this (sampledOutListeners should be a subset of sampledListeners, plus + // getRegistration does a lookup for every listener, and this is in the critical path). + updateListenersState(sampledListeners, + currentSelectedTrackId, localLastSeenLocationId, pointSamplingFrequency); + updateListenersState(sampledOutListeners, + currentSelectedTrackId, localLastSeenLocationId, pointSamplingFrequency); + for (TrackDataListener listener : sampledListeners) { listener.onNewTrackPointsDone(); } } + private void updateListenersState(Set sampledListeners, + long trackId, long lastPointId, int samplingFrequency) { + synchronized (listeners) { + for (TrackDataListener listener : sampledListeners) { + ListenerRegistration registration = listeners.getRegistration(listener); + if (registration != null) { + registration.lastTrackId = trackId; + registration.lastPointId = lastPointId; + registration.lastSamplingFrequency = samplingFrequency; + } + } + } + } + private void notifyNewPoint(Location location, long locationId, long lastStoredLocationId, diff --git a/MyTracks/src/com/google/android/apps/mytracks/content/TrackDataListeners.java b/MyTracks/src/com/google/android/apps/mytracks/content/TrackDataListeners.java index 9fb6c0c1a..765033866 100644 --- a/MyTracks/src/com/google/android/apps/mytracks/content/TrackDataListeners.java +++ b/MyTracks/src/com/google/android/apps/mytracks/content/TrackDataListeners.java @@ -27,6 +27,7 @@ import java.util.HashMap; import java.util.LinkedHashSet; import java.util.Map; import java.util.Set; +import java.util.WeakHashMap; /** * Manager for the external data listeners and their listening types. @@ -39,7 +40,11 @@ class TrackDataListeners { static class ListenerRegistration { final TrackDataListener listener; final EnumSet types; - // TODO: Add the last-notified point ID here, to allow pausing/resuming. + + // State that was last notified to the listener, for resuming after a pause. + long lastTrackId; + long lastPointId; + int lastSamplingFrequency; public ListenerRegistration(TrackDataListener listener, EnumSet types) { @@ -50,12 +55,26 @@ class TrackDataListeners { public boolean isInterestedIn(ListenerDataType type) { return types.contains(type); } + + @Override + public String toString() { + return "ListenerRegistration [listener=" + listener + ", types=" + types + + ", lastTrackId=" + lastTrackId + ", lastPointId=" + lastPointId + + ", lastSamplingFrequency=" + lastSamplingFrequency + "]"; + } } /** Map of external listener to its registration details. */ private final Map registeredListeners = new HashMap(); + /** + * Map of external paused listener to its registration details. + * This will automatically discard listeners which are GCed. + */ + private final WeakHashMap oldListeners = + new WeakHashMap(); + /** Map of data type to external listeners interested in it. */ private final Map> listenerSetsPerType = new EnumMap>(ListenerDataType.class); @@ -77,11 +96,14 @@ class TrackDataListeners { */ public ListenerRegistration registerTrackDataListener(final TrackDataListener listener, EnumSet dataTypes) { Log.d(TAG, "Registered track data listener: " + listener); - ListenerRegistration registration = new ListenerRegistration(listener, dataTypes); - - if (registeredListeners.get(listener) != null) { + if (registeredListeners.containsKey(listener)) { throw new IllegalStateException("Listener already registered"); } + + ListenerRegistration registration = oldListeners.remove(listener); + if (registration == null) { + registration = new ListenerRegistration(listener, dataTypes); + } registeredListeners.put(listener, registration); for (ListenerDataType type : dataTypes) { @@ -111,10 +133,17 @@ class TrackDataListeners { for (ListenerDataType type : match.types) { listenerSetsPerType.get(type).remove(listener); } + + // Keep it around in case it's re-registered soon + oldListeners.put(listener, match); } public ListenerRegistration getRegistration(TrackDataListener listener) { - return registeredListeners.get(listener); + ListenerRegistration registration = registeredListeners.get(listener); + if (registration == null) { + registration = oldListeners.get(listener); + } + return registration; } public Set getListenersFor(ListenerDataType type) { diff --git a/MyTracksTest/src/com/google/android/apps/mytracks/content/TrackDataHubTest.java b/MyTracksTest/src/com/google/android/apps/mytracks/content/TrackDataHubTest.java index 991caf5ec..81e3401a2 100644 --- a/MyTracksTest/src/com/google/android/apps/mytracks/content/TrackDataHubTest.java +++ b/MyTracksTest/src/com/google/android/apps/mytracks/content/TrackDataHubTest.java @@ -478,6 +478,14 @@ public class TrackDataHubTest extends AndroidTestCase { // TODO: test loading a track, getting updates, loading another, unloading } + public void testRelisten() { + // TODO: test re-registering an old points listener + } + + public void testRelisten_changed() { + // TODO: test register, get points, unregister, change track, register + } + private void expectStart() { dataSources.registerOnSharedPreferenceChangeListener(capture(preferenceListenerCapture)); }