Merge pull request #1631 from smartdevicelink/bugfix/issue_1630_fix_choice_set_default_timeout

Bugfix/issue 1630 fix choice set default timeout

3 files changed, 67 insertions, 27 deletions

diff --git a/android/sdl_android/src/androidTest/java/com/smartdevicelink/managers/screen/choiceset/ChoiceSetTests.java b/android/sdl_android/src/androidTest/java/com/smartdevicelink/managers/screen/choiceset/ChoiceSetTests.java
index 4eeb04927..1e2c7359a 100644
--- a/android/sdl_android/src/androidTest/java/com/smartdevicelink/managers/screen/choiceset/ChoiceSetTests.java
+++ b/android/sdl_android/src/androidTest/java/com/smartdevicelink/managers/screen/choiceset/ChoiceSetTests.java
@@ -79,6 +79,23 @@ public class ChoiceSetTests {
         assertEquals(choiceSet.getTimeout(), defaultTimeout);
         assertEquals(choiceSet.getChoices(), choices);
         assertEquals(choiceSet.getChoiceSetSelectionListener(), listener);
+
+        // Test timeout and default timeout
+        choiceSet.setDefaultTimeout(20);
+        assertEquals(choiceSet.getDefaultTimeout(), 20);
+        choiceSet.setDefaultTimeout(1);
+        assertEquals(choiceSet.getDefaultTimeout(), 5);
+        choiceSet.setDefaultTimeout(101);
+        assertEquals(choiceSet.getDefaultTimeout(), 100);
+
+        choiceSet.setTimeout(20);
+        assertEquals(choiceSet.getTimeout().intValue(), 20);
+        choiceSet.setTimeout(1);
+        assertEquals(choiceSet.getTimeout().intValue(), 5);
+        choiceSet.setTimeout(101);
+        assertEquals(choiceSet.getTimeout().intValue(), 100);
+        // Reset default value for other unit test
+        choiceSet.setDefaultTimeout(10);
     }
 
     @Test
@@ -86,6 +103,7 @@ public class ChoiceSetTests {
 
         // first constructor was tested in previous method, use the rest here
         ChoiceSet choiceSet = new ChoiceSet(TestValues.GENERAL_STRING, layout, TestValues.GENERAL_INTEGER, TestValues.GENERAL_STRING, TestValues.GENERAL_STRING, TestValues.GENERAL_STRING, TestValues.GENERAL_VRHELPITEM_LIST, TestValues.GENERAL_KEYBOARDPROPERTIES, choices, listener);
+
         assertEquals(choiceSet.getTitle(), TestValues.GENERAL_STRING);
         assertEquals(choiceSet.getInitialPrompt().get(0).getText(), TestValues.GENERAL_STRING);
         assertEquals(choiceSet.getHelpPrompt().get(0).getText(), TestValues.GENERAL_STRING);
@@ -95,6 +113,7 @@ public class ChoiceSetTests {
         assertEquals(choiceSet.getChoices(), choices);
         assertEquals(choiceSet.getChoiceSetSelectionListener(), listener);
 
+
         ChoiceSet choiceSet2 = new ChoiceSet(TestValues.GENERAL_STRING, layout, TestValues.GENERAL_INTEGER, TestValues.GENERAL_TTSCHUNK_LIST, TestValues.GENERAL_TTSCHUNK_LIST, TestValues.GENERAL_TTSCHUNK_LIST, TestValues.GENERAL_VRHELPITEM_LIST, TestValues.GENERAL_KEYBOARDPROPERTIES, choices, listener);
         assertEquals(choiceSet2.getTitle(), TestValues.GENERAL_STRING);
         assertEquals(choiceSet2.getInitialPrompt(), TestValues.GENERAL_TTSCHUNK_LIST);
diff --git a/base/src/main/java/com/smartdevicelink/managers/screen/AlertView.java b/base/src/main/java/com/smartdevicelink/managers/screen/AlertView.java
index 4e009147b..f69011170 100644
--- a/base/src/main/java/com/smartdevicelink/managers/screen/AlertView.java
+++ b/base/src/main/java/com/smartdevicelink/managers/screen/AlertView.java
@@ -39,10 +39,11 @@ import java.util.ArrayList;
 import java.util.List;
 
 public class AlertView implements Cloneable {
-    
-    private static Integer defaultTimeout = 5;
+
+    private static final int TIMEOUT_DEFAULT = 0;
     private static final int TIMEOUT_MIN = 3;
     private static final int TIMEOUT_MAX = 10;
+    private static Integer defaultTimeout = 5;
     private String text, secondaryText, tertiaryText;
     private Integer timeout;
     private AlertAudioData audio;
@@ -53,7 +54,6 @@ public class AlertView implements Cloneable {
 
 
     private AlertView() {
-        this.timeout = defaultTimeout;
     }
 
     public static class Builder {
@@ -62,6 +62,9 @@ public class AlertView implements Cloneable {
 
         public Builder() {
             alertView = new AlertView();
+            if (alertView.timeout == null) {
+                alertView.timeout = TIMEOUT_DEFAULT;
+            }
         }
 
         /**
@@ -69,7 +72,7 @@ public class AlertView implements Cloneable {
          * on the head unit, the screen manager will automatically concatenate some of the lines together.
          */
         public Builder setText(String text) {
-            this.alertView.text = text;
+            alertView.text = text;
             return this;
         }
 
@@ -165,8 +168,8 @@ public class AlertView implements Cloneable {
     }
 
     public Integer getTimeout() {
-        if (timeout == null) {
-            timeout = defaultTimeout;
+        if (timeout == TIMEOUT_DEFAULT) {
+            timeout = getDefaultTimeout();
         } else if (timeout < TIMEOUT_MIN) {
             return TIMEOUT_MIN;
         } else if (timeout > TIMEOUT_MAX) {
@@ -237,17 +240,15 @@ public class AlertView implements Cloneable {
     }
 
     public int getDefaultTimeout() {
+        if (defaultTimeout < TIMEOUT_MIN) {
+            return TIMEOUT_MIN;
+        } else if (defaultTimeout > TIMEOUT_MAX) {
+            return TIMEOUT_MAX;
+        }
         return defaultTimeout;
     }
 
     public void setDefaultTimeout(int defaultTimeout) {
-        if (defaultTimeout <= TIMEOUT_MIN) {
-            AlertView.defaultTimeout = TIMEOUT_MIN;
-            return;
-        } else if (defaultTimeout >= TIMEOUT_MAX) {
-            AlertView.defaultTimeout = TIMEOUT_MAX;
-            return;
-        }
         AlertView.defaultTimeout = defaultTimeout;
     }
 
diff --git a/base/src/main/java/com/smartdevicelink/managers/screen/choiceset/ChoiceSet.java b/base/src/main/java/com/smartdevicelink/managers/screen/choiceset/ChoiceSet.java
index b8d7412ee..d38cf9713 100644
--- a/base/src/main/java/com/smartdevicelink/managers/screen/choiceset/ChoiceSet.java
+++ b/base/src/main/java/com/smartdevicelink/managers/screen/choiceset/ChoiceSet.java
@@ -59,7 +59,10 @@ public class ChoiceSet {
     ChoiceSetCanceledListener canceledListener;
 
     // defaults
-    private final Integer defaultTimeout = 10;
+    private static final int TIMEOUT_DEFAULT = 0;
+    private static final int TIMEOUT_MIN = 5;
+    private static final int TIMEOUT_MAX = 100;
+    private static Integer defaultTimeout = 10;
     private final ChoiceSetLayout defaultLayout = ChoiceSetLayout.CHOICE_SET_LAYOUT_LIST;
 
     /**
@@ -79,10 +82,10 @@ public class ChoiceSet {
         setTitle(title);
         setChoiceSetSelectionListener(listener);
         setChoices(choices);
+        setTimeout(TIMEOUT_DEFAULT);
 
         // defaults
         setLayout(defaultLayout);
-        setTimeout(defaultTimeout);
 
         // things to do
         checkChoiceSetParameters();
@@ -301,21 +304,43 @@ public class ChoiceSet {
      * @return The Timeout
      */
     public Integer getTimeout() {
+        if (timeout == TIMEOUT_DEFAULT) {
+            timeout = getDefaultTimeout();
+        } else if (timeout < TIMEOUT_MIN) {
+            return TIMEOUT_MIN;
+        } else if (timeout > TIMEOUT_MAX) {
+            return TIMEOUT_MAX;
+        }
         return timeout;
     }
 
     /**
-     * @param timeout - Maps to PerformInteraction.timeout. This applies only to a manual selection
-     *                (not a voice selection, which has its timeout handled by the system). Defaults to `defaultTimeout`.
-     *                <strong>This is set to seconds if using the screen manager.</strong>
+     * Maps to PerformInteraction.timeout. Timeout in seconds. Defaults to 0, which will use `defaultTimeout`.
+     * If this is set below the minimum, it will be capped at 5 seconds. Minimum 5 seconds, maximum 100 seconds.
+     * If this is set above the maximum, it will be capped at 100 seconds.
      */
     public void setTimeout(Integer timeout) {
-        if (timeout == null) {
-            this.timeout = defaultTimeout;
-        } else {
             this.timeout = timeout;
+    }
+
+    public int getDefaultTimeout() {
+        if (defaultTimeout < TIMEOUT_MIN) {
+            return TIMEOUT_MIN;
+        } else if (defaultTimeout > TIMEOUT_MAX) {
+            return TIMEOUT_MAX;
         }
-        checkChoiceSetParameters();
+        return defaultTimeout;
+    }
+
+    /**
+     * Set this to change the default timeout for all ChoiceSets. If a timeout is not set on an individual
+     * ChoiceSet object (or if it is set to 0), then it will use this timeout instead. See `timeout`
+     * for more details. If this is not set by you, it will default to 10 seconds. The minimum is
+     * 5 seconds, the maximum is 100 seconds. If this is set below the minimum, it will be capped
+     * at 3 seconds. If this is set above the maximum, it will be capped at 10 seconds.
+     */
+    public void setDefaultTimeout(int defaultTimeout) {
+        this.defaultTimeout = defaultTimeout;
     }
 
     /**
@@ -388,11 +413,6 @@ public class ChoiceSet {
                     DebugTool.logWarning(TAG, "Attempted to create a choice set with a title of " + getTitle().length() + " length. Only 500 characters are supported.");
                 }
             }
-            if (getTimeout() != null) {
-                if (getTimeout() < 5 || getTimeout() > 100) {
-                    DebugTool.logWarning(TAG, "Attempted to create a choice set with a " + getTimeout() + " second timeout; Only 5 - 100 seconds is valid");
-                }
-            }
             if (getChoices() != null) {
                 if (getChoices().size() == 0 || getChoices().size() > 100) {
                     DebugTool.logWarning(TAG, "Attempted to create a choice set with " + getChoices().size() + " choices; Only 1 - 100 choices are valid");