From b03386b9eaaa4f1771392e49b70a6db1665300ad Mon Sep 17 00:00:00 2001 From: Julian Kast Date: Fri, 7 May 2021 14:37:24 -0400 Subject: [PATCH 01/28] BaseChoiceSetManager, Strip away unsupported textFields from choices --- .../choiceset/BaseChoiceSetManager.java | 21 +++++++++++++++++++ 1 file changed, 21 insertions(+) diff --git a/base/src/main/java/com/smartdevicelink/managers/screen/choiceset/BaseChoiceSetManager.java b/base/src/main/java/com/smartdevicelink/managers/screen/choiceset/BaseChoiceSetManager.java index 746d498659..f00080215c 100644 --- a/base/src/main/java/com/smartdevicelink/managers/screen/choiceset/BaseChoiceSetManager.java +++ b/base/src/main/java/com/smartdevicelink/managers/screen/choiceset/BaseChoiceSetManager.java @@ -42,6 +42,7 @@ import com.smartdevicelink.managers.BaseSubManager; import com.smartdevicelink.managers.CompletionListener; import com.smartdevicelink.managers.ISdl; +import com.smartdevicelink.managers.ManagerUtility; import com.smartdevicelink.managers.file.FileManager; import com.smartdevicelink.managers.lifecycle.OnSystemCapabilityListener; import com.smartdevicelink.managers.lifecycle.SystemCapabilityManager; @@ -61,6 +62,7 @@ import com.smartdevicelink.proxy.rpc.enums.PredefinedWindows; import com.smartdevicelink.proxy.rpc.enums.SystemCapabilityType; import com.smartdevicelink.proxy.rpc.enums.SystemContext; +import com.smartdevicelink.proxy.rpc.enums.TextFieldName; import com.smartdevicelink.proxy.rpc.enums.TriggerSource; import com.smartdevicelink.proxy.rpc.listeners.OnRPCNotificationListener; import com.smartdevicelink.util.DebugTool; @@ -672,6 +674,18 @@ public void onNotified(RPCNotification notification) { // ADDITIONAL HELPERS + boolean shouldSendChoiceSecondaryText() { + return templateSupportsTextField(TextFieldName.secondaryText); + } + + boolean shouldSendChoiceTertiaryText() { + return templateSupportsTextField(TextFieldName.tertiaryText); + } + + boolean templateSupportsTextField(TextFieldName name) { + return defaultMainWindowCapability == null || ManagerUtility.WindowCapabilityUtility.hasTextFieldOfName(defaultMainWindowCapability, name); + } + boolean setUpChoiceSet(ChoiceSet choiceSet) { List choices = choiceSet.getChoices(); @@ -695,6 +709,13 @@ boolean setUpChoiceSet(ChoiceSet choiceSet) { int choiceCellWithVoiceCommandCount = 0; for (ChoiceCell cell : choices) { + if (!shouldSendChoiceSecondaryText() && cell.getSecondaryText() != null) { + cell.setSecondaryText(null); + } + + if (!shouldSendChoiceTertiaryText() && cell.getTertiaryText() != null) { + cell.setSecondaryText(null); + } uniqueChoiceCells.add(cell); From 087f63499e4daa104f66de874ce335986ecd368c Mon Sep 17 00:00:00 2001 From: Julian Kast Date: Mon, 10 May 2021 16:32:20 -0400 Subject: [PATCH 02/28] Add image checking to ChoiceSetManager uniqueness check --- .../choiceset/BaseChoiceSetManager.java | 25 +++++++++++-------- 1 file changed, 14 insertions(+), 11 deletions(-) diff --git a/base/src/main/java/com/smartdevicelink/managers/screen/choiceset/BaseChoiceSetManager.java b/base/src/main/java/com/smartdevicelink/managers/screen/choiceset/BaseChoiceSetManager.java index f00080215c..36a89d79e1 100644 --- a/base/src/main/java/com/smartdevicelink/managers/screen/choiceset/BaseChoiceSetManager.java +++ b/base/src/main/java/com/smartdevicelink/managers/screen/choiceset/BaseChoiceSetManager.java @@ -55,6 +55,7 @@ import com.smartdevicelink.proxy.rpc.OnHMIStatus; import com.smartdevicelink.proxy.rpc.WindowCapability; import com.smartdevicelink.proxy.rpc.enums.HMILevel; +import com.smartdevicelink.proxy.rpc.enums.ImageFieldName; import com.smartdevicelink.proxy.rpc.enums.InteractionMode; import com.smartdevicelink.proxy.rpc.enums.KeyboardLayout; import com.smartdevicelink.proxy.rpc.enums.KeypressMode; @@ -674,16 +675,12 @@ public void onNotified(RPCNotification notification) { // ADDITIONAL HELPERS - boolean shouldSendChoiceSecondaryText() { - return templateSupportsTextField(TextFieldName.secondaryText); + private boolean hasImageFieldOfName(ImageFieldName imageFieldName) { + return defaultMainWindowCapability == null || ManagerUtility.WindowCapabilityUtility.hasImageFieldOfName(defaultMainWindowCapability, imageFieldName); } - boolean shouldSendChoiceTertiaryText() { - return templateSupportsTextField(TextFieldName.tertiaryText); - } - - boolean templateSupportsTextField(TextFieldName name) { - return defaultMainWindowCapability == null || ManagerUtility.WindowCapabilityUtility.hasTextFieldOfName(defaultMainWindowCapability, name); + private boolean hasTextFieldOfName(TextFieldName textFieldName) { + return defaultMainWindowCapability == null || ManagerUtility.WindowCapabilityUtility.hasTextFieldOfName(defaultMainWindowCapability, textFieldName); } boolean setUpChoiceSet(ChoiceSet choiceSet) { @@ -709,13 +706,19 @@ boolean setUpChoiceSet(ChoiceSet choiceSet) { int choiceCellWithVoiceCommandCount = 0; for (ChoiceCell cell : choices) { - if (!shouldSendChoiceSecondaryText() && cell.getSecondaryText() != null) { + // Strip cell parameters that are not supported on head unit to support uniqueness. + if (cell.getSecondaryText() != null && !hasTextFieldOfName(TextFieldName.secondaryText)) { cell.setSecondaryText(null); } - - if (!shouldSendChoiceTertiaryText() && cell.getTertiaryText() != null) { + if (cell.getTertiaryText() != null && !hasTextFieldOfName(TextFieldName.tertiaryText)) { cell.setSecondaryText(null); } + if (cell.getArtwork() != null && hasImageFieldOfName(ImageFieldName.choiceImage)) { + cell.setArtwork(null); + } + if (cell.getSecondaryArtwork() != null && hasImageFieldOfName(ImageFieldName.choiceSecondaryImage)) { + cell.setSecondaryArtwork(null); + } uniqueChoiceCells.add(cell); From 61e884a49b629a7dba2e95783c196360dd59d87d Mon Sep 17 00:00:00 2001 From: Julian Kast Date: Mon, 10 May 2021 16:33:24 -0400 Subject: [PATCH 03/28] MenuManager strip parameters away from uniqueness check if hmi does not support fields --- .../managers/screen/menu/BaseMenuManager.java | 20 +++++++++++++++++++ 1 file changed, 20 insertions(+) diff --git a/base/src/main/java/com/smartdevicelink/managers/screen/menu/BaseMenuManager.java b/base/src/main/java/com/smartdevicelink/managers/screen/menu/BaseMenuManager.java index f39423b1af..c79236ccfa 100644 --- a/base/src/main/java/com/smartdevicelink/managers/screen/menu/BaseMenuManager.java +++ b/base/src/main/java/com/smartdevicelink/managers/screen/menu/BaseMenuManager.java @@ -1474,6 +1474,26 @@ private boolean menuCellsAreUnique(List cells, ArrayList allVo HashSet identicalCellsCheckSet = new HashSet<>(); for (MenuCell cell : cells) { + if (cell.getSecondaryText() != null && !hasTextFieldOfName(TextFieldName.menuCommandSecondaryText)) { + cell.setSecondaryText(null); + } + if (cell.getTertiaryText() != null && !hasTextFieldOfName(TextFieldName.menuCommandTertiaryText)) { + cell.setTertiaryText(null); + } + if(cell.getIcon() != null && !hasImageFieldOfName(ImageFieldName.cmdIcon)) { + cell.setIcon(null); + } + + if (cell.getSecondaryArtwork() != null) { + if (cell.getParentCellId() == parentIdNotFound) { //cell is not a subcell + if (!hasImageFieldOfName(ImageFieldName.menuCommandSecondaryImage)) { + cell.setSecondaryArtwork(null); + } + } else if (!hasImageFieldOfName(ImageFieldName.menuSubMenuSecondaryImage)) { // Cell is a subcell + cell.setSecondaryArtwork(null); + } + } + identicalCellsCheckSet.add(cell); // Recursively check the subcell lists to see if they are all unique as well. If anything is not, this will chain back up the list to return false. From 8e9dc8e70fada09f4c8233d6c09f94dd02ef3da3 Mon Sep 17 00:00:00 2001 From: Julian Kast Date: Tue, 11 May 2021 09:05:02 -0400 Subject: [PATCH 04/28] added logic to check subcells properly --- .../managers/screen/menu/BaseMenuManager.java | 12 ++++++------ 1 file changed, 6 insertions(+), 6 deletions(-) diff --git a/base/src/main/java/com/smartdevicelink/managers/screen/menu/BaseMenuManager.java b/base/src/main/java/com/smartdevicelink/managers/screen/menu/BaseMenuManager.java index c79236ccfa..a0c415377f 100644 --- a/base/src/main/java/com/smartdevicelink/managers/screen/menu/BaseMenuManager.java +++ b/base/src/main/java/com/smartdevicelink/managers/screen/menu/BaseMenuManager.java @@ -206,7 +206,7 @@ public void setMenuCells(@NonNull List cells) { } // Check for cell lists with completely duplicate information, or any duplicate voiceCommands and return if it fails (logs are in the called method). - if (!menuCellsAreUnique(menuCells, new ArrayList())) { + if (!menuCellsAreUnique(menuCells, new ArrayList(), false)) { return; } @@ -1469,7 +1469,7 @@ private void addUniqueNamesToCells(List cells) { * @param allVoiceCommands List of String's for VoiceCommands (Used for recursive calls to check voiceCommands of the cells) * @return Boolean that indicates whether menuCells are unique or not */ - private boolean menuCellsAreUnique(List cells, ArrayList allVoiceCommands) { + private boolean menuCellsAreUnique(List cells, ArrayList allVoiceCommands, boolean isSubCell) { //Check all voice commands for identical items and check each list of cells for identical cells HashSet identicalCellsCheckSet = new HashSet<>(); @@ -1485,11 +1485,11 @@ private boolean menuCellsAreUnique(List cells, ArrayList allVo } if (cell.getSecondaryArtwork() != null) { - if (cell.getParentCellId() == parentIdNotFound) { //cell is not a subcell - if (!hasImageFieldOfName(ImageFieldName.menuCommandSecondaryImage)) { + if (isSubCell) { + if (!hasImageFieldOfName(ImageFieldName.menuSubMenuSecondaryImage)) { cell.setSecondaryArtwork(null); } - } else if (!hasImageFieldOfName(ImageFieldName.menuSubMenuSecondaryImage)) { // Cell is a subcell + } else if (!hasImageFieldOfName(ImageFieldName.menuCommandSecondaryImage)) { cell.setSecondaryArtwork(null); } } @@ -1498,7 +1498,7 @@ private boolean menuCellsAreUnique(List cells, ArrayList allVo // Recursively check the subcell lists to see if they are all unique as well. If anything is not, this will chain back up the list to return false. if (cell.getSubCells() != null && cell.getSubCells().size() > 0) { - boolean subCellsAreUnique = menuCellsAreUnique(cell.getSubCells(), allVoiceCommands); + boolean subCellsAreUnique = menuCellsAreUnique(cell.getSubCells(), allVoiceCommands, true); if (!subCellsAreUnique) { DebugTool.logError(TAG, "Not all subCells are unique. The menu will not be set."); From ad66dcc703f91917a32eb789145a91ecc382686b Mon Sep 17 00:00:00 2001 From: Julian Kast Date: Tue, 11 May 2021 14:01:08 -0400 Subject: [PATCH 05/28] Add missing ! --- .../managers/screen/choiceset/BaseChoiceSetManager.java | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/base/src/main/java/com/smartdevicelink/managers/screen/choiceset/BaseChoiceSetManager.java b/base/src/main/java/com/smartdevicelink/managers/screen/choiceset/BaseChoiceSetManager.java index 36a89d79e1..1fc9a3e310 100644 --- a/base/src/main/java/com/smartdevicelink/managers/screen/choiceset/BaseChoiceSetManager.java +++ b/base/src/main/java/com/smartdevicelink/managers/screen/choiceset/BaseChoiceSetManager.java @@ -713,10 +713,10 @@ boolean setUpChoiceSet(ChoiceSet choiceSet) { if (cell.getTertiaryText() != null && !hasTextFieldOfName(TextFieldName.tertiaryText)) { cell.setSecondaryText(null); } - if (cell.getArtwork() != null && hasImageFieldOfName(ImageFieldName.choiceImage)) { + if (cell.getArtwork() != null && !hasImageFieldOfName(ImageFieldName.choiceImage)) { cell.setArtwork(null); } - if (cell.getSecondaryArtwork() != null && hasImageFieldOfName(ImageFieldName.choiceSecondaryImage)) { + if (cell.getSecondaryArtwork() != null && !hasImageFieldOfName(ImageFieldName.choiceSecondaryImage)) { cell.setSecondaryArtwork(null); } From 83ae4dc7475e87cd8574411594e299308fe7cd08 Mon Sep 17 00:00:00 2001 From: Julian Kast Date: Tue, 11 May 2021 16:13:31 -0400 Subject: [PATCH 06/28] Fix copy paste error with naming --- .../managers/screen/choiceset/BaseChoiceSetManager.java | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/base/src/main/java/com/smartdevicelink/managers/screen/choiceset/BaseChoiceSetManager.java b/base/src/main/java/com/smartdevicelink/managers/screen/choiceset/BaseChoiceSetManager.java index 1fc9a3e310..0248a53488 100644 --- a/base/src/main/java/com/smartdevicelink/managers/screen/choiceset/BaseChoiceSetManager.java +++ b/base/src/main/java/com/smartdevicelink/managers/screen/choiceset/BaseChoiceSetManager.java @@ -711,7 +711,7 @@ boolean setUpChoiceSet(ChoiceSet choiceSet) { cell.setSecondaryText(null); } if (cell.getTertiaryText() != null && !hasTextFieldOfName(TextFieldName.tertiaryText)) { - cell.setSecondaryText(null); + cell.setTertiaryText(null); } if (cell.getArtwork() != null && !hasImageFieldOfName(ImageFieldName.choiceImage)) { cell.setArtwork(null); From a774543a4e76472b13ead65fb6deb1d332741ae5 Mon Sep 17 00:00:00 2001 From: Julian Kast Date: Wed, 12 May 2021 14:45:55 -0400 Subject: [PATCH 07/28] Clone choiceCell when comparing for visual uniqueness --- .../choiceset/BaseChoiceSetManager.java | 24 +++++++++++-------- 1 file changed, 14 insertions(+), 10 deletions(-) diff --git a/base/src/main/java/com/smartdevicelink/managers/screen/choiceset/BaseChoiceSetManager.java b/base/src/main/java/com/smartdevicelink/managers/screen/choiceset/BaseChoiceSetManager.java index 0248a53488..f936870009 100644 --- a/base/src/main/java/com/smartdevicelink/managers/screen/choiceset/BaseChoiceSetManager.java +++ b/base/src/main/java/com/smartdevicelink/managers/screen/choiceset/BaseChoiceSetManager.java @@ -706,22 +706,26 @@ boolean setUpChoiceSet(ChoiceSet choiceSet) { int choiceCellWithVoiceCommandCount = 0; for (ChoiceCell cell : choices) { + ChoiceCell cellClone = cell.clone(); // Strip cell parameters that are not supported on head unit to support uniqueness. - if (cell.getSecondaryText() != null && !hasTextFieldOfName(TextFieldName.secondaryText)) { - cell.setSecondaryText(null); + if (cellClone.getVoiceCommands() != null) { + cellClone.setVoiceCommands(null); } - if (cell.getTertiaryText() != null && !hasTextFieldOfName(TextFieldName.tertiaryText)) { - cell.setTertiaryText(null); + if (cellClone.getSecondaryText() != null && !hasTextFieldOfName(TextFieldName.secondaryText)) { + cellClone.setSecondaryText(null); } - if (cell.getArtwork() != null && !hasImageFieldOfName(ImageFieldName.choiceImage)) { - cell.setArtwork(null); + if (cellClone.getTertiaryText() != null && !hasTextFieldOfName(TextFieldName.tertiaryText)) { + cellClone.setTertiaryText(null); } - if (cell.getSecondaryArtwork() != null && !hasImageFieldOfName(ImageFieldName.choiceSecondaryImage)) { - cell.setSecondaryArtwork(null); + if (cellClone.getArtwork() != null && !hasImageFieldOfName(ImageFieldName.choiceImage)) { + cellClone.setArtwork(null); + } + if (cellClone.getSecondaryArtwork() != null && !hasImageFieldOfName(ImageFieldName.choiceSecondaryImage)) { + cellClone.setSecondaryArtwork(null); } - uniqueChoiceCells.add(cell); - + uniqueChoiceCells.add(cellClone); + // Not using cloned cell here because we set the clone's VoiceCommands to null for visual check only if (cell.getVoiceCommands() != null) { uniqueVoiceCommands.addAll(cell.getVoiceCommands()); choiceCellWithVoiceCommandCount += 1; From 2b9e05a28024cec07f99df8391334c3915830c96 Mon Sep 17 00:00:00 2001 From: Julian Kast Date: Wed, 12 May 2021 14:46:40 -0400 Subject: [PATCH 08/28] Clone menuCell when comparing for visual uniqueness --- .../managers/screen/menu/BaseMenuManager.java | 29 +++++++++++-------- 1 file changed, 17 insertions(+), 12 deletions(-) diff --git a/base/src/main/java/com/smartdevicelink/managers/screen/menu/BaseMenuManager.java b/base/src/main/java/com/smartdevicelink/managers/screen/menu/BaseMenuManager.java index a0c415377f..f00df123c2 100644 --- a/base/src/main/java/com/smartdevicelink/managers/screen/menu/BaseMenuManager.java +++ b/base/src/main/java/com/smartdevicelink/managers/screen/menu/BaseMenuManager.java @@ -1474,31 +1474,36 @@ private boolean menuCellsAreUnique(List cells, ArrayList allVo HashSet identicalCellsCheckSet = new HashSet<>(); for (MenuCell cell : cells) { - if (cell.getSecondaryText() != null && !hasTextFieldOfName(TextFieldName.menuCommandSecondaryText)) { - cell.setSecondaryText(null); + // Strip away fields that cannot be used to determine uniqueness visually including fields not supported by hmi + MenuCell cellClone = cell.clone(); + if (cellClone.getVoiceCommands() != null) { + cellClone.setVoiceCommands(null); } - if (cell.getTertiaryText() != null && !hasTextFieldOfName(TextFieldName.menuCommandTertiaryText)) { - cell.setTertiaryText(null); + if (cellClone.getSecondaryText() != null && !hasTextFieldOfName(TextFieldName.menuCommandSecondaryText)) { + cellClone.setSecondaryText(null); } - if(cell.getIcon() != null && !hasImageFieldOfName(ImageFieldName.cmdIcon)) { - cell.setIcon(null); + if (cellClone.getTertiaryText() != null && !hasTextFieldOfName(TextFieldName.menuCommandTertiaryText)) { + cellClone.setTertiaryText(null); + } + if (cellClone.getIcon() != null && !hasImageFieldOfName(ImageFieldName.cmdIcon)) { + cellClone.setIcon(null); } - if (cell.getSecondaryArtwork() != null) { + if (cellClone.getSecondaryArtwork() != null) { if (isSubCell) { if (!hasImageFieldOfName(ImageFieldName.menuSubMenuSecondaryImage)) { - cell.setSecondaryArtwork(null); + cellClone.setSecondaryArtwork(null); } } else if (!hasImageFieldOfName(ImageFieldName.menuCommandSecondaryImage)) { - cell.setSecondaryArtwork(null); + cellClone.setSecondaryArtwork(null); } } - identicalCellsCheckSet.add(cell); + identicalCellsCheckSet.add(cellClone); // Recursively check the subcell lists to see if they are all unique as well. If anything is not, this will chain back up the list to return false. - if (cell.getSubCells() != null && cell.getSubCells().size() > 0) { - boolean subCellsAreUnique = menuCellsAreUnique(cell.getSubCells(), allVoiceCommands, true); + if (cellClone.getSubCells() != null && cellClone.getSubCells().size() > 0) { + boolean subCellsAreUnique = menuCellsAreUnique(cellClone.getSubCells(), allVoiceCommands, true); if (!subCellsAreUnique) { DebugTool.logError(TAG, "Not all subCells are unique. The menu will not be set."); From ff38061ec30dbe7e2aea1e2e49c7bd376b58b20d Mon Sep 17 00:00:00 2001 From: Julian Kast Date: Wed, 12 May 2021 14:47:00 -0400 Subject: [PATCH 09/28] update comment --- .../smartdevicelink/managers/screen/menu/BaseMenuManager.java | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/base/src/main/java/com/smartdevicelink/managers/screen/menu/BaseMenuManager.java b/base/src/main/java/com/smartdevicelink/managers/screen/menu/BaseMenuManager.java index f00df123c2..7010a9c3f3 100644 --- a/base/src/main/java/com/smartdevicelink/managers/screen/menu/BaseMenuManager.java +++ b/base/src/main/java/com/smartdevicelink/managers/screen/menu/BaseMenuManager.java @@ -1511,7 +1511,7 @@ private boolean menuCellsAreUnique(List cells, ArrayList allVo } } - // Voice commands have to be identical across all lists + // Voice commands have to be identical across all lists, Not using cloned cell here because we set the clone's VoiceCommands to null for visual check only if (cell.getVoiceCommands() == null) { continue; } From 0a25c36dfe04168e766b21fb177ad3cb4a8b69da Mon Sep 17 00:00:00 2001 From: Julian Kast Date: Wed, 12 May 2021 15:10:27 -0400 Subject: [PATCH 10/28] Add check for submenu --- .../managers/screen/menu/BaseMenuManager.java | 18 ++++++++++++++---- 1 file changed, 14 insertions(+), 4 deletions(-) diff --git a/base/src/main/java/com/smartdevicelink/managers/screen/menu/BaseMenuManager.java b/base/src/main/java/com/smartdevicelink/managers/screen/menu/BaseMenuManager.java index 7010a9c3f3..cde25e1107 100644 --- a/base/src/main/java/com/smartdevicelink/managers/screen/menu/BaseMenuManager.java +++ b/base/src/main/java/com/smartdevicelink/managers/screen/menu/BaseMenuManager.java @@ -1479,11 +1479,21 @@ private boolean menuCellsAreUnique(List cells, ArrayList allVo if (cellClone.getVoiceCommands() != null) { cellClone.setVoiceCommands(null); } - if (cellClone.getSecondaryText() != null && !hasTextFieldOfName(TextFieldName.menuCommandSecondaryText)) { - cellClone.setSecondaryText(null); + + if (cellClone.getSecondaryText() != null) { + if (isSubCell && !hasTextFieldOfName(TextFieldName.menuSubMenuSecondaryText)) { + cellClone.setSecondaryText(null); + } else if (!hasTextFieldOfName(TextFieldName.menuCommandSecondaryText)) { + cellClone.setSecondaryText(null); + } } - if (cellClone.getTertiaryText() != null && !hasTextFieldOfName(TextFieldName.menuCommandTertiaryText)) { - cellClone.setTertiaryText(null); + + if (cellClone.getTertiaryText() != null) { + if (isSubCell && !hasTextFieldOfName(TextFieldName.menuSubMenuTertiaryText)) { + cellClone.setTertiaryText(null); + } else if (!hasTextFieldOfName(TextFieldName.menuCommandTertiaryText)) { + cellClone.setTertiaryText(null); + } } if (cellClone.getIcon() != null && !hasImageFieldOfName(ImageFieldName.cmdIcon)) { cellClone.setIcon(null); From 48ea753a4f3037cb558b9416db787581c252ded5 Mon Sep 17 00:00:00 2001 From: Julian Kast Date: Wed, 12 May 2021 16:11:30 -0400 Subject: [PATCH 11/28] Refactor check for uniqueness --- .../managers/screen/menu/BaseMenuManager.java | 48 ++++++++----------- 1 file changed, 21 insertions(+), 27 deletions(-) diff --git a/base/src/main/java/com/smartdevicelink/managers/screen/menu/BaseMenuManager.java b/base/src/main/java/com/smartdevicelink/managers/screen/menu/BaseMenuManager.java index cde25e1107..b735f52be3 100644 --- a/base/src/main/java/com/smartdevicelink/managers/screen/menu/BaseMenuManager.java +++ b/base/src/main/java/com/smartdevicelink/managers/screen/menu/BaseMenuManager.java @@ -206,7 +206,7 @@ public void setMenuCells(@NonNull List cells) { } // Check for cell lists with completely duplicate information, or any duplicate voiceCommands and return if it fails (logs are in the called method). - if (!menuCellsAreUnique(menuCells, new ArrayList(), false)) { + if (!menuCellsAreUnique(menuCells, new ArrayList())) { return; } @@ -1469,51 +1469,45 @@ private void addUniqueNamesToCells(List cells) { * @param allVoiceCommands List of String's for VoiceCommands (Used for recursive calls to check voiceCommands of the cells) * @return Boolean that indicates whether menuCells are unique or not */ - private boolean menuCellsAreUnique(List cells, ArrayList allVoiceCommands, boolean isSubCell) { + private boolean menuCellsAreUnique(List cells, ArrayList allVoiceCommands) { //Check all voice commands for identical items and check each list of cells for identical cells HashSet identicalCellsCheckSet = new HashSet<>(); for (MenuCell cell : cells) { // Strip away fields that cannot be used to determine uniqueness visually including fields not supported by hmi MenuCell cellClone = cell.clone(); - if (cellClone.getVoiceCommands() != null) { - cellClone.setVoiceCommands(null); + cellClone.setVoiceCommands(null); + // Check for menuCells and subMenu icon + if (!hasImageFieldOfName(ImageFieldName.cmdIcon)) { + cellClone.setIcon(null); } - - if (cellClone.getSecondaryText() != null) { - if (isSubCell && !hasTextFieldOfName(TextFieldName.menuSubMenuSecondaryText)) { - cellClone.setSecondaryText(null); - } else if (!hasTextFieldOfName(TextFieldName.menuCommandSecondaryText)) { + // Check for subMenu fields supported + if (cellClone.getSubCells() != null) { + if (!hasTextFieldOfName(TextFieldName.menuSubMenuSecondaryText)) { cellClone.setSecondaryText(null); } - } - - if (cellClone.getTertiaryText() != null) { - if (isSubCell && !hasTextFieldOfName(TextFieldName.menuSubMenuTertiaryText)) { + if (!hasTextFieldOfName(TextFieldName.menuSubMenuTertiaryText)) { cellClone.setTertiaryText(null); - } else if (!hasTextFieldOfName(TextFieldName.menuCommandTertiaryText)) { + } + if (!hasImageFieldOfName(ImageFieldName.menuSubMenuSecondaryImage)) { + cellClone.setSecondaryArtwork(null); + } + } else { + if (!hasTextFieldOfName(TextFieldName.menuCommandSecondaryText)) { + cellClone.setSecondaryText(null); + } + if (!hasTextFieldOfName(TextFieldName.menuCommandTertiaryText)) { cellClone.setTertiaryText(null); } - } - if (cellClone.getIcon() != null && !hasImageFieldOfName(ImageFieldName.cmdIcon)) { - cellClone.setIcon(null); - } - - if (cellClone.getSecondaryArtwork() != null) { - if (isSubCell) { - if (!hasImageFieldOfName(ImageFieldName.menuSubMenuSecondaryImage)) { - cellClone.setSecondaryArtwork(null); - } - } else if (!hasImageFieldOfName(ImageFieldName.menuCommandSecondaryImage)) { + if (!hasImageFieldOfName(ImageFieldName.menuCommandSecondaryImage)) { cellClone.setSecondaryArtwork(null); } } - identicalCellsCheckSet.add(cellClone); // Recursively check the subcell lists to see if they are all unique as well. If anything is not, this will chain back up the list to return false. if (cellClone.getSubCells() != null && cellClone.getSubCells().size() > 0) { - boolean subCellsAreUnique = menuCellsAreUnique(cellClone.getSubCells(), allVoiceCommands, true); + boolean subCellsAreUnique = menuCellsAreUnique(cellClone.getSubCells(), allVoiceCommands); if (!subCellsAreUnique) { DebugTool.logError(TAG, "Not all subCells are unique. The menu will not be set."); From 60b60b39f1e521039b1f3aabd2139309b8a477aa Mon Sep 17 00:00:00 2001 From: Julian Kast Date: Thu, 13 May 2021 14:05:05 -0400 Subject: [PATCH 12/28] Add WindowCapability to menuManagerTest --- .../managers/screen/menu/MenuManagerTests.java | 17 +++++++++++++++++ 1 file changed, 17 insertions(+) diff --git a/android/sdl_android/src/androidTest/java/com/smartdevicelink/managers/screen/menu/MenuManagerTests.java b/android/sdl_android/src/androidTest/java/com/smartdevicelink/managers/screen/menu/MenuManagerTests.java index 2419023f60..ffc707abf7 100644 --- a/android/sdl_android/src/androidTest/java/com/smartdevicelink/managers/screen/menu/MenuManagerTests.java +++ b/android/sdl_android/src/androidTest/java/com/smartdevicelink/managers/screen/menu/MenuManagerTests.java @@ -38,6 +38,7 @@ import com.smartdevicelink.managers.BaseSubManager; import com.smartdevicelink.managers.CompletionListener; import com.smartdevicelink.managers.ISdl; +import com.smartdevicelink.managers.ManagerUtility; import com.smartdevicelink.managers.file.FileManager; import com.smartdevicelink.managers.file.filetypes.SdlArtwork; import com.smartdevicelink.protocol.enums.FunctionID; @@ -54,6 +55,7 @@ import com.smartdevicelink.proxy.rpc.enums.SystemContext; import com.smartdevicelink.proxy.rpc.enums.TriggerSource; import com.smartdevicelink.proxy.rpc.listeners.OnRPCNotificationListener; +import com.smartdevicelink.test.TestValues; import org.junit.After; import org.junit.Before; @@ -160,6 +162,21 @@ public Void answer(InvocationOnMock invocation) { assertNotNull(menuManager.hmiListener); assertNotNull(menuManager.commandListener); assertNotNull(menuManager.onDisplaysCapabilityListener); + WindowCapability windowCapability = new WindowCapability(); + windowCapability = new WindowCapability(); + windowCapability.setWindowID(TestValues.GENERAL_INT); + windowCapability.setTextFields(TestValues.GENERAL_TEXTFIELD_LIST); + windowCapability.setImageFields(TestValues.GENERAL_IMAGEFIELD_LIST); + windowCapability.setImageTypeSupported(TestValues.GENERAL_IMAGE_TYPE_LIST); + windowCapability.setTemplatesAvailable(TestValues.GENERAL_STRING_LIST); + windowCapability.setNumCustomPresetsAvailable(TestValues.GENERAL_INT); + windowCapability.setButtonCapabilities(TestValues.GENERAL_BUTTONCAPABILITIES_LIST); + windowCapability.setSoftButtonCapabilities(TestValues.GENERAL_SOFTBUTTONCAPABILITIES_LIST); + windowCapability.setMenuLayoutsAvailable(TestValues.GENERAL_MENU_LAYOUT_LIST); + windowCapability.setDynamicUpdateCapabilities(TestValues.GENERAL_DYNAMICUPDATECAPABILITIES); + windowCapability.setKeyboardCapabilities(TestValues.GENERAL_KEYBOARD_CAPABILITIES); + + menuManager.defaultMainWindowCapability = windowCapability; } From e956e971d65ba7920f00db1c5729f2e1f8d38ca3 Mon Sep 17 00:00:00 2001 From: Julian Kast Date: Thu, 13 May 2021 14:41:03 -0400 Subject: [PATCH 13/28] Change menuCellsAreUnique to package private and add unit test --- .../screen/menu/MenuManagerTests.java | 62 ++++++++++++++----- .../managers/screen/menu/BaseMenuManager.java | 2 +- 2 files changed, 48 insertions(+), 16 deletions(-) diff --git a/android/sdl_android/src/androidTest/java/com/smartdevicelink/managers/screen/menu/MenuManagerTests.java b/android/sdl_android/src/androidTest/java/com/smartdevicelink/managers/screen/menu/MenuManagerTests.java index ffc707abf7..eac08932d4 100644 --- a/android/sdl_android/src/androidTest/java/com/smartdevicelink/managers/screen/menu/MenuManagerTests.java +++ b/android/sdl_android/src/androidTest/java/com/smartdevicelink/managers/screen/menu/MenuManagerTests.java @@ -44,15 +44,19 @@ import com.smartdevicelink.protocol.enums.FunctionID; import com.smartdevicelink.proxy.RPCRequest; import com.smartdevicelink.proxy.RPCResponse; +import com.smartdevicelink.proxy.rpc.ImageField; import com.smartdevicelink.proxy.rpc.OnCommand; import com.smartdevicelink.proxy.rpc.OnHMIStatus; import com.smartdevicelink.proxy.rpc.SdlMsgVersion; import com.smartdevicelink.proxy.rpc.SetGlobalProperties; +import com.smartdevicelink.proxy.rpc.TextField; import com.smartdevicelink.proxy.rpc.WindowCapability; import com.smartdevicelink.proxy.rpc.enums.FileType; import com.smartdevicelink.proxy.rpc.enums.HMILevel; +import com.smartdevicelink.proxy.rpc.enums.ImageFieldName; import com.smartdevicelink.proxy.rpc.enums.MenuLayout; import com.smartdevicelink.proxy.rpc.enums.SystemContext; +import com.smartdevicelink.proxy.rpc.enums.TextFieldName; import com.smartdevicelink.proxy.rpc.enums.TriggerSource; import com.smartdevicelink.proxy.rpc.listeners.OnRPCNotificationListener; import com.smartdevicelink.test.TestValues; @@ -65,6 +69,7 @@ import org.mockito.invocation.InvocationOnMock; import org.mockito.stubbing.Answer; +import java.util.ArrayList; import java.util.Arrays; import java.util.Collections; import java.util.List; @@ -162,21 +167,6 @@ public Void answer(InvocationOnMock invocation) { assertNotNull(menuManager.hmiListener); assertNotNull(menuManager.commandListener); assertNotNull(menuManager.onDisplaysCapabilityListener); - WindowCapability windowCapability = new WindowCapability(); - windowCapability = new WindowCapability(); - windowCapability.setWindowID(TestValues.GENERAL_INT); - windowCapability.setTextFields(TestValues.GENERAL_TEXTFIELD_LIST); - windowCapability.setImageFields(TestValues.GENERAL_IMAGEFIELD_LIST); - windowCapability.setImageTypeSupported(TestValues.GENERAL_IMAGE_TYPE_LIST); - windowCapability.setTemplatesAvailable(TestValues.GENERAL_STRING_LIST); - windowCapability.setNumCustomPresetsAvailable(TestValues.GENERAL_INT); - windowCapability.setButtonCapabilities(TestValues.GENERAL_BUTTONCAPABILITIES_LIST); - windowCapability.setSoftButtonCapabilities(TestValues.GENERAL_SOFTBUTTONCAPABILITIES_LIST); - windowCapability.setMenuLayoutsAvailable(TestValues.GENERAL_MENU_LAYOUT_LIST); - windowCapability.setDynamicUpdateCapabilities(TestValues.GENERAL_DYNAMICUPDATECAPABILITIES); - windowCapability.setKeyboardCapabilities(TestValues.GENERAL_KEYBOARD_CAPABILITIES); - - menuManager.defaultMainWindowCapability = windowCapability; } @@ -614,6 +604,48 @@ public void testAllowingNonUniqueTitles() { assertEquals(menuManager.menuCells.get(3).getSubCells().get(2).getUniqueTitle(), "A"); assertEquals(menuManager.menuCells.get(3).getSubCells().get(3).getUniqueTitle(), "A"); } + @Test + public void testUniquenessForAvailableFields() { + + WindowCapability windowCapability = new WindowCapability(); + TextField menuSubMenuSecondaryText = new TextField(); + menuSubMenuSecondaryText.setName(TextFieldName.menuSubMenuSecondaryText); + TextField menuSubMenuTertiaryText = new TextField(); + menuSubMenuTertiaryText.setName(TextFieldName.menuSubMenuTertiaryText); + TextField menuCommandSecondaryText = new TextField(); + menuCommandSecondaryText.setName(TextFieldName.menuCommandSecondaryText); + TextField menuCommandTertiaryText = new TextField(); + menuCommandTertiaryText.setName(TextFieldName.menuCommandTertiaryText); + List textFields = new ArrayList<>(); + textFields.add(menuSubMenuSecondaryText); + textFields.add(menuSubMenuTertiaryText); + textFields.add(menuCommandSecondaryText); + textFields.add(menuCommandTertiaryText); + + windowCapability.setTextFields(textFields); + + + ImageField cmdIcon = new ImageField(); + cmdIcon.setName(ImageFieldName.cmdIcon); + ImageField menuSubMenuSecondaryImage = new ImageField(); + menuSubMenuSecondaryImage.setName(ImageFieldName.menuSubMenuSecondaryImage); + ImageField menuCommandSecondaryImage = new ImageField(); + menuCommandSecondaryImage.setName(ImageFieldName.menuCommandSecondaryImage); + + List imageFieldList = new ArrayList<>(); + imageFieldList.add(cmdIcon); + imageFieldList.add(menuSubMenuSecondaryImage); + imageFieldList.add(menuCommandSecondaryImage); + windowCapability.setImageFields(imageFieldList); + + + menuManager.defaultMainWindowCapability = windowCapability; + menuManager.currentHMILevel = HMILevel.HMI_FULL; + // send new cells. They should set the old way + List oldMenu = createDynamicMenu6_forUniqueNamesTest(); + assertTrue(menuManager.menuCellsAreUnique(oldMenu, new ArrayList())); + } + // HELPERS diff --git a/base/src/main/java/com/smartdevicelink/managers/screen/menu/BaseMenuManager.java b/base/src/main/java/com/smartdevicelink/managers/screen/menu/BaseMenuManager.java index b735f52be3..d6c1b277c1 100644 --- a/base/src/main/java/com/smartdevicelink/managers/screen/menu/BaseMenuManager.java +++ b/base/src/main/java/com/smartdevicelink/managers/screen/menu/BaseMenuManager.java @@ -1469,7 +1469,7 @@ private void addUniqueNamesToCells(List cells) { * @param allVoiceCommands List of String's for VoiceCommands (Used for recursive calls to check voiceCommands of the cells) * @return Boolean that indicates whether menuCells are unique or not */ - private boolean menuCellsAreUnique(List cells, ArrayList allVoiceCommands) { + boolean menuCellsAreUnique(List cells, ArrayList allVoiceCommands) { //Check all voice commands for identical items and check each list of cells for identical cells HashSet identicalCellsCheckSet = new HashSet<>(); From 2babb3cf27992bb6d487a29598b7ccf9221c4087 Mon Sep 17 00:00:00 2001 From: Julian Kast Date: Thu, 13 May 2021 15:40:52 -0400 Subject: [PATCH 14/28] Update unit test --- .../screen/menu/MenuManagerTests.java | 42 +++++++++++++++++-- 1 file changed, 39 insertions(+), 3 deletions(-) diff --git a/android/sdl_android/src/androidTest/java/com/smartdevicelink/managers/screen/menu/MenuManagerTests.java b/android/sdl_android/src/androidTest/java/com/smartdevicelink/managers/screen/menu/MenuManagerTests.java index eac08932d4..6ef482a6a2 100644 --- a/android/sdl_android/src/androidTest/java/com/smartdevicelink/managers/screen/menu/MenuManagerTests.java +++ b/android/sdl_android/src/androidTest/java/com/smartdevicelink/managers/screen/menu/MenuManagerTests.java @@ -38,7 +38,6 @@ import com.smartdevicelink.managers.BaseSubManager; import com.smartdevicelink.managers.CompletionListener; import com.smartdevicelink.managers.ISdl; -import com.smartdevicelink.managers.ManagerUtility; import com.smartdevicelink.managers.file.FileManager; import com.smartdevicelink.managers.file.filetypes.SdlArtwork; import com.smartdevicelink.protocol.enums.FunctionID; @@ -616,6 +615,7 @@ public void testUniquenessForAvailableFields() { menuCommandSecondaryText.setName(TextFieldName.menuCommandSecondaryText); TextField menuCommandTertiaryText = new TextField(); menuCommandTertiaryText.setName(TextFieldName.menuCommandTertiaryText); + List textFields = new ArrayList<>(); textFields.add(menuSubMenuSecondaryText); textFields.add(menuSubMenuTertiaryText); @@ -638,12 +638,48 @@ public void testUniquenessForAvailableFields() { imageFieldList.add(menuCommandSecondaryImage); windowCapability.setImageFields(imageFieldList); - menuManager.defaultMainWindowCapability = windowCapability; menuManager.currentHMILevel = HMILevel.HMI_FULL; // send new cells. They should set the old way List oldMenu = createDynamicMenu6_forUniqueNamesTest(); - assertTrue(menuManager.menuCellsAreUnique(oldMenu, new ArrayList())); + + + MenuCell cell1 = new MenuCell("Text1", "SecondaryText", "TText", TestValues.GENERAL_ARTWORK, TestValues.GENERAL_ARTWORK, null, new MenuSelectionListener() { + @Override + public void onTriggered(TriggerSource trigger) { + + } + }); + + MenuCell cell2 = new MenuCell("Text2", "SecondaryText", "TText",TestValues.GENERAL_ARTWORK , TestValues.GENERAL_ARTWORK, null, new MenuSelectionListener() { + @Override + public void onTriggered(TriggerSource trigger) { + + } + }); + + List menuCellList = new ArrayList<>(); + menuCellList.add(cell1); + menuCellList.add(cell2); + + assertTrue(menuManager.menuCellsAreUnique(menuCellList, new ArrayList())); + + cell2.setTitle("Text1"); + + assertFalse(menuManager.menuCellsAreUnique(menuCellList, new ArrayList())); + + cell2.setSecondaryText("text2"); + assertTrue(menuManager.menuCellsAreUnique(menuCellList, new ArrayList())); + textFields.remove(menuCommandSecondaryText); + assertFalse(menuManager.menuCellsAreUnique(menuCellList, new ArrayList())); + + cell2.setTertiaryText("text3"); + + assertTrue(menuManager.menuCellsAreUnique(menuCellList, new ArrayList())); + + textFields.remove(menuCommandTertiaryText); + assertFalse(menuManager.menuCellsAreUnique(menuCellList, new ArrayList())); + } From dffed17c484435a962c2acbe5b94bb6a66faae4c Mon Sep 17 00:00:00 2001 From: Julian Kast Date: Thu, 13 May 2021 16:02:22 -0400 Subject: [PATCH 15/28] add menuManager test --- .../screen/menu/MenuManagerTests.java | 46 ++++++++++++++++++- 1 file changed, 45 insertions(+), 1 deletion(-) diff --git a/android/sdl_android/src/androidTest/java/com/smartdevicelink/managers/screen/menu/MenuManagerTests.java b/android/sdl_android/src/androidTest/java/com/smartdevicelink/managers/screen/menu/MenuManagerTests.java index 6ef482a6a2..9c97db2491 100644 --- a/android/sdl_android/src/androidTest/java/com/smartdevicelink/managers/screen/menu/MenuManagerTests.java +++ b/android/sdl_android/src/androidTest/java/com/smartdevicelink/managers/screen/menu/MenuManagerTests.java @@ -603,6 +603,7 @@ public void testAllowingNonUniqueTitles() { assertEquals(menuManager.menuCells.get(3).getSubCells().get(2).getUniqueTitle(), "A"); assertEquals(menuManager.menuCells.get(3).getSubCells().get(3).getUniqueTitle(), "A"); } + @Test public void testUniquenessForAvailableFields() { @@ -651,12 +652,31 @@ public void onTriggered(TriggerSource trigger) { } }); - MenuCell cell2 = new MenuCell("Text2", "SecondaryText", "TText",TestValues.GENERAL_ARTWORK , TestValues.GENERAL_ARTWORK, null, new MenuSelectionListener() { + MenuCell cell2 = new MenuCell("Text2", "SecondaryText", "TText", TestValues.GENERAL_ARTWORK, TestValues.GENERAL_ARTWORK, null, new MenuSelectionListener() { + @Override + public void onTriggered(TriggerSource trigger) { + + } + }); + + + MenuCell subCell1 = new MenuCell("SubCell 1", "Secondary Text", "Tertiary Text", TestValues.GENERAL_ARTWORK, TestValues.GENERAL_ARTWORK, null, new MenuSelectionListener() { @Override public void onTriggered(TriggerSource trigger) { + } + }); + MenuCell subCell2 = new MenuCell("SubCell 2", "Secondary Text", "Tertiary Text", TestValues.GENERAL_ARTWORK, TestValues.GENERAL_ARTWORK, null, new MenuSelectionListener() { + @Override + public void onTriggered(TriggerSource trigger) { } }); + MenuCell mainCell1 = new MenuCell("Test Cell 1", "null", "null", MenuLayout.LIST, TestValues.GENERAL_ARTWORK, TestValues.GENERAL_ARTWORK, Arrays.asList(subCell1, subCell2)); + MenuCell mainCell2 = new MenuCell("Test Cell 2", "null", "null", MenuLayout.LIST, TestValues.GENERAL_ARTWORK, TestValues.GENERAL_ARTWORK, Arrays.asList(subCell1, subCell2)); + + List menuCellSubMenuList = new ArrayList<>(); + menuCellSubMenuList.add(mainCell1); + menuCellSubMenuList.add(mainCell2); List menuCellList = new ArrayList<>(); menuCellList.add(cell1); @@ -680,6 +700,30 @@ public void onTriggered(TriggerSource trigger) { textFields.remove(menuCommandTertiaryText); assertFalse(menuManager.menuCellsAreUnique(menuCellList, new ArrayList())); + cell2.setIcon(null); + imageFieldList.remove(cmdIcon); + assertFalse(menuManager.menuCellsAreUnique(menuCellList, new ArrayList())); + + cell2.setSecondaryArtwork(null); + imageFieldList.remove(menuCommandSecondaryImage); + assertFalse(menuManager.menuCellsAreUnique(menuCellList, new ArrayList())); + + assertTrue(menuManager.menuCellsAreUnique(menuCellSubMenuList, new ArrayList())); + mainCell2.setTitle("Test Cell 1"); + assertFalse(menuManager.menuCellsAreUnique(menuCellSubMenuList, new ArrayList())); + + mainCell2.setSecondaryText("changed text"); + assertTrue(menuManager.menuCellsAreUnique(menuCellSubMenuList, new ArrayList())); + textFields.remove(menuSubMenuSecondaryText); + assertFalse(menuManager.menuCellsAreUnique(menuCellSubMenuList, new ArrayList())); + + mainCell2.setTertiaryText("changed text"); + textFields.remove(menuSubMenuTertiaryText); + assertFalse(menuManager.menuCellsAreUnique(menuCellSubMenuList, new ArrayList())); + + mainCell2.setSecondaryArtwork(null); + assertTrue(menuManager.menuCellsAreUnique(menuCellSubMenuList, new ArrayList())); + imageFieldList.remove(menuSubMenuSecondaryImage); } From 55b42674b91ddd7e4472f5beeca7cfeefd2efdd6 Mon Sep 17 00:00:00 2001 From: Julian Kast Date: Thu, 13 May 2021 16:48:54 -0400 Subject: [PATCH 16/28] Clean up BaseChoiceSetManager logic --- .../screen/choiceset/BaseChoiceSetManager.java | 13 ++++++------- 1 file changed, 6 insertions(+), 7 deletions(-) diff --git a/base/src/main/java/com/smartdevicelink/managers/screen/choiceset/BaseChoiceSetManager.java b/base/src/main/java/com/smartdevicelink/managers/screen/choiceset/BaseChoiceSetManager.java index f936870009..feaf9007ad 100644 --- a/base/src/main/java/com/smartdevicelink/managers/screen/choiceset/BaseChoiceSetManager.java +++ b/base/src/main/java/com/smartdevicelink/managers/screen/choiceset/BaseChoiceSetManager.java @@ -708,19 +708,18 @@ boolean setUpChoiceSet(ChoiceSet choiceSet) { for (ChoiceCell cell : choices) { ChoiceCell cellClone = cell.clone(); // Strip cell parameters that are not supported on head unit to support uniqueness. - if (cellClone.getVoiceCommands() != null) { - cellClone.setVoiceCommands(null); - } - if (cellClone.getSecondaryText() != null && !hasTextFieldOfName(TextFieldName.secondaryText)) { + cellClone.setVoiceCommands(null); + + if (!hasTextFieldOfName(TextFieldName.secondaryText)) { cellClone.setSecondaryText(null); } - if (cellClone.getTertiaryText() != null && !hasTextFieldOfName(TextFieldName.tertiaryText)) { + if (!hasTextFieldOfName(TextFieldName.tertiaryText)) { cellClone.setTertiaryText(null); } - if (cellClone.getArtwork() != null && !hasImageFieldOfName(ImageFieldName.choiceImage)) { + if (!hasImageFieldOfName(ImageFieldName.choiceImage)) { cellClone.setArtwork(null); } - if (cellClone.getSecondaryArtwork() != null && !hasImageFieldOfName(ImageFieldName.choiceSecondaryImage)) { + if (!hasImageFieldOfName(ImageFieldName.choiceSecondaryImage)) { cellClone.setSecondaryArtwork(null); } From 919cb063f7688b5460b07a1318365dc863055251 Mon Sep 17 00:00:00 2001 From: Julian Kast Date: Thu, 13 May 2021 17:03:36 -0400 Subject: [PATCH 17/28] Add more unit test --- .../choiceset/ChoiceSetManagerTests.java | 63 +++++++++++++++++++ .../screen/menu/MenuManagerTests.java | 3 +- 2 files changed, 65 insertions(+), 1 deletion(-) diff --git a/android/sdl_android/src/androidTest/java/com/smartdevicelink/managers/screen/choiceset/ChoiceSetManagerTests.java b/android/sdl_android/src/androidTest/java/com/smartdevicelink/managers/screen/choiceset/ChoiceSetManagerTests.java index 16ea857f55..51009ab1b8 100644 --- a/android/sdl_android/src/androidTest/java/com/smartdevicelink/managers/screen/choiceset/ChoiceSetManagerTests.java +++ b/android/sdl_android/src/androidTest/java/com/smartdevicelink/managers/screen/choiceset/ChoiceSetManagerTests.java @@ -40,18 +40,23 @@ import com.smartdevicelink.managers.BaseSubManager; import com.smartdevicelink.managers.ISdl; import com.smartdevicelink.managers.file.FileManager; +import com.smartdevicelink.proxy.rpc.ImageField; import com.smartdevicelink.proxy.rpc.KeyboardCapabilities; import com.smartdevicelink.proxy.rpc.KeyboardLayoutCapability; import com.smartdevicelink.proxy.rpc.KeyboardProperties; import com.smartdevicelink.proxy.rpc.SdlMsgVersion; +import com.smartdevicelink.proxy.rpc.TextField; import com.smartdevicelink.proxy.rpc.WindowCapability; import com.smartdevicelink.proxy.rpc.enums.HMILevel; +import com.smartdevicelink.proxy.rpc.enums.ImageFieldName; import com.smartdevicelink.proxy.rpc.enums.KeyboardInputMask; import com.smartdevicelink.proxy.rpc.enums.KeyboardLayout; import com.smartdevicelink.proxy.rpc.enums.KeypressMode; import com.smartdevicelink.proxy.rpc.enums.Language; import com.smartdevicelink.proxy.rpc.enums.SystemContext; +import com.smartdevicelink.proxy.rpc.enums.TextFieldName; import com.smartdevicelink.proxy.rpc.enums.TriggerSource; +import com.smartdevicelink.test.TestValues; import org.junit.After; import org.junit.Before; @@ -468,4 +473,62 @@ public void testDismissingQueuedKeyboard() { verify(testKeyboardOp, times(0)).dismissKeyboard(); verify(testKeyboardOp2, times(1)).dismissKeyboard(); } + + @Test + public void testUniquenessForAvailableFields() { + WindowCapability windowCapability = new WindowCapability(); + TextField secondaryText = new TextField(); + secondaryText.setName(TextFieldName.secondaryText); + TextField tertiaryText = new TextField(); + tertiaryText.setName(TextFieldName.tertiaryText); + + List textFields = new ArrayList<>(); + textFields.add(secondaryText); + textFields.add(tertiaryText); + + windowCapability.setTextFields(textFields); + + ImageField choiceImage = new ImageField(); + choiceImage.setName(ImageFieldName.choiceImage); + ImageField choiceSecondaryImage = new ImageField(); + choiceSecondaryImage.setName(ImageFieldName.choiceSecondaryImage); + + List imageFieldList = new ArrayList<>(); + imageFieldList.add(choiceImage); + imageFieldList.add(choiceSecondaryImage); + windowCapability.setImageFields(imageFieldList); + + csm.defaultMainWindowCapability = windowCapability; + + ChoiceCell cell1 = new ChoiceCell("Item 1", "null" , "tertiaryText", null, TestValues.GENERAL_ARTWORK, TestValues.GENERAL_ARTWORK ); + ChoiceCell cell2 = new ChoiceCell("Item 2", "null" , "tertiaryText", null, TestValues.GENERAL_ARTWORK ,TestValues.GENERAL_ARTWORK ); + List choiceCellList = new ArrayList<>(); + choiceCellList.add(cell1); + choiceCellList.add(cell2); + ChoiceSet choiceSet = new ChoiceSet("choicSet" , choiceCellList, null); + + assertTrue(csm.setUpChoiceSet(choiceSet)); + + cell2.setText("Item 1"); + assertFalse(csm.setUpChoiceSet(choiceSet)); + cell2.setSecondaryText("changed text"); + assertTrue(csm.setUpChoiceSet(choiceSet)); + textFields.remove(secondaryText); + assertFalse(csm.setUpChoiceSet(choiceSet)); + cell2.setTertiaryText("changed text"); + assertTrue(csm.setUpChoiceSet(choiceSet)); + textFields.remove(tertiaryText); + assertFalse(csm.setUpChoiceSet(choiceSet)); + cell2.setArtwork(null); + assertTrue(csm.setUpChoiceSet(choiceSet)); + imageFieldList.remove(choiceImage); + assertFalse(csm.setUpChoiceSet(choiceSet)); + cell2.setSecondaryArtwork(null); + assertTrue(csm.setUpChoiceSet(choiceSet)); + imageFieldList.remove(choiceSecondaryImage); + assertFalse(csm.setUpChoiceSet(choiceSet)); + + + + } } diff --git a/android/sdl_android/src/androidTest/java/com/smartdevicelink/managers/screen/menu/MenuManagerTests.java b/android/sdl_android/src/androidTest/java/com/smartdevicelink/managers/screen/menu/MenuManagerTests.java index 9c97db2491..2909f3e0cf 100644 --- a/android/sdl_android/src/androidTest/java/com/smartdevicelink/managers/screen/menu/MenuManagerTests.java +++ b/android/sdl_android/src/androidTest/java/com/smartdevicelink/managers/screen/menu/MenuManagerTests.java @@ -625,7 +625,6 @@ public void testUniquenessForAvailableFields() { windowCapability.setTextFields(textFields); - ImageField cmdIcon = new ImageField(); cmdIcon.setName(ImageFieldName.cmdIcon); ImageField menuSubMenuSecondaryImage = new ImageField(); @@ -724,6 +723,8 @@ public void onTriggered(TriggerSource trigger) { mainCell2.setSecondaryArtwork(null); assertTrue(menuManager.menuCellsAreUnique(menuCellSubMenuList, new ArrayList())); imageFieldList.remove(menuSubMenuSecondaryImage); + assertFalse(menuManager.menuCellsAreUnique(menuCellSubMenuList, new ArrayList())); + } From a283c72bdb9734515151b645099affb647cdd906 Mon Sep 17 00:00:00 2001 From: Julian Kast Date: Thu, 13 May 2021 17:16:44 -0400 Subject: [PATCH 18/28] Modify menu manger unit test --- .../smartdevicelink/managers/screen/menu/MenuManagerTests.java | 3 +++ 1 file changed, 3 insertions(+) diff --git a/android/sdl_android/src/androidTest/java/com/smartdevicelink/managers/screen/menu/MenuManagerTests.java b/android/sdl_android/src/androidTest/java/com/smartdevicelink/managers/screen/menu/MenuManagerTests.java index 2909f3e0cf..eec0f6a0fd 100644 --- a/android/sdl_android/src/androidTest/java/com/smartdevicelink/managers/screen/menu/MenuManagerTests.java +++ b/android/sdl_android/src/androidTest/java/com/smartdevicelink/managers/screen/menu/MenuManagerTests.java @@ -725,6 +725,9 @@ public void onTriggered(TriggerSource trigger) { imageFieldList.remove(menuSubMenuSecondaryImage); assertFalse(menuManager.menuCellsAreUnique(menuCellSubMenuList, new ArrayList())); + mainCell2.setSubCells(Collections.emptyList()); + assertFalse(menuManager.menuCellsAreUnique(menuCellSubMenuList, new ArrayList())); + } From 0c31a7bc9a7258405a7f0cd42066984540e4a7b9 Mon Sep 17 00:00:00 2001 From: Julian Kast Date: Fri, 14 May 2021 09:13:52 -0400 Subject: [PATCH 19/28] Formatting and cleanup of unitTest --- .../choiceset/ChoiceSetManagerTests.java | 17 ++++++++-------- .../screen/menu/MenuManagerTests.java | 20 ++++--------------- 2 files changed, 12 insertions(+), 25 deletions(-) diff --git a/android/sdl_android/src/androidTest/java/com/smartdevicelink/managers/screen/choiceset/ChoiceSetManagerTests.java b/android/sdl_android/src/androidTest/java/com/smartdevicelink/managers/screen/choiceset/ChoiceSetManagerTests.java index 51009ab1b8..dd76c833e8 100644 --- a/android/sdl_android/src/androidTest/java/com/smartdevicelink/managers/screen/choiceset/ChoiceSetManagerTests.java +++ b/android/sdl_android/src/androidTest/java/com/smartdevicelink/managers/screen/choiceset/ChoiceSetManagerTests.java @@ -485,14 +485,12 @@ public void testUniquenessForAvailableFields() { List textFields = new ArrayList<>(); textFields.add(secondaryText); textFields.add(tertiaryText); - windowCapability.setTextFields(textFields); ImageField choiceImage = new ImageField(); choiceImage.setName(ImageFieldName.choiceImage); ImageField choiceSecondaryImage = new ImageField(); choiceSecondaryImage.setName(ImageFieldName.choiceSecondaryImage); - List imageFieldList = new ArrayList<>(); imageFieldList.add(choiceImage); imageFieldList.add(choiceSecondaryImage); @@ -500,21 +498,25 @@ public void testUniquenessForAvailableFields() { csm.defaultMainWindowCapability = windowCapability; - ChoiceCell cell1 = new ChoiceCell("Item 1", "null" , "tertiaryText", null, TestValues.GENERAL_ARTWORK, TestValues.GENERAL_ARTWORK ); - ChoiceCell cell2 = new ChoiceCell("Item 2", "null" , "tertiaryText", null, TestValues.GENERAL_ARTWORK ,TestValues.GENERAL_ARTWORK ); + ChoiceCell cell1 = new ChoiceCell("Item 1", "null", "tertiaryText", null, TestValues.GENERAL_ARTWORK, TestValues.GENERAL_ARTWORK); + ChoiceCell cell2 = new ChoiceCell("Item 2", "null", "tertiaryText", null, TestValues.GENERAL_ARTWORK, TestValues.GENERAL_ARTWORK); List choiceCellList = new ArrayList<>(); choiceCellList.add(cell1); choiceCellList.add(cell2); - ChoiceSet choiceSet = new ChoiceSet("choicSet" , choiceCellList, null); + ChoiceSet choiceSet = new ChoiceSet("choiceSet", choiceCellList, null); assertTrue(csm.setUpChoiceSet(choiceSet)); - + // Identical cells cell2.setText("Item 1"); assertFalse(csm.setUpChoiceSet(choiceSet)); + // Changing secondary text on cell 2 to be different cell2.setSecondaryText("changed text"); assertTrue(csm.setUpChoiceSet(choiceSet)); + // Removing secondaryText as a supported TextField in the WindowCapability textFields.remove(secondaryText); + // Test that it does not take into account secondaryText as a unique factor assertFalse(csm.setUpChoiceSet(choiceSet)); + cell2.setTertiaryText("changed text"); assertTrue(csm.setUpChoiceSet(choiceSet)); textFields.remove(tertiaryText); @@ -527,8 +529,5 @@ public void testUniquenessForAvailableFields() { assertTrue(csm.setUpChoiceSet(choiceSet)); imageFieldList.remove(choiceSecondaryImage); assertFalse(csm.setUpChoiceSet(choiceSet)); - - - } } diff --git a/android/sdl_android/src/androidTest/java/com/smartdevicelink/managers/screen/menu/MenuManagerTests.java b/android/sdl_android/src/androidTest/java/com/smartdevicelink/managers/screen/menu/MenuManagerTests.java index eec0f6a0fd..9523b01450 100644 --- a/android/sdl_android/src/androidTest/java/com/smartdevicelink/managers/screen/menu/MenuManagerTests.java +++ b/android/sdl_android/src/androidTest/java/com/smartdevicelink/managers/screen/menu/MenuManagerTests.java @@ -606,7 +606,6 @@ public void testAllowingNonUniqueTitles() { @Test public void testUniquenessForAvailableFields() { - WindowCapability windowCapability = new WindowCapability(); TextField menuSubMenuSecondaryText = new TextField(); menuSubMenuSecondaryText.setName(TextFieldName.menuSubMenuSecondaryText); @@ -616,13 +615,11 @@ public void testUniquenessForAvailableFields() { menuCommandSecondaryText.setName(TextFieldName.menuCommandSecondaryText); TextField menuCommandTertiaryText = new TextField(); menuCommandTertiaryText.setName(TextFieldName.menuCommandTertiaryText); - List textFields = new ArrayList<>(); textFields.add(menuSubMenuSecondaryText); textFields.add(menuSubMenuTertiaryText); textFields.add(menuCommandSecondaryText); textFields.add(menuCommandTertiaryText); - windowCapability.setTextFields(textFields); ImageField cmdIcon = new ImageField(); @@ -631,18 +628,12 @@ public void testUniquenessForAvailableFields() { menuSubMenuSecondaryImage.setName(ImageFieldName.menuSubMenuSecondaryImage); ImageField menuCommandSecondaryImage = new ImageField(); menuCommandSecondaryImage.setName(ImageFieldName.menuCommandSecondaryImage); - List imageFieldList = new ArrayList<>(); imageFieldList.add(cmdIcon); imageFieldList.add(menuSubMenuSecondaryImage); imageFieldList.add(menuCommandSecondaryImage); windowCapability.setImageFields(imageFieldList); - menuManager.defaultMainWindowCapability = windowCapability; - menuManager.currentHMILevel = HMILevel.HMI_FULL; - // send new cells. They should set the old way - List oldMenu = createDynamicMenu6_forUniqueNamesTest(); - MenuCell cell1 = new MenuCell("Text1", "SecondaryText", "TText", TestValues.GENERAL_ARTWORK, TestValues.GENERAL_ARTWORK, null, new MenuSelectionListener() { @Override @@ -680,22 +671,20 @@ public void onTriggered(TriggerSource trigger) { List menuCellList = new ArrayList<>(); menuCellList.add(cell1); menuCellList.add(cell2); - + // Test that menu Cells are unique assertTrue(menuManager.menuCellsAreUnique(menuCellList, new ArrayList())); - + // Set the menu cells to not be unique and test cell2.setTitle("Text1"); - assertFalse(menuManager.menuCellsAreUnique(menuCellList, new ArrayList())); - + // Change secondary text to make cells unique and test cell2.setSecondaryText("text2"); assertTrue(menuManager.menuCellsAreUnique(menuCellList, new ArrayList())); + // Remove menuCommandSecondaryText as a supported TextField and test that cells are not unique now textFields.remove(menuCommandSecondaryText); assertFalse(menuManager.menuCellsAreUnique(menuCellList, new ArrayList())); cell2.setTertiaryText("text3"); - assertTrue(menuManager.menuCellsAreUnique(menuCellList, new ArrayList())); - textFields.remove(menuCommandTertiaryText); assertFalse(menuManager.menuCellsAreUnique(menuCellList, new ArrayList())); @@ -727,7 +716,6 @@ public void onTriggered(TriggerSource trigger) { mainCell2.setSubCells(Collections.emptyList()); assertFalse(menuManager.menuCellsAreUnique(menuCellSubMenuList, new ArrayList())); - } From 815c9d4487b6f1cceed72c0d74b177ecf10be500 Mon Sep 17 00:00:00 2001 From: Julian Kast Date: Thu, 27 May 2021 13:22:55 -0400 Subject: [PATCH 20/28] Added logic for : On RPC v7.1+ connections, we will no longer display duplicate menu cells if some property of the cell is different but that property isn't actually displayed on the head unit. --- .../managers/screen/menu/BaseMenuManager.java | 126 ++++++++++++------ 1 file changed, 88 insertions(+), 38 deletions(-) diff --git a/base/src/main/java/com/smartdevicelink/managers/screen/menu/BaseMenuManager.java b/base/src/main/java/com/smartdevicelink/managers/screen/menu/BaseMenuManager.java index 56af80b737..98f9d370d2 100644 --- a/base/src/main/java/com/smartdevicelink/managers/screen/menu/BaseMenuManager.java +++ b/base/src/main/java/com/smartdevicelink/managers/screen/menu/BaseMenuManager.java @@ -176,11 +176,10 @@ public void setMenuCells(@NonNull List cells) { // Create a deep copy of the list so future changes by developers don't affect the algorithm logic List clonedCells = cloneMenuCellsList(cells); - // If we're running on a connection < RPC 7.1, we need to de-duplicate cells because presenting them will fail if we have the same cell primary text. - if (clonedCells != null && internalInterface.getSdlMsgVersion() != null - && (internalInterface.getSdlMsgVersion().getMajorVersion() < 7 - || (internalInterface.getSdlMsgVersion().getMajorVersion() == 7 && internalInterface.getSdlMsgVersion().getMinorVersion() == 0))) { - addUniqueNamesToCells(clonedCells); + // Check for cell lists with completely duplicate information, or any duplicate voiceCommands and return if it fails (logs are in the called method). + //TODO test what happens when we send a null list to menuCellsAreUnique + if (clonedCells != null && !menuCellsAreUnique(clonedCells, new ArrayList())) { + return; } if (currentHMILevel == null || currentHMILevel.equals(HMILevel.HMI_NONE) || currentSystemContext.equals(SystemContext.SYSCTXT_MENU)) { @@ -194,6 +193,23 @@ public void setMenuCells(@NonNull List cells) { } waitingOnHMIUpdate = false; + // If we're running on a connection < RPC 7.1, we need to de-duplicate cells because presenting them will fail if we have the same cell primary text. + if (clonedCells != null && internalInterface.getSdlMsgVersion() != null + && (internalInterface.getSdlMsgVersion().getMajorVersion() < 7 + || (internalInterface.getSdlMsgVersion().getMajorVersion() == 7 && internalInterface.getSdlMsgVersion().getMinorVersion() == 0))) { + addUniqueNamesToCellsWithDuplicatePrimaryText(clonedCells); + } else { + // On > RPC 7.1, at this point all cells are unique when considering all properties, + // but we also need to check if any cells will _appear_ as duplicates when displayed on the screen. + // To check that, we'll remove properties from the set cells based on the system capabilities + // (we probably don't need to consider them changing between now and when they're actually sent to the HU unless the menu layout changes) + // and check for uniqueness again. Then we'll add unique identifiers to primary text if there are duplicates. + // Then we transfer the primary text identifiers back to the main cells and add those to an operation to be sent. + List cellsWithRemovedProperties = removeUnusedProperties(clonedCells); + addUniqueNamesBasedOnStrippedCells(cellsWithRemovedProperties, clonedCells); + + } + // Update our Lists // set old list if (menuCells != null) { @@ -205,11 +221,6 @@ public void setMenuCells(@NonNull List cells) { menuCells.addAll(clonedCells); } - // Check for cell lists with completely duplicate information, or any duplicate voiceCommands and return if it fails (logs are in the called method). - if (!menuCellsAreUnique(menuCells, new ArrayList())) { - return; - } - // Upload the Artworks List artworksToBeUploaded = findAllArtworksToBeUploadedFromCells(menuCells); if (artworksToBeUploaded.size() > 0 && fileManager.get() != null) { @@ -815,6 +826,9 @@ private boolean hasImageFieldOfName(ImageFieldName imageFieldName) { } private boolean hasTextFieldOfName(TextFieldName textFieldName) { + if(textFieldName == TextFieldName.menuCommandSecondaryText) { + return false; + } return defaultMainWindowCapability == null || ManagerUtility.WindowCapabilityUtility.hasTextFieldOfName(defaultMainWindowCapability, textFieldName); } @@ -1441,7 +1455,7 @@ private List cloneMenuCellsList(List originalList) { return clone; } - private void addUniqueNamesToCells(List cells) { + private void addUniqueNamesToCellsWithDuplicatePrimaryText(List cells) { HashMap dictCounter = new HashMap<>(); for (MenuCell cell : cells) { @@ -1456,58 +1470,94 @@ private void addUniqueNamesToCells(List cells) { } if (cell.getSubCells() != null && cell.getSubCells().size() > 0) { - addUniqueNamesToCells(cell.getSubCells()); + addUniqueNamesToCellsWithDuplicatePrimaryText(cell.getSubCells()); } } } + void addUniqueNamesBasedOnStrippedCells(List strippedCells, List unstrippedCells) { + if (strippedCells == null || unstrippedCells == null || strippedCells.size() != unstrippedCells.size()) { + return; + } - /** - * Check for cell lists with completely duplicate information, or any duplicate voiceCommands - * - * @param cells List of MenuCell's you will be adding - * @param allVoiceCommands List of String's for VoiceCommands (Used for recursive calls to check voiceCommands of the cells) - * @return Boolean that indicates whether menuCells are unique or not - */ - boolean menuCellsAreUnique(List cells, ArrayList allVoiceCommands) { - //Check all voice commands for identical items and check each list of cells for identical cells - HashSet identicalCellsCheckSet = new HashSet<>(); + HashMap dictCounter = new HashMap<>(); - for (MenuCell cell : cells) { - // Strip away fields that cannot be used to determine uniqueness visually including fields not supported by hmi - MenuCell cellClone = cell.clone(); - cellClone.setVoiceCommands(null); - // Check for menuCells and subMenu icon + for (int i = 0; i < strippedCells.size(); i++) { + MenuCell cell = strippedCells.get(i); + Integer counter = dictCounter.get(cell); + if (counter != null) { + counter = counter + 1; + dictCounter.put(cell, counter); + } else { + dictCounter.put(cell, 1); + } + counter = dictCounter.get(cell); + if (counter > 1) { + unstrippedCells.get(i).setUniqueTitle(unstrippedCells.get(i).getTitle() + " (" + counter + ")"); + } + + if (cell.getSubCells() != null && cell.getSubCells().size() > 0) { + addUniqueNamesToCellsWithDuplicatePrimaryText(cell.getSubCells()); + } + + } + + + } + + List removeUnusedProperties(List menuCells) { + List removePropertiesClone = cloneMenuCellsList(menuCells); + for (MenuCell cell : removePropertiesClone) { + cell.setVoiceCommands(null); + + // Don't check ImageFieldName.subMenuIcon because it was added in 7.0 when the feature was added in 5.0. + // Just assume that if cmdIcon is not available, the submenu icon is not either. if (!hasImageFieldOfName(ImageFieldName.cmdIcon)) { - cellClone.setIcon(null); + cell.setIcon(null); } // Check for subMenu fields supported - if (cellClone.getSubCells() != null) { + if (cell.getSubCells() != null) { if (!hasTextFieldOfName(TextFieldName.menuSubMenuSecondaryText)) { - cellClone.setSecondaryText(null); + cell.setSecondaryText(null); } if (!hasTextFieldOfName(TextFieldName.menuSubMenuTertiaryText)) { - cellClone.setTertiaryText(null); + cell.setTertiaryText(null); } if (!hasImageFieldOfName(ImageFieldName.menuSubMenuSecondaryImage)) { - cellClone.setSecondaryArtwork(null); + cell.setSecondaryArtwork(null); } } else { if (!hasTextFieldOfName(TextFieldName.menuCommandSecondaryText)) { - cellClone.setSecondaryText(null); + cell.setSecondaryText(null); } if (!hasTextFieldOfName(TextFieldName.menuCommandTertiaryText)) { - cellClone.setTertiaryText(null); + cell.setTertiaryText(null); } if (!hasImageFieldOfName(ImageFieldName.menuCommandSecondaryImage)) { - cellClone.setSecondaryArtwork(null); + cell.setSecondaryArtwork(null); } } - identicalCellsCheckSet.add(cellClone); + } + return removePropertiesClone; + } + + /** + * Check for cell lists with completely duplicate information, or any duplicate voiceCommands + * + * @param cells List of MenuCell's you will be adding + * @param allVoiceCommands List of String's for VoiceCommands (Used for recursive calls to check voiceCommands of the cells) + * @return Boolean that indicates whether menuCells are unique or not + */ + boolean menuCellsAreUnique(List cells, ArrayList allVoiceCommands) { + //Check all voice commands for identical items and check each list of cells for identical cells + HashSet identicalCellsCheckSet = new HashSet<>(); + + for (MenuCell cell : cells) { + identicalCellsCheckSet.add(cell); // Recursively check the subcell lists to see if they are all unique as well. If anything is not, this will chain back up the list to return false. - if (cellClone.getSubCells() != null && cellClone.getSubCells().size() > 0) { - boolean subCellsAreUnique = menuCellsAreUnique(cellClone.getSubCells(), allVoiceCommands); + if (cell.getSubCells() != null && cell.getSubCells().size() > 0) { + boolean subCellsAreUnique = menuCellsAreUnique(cell.getSubCells(), allVoiceCommands); if (!subCellsAreUnique) { DebugTool.logError(TAG, "Not all subCells are unique. The menu will not be set."); From 5c599198d27be98ed24ff40089e44cdfef7da51e Mon Sep 17 00:00:00 2001 From: Julian Kast Date: Thu, 27 May 2021 13:23:50 -0400 Subject: [PATCH 21/28] Remove testing logic that was not meant to be uploaded. --- .../smartdevicelink/managers/screen/menu/BaseMenuManager.java | 3 --- 1 file changed, 3 deletions(-) diff --git a/base/src/main/java/com/smartdevicelink/managers/screen/menu/BaseMenuManager.java b/base/src/main/java/com/smartdevicelink/managers/screen/menu/BaseMenuManager.java index 98f9d370d2..ad0b041329 100644 --- a/base/src/main/java/com/smartdevicelink/managers/screen/menu/BaseMenuManager.java +++ b/base/src/main/java/com/smartdevicelink/managers/screen/menu/BaseMenuManager.java @@ -826,9 +826,6 @@ private boolean hasImageFieldOfName(ImageFieldName imageFieldName) { } private boolean hasTextFieldOfName(TextFieldName textFieldName) { - if(textFieldName == TextFieldName.menuCommandSecondaryText) { - return false; - } return defaultMainWindowCapability == null || ManagerUtility.WindowCapabilityUtility.hasTextFieldOfName(defaultMainWindowCapability, textFieldName); } From f3a74df1a2ddc6e103ba5c1ba60efb07fd5f57f7 Mon Sep 17 00:00:00 2001 From: Julian Kast Date: Fri, 28 May 2021 13:35:36 -0400 Subject: [PATCH 22/28] Fix not checking available ChoiceCell properties --- .../choiceset/ChoiceSetManagerTests.java | 3 +- .../choiceset/BaseChoiceSetManager.java | 100 +++++++++++++----- 2 files changed, 78 insertions(+), 25 deletions(-) diff --git a/android/sdl_android/src/androidTest/java/com/smartdevicelink/managers/screen/choiceset/ChoiceSetManagerTests.java b/android/sdl_android/src/androidTest/java/com/smartdevicelink/managers/screen/choiceset/ChoiceSetManagerTests.java index dd76c833e8..99100e6abc 100644 --- a/android/sdl_android/src/androidTest/java/com/smartdevicelink/managers/screen/choiceset/ChoiceSetManagerTests.java +++ b/android/sdl_android/src/androidTest/java/com/smartdevicelink/managers/screen/choiceset/ChoiceSetManagerTests.java @@ -236,7 +236,8 @@ public void testAddUniqueNamesToCells() { ChoiceCell cell4 = new ChoiceCell("McDonalds", "4 mile away", null, null, null, null); ChoiceCell cell5 = new ChoiceCell("Starbucks", "5 mile away", null, null, null, null); ChoiceCell cell6 = new ChoiceCell("Meijer", "6 mile away", null, null, null, null); - LinkedHashSet cellList = new LinkedHashSet<>(); + List cellList = new ArrayList<>(); + cellList.add(cell1); cellList.add(cell2); cellList.add(cell3); diff --git a/base/src/main/java/com/smartdevicelink/managers/screen/choiceset/BaseChoiceSetManager.java b/base/src/main/java/com/smartdevicelink/managers/screen/choiceset/BaseChoiceSetManager.java index feaf9007ad..7bf67c257c 100644 --- a/base/src/main/java/com/smartdevicelink/managers/screen/choiceset/BaseChoiceSetManager.java +++ b/base/src/main/java/com/smartdevicelink/managers/screen/choiceset/BaseChoiceSetManager.java @@ -39,6 +39,7 @@ import com.livio.taskmaster.Queue; import com.livio.taskmaster.Task; +import com.smartdevicelink.exception.SdlException; import com.smartdevicelink.managers.BaseSubManager; import com.smartdevicelink.managers.CompletionListener; import com.smartdevicelink.managers.ISdl; @@ -69,6 +70,7 @@ import com.smartdevicelink.util.DebugTool; import java.lang.ref.WeakReference; +import java.util.ArrayList; import java.util.HashMap; import java.util.HashSet; import java.util.LinkedHashSet; @@ -543,7 +545,7 @@ HashSet choicesToBeRemovedFromPendingWithArray(List choi * E.g. Choices param contains 2 cells with text/title "Address" will be handled by updating the uniqueText/uniqueTitle of the second cell to "Address (2)". * @param choices The list of choiceCells to be uploaded. */ - void addUniqueNamesToCells(LinkedHashSet choices) { + void addUniqueNamesToCells(List choices) { HashMap dictCounter = new HashMap<>(); for (ChoiceCell cell : choices) { @@ -559,16 +561,80 @@ void addUniqueNamesToCells(LinkedHashSet choices) { } } + void addUniqueNamesBasedOnStrippedCells(List strippedCells, List unstrippedCells) { + if (strippedCells == null || unstrippedCells == null || strippedCells.size() != unstrippedCells.size()) { + // throw SdlException + } + // Tracks how many of each cell primary text there are so that we can append numbers to make each unique as necessary + HashMap dictCounter = new HashMap<>(); + for (int i = 0; i < strippedCells.size(); i++) { + ChoiceCell cell = strippedCells.get(i); + Integer counter = dictCounter.get(cell); + if (counter != null) { + counter++; + dictCounter.put(cell, counter); + } else { + dictCounter.put(cell, 1); + } + + counter = dictCounter.get(cell); + + if (counter > 1) { + unstrippedCells.get(i).setUniqueText(unstrippedCells.get(i).getText() + " (" + counter + ")"); + } + } + } + + private List cloneChoiceCellList(List originalList) { + if (originalList == null) { + return null; + } + + List clone = new ArrayList<>(); + for (ChoiceCell choiceCell : originalList) { + clone.add(choiceCell.clone()); + } + return clone; + } + private LinkedHashSet getChoicesToBeUploadedWithArray(List choices) { - LinkedHashSet choiceSet = new LinkedHashSet<>(choices); - // If we're running on a connection < RPC 7.1, we need to de-duplicate cells because presenting them will fail if we have the same cell primary text. + // Clone choices + List choicesClone = cloneChoiceCellList(choices); if (choices != null && internalInterface.getSdlMsgVersion() != null && (internalInterface.getSdlMsgVersion().getMajorVersion() < 7 || (internalInterface.getSdlMsgVersion().getMajorVersion() == 7 && internalInterface.getSdlMsgVersion().getMinorVersion() == 0))) { - addUniqueNamesToCells(choiceSet); + // If we're on < RPC 7.1, all primary texts need to be unique, so we don't need to check removed properties and duplicate cells + addUniqueNamesToCells(choicesClone); + } else { + List strippedCellsClone = removeUnusedProperties(choicesClone); + addUniqueNamesBasedOnStrippedCells(strippedCellsClone, choicesClone); } - choiceSet.removeAll(preloadedChoices); - return choiceSet; + LinkedHashSet choiceCloneLinkedHash = new LinkedHashSet<>(choicesClone); + choiceCloneLinkedHash.removeAll(preloadedChoices); + return choiceCloneLinkedHash; + } + + List removeUnusedProperties(List choiceCells) { + List strippedCellsClone = cloneChoiceCellList(choiceCells); + //Clone Cells + for (ChoiceCell cell : strippedCellsClone) { + // Strip away fields that cannot be used to determine uniqueness visually including fields not supported by the HMI + cell.setVoiceCommands(null); + + if (!hasImageFieldOfName(ImageFieldName.choiceImage)) { + cell.setArtwork(null); + } + if (!hasTextFieldOfName(TextFieldName.secondaryText)) { + cell.setSecondaryText(null); + } + if (!hasTextFieldOfName(TextFieldName.tertiaryText)) { + cell.setTertiaryText(null); + } + if (!hasImageFieldOfName(ImageFieldName.choiceSecondaryImage)) { + cell.setSecondaryArtwork(null); + } + } + return strippedCellsClone; } void updateIdsOnChoices(LinkedHashSet choices) { @@ -680,6 +746,9 @@ private boolean hasImageFieldOfName(ImageFieldName imageFieldName) { } private boolean hasTextFieldOfName(TextFieldName textFieldName) { + if (textFieldName == TextFieldName.secondaryText) { + return false; + } return defaultMainWindowCapability == null || ManagerUtility.WindowCapabilityUtility.hasTextFieldOfName(defaultMainWindowCapability, textFieldName); } @@ -706,24 +775,7 @@ boolean setUpChoiceSet(ChoiceSet choiceSet) { int choiceCellWithVoiceCommandCount = 0; for (ChoiceCell cell : choices) { - ChoiceCell cellClone = cell.clone(); - // Strip cell parameters that are not supported on head unit to support uniqueness. - cellClone.setVoiceCommands(null); - - if (!hasTextFieldOfName(TextFieldName.secondaryText)) { - cellClone.setSecondaryText(null); - } - if (!hasTextFieldOfName(TextFieldName.tertiaryText)) { - cellClone.setTertiaryText(null); - } - if (!hasImageFieldOfName(ImageFieldName.choiceImage)) { - cellClone.setArtwork(null); - } - if (!hasImageFieldOfName(ImageFieldName.choiceSecondaryImage)) { - cellClone.setSecondaryArtwork(null); - } - - uniqueChoiceCells.add(cellClone); + uniqueChoiceCells.add(cell); // Not using cloned cell here because we set the clone's VoiceCommands to null for visual check only if (cell.getVoiceCommands() != null) { uniqueVoiceCommands.addAll(cell.getVoiceCommands()); From 9c72b7d009c93c5a246da20bd487492a5b45b2c0 Mon Sep 17 00:00:00 2001 From: Julian Kast Date: Tue, 1 Jun 2021 11:11:32 -0400 Subject: [PATCH 23/28] Add null checks and remove added unitTest, --- .../choiceset/ChoiceSetManagerTests.java | 57 --------- .../screen/menu/MenuManagerTests.java | 114 ------------------ .../choiceset/BaseChoiceSetManager.java | 2 +- .../managers/screen/menu/BaseMenuManager.java | 3 + 4 files changed, 4 insertions(+), 172 deletions(-) diff --git a/android/sdl_android/src/androidTest/java/com/smartdevicelink/managers/screen/choiceset/ChoiceSetManagerTests.java b/android/sdl_android/src/androidTest/java/com/smartdevicelink/managers/screen/choiceset/ChoiceSetManagerTests.java index 99100e6abc..fe3b5cfce7 100644 --- a/android/sdl_android/src/androidTest/java/com/smartdevicelink/managers/screen/choiceset/ChoiceSetManagerTests.java +++ b/android/sdl_android/src/androidTest/java/com/smartdevicelink/managers/screen/choiceset/ChoiceSetManagerTests.java @@ -474,61 +474,4 @@ public void testDismissingQueuedKeyboard() { verify(testKeyboardOp, times(0)).dismissKeyboard(); verify(testKeyboardOp2, times(1)).dismissKeyboard(); } - - @Test - public void testUniquenessForAvailableFields() { - WindowCapability windowCapability = new WindowCapability(); - TextField secondaryText = new TextField(); - secondaryText.setName(TextFieldName.secondaryText); - TextField tertiaryText = new TextField(); - tertiaryText.setName(TextFieldName.tertiaryText); - - List textFields = new ArrayList<>(); - textFields.add(secondaryText); - textFields.add(tertiaryText); - windowCapability.setTextFields(textFields); - - ImageField choiceImage = new ImageField(); - choiceImage.setName(ImageFieldName.choiceImage); - ImageField choiceSecondaryImage = new ImageField(); - choiceSecondaryImage.setName(ImageFieldName.choiceSecondaryImage); - List imageFieldList = new ArrayList<>(); - imageFieldList.add(choiceImage); - imageFieldList.add(choiceSecondaryImage); - windowCapability.setImageFields(imageFieldList); - - csm.defaultMainWindowCapability = windowCapability; - - ChoiceCell cell1 = new ChoiceCell("Item 1", "null", "tertiaryText", null, TestValues.GENERAL_ARTWORK, TestValues.GENERAL_ARTWORK); - ChoiceCell cell2 = new ChoiceCell("Item 2", "null", "tertiaryText", null, TestValues.GENERAL_ARTWORK, TestValues.GENERAL_ARTWORK); - List choiceCellList = new ArrayList<>(); - choiceCellList.add(cell1); - choiceCellList.add(cell2); - ChoiceSet choiceSet = new ChoiceSet("choiceSet", choiceCellList, null); - - assertTrue(csm.setUpChoiceSet(choiceSet)); - // Identical cells - cell2.setText("Item 1"); - assertFalse(csm.setUpChoiceSet(choiceSet)); - // Changing secondary text on cell 2 to be different - cell2.setSecondaryText("changed text"); - assertTrue(csm.setUpChoiceSet(choiceSet)); - // Removing secondaryText as a supported TextField in the WindowCapability - textFields.remove(secondaryText); - // Test that it does not take into account secondaryText as a unique factor - assertFalse(csm.setUpChoiceSet(choiceSet)); - - cell2.setTertiaryText("changed text"); - assertTrue(csm.setUpChoiceSet(choiceSet)); - textFields.remove(tertiaryText); - assertFalse(csm.setUpChoiceSet(choiceSet)); - cell2.setArtwork(null); - assertTrue(csm.setUpChoiceSet(choiceSet)); - imageFieldList.remove(choiceImage); - assertFalse(csm.setUpChoiceSet(choiceSet)); - cell2.setSecondaryArtwork(null); - assertTrue(csm.setUpChoiceSet(choiceSet)); - imageFieldList.remove(choiceSecondaryImage); - assertFalse(csm.setUpChoiceSet(choiceSet)); - } } diff --git a/android/sdl_android/src/androidTest/java/com/smartdevicelink/managers/screen/menu/MenuManagerTests.java b/android/sdl_android/src/androidTest/java/com/smartdevicelink/managers/screen/menu/MenuManagerTests.java index 9523b01450..ecfe93671b 100644 --- a/android/sdl_android/src/androidTest/java/com/smartdevicelink/managers/screen/menu/MenuManagerTests.java +++ b/android/sdl_android/src/androidTest/java/com/smartdevicelink/managers/screen/menu/MenuManagerTests.java @@ -604,120 +604,6 @@ public void testAllowingNonUniqueTitles() { assertEquals(menuManager.menuCells.get(3).getSubCells().get(3).getUniqueTitle(), "A"); } - @Test - public void testUniquenessForAvailableFields() { - WindowCapability windowCapability = new WindowCapability(); - TextField menuSubMenuSecondaryText = new TextField(); - menuSubMenuSecondaryText.setName(TextFieldName.menuSubMenuSecondaryText); - TextField menuSubMenuTertiaryText = new TextField(); - menuSubMenuTertiaryText.setName(TextFieldName.menuSubMenuTertiaryText); - TextField menuCommandSecondaryText = new TextField(); - menuCommandSecondaryText.setName(TextFieldName.menuCommandSecondaryText); - TextField menuCommandTertiaryText = new TextField(); - menuCommandTertiaryText.setName(TextFieldName.menuCommandTertiaryText); - List textFields = new ArrayList<>(); - textFields.add(menuSubMenuSecondaryText); - textFields.add(menuSubMenuTertiaryText); - textFields.add(menuCommandSecondaryText); - textFields.add(menuCommandTertiaryText); - windowCapability.setTextFields(textFields); - - ImageField cmdIcon = new ImageField(); - cmdIcon.setName(ImageFieldName.cmdIcon); - ImageField menuSubMenuSecondaryImage = new ImageField(); - menuSubMenuSecondaryImage.setName(ImageFieldName.menuSubMenuSecondaryImage); - ImageField menuCommandSecondaryImage = new ImageField(); - menuCommandSecondaryImage.setName(ImageFieldName.menuCommandSecondaryImage); - List imageFieldList = new ArrayList<>(); - imageFieldList.add(cmdIcon); - imageFieldList.add(menuSubMenuSecondaryImage); - imageFieldList.add(menuCommandSecondaryImage); - windowCapability.setImageFields(imageFieldList); - menuManager.defaultMainWindowCapability = windowCapability; - - MenuCell cell1 = new MenuCell("Text1", "SecondaryText", "TText", TestValues.GENERAL_ARTWORK, TestValues.GENERAL_ARTWORK, null, new MenuSelectionListener() { - @Override - public void onTriggered(TriggerSource trigger) { - - } - }); - - MenuCell cell2 = new MenuCell("Text2", "SecondaryText", "TText", TestValues.GENERAL_ARTWORK, TestValues.GENERAL_ARTWORK, null, new MenuSelectionListener() { - @Override - public void onTriggered(TriggerSource trigger) { - - } - }); - - - MenuCell subCell1 = new MenuCell("SubCell 1", "Secondary Text", "Tertiary Text", TestValues.GENERAL_ARTWORK, TestValues.GENERAL_ARTWORK, null, new MenuSelectionListener() { - @Override - public void onTriggered(TriggerSource trigger) { - } - }); - - MenuCell subCell2 = new MenuCell("SubCell 2", "Secondary Text", "Tertiary Text", TestValues.GENERAL_ARTWORK, TestValues.GENERAL_ARTWORK, null, new MenuSelectionListener() { - @Override - public void onTriggered(TriggerSource trigger) { - } - }); - MenuCell mainCell1 = new MenuCell("Test Cell 1", "null", "null", MenuLayout.LIST, TestValues.GENERAL_ARTWORK, TestValues.GENERAL_ARTWORK, Arrays.asList(subCell1, subCell2)); - MenuCell mainCell2 = new MenuCell("Test Cell 2", "null", "null", MenuLayout.LIST, TestValues.GENERAL_ARTWORK, TestValues.GENERAL_ARTWORK, Arrays.asList(subCell1, subCell2)); - - List menuCellSubMenuList = new ArrayList<>(); - menuCellSubMenuList.add(mainCell1); - menuCellSubMenuList.add(mainCell2); - - List menuCellList = new ArrayList<>(); - menuCellList.add(cell1); - menuCellList.add(cell2); - // Test that menu Cells are unique - assertTrue(menuManager.menuCellsAreUnique(menuCellList, new ArrayList())); - // Set the menu cells to not be unique and test - cell2.setTitle("Text1"); - assertFalse(menuManager.menuCellsAreUnique(menuCellList, new ArrayList())); - // Change secondary text to make cells unique and test - cell2.setSecondaryText("text2"); - assertTrue(menuManager.menuCellsAreUnique(menuCellList, new ArrayList())); - // Remove menuCommandSecondaryText as a supported TextField and test that cells are not unique now - textFields.remove(menuCommandSecondaryText); - assertFalse(menuManager.menuCellsAreUnique(menuCellList, new ArrayList())); - - cell2.setTertiaryText("text3"); - assertTrue(menuManager.menuCellsAreUnique(menuCellList, new ArrayList())); - textFields.remove(menuCommandTertiaryText); - assertFalse(menuManager.menuCellsAreUnique(menuCellList, new ArrayList())); - - cell2.setIcon(null); - imageFieldList.remove(cmdIcon); - assertFalse(menuManager.menuCellsAreUnique(menuCellList, new ArrayList())); - - cell2.setSecondaryArtwork(null); - imageFieldList.remove(menuCommandSecondaryImage); - assertFalse(menuManager.menuCellsAreUnique(menuCellList, new ArrayList())); - - assertTrue(menuManager.menuCellsAreUnique(menuCellSubMenuList, new ArrayList())); - mainCell2.setTitle("Test Cell 1"); - assertFalse(menuManager.menuCellsAreUnique(menuCellSubMenuList, new ArrayList())); - - mainCell2.setSecondaryText("changed text"); - assertTrue(menuManager.menuCellsAreUnique(menuCellSubMenuList, new ArrayList())); - textFields.remove(menuSubMenuSecondaryText); - assertFalse(menuManager.menuCellsAreUnique(menuCellSubMenuList, new ArrayList())); - - mainCell2.setTertiaryText("changed text"); - textFields.remove(menuSubMenuTertiaryText); - assertFalse(menuManager.menuCellsAreUnique(menuCellSubMenuList, new ArrayList())); - - mainCell2.setSecondaryArtwork(null); - assertTrue(menuManager.menuCellsAreUnique(menuCellSubMenuList, new ArrayList())); - imageFieldList.remove(menuSubMenuSecondaryImage); - assertFalse(menuManager.menuCellsAreUnique(menuCellSubMenuList, new ArrayList())); - - mainCell2.setSubCells(Collections.emptyList()); - assertFalse(menuManager.menuCellsAreUnique(menuCellSubMenuList, new ArrayList())); - } - // HELPERS diff --git a/base/src/main/java/com/smartdevicelink/managers/screen/choiceset/BaseChoiceSetManager.java b/base/src/main/java/com/smartdevicelink/managers/screen/choiceset/BaseChoiceSetManager.java index 7bf67c257c..c69e4b83c2 100644 --- a/base/src/main/java/com/smartdevicelink/managers/screen/choiceset/BaseChoiceSetManager.java +++ b/base/src/main/java/com/smartdevicelink/managers/screen/choiceset/BaseChoiceSetManager.java @@ -563,7 +563,7 @@ void addUniqueNamesToCells(List choices) { void addUniqueNamesBasedOnStrippedCells(List strippedCells, List unstrippedCells) { if (strippedCells == null || unstrippedCells == null || strippedCells.size() != unstrippedCells.size()) { - // throw SdlException + return; } // Tracks how many of each cell primary text there are so that we can append numbers to make each unique as necessary HashMap dictCounter = new HashMap<>(); diff --git a/base/src/main/java/com/smartdevicelink/managers/screen/menu/BaseMenuManager.java b/base/src/main/java/com/smartdevicelink/managers/screen/menu/BaseMenuManager.java index ad0b041329..40b84ec833 100644 --- a/base/src/main/java/com/smartdevicelink/managers/screen/menu/BaseMenuManager.java +++ b/base/src/main/java/com/smartdevicelink/managers/screen/menu/BaseMenuManager.java @@ -1503,6 +1503,9 @@ void addUniqueNamesBasedOnStrippedCells(List strippedCells, List removeUnusedProperties(List menuCells) { + if (menuCells == null) { + return null; + } List removePropertiesClone = cloneMenuCellsList(menuCells); for (MenuCell cell : removePropertiesClone) { cell.setVoiceCommands(null); From bdc318c70f22b2dc71c5265c41c7aa238294a474 Mon Sep 17 00:00:00 2001 From: Julian Kast Date: Tue, 1 Jun 2021 13:35:00 -0400 Subject: [PATCH 24/28] Fix subCell logic for MenuCells --- .../smartdevicelink/managers/screen/menu/BaseMenuManager.java | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/base/src/main/java/com/smartdevicelink/managers/screen/menu/BaseMenuManager.java b/base/src/main/java/com/smartdevicelink/managers/screen/menu/BaseMenuManager.java index 40b84ec833..f7ad3f66f2 100644 --- a/base/src/main/java/com/smartdevicelink/managers/screen/menu/BaseMenuManager.java +++ b/base/src/main/java/com/smartdevicelink/managers/screen/menu/BaseMenuManager.java @@ -1494,7 +1494,7 @@ void addUniqueNamesBasedOnStrippedCells(List strippedCells, List 0) { - addUniqueNamesToCellsWithDuplicatePrimaryText(cell.getSubCells()); + addUniqueNamesBasedOnStrippedCells(cell.getSubCells(), unstrippedCells.get(i).getSubCells()); } } @@ -1526,6 +1526,7 @@ List removeUnusedProperties(List menuCells) { if (!hasImageFieldOfName(ImageFieldName.menuSubMenuSecondaryImage)) { cell.setSecondaryArtwork(null); } + cell.setSubCells(removeUnusedProperties(cell.getSubCells())); } else { if (!hasTextFieldOfName(TextFieldName.menuCommandSecondaryText)) { cell.setSecondaryText(null); From 14c36b52753bb95a12fbac950f81b6bd36e9337c Mon Sep 17 00:00:00 2001 From: Julian Kast Date: Tue, 1 Jun 2021 13:35:24 -0400 Subject: [PATCH 25/28] Add unit test to menuManager --- .../screen/menu/MenuManagerTests.java | 104 ++++++++++++++++++ 1 file changed, 104 insertions(+) diff --git a/android/sdl_android/src/androidTest/java/com/smartdevicelink/managers/screen/menu/MenuManagerTests.java b/android/sdl_android/src/androidTest/java/com/smartdevicelink/managers/screen/menu/MenuManagerTests.java index ecfe93671b..0ac13101ee 100644 --- a/android/sdl_android/src/androidTest/java/com/smartdevicelink/managers/screen/menu/MenuManagerTests.java +++ b/android/sdl_android/src/androidTest/java/com/smartdevicelink/managers/screen/menu/MenuManagerTests.java @@ -604,6 +604,110 @@ public void testAllowingNonUniqueTitles() { assertEquals(menuManager.menuCells.get(3).getSubCells().get(3).getUniqueTitle(), "A"); } + @Test + public void testUniquenessForAvailableFields() { + WindowCapability windowCapability = new WindowCapability(); + TextField menuSubMenuSecondaryText = new TextField(); + menuSubMenuSecondaryText.setName(TextFieldName.menuSubMenuSecondaryText); + TextField menuSubMenuTertiaryText = new TextField(); + menuSubMenuTertiaryText.setName(TextFieldName.menuSubMenuTertiaryText); + TextField menuCommandSecondaryText = new TextField(); + menuCommandSecondaryText.setName(TextFieldName.menuCommandSecondaryText); + TextField menuCommandTertiaryText = new TextField(); + menuCommandTertiaryText.setName(TextFieldName.menuCommandTertiaryText); + List textFields = new ArrayList<>(); + textFields.add(menuSubMenuSecondaryText); + textFields.add(menuSubMenuTertiaryText); + textFields.add(menuCommandSecondaryText); + textFields.add(menuCommandTertiaryText); + windowCapability.setTextFields(textFields); + + ImageField cmdIcon = new ImageField(); + cmdIcon.setName(ImageFieldName.cmdIcon); + ImageField menuSubMenuSecondaryImage = new ImageField(); + menuSubMenuSecondaryImage.setName(ImageFieldName.menuSubMenuSecondaryImage); + ImageField menuCommandSecondaryImage = new ImageField(); + menuCommandSecondaryImage.setName(ImageFieldName.menuCommandSecondaryImage); + List imageFieldList = new ArrayList<>(); + imageFieldList.add(cmdIcon); + imageFieldList.add(menuSubMenuSecondaryImage); + imageFieldList.add(menuCommandSecondaryImage); + windowCapability.setImageFields(imageFieldList); + menuManager.defaultMainWindowCapability = windowCapability; + + assertNull(menuManager.removeUnusedProperties(null)); + + MenuCell cell1 = new MenuCell("Text1", "SecondaryText", "TText", TestValues.GENERAL_ARTWORK, TestValues.GENERAL_ARTWORK, null, new MenuSelectionListener() { + @Override + public void onTriggered(TriggerSource trigger) { + + } + }); + + MenuCell cell2 = new MenuCell("Text1", "SecondaryText2", "TText2", null, null, null, new MenuSelectionListener() { + @Override + public void onTriggered(TriggerSource trigger) { + + } + }); + + MenuCell subCell1 = new MenuCell("SubCell1", "Secondary Text", "TText", TestValues.GENERAL_ARTWORK, TestValues.GENERAL_ARTWORK, null, new MenuSelectionListener() { + @Override + public void onTriggered(TriggerSource trigger) { + } + }); + + MenuCell subCell2 = new MenuCell("SubCell1", "Secondary Text2", "TText2", null, null, null, new MenuSelectionListener() { + @Override + public void onTriggered(TriggerSource trigger) { + } + }); + + List subCellList = new ArrayList<>(); + subCellList.add(subCell1); + subCellList.add(subCell2); + + + MenuCell cell3 = new MenuCell("Test Cell 3 (sub menu)", "SecondaryText", "TText", MenuLayout.LIST, TestValues.GENERAL_ARTWORK, TestValues.GENERAL_ARTWORK, subCellList); + MenuCell cell4 = new MenuCell("Test Cell 3 (sub menu)", null, null, MenuLayout.LIST, null, null, subCellList); + + List menuCellList = new ArrayList<>(); + menuCellList.add(cell1); + menuCellList.add(cell2); + menuCellList.add(cell3); + menuCellList.add(cell4); + + List removedProperties = menuManager.removeUnusedProperties(menuCellList); + assertNotNull(removedProperties.get(0).getSecondaryText()); + menuManager.addUniqueNamesBasedOnStrippedCells(removedProperties, menuCellList); + assertEquals(menuCellList.get(1).getUniqueTitle(), "Text1"); + + // Remove menuCommandSecondaryText as a supported TextField + textFields.remove(menuCommandSecondaryText); + textFields.remove(menuCommandTertiaryText); + imageFieldList.remove(cmdIcon); + imageFieldList.remove(menuCommandSecondaryImage); + imageFieldList.remove(menuSubMenuSecondaryImage); + textFields.remove(menuSubMenuSecondaryText); + textFields.remove(menuSubMenuTertiaryText); + textFields.remove(menuSubMenuSecondaryImage); + + // Test removeUnusedProperties + removedProperties = menuManager.removeUnusedProperties(menuCellList); + assertNull(removedProperties.get(0).getSecondaryText()); + assertNull(removedProperties.get(0).getTertiaryText()); + + menuManager.addUniqueNamesBasedOnStrippedCells(removedProperties, menuCellList); + assertEquals(menuCellList.get(1).getUniqueTitle(), "Text1 (2)"); + + // SubCell test + assertEquals(menuCellList.get(3).getUniqueTitle(), "Test Cell 3 (sub menu) (2)"); + assertEquals(menuCellList.get(2).getSubCells().get(1).getUniqueTitle(), "SubCell1 (2)"); + + + } + + // HELPERS From 1c10cb5ec950b620f26a36035a619201451f650b Mon Sep 17 00:00:00 2001 From: Julian Kast Date: Tue, 1 Jun 2021 14:02:43 -0400 Subject: [PATCH 26/28] Remove testing code and add unit test for the choiceSetManager --- .../choiceset/ChoiceSetManagerTests.java | 47 +++++++++++++++++++ .../screen/menu/MenuManagerTests.java | 3 +- .../choiceset/BaseChoiceSetManager.java | 3 -- 3 files changed, 49 insertions(+), 4 deletions(-) diff --git a/android/sdl_android/src/androidTest/java/com/smartdevicelink/managers/screen/choiceset/ChoiceSetManagerTests.java b/android/sdl_android/src/androidTest/java/com/smartdevicelink/managers/screen/choiceset/ChoiceSetManagerTests.java index fe3b5cfce7..81f9123616 100644 --- a/android/sdl_android/src/androidTest/java/com/smartdevicelink/managers/screen/choiceset/ChoiceSetManagerTests.java +++ b/android/sdl_android/src/androidTest/java/com/smartdevicelink/managers/screen/choiceset/ChoiceSetManagerTests.java @@ -474,4 +474,51 @@ public void testDismissingQueuedKeyboard() { verify(testKeyboardOp, times(0)).dismissKeyboard(); verify(testKeyboardOp2, times(1)).dismissKeyboard(); } + + @Test + public void testUniquenessForAvailableFields() { + WindowCapability windowCapability = new WindowCapability(); + TextField secondaryText = new TextField(); + secondaryText.setName(TextFieldName.secondaryText); + TextField tertiaryText = new TextField(); + tertiaryText.setName(TextFieldName.tertiaryText); + + List textFields = new ArrayList<>(); + textFields.add(secondaryText); + textFields.add(tertiaryText); + windowCapability.setTextFields(textFields); + + ImageField choiceImage = new ImageField(); + choiceImage.setName(ImageFieldName.choiceImage); + ImageField choiceSecondaryImage = new ImageField(); + choiceSecondaryImage.setName(ImageFieldName.choiceSecondaryImage); + List imageFieldList = new ArrayList<>(); + imageFieldList.add(choiceImage); + imageFieldList.add(choiceSecondaryImage); + windowCapability.setImageFields(imageFieldList); + + csm.defaultMainWindowCapability = windowCapability; + + ChoiceCell cell1 = new ChoiceCell("Item 1", "null", "tertiaryText", null, TestValues.GENERAL_ARTWORK, TestValues.GENERAL_ARTWORK); + ChoiceCell cell2 = new ChoiceCell("Item 1", "null2", "tertiaryText2", null, null, null); + List choiceCellList = new ArrayList<>(); + choiceCellList.add(cell1); + choiceCellList.add(cell2); + + List removedProperties = csm.removeUnusedProperties(choiceCellList); + assertNotNull(removedProperties.get(0).getSecondaryText()); + + textFields.remove(secondaryText); + textFields.remove(tertiaryText); + imageFieldList.remove(choiceImage); + imageFieldList.remove(choiceSecondaryImage); + + removedProperties = csm.removeUnusedProperties(choiceCellList); + csm.addUniqueNamesBasedOnStrippedCells(removedProperties, choiceCellList); + assertEquals(choiceCellList.get(1).getUniqueText(), "Item 1 (2)"); + + + } + + } diff --git a/android/sdl_android/src/androidTest/java/com/smartdevicelink/managers/screen/menu/MenuManagerTests.java b/android/sdl_android/src/androidTest/java/com/smartdevicelink/managers/screen/menu/MenuManagerTests.java index 0ac13101ee..e71d8295c7 100644 --- a/android/sdl_android/src/androidTest/java/com/smartdevicelink/managers/screen/menu/MenuManagerTests.java +++ b/android/sdl_android/src/androidTest/java/com/smartdevicelink/managers/screen/menu/MenuManagerTests.java @@ -696,7 +696,7 @@ public void onTriggered(TriggerSource trigger) { removedProperties = menuManager.removeUnusedProperties(menuCellList); assertNull(removedProperties.get(0).getSecondaryText()); assertNull(removedProperties.get(0).getTertiaryText()); - + menuManager.addUniqueNamesBasedOnStrippedCells(removedProperties, menuCellList); assertEquals(menuCellList.get(1).getUniqueTitle(), "Text1 (2)"); @@ -960,4 +960,5 @@ private List createDynamicMenu6_forUniqueNamesTest() { return Arrays.asList(A, B, C, D); } + } diff --git a/base/src/main/java/com/smartdevicelink/managers/screen/choiceset/BaseChoiceSetManager.java b/base/src/main/java/com/smartdevicelink/managers/screen/choiceset/BaseChoiceSetManager.java index c69e4b83c2..7199ccf569 100644 --- a/base/src/main/java/com/smartdevicelink/managers/screen/choiceset/BaseChoiceSetManager.java +++ b/base/src/main/java/com/smartdevicelink/managers/screen/choiceset/BaseChoiceSetManager.java @@ -746,9 +746,6 @@ private boolean hasImageFieldOfName(ImageFieldName imageFieldName) { } private boolean hasTextFieldOfName(TextFieldName textFieldName) { - if (textFieldName == TextFieldName.secondaryText) { - return false; - } return defaultMainWindowCapability == null || ManagerUtility.WindowCapabilityUtility.hasTextFieldOfName(defaultMainWindowCapability, textFieldName); } From 7f09631e6cc3022b9bf9bd47319b085fc14cc2f2 Mon Sep 17 00:00:00 2001 From: Julian Kast Date: Tue, 1 Jun 2021 15:55:39 -0400 Subject: [PATCH 27/28] Update comments/alignment --- .../managers/screen/menu/BaseMenuManager.java | 13 ++++++------- 1 file changed, 6 insertions(+), 7 deletions(-) diff --git a/base/src/main/java/com/smartdevicelink/managers/screen/menu/BaseMenuManager.java b/base/src/main/java/com/smartdevicelink/managers/screen/menu/BaseMenuManager.java index f7ad3f66f2..149d3fa2b8 100644 --- a/base/src/main/java/com/smartdevicelink/managers/screen/menu/BaseMenuManager.java +++ b/base/src/main/java/com/smartdevicelink/managers/screen/menu/BaseMenuManager.java @@ -177,7 +177,6 @@ public void setMenuCells(@NonNull List cells) { List clonedCells = cloneMenuCellsList(cells); // Check for cell lists with completely duplicate information, or any duplicate voiceCommands and return if it fails (logs are in the called method). - //TODO test what happens when we send a null list to menuCellsAreUnique if (clonedCells != null && !menuCellsAreUnique(clonedCells, new ArrayList())) { return; } @@ -205,8 +204,8 @@ public void setMenuCells(@NonNull List cells) { // (we probably don't need to consider them changing between now and when they're actually sent to the HU unless the menu layout changes) // and check for uniqueness again. Then we'll add unique identifiers to primary text if there are duplicates. // Then we transfer the primary text identifiers back to the main cells and add those to an operation to be sent. - List cellsWithRemovedProperties = removeUnusedProperties(clonedCells); - addUniqueNamesBasedOnStrippedCells(cellsWithRemovedProperties, clonedCells); + List strippedCellsClone = removeUnusedProperties(clonedCells); + addUniqueNamesBasedOnStrippedCells(strippedCellsClone, clonedCells); } @@ -1476,9 +1475,8 @@ void addUniqueNamesBasedOnStrippedCells(List strippedCells, List dictCounter = new HashMap<>(); - for (int i = 0; i < strippedCells.size(); i++) { MenuCell cell = strippedCells.get(i); Integer counter = dictCounter.get(cell); @@ -1508,6 +1506,7 @@ List removeUnusedProperties(List menuCells) { } List removePropertiesClone = cloneMenuCellsList(menuCells); for (MenuCell cell : removePropertiesClone) { + // Strip away fields that cannot be used to determine uniqueness visually including fields not supported by the HMI cell.setVoiceCommands(null); // Don't check ImageFieldName.subMenuIcon because it was added in 7.0 when the feature was added in 5.0. @@ -1549,7 +1548,7 @@ List removeUnusedProperties(List menuCells) { * @param allVoiceCommands List of String's for VoiceCommands (Used for recursive calls to check voiceCommands of the cells) * @return Boolean that indicates whether menuCells are unique or not */ - boolean menuCellsAreUnique(List cells, ArrayList allVoiceCommands) { + private boolean menuCellsAreUnique(List cells, ArrayList allVoiceCommands) { //Check all voice commands for identical items and check each list of cells for identical cells HashSet identicalCellsCheckSet = new HashSet<>(); @@ -1566,7 +1565,7 @@ boolean menuCellsAreUnique(List cells, ArrayList allVoiceComma } } - // Voice commands have to be identical across all lists, Not using cloned cell here because we set the clone's VoiceCommands to null for visual check only + // Voice commands have to be identical across all lists if (cell.getVoiceCommands() == null) { continue; } From 108f31530caa712d4280ac18c3bed456d00157f1 Mon Sep 17 00:00:00 2001 From: Julian Kast Date: Tue, 1 Jun 2021 15:57:26 -0400 Subject: [PATCH 28/28] remove unused import --- .../managers/screen/choiceset/BaseChoiceSetManager.java | 1 - 1 file changed, 1 deletion(-) diff --git a/base/src/main/java/com/smartdevicelink/managers/screen/choiceset/BaseChoiceSetManager.java b/base/src/main/java/com/smartdevicelink/managers/screen/choiceset/BaseChoiceSetManager.java index 7199ccf569..8939097185 100644 --- a/base/src/main/java/com/smartdevicelink/managers/screen/choiceset/BaseChoiceSetManager.java +++ b/base/src/main/java/com/smartdevicelink/managers/screen/choiceset/BaseChoiceSetManager.java @@ -39,7 +39,6 @@ import com.livio.taskmaster.Queue; import com.livio.taskmaster.Task; -import com.smartdevicelink.exception.SdlException; import com.smartdevicelink.managers.BaseSubManager; import com.smartdevicelink.managers.CompletionListener; import com.smartdevicelink.managers.ISdl;