From 128b95cd8381202fd8d02c3976617c170dc62fdf Mon Sep 17 00:00:00 2001 From: William Pettersson Date: Thu, 16 Jun 2022 17:18:01 +0100 Subject: [PATCH] Fix voice announcement rounding problem We extracted integral parts before rounding, but String.format does rounding which caused occasional issues. To alleviate, we need to round before passing to String.format. All this rounding logic has now been moved into appendDecimalUnit, and tests have been added to check that rounding works correctly. Fixes: #1285 --- .../VoiceAnnouncementUtilsTest.java | 48 +++++++++++++++++++ .../announcement/VoiceAnnouncementUtils.java | 39 +++++++-------- 2 files changed, 65 insertions(+), 22 deletions(-) diff --git a/src/androidTest/java/de/dennisguse/opentracks/services/announcement/VoiceAnnouncementUtilsTest.java b/src/androidTest/java/de/dennisguse/opentracks/services/announcement/VoiceAnnouncementUtilsTest.java index 3892ba0eb..aeb4b5c3c 100644 --- a/src/androidTest/java/de/dennisguse/opentracks/services/announcement/VoiceAnnouncementUtilsTest.java +++ b/src/androidTest/java/de/dennisguse/opentracks/services/announcement/VoiceAnnouncementUtilsTest.java @@ -67,6 +67,54 @@ public class VoiceAnnouncementUtilsTest { assertEquals("Total distance 20.00 kilometers. 1 hour 5 minutes 10 seconds. Speed 18.4 kilometers per hour.", announcement); } + @Test + public void getAnnouncement_metric_speed_rounding_check() { + TrackStatistics stats = new TrackStatistics(); + stats.setTotalDistance(Distance.of(20000)); + stats.setTotalTime(Duration.ofHours(1).plusMinutes(5).plusSeconds(10)); + stats.setMovingTime(Duration.ofHours(1).plusSeconds(1)); + stats.setMaxSpeed(Speed.of(100)); + stats.setTotalAltitudeGain(6000f); + + // when + String announcement = VoiceAnnouncementUtils.getAnnouncement(context, stats, UnitSystem.METRIC, true, null, null).toString(); + + // then + assertEquals("Total distance 20.00 kilometers. 1 hour 1 second. Speed 20.0 kilometers per hour.", announcement); + } + + @Test + public void getAnnouncement_metric_distance_rounding_check() { + TrackStatistics stats = new TrackStatistics(); + stats.setTotalDistance(Distance.of(19999)); + stats.setTotalTime(Duration.ofHours(1).plusMinutes(5).plusSeconds(10)); + stats.setMovingTime(Duration.ofHours(1)); + stats.setMaxSpeed(Speed.of(100)); + stats.setTotalAltitudeGain(6000f); + + // when + String announcement = VoiceAnnouncementUtils.getAnnouncement(context, stats, UnitSystem.METRIC, true, null, null).toString(); + + // then + assertEquals("Total distance 20.00 kilometers. 1 hour. Speed 20.0 kilometers per hour.", announcement); + } + + @Test + public void getAnnouncement_metric_distance_rounding_check_two() { + TrackStatistics stats = new TrackStatistics(); + stats.setTotalDistance(Distance.of(19990)); + stats.setTotalTime(Duration.ofHours(1).plusMinutes(5).plusSeconds(10)); + stats.setMovingTime(Duration.ofHours(1)); + stats.setMaxSpeed(Speed.of(100)); + stats.setTotalAltitudeGain(6000f); + + // when + String announcement = VoiceAnnouncementUtils.getAnnouncement(context, stats, UnitSystem.METRIC, true, null, null).toString(); + + // then + assertEquals("Total distance 19.99 kilometers. 1 hour. Speed 20.0 kilometers per hour.", announcement); + } + @Test public void getAnnouncement_withInterval_metric_speed() { // given diff --git a/src/main/java/de/dennisguse/opentracks/services/announcement/VoiceAnnouncementUtils.java b/src/main/java/de/dennisguse/opentracks/services/announcement/VoiceAnnouncementUtils.java index d2a8767ef..ddbe2602c 100644 --- a/src/main/java/de/dennisguse/opentracks/services/announcement/VoiceAnnouncementUtils.java +++ b/src/main/java/de/dennisguse/opentracks/services/announcement/VoiceAnnouncementUtils.java @@ -72,12 +72,9 @@ class VoiceAnnouncementUtils { if (shouldVoiceAnnounceTotalDistance()) { builder.append(context.getString(R.string.total_distance)); - long distanceIntegerPart = (long) distanceInUnit; - // Extract the decimal part - String distanceFractionalPart = String.format("%.2f", (distanceInUnit - distanceIntegerPart)).substring(2); // Units should always be english singular for TTS. // See https://developer.android.com/reference/android/text/style/TtsSpan?hl=en#TYPE_MEASURE - appendDecimalUnit(builder, context.getResources().getQuantityString(distanceId, getQuantityCount(distanceInUnit), distanceInUnit), distanceIntegerPart, distanceFractionalPart, unitDistanceTTS); + appendDecimalUnit(builder, context.getResources().getQuantityString(distanceId, getQuantityCount(distanceInUnit), distanceInUnit), distanceInUnit, 2, unitDistanceTTS); // Punctuation helps introduce natural pauses in TTS builder.append("."); } @@ -95,27 +92,17 @@ class VoiceAnnouncementUtils { if (isReportSpeed) { if (shouldVoiceAnnounceAverageSpeedPace()) { double speedInUnit = distancePerTime.to(unitSystem); - builder.append(" ") .append(context.getString(R.string.speed)); - long speedIntegerPart = (long) speedInUnit; - // Extract the decimal part - String speedFractionalPart = String.format("%.1f", (speedInUnit - speedIntegerPart)).substring(2); - appendDecimalUnit(builder, context.getResources().getQuantityString(speedId, getQuantityCount(speedInUnit), speedInUnit), speedIntegerPart, speedFractionalPart, unitSpeedTTS); + appendDecimalUnit(builder, context.getResources().getQuantityString(speedId, getQuantityCount(speedInUnit), speedInUnit), speedInUnit, 1, unitSpeedTTS); builder.append("."); } - if (shouldVoiceAnnounceLapSpeedPace() && currentDistancePerTime != null) { double currentDistancePerTimeInUnit = currentDistancePerTime.to(unitSystem); - if (currentDistancePerTimeInUnit > 0) { - builder.append(" ") .append(context.getString(R.string.lap_speed)); - long currentDistanceIntegerPart = (long) currentDistancePerTimeInUnit; - // Extract the decimal part - String currentDistanceFractionalPart = String.format("%.1f", (currentDistancePerTimeInUnit - currentDistanceIntegerPart)).substring(2); - appendDecimalUnit(builder, context.getResources().getQuantityString(speedId, getQuantityCount(currentDistancePerTimeInUnit), currentDistancePerTimeInUnit), currentDistanceIntegerPart, currentDistanceFractionalPart, unitSpeedTTS); + appendDecimalUnit(builder, context.getResources().getQuantityString(speedId, getQuantityCount(currentDistancePerTimeInUnit), currentDistancePerTimeInUnit), currentDistancePerTimeInUnit, 1, unitSpeedTTS); builder.append("."); } } @@ -171,26 +158,34 @@ class VoiceAnnouncementUtils { int seconds = (int) (duration.getSeconds() % 60); if (hours > 0) { - appendDecimalUnit(builder, context.getResources().getQuantityString(R.plurals.voiceHours, hours, hours), hours, null, "hour"); + appendDecimalUnit(builder, context.getResources().getQuantityString(R.plurals.voiceHours, hours, hours), hours, 0, "hour"); } if (minutes > 0) { - appendDecimalUnit(builder, context.getResources().getQuantityString(R.plurals.voiceMinutes, minutes, minutes), minutes, null, "minute"); + appendDecimalUnit(builder, context.getResources().getQuantityString(R.plurals.voiceMinutes, minutes, minutes), minutes, 0, "minute"); } if (seconds > 0 || duration.isZero()) { - appendDecimalUnit(builder, context.getResources().getQuantityString(R.plurals.voiceSeconds, seconds, seconds), seconds, null, "second"); + appendDecimalUnit(builder, context.getResources().getQuantityString(R.plurals.voiceSeconds, seconds, seconds), seconds, 0, "second"); } } /** * Speaks as: 98.14 [UNIT] - ninety eight point one four [UNIT with correct plural form] + * + * @param number The number to speak + * @param precision The number of decimal places to announce */ - private static void appendDecimalUnit(@NonNull SpannableStringBuilder builder, @NonNull String localizedText, long integerPart, @Nullable String fractionalPart, @NonNull String unit) { + private static void appendDecimalUnit(@NonNull SpannableStringBuilder builder, @NonNull String localizedText, double number, int precision, @NonNull String unit) { TtsSpan.MeasureBuilder measureBuilder = new TtsSpan.MeasureBuilder() .setUnit(unit); - if (fractionalPart == null) { - measureBuilder.setNumber(integerPart); + if (precision == 0) { + measureBuilder.setNumber((long)number); } else { + // Round before extracting integral and decimal parts + double number = Math.round(Math.pow(10, precision) * number) / Math.pow(10.0, precision); + long integerPart = (long) number; + // Extract the decimal part + String fractionalPart = String.format("%." + precision + "f", (number - integerPart)).substring(2); measureBuilder.setIntegerPart(integerPart) .setFractionalPart(fractionalPart); }