diff --git a/MyTracks/src/com/google/android/apps/mytracks/services/PeriodicTaskExecuter.java b/MyTracks/src/com/google/android/apps/mytracks/services/PeriodicTaskExecuter.java index f476c5983..737d051d7 100644 --- a/MyTracks/src/com/google/android/apps/mytracks/services/PeriodicTaskExecuter.java +++ b/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 " - + interval + " milliseconds."); + task.getClass().getSimpleName() + " scheduled to start at " + start + + " 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; diff --git a/MyTracks/src/com/google/android/apps/mytracks/services/SplitManager.java b/MyTracks/src/com/google/android/apps/mytracks/services/SplitManager.java index 857d48316..39f4c61ee 100644 --- a/MyTracks/src/com/google/android/apps/mytracks/services/SplitManager.java +++ b/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(); diff --git a/MyTracks/src/com/google/android/apps/mytracks/services/StatusAnnouncerTask.java b/MyTracks/src/com/google/android/apps/mytracks/services/StatusAnnouncerTask.java index 3eccff912..5d2186d05 100644 --- a/MyTracks/src/com/google/android/apps/mytracks/services/StatusAnnouncerTask.java +++ b/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(); - tts = null; + if (tts != null) { + tts.shutdown(); + tts = null; + } } /** diff --git a/MyTracks/src/com/google/android/apps/mytracks/services/TrackRecordingService.java b/MyTracks/src/com/google/android/apps/mytracks/services/TrackRecordingService.java index f7820f43f..788febe5d 100644 --- a/MyTracks/src/com/google/android/apps/mytracks/services/TrackRecordingService.java +++ b/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. - Log.w(MyTracksConstants.TAG, "TrackRecordingService.onCreate: Resetting " - + "an orphaned recording track: " + recordingTrackId); + if (recordingTrackId != -1) { + // Make sure we have consistent state in shared preferences. + 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) { - announcementExecuter.shutdown(); - } + shutdownAnnouncer(); 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) { - announcementExecuter.shutdown(); - announcementExecuter = null; - } + shutdownAnnouncer(); } else { setUpAnnouncer(); } diff --git a/MyTracksTest/src/com/google/android/apps/mytracks/services/TrackRecordingServiceTest.java b/MyTracksTest/src/com/google/android/apps/mytracks/services/TrackRecordingServiceTest.java index c59a1f975..1fa7a581d 100644 --- a/MyTracksTest/src/com/google/android/apps/mytracks/services/TrackRecordingServiceTest.java +++ b/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 { @@ -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 tracks = providerUtils.getAllTracks(); assertTrue(tracks.isEmpty()); - - 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()); + fullRecordingSession(); } @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()); + } }