From b395c19832723324d544e5d513dde3966ec6ba60 Mon Sep 17 00:00:00 2001 From: Dennis Guse Date: Mon, 3 May 2021 22:31:11 +0200 Subject: [PATCH] Bugfix: voice announcements need to use KM/Miles (instead of m/feet). Fixes #722. --- .../services/TrackRecordingServiceTest.java | 5 ++ .../AnnouncementPeriodicTaskFactoryTest.java | 59 ---------------- .../tasks/PeriodicTaskExecutorTest.java | 69 +++++++++++++++++++ .../services/TrackRecordingService.java | 4 +- .../tasks/AnnouncementPeriodicTask.java | 18 ++++- .../AnnouncementPeriodicTaskFactory.java | 31 --------- .../services/tasks/PeriodicTask.java | 4 +- .../services/tasks/PeriodicTaskExecutor.java | 59 +++++++--------- .../services/tasks/PeriodicTaskFactory.java | 3 + src/main/res/values/settings.xml | 20 ++++++ src/main/res/values/settings_deprecated.xml | 19 ----- 11 files changed, 144 insertions(+), 147 deletions(-) delete mode 100644 src/androidTest/java/de/dennisguse/opentracks/services/tasks/AnnouncementPeriodicTaskFactoryTest.java create mode 100644 src/androidTest/java/de/dennisguse/opentracks/services/tasks/PeriodicTaskExecutorTest.java delete mode 100644 src/main/java/de/dennisguse/opentracks/services/tasks/AnnouncementPeriodicTaskFactory.java diff --git a/src/androidTest/java/de/dennisguse/opentracks/services/TrackRecordingServiceTest.java b/src/androidTest/java/de/dennisguse/opentracks/services/TrackRecordingServiceTest.java index 14882a4af..2094d9da8 100644 --- a/src/androidTest/java/de/dennisguse/opentracks/services/TrackRecordingServiceTest.java +++ b/src/androidTest/java/de/dennisguse/opentracks/services/TrackRecordingServiceTest.java @@ -31,6 +31,7 @@ import androidx.test.rule.GrantPermissionRule; import androidx.test.rule.ServiceTestRule; import org.junit.After; +import org.junit.AfterClass; import org.junit.Before; import org.junit.BeforeClass; import org.junit.Rule; @@ -83,6 +84,10 @@ public class TrackRecordingServiceTest { if (Looper.myLooper() == null) Looper.prepare(); } + @AfterClass + public static void finalTearDown() { + if (Looper.myLooper() != null) Looper.myLooper().quit(); + } private final Context context = ApplicationProvider.getApplicationContext(); private final SharedPreferences sharedPreferences = PreferencesUtils.getSharedPreferences(context); diff --git a/src/androidTest/java/de/dennisguse/opentracks/services/tasks/AnnouncementPeriodicTaskFactoryTest.java b/src/androidTest/java/de/dennisguse/opentracks/services/tasks/AnnouncementPeriodicTaskFactoryTest.java deleted file mode 100644 index 32f75b572..000000000 --- a/src/androidTest/java/de/dennisguse/opentracks/services/tasks/AnnouncementPeriodicTaskFactoryTest.java +++ /dev/null @@ -1,59 +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 de.dennisguse.opentracks.services.tasks; - -import android.content.Context; -import android.os.Looper; - -import androidx.test.core.app.ApplicationProvider; -import androidx.test.ext.junit.runners.AndroidJUnit4; - -import org.junit.AfterClass; -import org.junit.BeforeClass; -import org.junit.Test; -import org.junit.runner.RunWith; - -import static org.junit.Assert.assertTrue; - -/** - * Tests for {@link AnnouncementPeriodicTaskFactory}. - * - * @author Rodrigo Damazio - */ -@RunWith(AndroidJUnit4.class) -public class AnnouncementPeriodicTaskFactoryTest { - - private final Context context = ApplicationProvider.getApplicationContext(); - - @BeforeClass - public static void preSetUp() { - // Prepare looper for Android's message queue - if (Looper.myLooper() == null) Looper.prepare(); - } - - @AfterClass - public static void finalTearDown() { - if (Looper.myLooper() != null) Looper.myLooper().quit(); - } - - @Test - public void testCreate() { - PeriodicTaskFactory factory = new AnnouncementPeriodicTaskFactory(); - PeriodicTask task = factory.create(context); - assertTrue(task instanceof AnnouncementPeriodicTask); - } -} diff --git a/src/androidTest/java/de/dennisguse/opentracks/services/tasks/PeriodicTaskExecutorTest.java b/src/androidTest/java/de/dennisguse/opentracks/services/tasks/PeriodicTaskExecutorTest.java new file mode 100644 index 000000000..0f54013f6 --- /dev/null +++ b/src/androidTest/java/de/dennisguse/opentracks/services/tasks/PeriodicTaskExecutorTest.java @@ -0,0 +1,69 @@ +package de.dennisguse.opentracks.services.tasks; + +import android.content.Context; +import android.content.Intent; +import android.os.Looper; + +import androidx.test.core.app.ApplicationProvider; +import androidx.test.ext.junit.runners.AndroidJUnit4; +import androidx.test.rule.GrantPermissionRule; +import androidx.test.rule.ServiceTestRule; + +import org.junit.AfterClass; +import org.junit.BeforeClass; +import org.junit.Rule; +import org.junit.Test; +import org.junit.runner.RunWith; + +import java.util.concurrent.TimeUnit; +import java.util.concurrent.TimeoutException; + +import de.dennisguse.opentracks.content.data.Distance; +import de.dennisguse.opentracks.services.TrackRecordingService; +import de.dennisguse.opentracks.stats.TrackStatistics; + +import static org.junit.Assert.assertEquals; + +@RunWith(AndroidJUnit4.class) +public class PeriodicTaskExecutorTest { + + @Rule + public final ServiceTestRule mServiceRule = ServiceTestRule.withTimeout(5, TimeUnit.SECONDS); + + @Rule + public GrantPermissionRule mRuntimePermissionRule = GrantPermissionRule.grant(android.Manifest.permission.ACCESS_FINE_LOCATION); + + @BeforeClass + public static void preSetUp() { + // Prepare looper for Android's message queue + if (Looper.myLooper() == null) Looper.prepare(); + } + + @AfterClass + public static void finalTearDown() { + if (Looper.myLooper() != null) Looper.myLooper().quit(); + } + + + private final Context context = ApplicationProvider.getApplicationContext(); + + @Test + public void calculateNextTaskDistance() throws TimeoutException { + // given + TrackRecordingService service = ((TrackRecordingService.Binder) mServiceRule.bindService(new Intent(context, TrackRecordingService.class))) + .getService(); + + PeriodicTaskExecutor periodicTaskExecutor = new PeriodicTaskExecutor(service, new AnnouncementPeriodicTask.Factory()); + periodicTaskExecutor.setMetricUnits(true); + periodicTaskExecutor.setTaskFrequency(-5); + + // when + TrackStatistics statistics = new TrackStatistics(); + statistics.setTotalDistance(Distance.of(13000)); + assertEquals(Distance.of(15000), periodicTaskExecutor.calculateNextTaskDistance(statistics)); + + statistics.setTotalDistance(Distance.of(15100)); + assertEquals(Distance.of(20000), periodicTaskExecutor.calculateNextTaskDistance(statistics)); + } + +} \ No newline at end of file diff --git a/src/main/java/de/dennisguse/opentracks/services/TrackRecordingService.java b/src/main/java/de/dennisguse/opentracks/services/TrackRecordingService.java index bc823184d..728f21f43 100644 --- a/src/main/java/de/dennisguse/opentracks/services/TrackRecordingService.java +++ b/src/main/java/de/dennisguse/opentracks/services/TrackRecordingService.java @@ -54,7 +54,7 @@ import de.dennisguse.opentracks.services.handlers.GpsStatusValue; import de.dennisguse.opentracks.services.handlers.HandlerServer; import de.dennisguse.opentracks.services.sensors.AltitudeSumManager; import de.dennisguse.opentracks.services.sensors.BluetoothRemoteSensorManager; -import de.dennisguse.opentracks.services.tasks.AnnouncementPeriodicTaskFactory; +import de.dennisguse.opentracks.services.tasks.AnnouncementPeriodicTask; import de.dennisguse.opentracks.services.tasks.PeriodicTaskExecutor; import de.dennisguse.opentracks.settings.SettingsActivity; import de.dennisguse.opentracks.stats.TrackStatistics; @@ -139,7 +139,7 @@ public class TrackRecordingService extends Service implements HandlerServer.Hand handlerServer = new HandlerServer(this); contentProviderUtils = new ContentProviderUtils(this); - voiceExecutor = new PeriodicTaskExecutor(this, new AnnouncementPeriodicTaskFactory()); + voiceExecutor = new PeriodicTaskExecutor(this, new AnnouncementPeriodicTask.Factory()); notificationManager = new TrackRecordingServiceNotificationManager(this); diff --git a/src/main/java/de/dennisguse/opentracks/services/tasks/AnnouncementPeriodicTask.java b/src/main/java/de/dennisguse/opentracks/services/tasks/AnnouncementPeriodicTask.java index 1fdceb588..6db83ec34 100644 --- a/src/main/java/de/dennisguse/opentracks/services/tasks/AnnouncementPeriodicTask.java +++ b/src/main/java/de/dennisguse/opentracks/services/tasks/AnnouncementPeriodicTask.java @@ -23,6 +23,8 @@ import android.speech.tts.TextToSpeech; import android.speech.tts.UtteranceProgressListener; import android.util.Log; +import androidx.annotation.NonNull; + import java.util.ArrayList; import java.util.Arrays; import java.util.Locale; @@ -135,7 +137,7 @@ public class AnnouncementPeriodicTask implements PeriodicTask { } @Override - public void run(TrackRecordingService trackRecordingService) { + public void run(@NonNull TrackRecordingService trackRecordingService) { if (trackRecordingService == null) { Log.e(TAG, "TrackRecordingService is null."); return; @@ -231,4 +233,18 @@ public class AnnouncementPeriodicTask implements PeriodicTask { // We don't care about the utterance id. It is supplied here to force onUtteranceCompleted to be called. tts.speak(announcement, TextToSpeech.QUEUE_FLUSH, null, "not used"); } + + /** + * A {@link PeriodicTaskFactory} for text-to-speech announcement periodic task. + * + * @author Rodrigo Damazio + */ + public static class Factory implements PeriodicTaskFactory { + + @Override + @NonNull + public PeriodicTask create(Context context) { + return new AnnouncementPeriodicTask(context); + } + } } diff --git a/src/main/java/de/dennisguse/opentracks/services/tasks/AnnouncementPeriodicTaskFactory.java b/src/main/java/de/dennisguse/opentracks/services/tasks/AnnouncementPeriodicTaskFactory.java deleted file mode 100644 index 86b5a44ef..000000000 --- a/src/main/java/de/dennisguse/opentracks/services/tasks/AnnouncementPeriodicTaskFactory.java +++ /dev/null @@ -1,31 +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 de.dennisguse.opentracks.services.tasks; - -import android.content.Context; - -/** - * A {@link PeriodicTaskFactory} for text-to-speech announcement periodic task. - * - * @author Rodrigo Damazio - */ -public class AnnouncementPeriodicTaskFactory implements PeriodicTaskFactory { - - @Override - public PeriodicTask create(Context context) { - return new AnnouncementPeriodicTask(context); - } -} diff --git a/src/main/java/de/dennisguse/opentracks/services/tasks/PeriodicTask.java b/src/main/java/de/dennisguse/opentracks/services/tasks/PeriodicTask.java index 64395614e..7cc575113 100644 --- a/src/main/java/de/dennisguse/opentracks/services/tasks/PeriodicTask.java +++ b/src/main/java/de/dennisguse/opentracks/services/tasks/PeriodicTask.java @@ -16,6 +16,8 @@ package de.dennisguse.opentracks.services.tasks; +import androidx.annotation.NonNull; + import de.dennisguse.opentracks.services.TrackRecordingService; /** @@ -35,7 +37,7 @@ public interface PeriodicTask { * * @param trackRecordingService the track recording service */ - void run(TrackRecordingService trackRecordingService); + void run(@NonNull TrackRecordingService trackRecordingService); /** * Shuts down this task and clean up resources. diff --git a/src/main/java/de/dennisguse/opentracks/services/tasks/PeriodicTaskExecutor.java b/src/main/java/de/dennisguse/opentracks/services/tasks/PeriodicTaskExecutor.java index 6c3692281..623132288 100644 --- a/src/main/java/de/dennisguse/opentracks/services/tasks/PeriodicTaskExecutor.java +++ b/src/main/java/de/dennisguse/opentracks/services/tasks/PeriodicTaskExecutor.java @@ -17,6 +17,9 @@ package de.dennisguse.opentracks.services.tasks; import android.util.Log; +import androidx.annotation.NonNull; +import androidx.annotation.VisibleForTesting; + import java.time.Duration; import de.dennisguse.opentracks.R; @@ -52,14 +55,13 @@ public class PeriodicTaskExecutor { private boolean metricUnits; - // The next distance for the distance periodic task - private double nextTaskDistance = Double.MAX_VALUE; + private Distance nextTaskDistance = Distance.of(Double.MAX_VALUE); - public PeriodicTaskExecutor(TrackRecordingService trackRecordingService, PeriodicTaskFactory periodicTaskFactory) { + public PeriodicTaskExecutor(@NonNull TrackRecordingService trackRecordingService, PeriodicTaskFactory periodicTaskFactory) { this.trackRecordingService = trackRecordingService; this.periodicTaskFactory = periodicTaskFactory; - TASK_FREQUENCY_OFF = Integer.parseInt(trackRecordingService.getBaseContext().getResources().getString(R.string.frequency_off)); + TASK_FREQUENCY_OFF = Integer.parseInt(trackRecordingService.getString(R.string.frequency_off)); taskFrequency = TASK_FREQUENCY_OFF; } @@ -97,7 +99,7 @@ public class PeriodicTaskExecutor { timerTaskExecutor.scheduleTask(Duration.ofMinutes(taskFrequency)); } else { // For distance periodic task - calculateNextTaskDistance(); + updateNextTaskDistance(); } } @@ -128,59 +130,48 @@ public class PeriodicTaskExecutor { return; } - Distance distance = trackStatistics.getTotalDistance(); - - if (distance.greaterThan(Distance.of(nextTaskDistance))) { + if (trackStatistics.getTotalDistance().greaterThan(nextTaskDistance)) { periodicTask.run(trackRecordingService); - calculateNextTaskDistance(); + updateNextTaskDistance(); } } - /** - * Sets task frequency. - * - * @param taskFrequency the task frequency - */ public void setTaskFrequency(int taskFrequency) { this.taskFrequency = taskFrequency; restore(); } - /** - * Sets metricUnits. - * - * @param metricUnits true to use metric units - */ public void setMetricUnits(boolean metricUnits) { this.metricUnits = metricUnits; - calculateNextTaskDistance(); + updateNextTaskDistance(); } - /** - * Calculates the next distance for the distance periodic task. - */ - private void calculateNextTaskDistance() { + private void updateNextTaskDistance() { if (!trackRecordingService.isRecording() || trackRecordingService.isPaused() || periodicTask == null) { return; } + if (!isDistanceFrequency()) { + nextTaskDistance = Distance.of(Double.MAX_VALUE); + Log.d(TAG, "SplitManager: Distance splits disabled."); + return; + } + TrackStatistics trackStatistics = trackRecordingService.getTrackStatistics(); if (trackStatistics == null) { return; } - if (!isDistanceFrequency()) { - nextTaskDistance = Double.MAX_VALUE; - Log.d(TAG, "SplitManager: Distance splits disabled."); - return; - } + nextTaskDistance = calculateNextTaskDistance(trackStatistics); + } - double distance = trackStatistics.getTotalDistance().toKM_Miles(metricUnits); + @VisibleForTesting + public Distance calculateNextTaskDistance(TrackStatistics trackStatistics) { + Distance distance = trackStatistics.getTotalDistance(); - // The index will be negative since the frequency is negative. - int index = (int) (distance / taskFrequency); - index -= 1; - nextTaskDistance = taskFrequency * index; + Distance announcementInterval = Distance.one(metricUnits).multipliedBy(Math.abs(taskFrequency)); + int index = (int) (distance.dividedBy(announcementInterval)); + return announcementInterval.multipliedBy(index + 1); } /** diff --git a/src/main/java/de/dennisguse/opentracks/services/tasks/PeriodicTaskFactory.java b/src/main/java/de/dennisguse/opentracks/services/tasks/PeriodicTaskFactory.java index ad30beb31..416e96f6e 100644 --- a/src/main/java/de/dennisguse/opentracks/services/tasks/PeriodicTaskFactory.java +++ b/src/main/java/de/dennisguse/opentracks/services/tasks/PeriodicTaskFactory.java @@ -18,6 +18,8 @@ package de.dennisguse.opentracks.services.tasks; import android.content.Context; +import androidx.annotation.NonNull; + /** * An interface for classes that can create {@link PeriodicTask}. * @@ -30,5 +32,6 @@ interface PeriodicTaskFactory { * * @return the task, or null if the task is not supported */ + @NonNull PeriodicTask create(Context context); } \ No newline at end of file diff --git a/src/main/res/values/settings.xml b/src/main/res/values/settings.xml index fbce92392..e3b611699 100644 --- a/src/main/res/values/settings.xml +++ b/src/main/res/values/settings.xml @@ -219,6 +219,26 @@ @string/settings_recording_track_name_number_option + voiceFrequency + @string/frequency_off + 0 + + @string/frequency_off + 1 + 2 + 5 + 10 + 15 + 30 + 60 + -1 + -5 + -10 + -25 + -50 + -100 + + exportTrackFileFormat KMZ_WITH_TRACKDETAIL_AND_SENSORDATA_AND_PICTURES diff --git a/src/main/res/values/settings_deprecated.xml b/src/main/res/values/settings_deprecated.xml index 69eb2061a..8455e60e0 100644 --- a/src/main/res/values/settings_deprecated.xml +++ b/src/main/res/values/settings_deprecated.xml @@ -18,25 +18,6 @@ splitFrequency @string/frequency_off - voiceFrequency - @string/frequency_off - 0 - - @string/frequency_off - 1 - 2 - 5 - 10 - 15 - 30 - 60 - -1 - -5 - -10 - -25 - -50 - -100 - chartShowCadence