From 04f6f58034893cd13b24171bc927cbe0925113dc Mon Sep 17 00:00:00 2001 From: Julian Kast Date: Tue, 18 May 2021 15:36:26 -0400 Subject: [PATCH 1/9] Update sending VoiceCommandManager to strip extra whiteSpace from voiceCommands and avoid sending empty strings in the update operation --- .../screen/menu/BaseVoiceCommandManager.java | 34 ++++++++++++++++--- 1 file changed, 30 insertions(+), 4 deletions(-) diff --git a/base/src/main/java/com/smartdevicelink/managers/screen/menu/BaseVoiceCommandManager.java b/base/src/main/java/com/smartdevicelink/managers/screen/menu/BaseVoiceCommandManager.java index 7513b728dd..2a4ed1b02e 100644 --- a/base/src/main/java/com/smartdevicelink/managers/screen/menu/BaseVoiceCommandManager.java +++ b/base/src/main/java/com/smartdevicelink/managers/screen/menu/BaseVoiceCommandManager.java @@ -56,7 +56,7 @@ abstract class BaseVoiceCommandManager extends BaseSubManager { private static final String TAG = "BaseVoiceCommandManager"; - List voiceCommands, currentVoiceCommands; + List voiceCommands, currentVoiceCommands, originalVoiceCommands; int lastVoiceCommandId; private static final int voiceCommandIdMin = 1900000000; @@ -132,7 +132,7 @@ private void updateTransactionQueueSuspended() { public void setVoiceCommands(List voiceCommands) { // we actually need voice commands to set. - if (voiceCommands == null || voiceCommands.equals(this.voiceCommands)) { + if (voiceCommands == null || voiceCommands.equals(this.originalVoiceCommands)) { DebugTool.logInfo(TAG, "Voice commands list was null or matches the current voice commands"); return; } @@ -142,11 +142,19 @@ public void setVoiceCommands(List voiceCommands) { return; } + List validatedVoiceCommands = removeEmptyVoiceCommands(voiceCommands); + + if (validatedVoiceCommands.size() == 0 && voiceCommands.size() > 0) { + DebugTool.logError(TAG, "New voice commands are invalid, skipping..."); + return; + } + + this.originalVoiceCommands = new ArrayList<>(voiceCommands); + this.voiceCommands = validatedVoiceCommands; updateIdsOnVoiceCommands(voiceCommands); - this.voiceCommands = new ArrayList<>(voiceCommands); cleanTransactionQueue(); - updateOperation = new VoiceCommandUpdateOperation(internalInterface, currentVoiceCommands, voiceCommands, new VoiceCommandUpdateOperation.VoiceCommandChangesListener() { + updateOperation = new VoiceCommandUpdateOperation(internalInterface, currentVoiceCommands, this.voiceCommands, new VoiceCommandUpdateOperation.VoiceCommandChangesListener() { @Override public void updateVoiceCommands(List newCurrentVoiceCommands, HashMap errorObject) { DebugTool.logInfo(TAG, "The updated list of VoiceCommands: " + newCurrentVoiceCommands); @@ -194,6 +202,24 @@ private void updateIdsOnVoiceCommands(List voiceCommands) { } } + List removeEmptyVoiceCommands(List voiceCommands) { + List validatedVoiceCommands = new ArrayList<>(); + for (VoiceCommand voiceCommand : voiceCommands) { + List voiceCommandStrings = new ArrayList<>(); + for (String voiceCommandString : voiceCommand.getVoiceCommands()) { + String trimmedString = voiceCommandString.trim(); + if (trimmedString.length() > 0) { + voiceCommandStrings.add(trimmedString); + } + } + if (voiceCommandStrings.size() > 0) { + voiceCommand.setVoiceCommands(voiceCommandStrings); + validatedVoiceCommands.add(voiceCommand); + } + } + return validatedVoiceCommands; + } + // LISTENERS private void addListeners() { From 59db1a7d1bdf3f222dd246f5770920a127250d1a Mon Sep 17 00:00:00 2001 From: Julian Kast Date: Tue, 18 May 2021 16:02:15 -0400 Subject: [PATCH 2/9] Add unit test for empty strings and trimmed whiteSpace for voiceCommands --- .../screen/menu/VoiceCommandManagerTests.java | 28 +++++++++++++++++++ 1 file changed, 28 insertions(+) diff --git a/android/sdl_android/src/androidTest/java/com/smartdevicelink/managers/screen/menu/VoiceCommandManagerTests.java b/android/sdl_android/src/androidTest/java/com/smartdevicelink/managers/screen/menu/VoiceCommandManagerTests.java index a315bd0c99..dccf019d6c 100644 --- a/android/sdl_android/src/androidTest/java/com/smartdevicelink/managers/screen/menu/VoiceCommandManagerTests.java +++ b/android/sdl_android/src/androidTest/java/com/smartdevicelink/managers/screen/menu/VoiceCommandManagerTests.java @@ -210,4 +210,32 @@ public void testUniqueStrings() { assertNull(voiceCommandManager.voiceCommands); } + /** + * Test trim whitespace from voice commands and not uploading empty strings. + */ + @Test + public void testEmptyStringsAndWhiteSpaceRemoval() { + List voiceCommandList = new ArrayList<>(); + VoiceCommand command1 = new VoiceCommand(Arrays.asList(" Command one "), null); + + voiceCommandList.add(command1); + voiceCommandManager.currentHMILevel = HMILevel.HMI_NONE; + voiceCommandManager.setVoiceCommands(voiceCommandList); + assertEquals(voiceCommandManager.voiceCommands.get(0).getVoiceCommands().get(0), "Command one"); + + voiceCommandManager.voiceCommands = null; + voiceCommandList = new ArrayList<>(); + command1 = new VoiceCommand(Arrays.asList(" "), null); + + voiceCommandList.add(command1); + voiceCommandManager.setVoiceCommands(voiceCommandList); + assertNull(voiceCommandManager.voiceCommands); + + VoiceCommand command2 = new VoiceCommand(Arrays.asList("Command two"), null); + voiceCommandList.add(command2); + voiceCommandManager.setVoiceCommands(voiceCommandList); + assertEquals(voiceCommandManager.voiceCommands.size(), 1); + } + + } From 15d838365cd07a994b79ccf190d24c6ac33fc4e8 Mon Sep 17 00:00:00 2001 From: Julian Kast Date: Tue, 18 May 2021 16:16:51 -0400 Subject: [PATCH 3/9] update unit test --- .../managers/screen/menu/VoiceCommandManagerTests.java | 4 ++++ 1 file changed, 4 insertions(+) diff --git a/android/sdl_android/src/androidTest/java/com/smartdevicelink/managers/screen/menu/VoiceCommandManagerTests.java b/android/sdl_android/src/androidTest/java/com/smartdevicelink/managers/screen/menu/VoiceCommandManagerTests.java index dccf019d6c..43892da33f 100644 --- a/android/sdl_android/src/androidTest/java/com/smartdevicelink/managers/screen/menu/VoiceCommandManagerTests.java +++ b/android/sdl_android/src/androidTest/java/com/smartdevicelink/managers/screen/menu/VoiceCommandManagerTests.java @@ -235,6 +235,10 @@ public void testEmptyStringsAndWhiteSpaceRemoval() { voiceCommandList.add(command2); voiceCommandManager.setVoiceCommands(voiceCommandList); assertEquals(voiceCommandManager.voiceCommands.size(), 1); + + voiceCommandManager.setVoiceCommands(voiceCommandList); + assertEquals(voiceCommandManager.voiceCommands.size(), 1); + } From af7c4f5845dfc9bc8bb645a8a1ca93111f8b5bb2 Mon Sep 17 00:00:00 2001 From: Julian Kast Date: Tue, 18 May 2021 16:34:12 -0400 Subject: [PATCH 4/9] Add null unit test check to voiceCommandManager --- .../managers/screen/menu/VoiceCommandManagerTests.java | 4 ++++ 1 file changed, 4 insertions(+) diff --git a/android/sdl_android/src/androidTest/java/com/smartdevicelink/managers/screen/menu/VoiceCommandManagerTests.java b/android/sdl_android/src/androidTest/java/com/smartdevicelink/managers/screen/menu/VoiceCommandManagerTests.java index 43892da33f..9868958ada 100644 --- a/android/sdl_android/src/androidTest/java/com/smartdevicelink/managers/screen/menu/VoiceCommandManagerTests.java +++ b/android/sdl_android/src/androidTest/java/com/smartdevicelink/managers/screen/menu/VoiceCommandManagerTests.java @@ -239,6 +239,10 @@ public void testEmptyStringsAndWhiteSpaceRemoval() { voiceCommandManager.setVoiceCommands(voiceCommandList); assertEquals(voiceCommandManager.voiceCommands.size(), 1); + voiceCommandManager.voiceCommands = null; + voiceCommandManager.setVoiceCommands(null); + assertNull(voiceCommandManager.voiceCommands); + } From 9a081d1f9af53d72d030f803b301b08c1dc97282 Mon Sep 17 00:00:00 2001 From: Julian Kast Date: Thu, 20 May 2021 09:35:09 -0400 Subject: [PATCH 5/9] Implement suggestions from code review --- .../managers/screen/menu/BaseVoiceCommandManager.java | 8 +++++++- 1 file changed, 7 insertions(+), 1 deletion(-) diff --git a/base/src/main/java/com/smartdevicelink/managers/screen/menu/BaseVoiceCommandManager.java b/base/src/main/java/com/smartdevicelink/managers/screen/menu/BaseVoiceCommandManager.java index 2a4ed1b02e..e6d962a25d 100644 --- a/base/src/main/java/com/smartdevicelink/managers/screen/menu/BaseVoiceCommandManager.java +++ b/base/src/main/java/com/smartdevicelink/managers/screen/menu/BaseVoiceCommandManager.java @@ -144,7 +144,7 @@ public void setVoiceCommands(List voiceCommands) { List validatedVoiceCommands = removeEmptyVoiceCommands(voiceCommands); - if (validatedVoiceCommands.size() == 0 && voiceCommands.size() > 0) { + if (validatedVoiceCommands.size() == 0) { DebugTool.logError(TAG, "New voice commands are invalid, skipping..."); return; } @@ -205,8 +205,14 @@ private void updateIdsOnVoiceCommands(List voiceCommands) { List removeEmptyVoiceCommands(List voiceCommands) { List validatedVoiceCommands = new ArrayList<>(); for (VoiceCommand voiceCommand : voiceCommands) { + if (voiceCommand == null) { + continue; + } List voiceCommandStrings = new ArrayList<>(); for (String voiceCommandString : voiceCommand.getVoiceCommands()) { + if (voiceCommandString == null) { + continue; + } String trimmedString = voiceCommandString.trim(); if (trimmedString.length() > 0) { voiceCommandStrings.add(trimmedString); From 840ead04a7328209187fc6c2bb51ef513bfee2b0 Mon Sep 17 00:00:00 2001 From: Julian Kast Date: Thu, 20 May 2021 11:56:23 -0400 Subject: [PATCH 6/9] Removed null voiceCommands from VoiceCommandList. Update unit test --- .../managers/screen/menu/VoiceCommandManagerTests.java | 7 +++++++ .../managers/screen/menu/BaseVoiceCommandManager.java | 7 ++++--- 2 files changed, 11 insertions(+), 3 deletions(-) diff --git a/android/sdl_android/src/androidTest/java/com/smartdevicelink/managers/screen/menu/VoiceCommandManagerTests.java b/android/sdl_android/src/androidTest/java/com/smartdevicelink/managers/screen/menu/VoiceCommandManagerTests.java index 9868958ada..05c5375b9b 100644 --- a/android/sdl_android/src/androidTest/java/com/smartdevicelink/managers/screen/menu/VoiceCommandManagerTests.java +++ b/android/sdl_android/src/androidTest/java/com/smartdevicelink/managers/screen/menu/VoiceCommandManagerTests.java @@ -243,6 +243,13 @@ public void testEmptyStringsAndWhiteSpaceRemoval() { voiceCommandManager.setVoiceCommands(null); assertNull(voiceCommandManager.voiceCommands); + voiceCommandList = new ArrayList<>(); + VoiceCommand command3 = new VoiceCommand(Arrays.asList("Command three", null, " "), null); + voiceCommandList.add(command3); + voiceCommandList.add(null); + voiceCommandManager.setVoiceCommands(voiceCommandList); + assertEquals(voiceCommandManager.voiceCommands.size(), 1); + } diff --git a/base/src/main/java/com/smartdevicelink/managers/screen/menu/BaseVoiceCommandManager.java b/base/src/main/java/com/smartdevicelink/managers/screen/menu/BaseVoiceCommandManager.java index e6d962a25d..eb6f2142da 100644 --- a/base/src/main/java/com/smartdevicelink/managers/screen/menu/BaseVoiceCommandManager.java +++ b/base/src/main/java/com/smartdevicelink/managers/screen/menu/BaseVoiceCommandManager.java @@ -205,9 +205,6 @@ private void updateIdsOnVoiceCommands(List voiceCommands) { List removeEmptyVoiceCommands(List voiceCommands) { List validatedVoiceCommands = new ArrayList<>(); for (VoiceCommand voiceCommand : voiceCommands) { - if (voiceCommand == null) { - continue; - } List voiceCommandStrings = new ArrayList<>(); for (String voiceCommandString : voiceCommand.getVoiceCommands()) { if (voiceCommandString == null) { @@ -275,6 +272,10 @@ private boolean isVoiceCommandsUnique(List voiceCommands) { int voiceCommandCount = 0; for (VoiceCommand voiceCommand : voiceCommands) { + if (voiceCommand == null) { + voiceCommands.remove(voiceCommand); + continue; + } voiceCommandHashSet.addAll(voiceCommand.getVoiceCommands()); voiceCommandCount += voiceCommand.getVoiceCommands().size(); } From d59080f2959cb4d37222ee2e99cf52f0b4aad1c9 Mon Sep 17 00:00:00 2001 From: Julian Kast Date: Thu, 20 May 2021 15:25:19 -0400 Subject: [PATCH 7/9] Fix suggestions from code review --- .../managers/screen/menu/BaseVoiceCommandManager.java | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) diff --git a/base/src/main/java/com/smartdevicelink/managers/screen/menu/BaseVoiceCommandManager.java b/base/src/main/java/com/smartdevicelink/managers/screen/menu/BaseVoiceCommandManager.java index eb6f2142da..2ca00452e4 100644 --- a/base/src/main/java/com/smartdevicelink/managers/screen/menu/BaseVoiceCommandManager.java +++ b/base/src/main/java/com/smartdevicelink/managers/screen/menu/BaseVoiceCommandManager.java @@ -205,6 +205,9 @@ private void updateIdsOnVoiceCommands(List voiceCommands) { List removeEmptyVoiceCommands(List voiceCommands) { List validatedVoiceCommands = new ArrayList<>(); for (VoiceCommand voiceCommand : voiceCommands) { + if (voiceCommand == null) { + continue; + } List voiceCommandStrings = new ArrayList<>(); for (String voiceCommandString : voiceCommand.getVoiceCommands()) { if (voiceCommandString == null) { @@ -273,7 +276,6 @@ private boolean isVoiceCommandsUnique(List voiceCommands) { for (VoiceCommand voiceCommand : voiceCommands) { if (voiceCommand == null) { - voiceCommands.remove(voiceCommand); continue; } voiceCommandHashSet.addAll(voiceCommand.getVoiceCommands()); From 4bae54606a110e35fe2fc5d42c8217a1c044b4fe Mon Sep 17 00:00:00 2001 From: Julian Kast Date: Thu, 20 May 2021 15:49:29 -0400 Subject: [PATCH 8/9] Update ID's for voice commands in proper list --- .../managers/screen/menu/BaseVoiceCommandManager.java | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/base/src/main/java/com/smartdevicelink/managers/screen/menu/BaseVoiceCommandManager.java b/base/src/main/java/com/smartdevicelink/managers/screen/menu/BaseVoiceCommandManager.java index 2ca00452e4..1c7d93ad75 100644 --- a/base/src/main/java/com/smartdevicelink/managers/screen/menu/BaseVoiceCommandManager.java +++ b/base/src/main/java/com/smartdevicelink/managers/screen/menu/BaseVoiceCommandManager.java @@ -151,7 +151,7 @@ public void setVoiceCommands(List voiceCommands) { this.originalVoiceCommands = new ArrayList<>(voiceCommands); this.voiceCommands = validatedVoiceCommands; - updateIdsOnVoiceCommands(voiceCommands); + updateIdsOnVoiceCommands(this.voiceCommands); cleanTransactionQueue(); updateOperation = new VoiceCommandUpdateOperation(internalInterface, currentVoiceCommands, this.voiceCommands, new VoiceCommandUpdateOperation.VoiceCommandChangesListener() { From 9ee6babf8e8808edbe8c668c5f0b8e09131974aa Mon Sep 17 00:00:00 2001 From: Julian Kast Date: Mon, 24 May 2021 12:31:58 -0400 Subject: [PATCH 9/9] Strip white Space before checking if commands are unique --- .../managers/screen/menu/BaseVoiceCommandManager.java | 10 +++++----- 1 file changed, 5 insertions(+), 5 deletions(-) diff --git a/base/src/main/java/com/smartdevicelink/managers/screen/menu/BaseVoiceCommandManager.java b/base/src/main/java/com/smartdevicelink/managers/screen/menu/BaseVoiceCommandManager.java index 1c7d93ad75..e51c8b0a66 100644 --- a/base/src/main/java/com/smartdevicelink/managers/screen/menu/BaseVoiceCommandManager.java +++ b/base/src/main/java/com/smartdevicelink/managers/screen/menu/BaseVoiceCommandManager.java @@ -137,11 +137,6 @@ public void setVoiceCommands(List voiceCommands) { return; } - if (!isVoiceCommandsUnique(voiceCommands)) { - DebugTool.logError(TAG, "Not all voice command strings are unique across all voice commands. Voice commands will not be set."); - return; - } - List validatedVoiceCommands = removeEmptyVoiceCommands(voiceCommands); if (validatedVoiceCommands.size() == 0) { @@ -149,6 +144,11 @@ public void setVoiceCommands(List voiceCommands) { return; } + if (!isVoiceCommandsUnique(validatedVoiceCommands)) { + DebugTool.logError(TAG, "Not all voice command strings are unique across all voice commands. Voice commands will not be set."); + return; + } + this.originalVoiceCommands = new ArrayList<>(voiceCommands); this.voiceCommands = validatedVoiceCommands; updateIdsOnVoiceCommands(this.voiceCommands);