diff options
author | Robert Henigan <robert.henigan@livio.io> | 2021-03-04 10:09:56 -0500 |
---|---|---|
committer | GitHub <noreply@github.com> | 2021-03-04 10:09:56 -0500 |
commit | a70d82d4219a80412fb092cbb4989a722083338b (patch) | |
tree | e9305e9d6b3146bf9a6159d60753628653aa0a25 | |
parent | c9f1903800f51835cc777be643f196f5cfa81a26 (diff) | |
download | sdl_android-a70d82d4219a80412fb092cbb4989a722083338b.tar.gz |
Feature/issue 811 broaden choiceset uniqueness (#1599)
* Update ChoiceCell with uniqueText field
uniqueText will be updated when SdlMsgVersion is older than 7.1 and if
two choiceCells have the same primary text
* Remove a check that isnt needed
* update unit tests for ChoiceCell uniqueText
* Add test for addUniqueNamesToCells
* Add uniqueTitle to subMenu cells
* Changes to better align with iOS lib
* Update subCells uniqueNames if available
* Updates based on iOS review
* Update based on iOS changes
* Add comment that was added to choicesetmanager
* Add duplicate check to menuManager
* Add tests and null checks
* iOS Alignment
* iOS Alignment changes
* Review Feedback fixes
Co-authored-by: Henigan <rheniga1@MGC12Z921DLVCG.fbpld77.ford.com>
11 files changed, 395 insertions, 54 deletions
diff --git a/android/sdl_android/src/androidTest/java/com/smartdevicelink/managers/screen/choiceset/ChoiceCellTests.java b/android/sdl_android/src/androidTest/java/com/smartdevicelink/managers/screen/choiceset/ChoiceCellTests.java index 4bdd3ff93..0dde875ea 100644 --- a/android/sdl_android/src/androidTest/java/com/smartdevicelink/managers/screen/choiceset/ChoiceCellTests.java +++ b/android/sdl_android/src/androidTest/java/com/smartdevicelink/managers/screen/choiceset/ChoiceCellTests.java @@ -61,6 +61,7 @@ public class ChoiceCellTests { choiceCell.setVoiceCommands(TestValues.GENERAL_STRING_LIST); choiceCell.setArtwork(artwork); choiceCell.setSecondaryArtwork(artwork); + choiceCell.setUniqueText(TestValues.GENERAL_STRING); // use getters and assert equality assertEquals(choiceCell.getText(), TestValues.GENERAL_STRING); @@ -70,6 +71,7 @@ public class ChoiceCellTests { assertEquals(choiceCell.getArtwork(), artwork); assertEquals(choiceCell.getSecondaryArtwork(), artwork); assertEquals(choiceCell.getChoiceId(), MAX_ID); + assertEquals(choiceCell.getUniqueText(), TestValues.GENERAL_STRING); } @Test @@ -87,6 +89,7 @@ public class ChoiceCellTests { assertEquals(choiceCell.getArtwork(), artwork); assertEquals(choiceCell.getSecondaryArtwork(), artwork); assertEquals(choiceCell.getChoiceId(), MAX_ID); + assertEquals(choiceCell.getUniqueText(), choiceCell.getText()); choiceCell = new ChoiceCell(TestValues.GENERAL_STRING, TestValues.GENERAL_STRING, TestValues.GENERAL_STRING, TestValues.GENERAL_STRING_LIST, artwork, artwork); @@ -97,6 +100,8 @@ public class ChoiceCellTests { assertEquals(choiceCell.getArtwork(), artwork); assertEquals(choiceCell.getSecondaryArtwork(), artwork); assertEquals(choiceCell.getChoiceId(), MAX_ID); + assertEquals(choiceCell.getUniqueText(), choiceCell.getText()); + } @Test @@ -116,6 +121,11 @@ public class ChoiceCellTests { choiceCell3.setSecondaryText(TestValues.GENERAL_STRING); choiceCell3.setTertiaryText(TestValues.GENERAL_STRING); + //UniqueText should not be taken into consideration when checking equality + choiceCell.setUniqueText(TestValues.GENERAL_STRING); + choiceCell2.setUniqueText(TestValues.GENERAL_STRING); + choiceCell3.setUniqueText(TestValues.GENERAL_STRING); + // Make sure our overridden method works, even though these are different objects in memory assertTrue(choiceCell.equals(choiceCell2)); assertFalse(choiceCell.equals(choiceCell3)); 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 59afe220b..16ea857f5 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 @@ -43,6 +43,7 @@ import com.smartdevicelink.managers.file.FileManager; 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.WindowCapability; import com.smartdevicelink.proxy.rpc.enums.HMILevel; import com.smartdevicelink.proxy.rpc.enums.KeyboardInputMask; @@ -62,6 +63,7 @@ import java.util.ArrayList; import java.util.Arrays; import java.util.Collections; import java.util.HashSet; +import java.util.LinkedHashSet; import java.util.List; import static junit.framework.TestCase.assertEquals; @@ -90,6 +92,7 @@ public class ChoiceSetManagerTests { FileManager fileManager = mock(FileManager.class); taskmaster = new Taskmaster.Builder().build(); when(internalInterface.getTaskmaster()).thenReturn(taskmaster); + when(internalInterface.getSdlMsgVersion()).thenReturn(new SdlMsgVersion(7, 0)); csm = new ChoiceSetManager(internalInterface, fileManager); assertEquals(csm.getState(), BaseSubManager.SETTING_UP); @@ -152,29 +155,37 @@ public class ChoiceSetManagerTests { ChoiceSet choiceSet1 = new ChoiceSet("test", Collections.<ChoiceCell>emptyList(), choiceSetSelectionListener); assertFalse(csm.setUpChoiceSet(choiceSet1)); - // cells cant have duplicate text + // Identical cells will not be allowed ChoiceCell cell1 = new ChoiceCell("test"); ChoiceCell cell2 = new ChoiceCell("test"); ChoiceSet choiceSet2 = new ChoiceSet("test", Arrays.asList(cell1, cell2), choiceSetSelectionListener); assertFalse(csm.setUpChoiceSet(choiceSet2)); - // cells cannot mix and match VR / non-VR - ChoiceCell cell3 = new ChoiceCell("test", Collections.singletonList("Test"), null); - ChoiceCell cell4 = new ChoiceCell("test2"); + // cells that have duplicate text will be allowed if there is another property to make them unique because a unique name will be assigned and used + ChoiceCell cell3 = new ChoiceCell("test"); + cell3.setSecondaryText("text 1"); + ChoiceCell cell4 = new ChoiceCell("test"); + cell4.setSecondaryText("text 2"); ChoiceSet choiceSet3 = new ChoiceSet("test", Arrays.asList(cell3, cell4), choiceSetSelectionListener); - assertFalse(csm.setUpChoiceSet(choiceSet3)); + assertTrue(csm.setUpChoiceSet(choiceSet3)); - // VR Commands must be unique + // cells cannot mix and match VR / non-VR ChoiceCell cell5 = new ChoiceCell("test", Collections.singletonList("Test"), null); - ChoiceCell cell6 = new ChoiceCell("test2", Collections.singletonList("Test"), null); + ChoiceCell cell6 = new ChoiceCell("test2"); ChoiceSet choiceSet4 = new ChoiceSet("test", Arrays.asList(cell5, cell6), choiceSetSelectionListener); assertFalse(csm.setUpChoiceSet(choiceSet4)); - // Passing Case + // VR Commands must be unique ChoiceCell cell7 = new ChoiceCell("test", Collections.singletonList("Test"), null); - ChoiceCell cell8 = new ChoiceCell("test2", Collections.singletonList("Test2"), null); + ChoiceCell cell8 = new ChoiceCell("test2", Collections.singletonList("Test"), null); ChoiceSet choiceSet5 = new ChoiceSet("test", Arrays.asList(cell7, cell8), choiceSetSelectionListener); - assertTrue(csm.setUpChoiceSet(choiceSet5)); + assertFalse(csm.setUpChoiceSet(choiceSet5)); + + // Passing Case + ChoiceCell cell9 = new ChoiceCell("test", Collections.singletonList("Test"), null); + ChoiceCell cell10 = new ChoiceCell("test2", Collections.singletonList("Test2"), null); + ChoiceSet choiceSet6 = new ChoiceSet("test", Arrays.asList(cell9, cell10), choiceSetSelectionListener); + assertTrue(csm.setUpChoiceSet(choiceSet6)); } @Test @@ -197,7 +208,7 @@ public class ChoiceSetManagerTests { ChoiceCell cell1 = new ChoiceCell("test"); ChoiceCell cell2 = new ChoiceCell("test2"); ChoiceCell cell3 = new ChoiceCell("test3"); - HashSet<ChoiceCell> cellSet = new HashSet<>(); + LinkedHashSet<ChoiceCell> cellSet = new LinkedHashSet<>(); cellSet.add(cell1); cellSet.add(cell2); cellSet.add(cell3); @@ -213,6 +224,32 @@ public class ChoiceSetManagerTests { } @Test + public void testAddUniqueNamesToCells() { + ChoiceCell cell1 = new ChoiceCell("McDonalds", "1 mile away", null, null, null, null); + ChoiceCell cell2 = new ChoiceCell("McDonalds", "2 mile away", null, null, null, null); + ChoiceCell cell3 = new ChoiceCell("Starbucks", "3 mile away", null, null, null, null); + 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<ChoiceCell> cellList = new LinkedHashSet<>(); + cellList.add(cell1); + cellList.add(cell2); + cellList.add(cell3); + cellList.add(cell4); + cellList.add(cell5); + cellList.add(cell6); + + csm.addUniqueNamesToCells(cellList); + + assertEquals(cell1.getUniqueText(), "McDonalds"); + assertEquals(cell2.getUniqueText(), "McDonalds (2)"); + assertEquals(cell3.getUniqueText(), "Starbucks"); + assertEquals(cell4.getUniqueText(), "McDonalds (3)"); + assertEquals(cell5.getUniqueText(), "Starbucks (2)"); + assertEquals(cell6.getUniqueText(), "Meijer"); + } + + @Test public void testChoicesToBeRemovedFromPendingWithArray() { ChoiceCell cell1 = new ChoiceCell("test"); diff --git a/android/sdl_android/src/androidTest/java/com/smartdevicelink/managers/screen/choiceset/PreloadChoicesOperationTests.java b/android/sdl_android/src/androidTest/java/com/smartdevicelink/managers/screen/choiceset/PreloadChoicesOperationTests.java index b8ab310ca..9e879a73a 100644 --- a/android/sdl_android/src/androidTest/java/com/smartdevicelink/managers/screen/choiceset/PreloadChoicesOperationTests.java +++ b/android/sdl_android/src/androidTest/java/com/smartdevicelink/managers/screen/choiceset/PreloadChoicesOperationTests.java @@ -58,6 +58,7 @@ import org.junit.runner.RunWith; import java.util.Arrays; import java.util.Collections; import java.util.HashSet; +import java.util.LinkedHashSet; import java.util.List; import static junit.framework.TestCase.assertEquals; @@ -81,7 +82,7 @@ public class PreloadChoicesOperationTests { ChoiceCell cell1 = new ChoiceCell("cell 1"); ChoiceCell cell2 = new ChoiceCell("cell 2", null, TestValues.GENERAL_ARTWORK); - HashSet<ChoiceCell> cellsToPreload = new HashSet<>(); + LinkedHashSet<ChoiceCell> cellsToPreload = new LinkedHashSet<>(); cellsToPreload.add(cell1); cellsToPreload.add(cell2); @@ -118,7 +119,7 @@ public class PreloadChoicesOperationTests { ChoiceCell cell1 = new ChoiceCell("cell 1"); ChoiceCell cell2 = new ChoiceCell("cell 2", null, TestValues.GENERAL_ARTWORK); - HashSet<ChoiceCell> cellsToPreload = new HashSet<>(); + LinkedHashSet<ChoiceCell> cellsToPreload = new LinkedHashSet<>(); cellsToPreload.add(cell1); cellsToPreload.add(cell2); @@ -135,7 +136,7 @@ public class PreloadChoicesOperationTests { ChoiceCell cell1 = new ChoiceCell("cell 1"); ChoiceCell cell2 = new ChoiceCell("cell 2", null, TestValues.GENERAL_ARTWORK); - HashSet<ChoiceCell> cellsToPreload = new HashSet<>(); + LinkedHashSet<ChoiceCell> cellsToPreload = new LinkedHashSet<>(); cellsToPreload.add(cell1); cellsToPreload.add(cell2); diff --git a/android/sdl_android/src/androidTest/java/com/smartdevicelink/managers/screen/menu/MenuCellTests.java b/android/sdl_android/src/androidTest/java/com/smartdevicelink/managers/screen/menu/MenuCellTests.java index f6d5d5adb..b0e703e33 100644 --- a/android/sdl_android/src/androidTest/java/com/smartdevicelink/managers/screen/menu/MenuCellTests.java +++ b/android/sdl_android/src/androidTest/java/com/smartdevicelink/managers/screen/menu/MenuCellTests.java @@ -32,8 +32,6 @@ package com.smartdevicelink.managers.screen.menu; -import androidx.annotation.NonNull; -import androidx.annotation.Nullable; import androidx.test.ext.junit.runners.AndroidJUnit4; import com.smartdevicelink.managers.file.filetypes.SdlArtworkTests; @@ -75,6 +73,7 @@ public class MenuCellTests { menuCell.setSecondaryText(TestValues.GENERAL_STRING); menuCell.setTertiaryText(TestValues.GENERAL_STRING); menuCell.setSecondaryArtwork(TestValues.GENERAL_ARTWORK); + menuCell.setUniqueTitle(TestValues.GENERAL_STRING); // use getters and assert equality assertEquals(menuCell.getTitle(), TestValues.GENERAL_STRING); @@ -87,6 +86,7 @@ public class MenuCellTests { assertEquals(menuCell.getSecondaryText(), TestValues.GENERAL_STRING); assertEquals(menuCell.getTertiaryText(), TestValues.GENERAL_STRING); assertEquals(menuCell.getSecondaryArtwork(), TestValues.GENERAL_ARTWORK); + assertEquals(menuCell.getUniqueTitle(), TestValues.GENERAL_STRING); } @Test @@ -99,16 +99,20 @@ public class MenuCellTests { assertEquals(menuCell3.getIcon(), TestValues.GENERAL_ARTWORK); assertEquals(menuCell3.getVoiceCommands(), TestValues.GENERAL_STRING_LIST); assertEquals(menuCell3.getMenuSelectionListener(), menuSelectionListener); + assertEquals(menuCell3.getUniqueTitle(), TestValues.GENERAL_STRING); MenuCell menuCell4 = new MenuCell(TestValues.GENERAL_STRING, null, null, menuSelectionListener); assertEquals(menuCell4.getTitle(), TestValues.GENERAL_STRING); assertEquals(menuCell4.getMenuSelectionListener(), menuSelectionListener); + assertEquals(menuCell4.getUniqueTitle(), TestValues.GENERAL_STRING); MenuCell menuCell5 = new MenuCell(TestValues.GENERAL_STRING, TestValues.GENERAL_MENU_LAYOUT, TestValues.GENERAL_ARTWORK, TestValues.GENERAL_MENUCELL_LIST); assertEquals(menuCell5.getTitle(), TestValues.GENERAL_STRING); assertEquals(menuCell5.getIcon(), TestValues.GENERAL_ARTWORK); assertEquals(menuCell5.getSubMenuLayout(), TestValues.GENERAL_MENU_LAYOUT); assertEquals(menuCell5.getSubCells(), TestValues.GENERAL_MENUCELL_LIST); + assertEquals(menuCell5.getUniqueTitle(), TestValues.GENERAL_STRING); + MenuCell menuCell6 = new MenuCell(TestValues.GENERAL_STRING, TestValues.GENERAL_STRING, TestValues.GENERAL_STRING, TestValues.GENERAL_ARTWORK, TestValues.GENERAL_ARTWORK, TestValues.GENERAL_STRING_LIST, menuSelectionListener); assertEquals(menuCell6.getTitle(), TestValues.GENERAL_STRING); @@ -173,6 +177,7 @@ public class MenuCellTests { assertEquals(original.getTitle(), clone.getTitle()); assertEquals(original.getCellId(), clone.getCellId()); assertEquals(original.getParentCellId(), clone.getParentCellId()); + assertEquals(original.getUniqueTitle(), clone.getUniqueTitle()); assertEquals(original.getSecondaryText(), clone.getSecondaryText()); assertEquals(original.getTertiaryText(), clone.getTertiaryText()); 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 43392b626..2419023f6 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 @@ -75,6 +75,7 @@ import static junit.framework.TestCase.assertTrue; import static org.mockito.ArgumentMatchers.any; import static org.mockito.ArgumentMatchers.eq; import static org.mockito.Mockito.doAnswer; +import static org.mockito.Mockito.doReturn; import static org.mockito.Mockito.mock; import static org.mockito.Mockito.times; import static org.mockito.Mockito.verify; @@ -89,6 +90,7 @@ public class MenuManagerTests { private MenuManager menuManager; private List<MenuCell> cells; private MenuCell mainCell1, mainCell4; + final ISdl internalInterface = mock(ISdl.class); // SETUP / HELPERS @@ -97,7 +99,6 @@ public class MenuManagerTests { cells = createTestCells(); - final ISdl internalInterface = mock(ISdl.class); FileManager fileManager = mock(FileManager.class); // When internalInterface.addOnRPCNotificationListener(FunctionID.ON_HMI_STATUS, OnRPCNotificationListener) is called @@ -136,6 +137,11 @@ public class MenuManagerTests { }; doAnswer(answer).when(internalInterface).sendRPC(any(SetGlobalProperties.class)); + SdlMsgVersion version = new SdlMsgVersion(); + version.setMajorVersion(7); + version.setMinorVersion(0); + doReturn(version).when(internalInterface).getSdlMsgVersion(); + menuManager = new MenuManager(internalInterface, fileManager); // Check some stuff during setup @@ -530,6 +536,68 @@ public class MenuManagerTests { } + @Test + public void testSettingUniqueMenuNames() { + //Testing using SDLMsgVersion 7.0, at this version uniqueTitles will be set + + // Make sure we can send an empty menu with no issues + // start fresh + menuManager.oldMenuCells = null; + menuManager.menuCells = null; + menuManager.inProgressUpdate = null; + menuManager.waitingUpdateMenuCells = null; + menuManager.waitingOnHMIUpdate = false; + + menuManager.currentHMILevel = HMILevel.HMI_FULL; + // send new cells. They should set the old way + List<MenuCell> oldMenu = createDynamicMenu6_forUniqueNamesTest(); + menuManager.setMenuCells(oldMenu); + assertEquals(menuManager.menuCells.size(), 4); + assertEquals(menuManager.menuCells.get(0).getUniqueTitle(), "A"); + assertEquals(menuManager.menuCells.get(1).getUniqueTitle(), "A (2)"); + assertEquals(menuManager.menuCells.get(2).getUniqueTitle(), "A (3)"); + assertEquals(menuManager.menuCells.get(3).getUniqueTitle(), "A (4)"); + + assertEquals((menuManager.menuCells.get(3).getSubCells().size()), 4); + assertEquals(menuManager.menuCells.get(3).getSubCells().get(0).getUniqueTitle(), "A"); + assertEquals(menuManager.menuCells.get(3).getSubCells().get(1).getUniqueTitle(), "A (2)"); + assertEquals(menuManager.menuCells.get(3).getSubCells().get(2).getUniqueTitle(), "A (3)"); + assertEquals(menuManager.menuCells.get(3).getSubCells().get(3).getUniqueTitle(), "A (4)"); + } + + @Test + public void testAllowingNonUniqueTitles() { + //Testing using SDLMsgVersion 7.1, at this version uniqueTitles will be set + SdlMsgVersion version = new SdlMsgVersion(); + version.setMajorVersion(7); + version.setMinorVersion(1); + doReturn(version).when(internalInterface).getSdlMsgVersion(); + + // Make sure we can send an empty menu with no issues + // start fresh + menuManager.oldMenuCells = null; + menuManager.menuCells = null; + menuManager.inProgressUpdate = null; + menuManager.waitingUpdateMenuCells = null; + menuManager.waitingOnHMIUpdate = false; + + menuManager.currentHMILevel = HMILevel.HMI_FULL; + // send new cells. They should set the old way + List<MenuCell> oldMenu = createDynamicMenu6_forUniqueNamesTest(); + menuManager.setMenuCells(oldMenu); + assertEquals(menuManager.menuCells.size(), 4); + assertEquals(menuManager.menuCells.get(0).getUniqueTitle(), "A"); + assertEquals(menuManager.menuCells.get(1).getUniqueTitle(), "A"); + assertEquals(menuManager.menuCells.get(2).getUniqueTitle(), "A"); + assertEquals(menuManager.menuCells.get(3).getUniqueTitle(), "A"); + + assertEquals((menuManager.menuCells.get(3).getSubCells().size()), 4); + assertEquals(menuManager.menuCells.get(3).getSubCells().get(0).getUniqueTitle(), "A"); + assertEquals(menuManager.menuCells.get(3).getSubCells().get(1).getUniqueTitle(), "A"); + assertEquals(menuManager.menuCells.get(3).getSubCells().get(2).getUniqueTitle(), "A"); + assertEquals(menuManager.menuCells.get(3).getSubCells().get(3).getUniqueTitle(), "A"); + } + // HELPERS // Emulate what happens when Core sends OnHMIStatus notification @@ -754,4 +822,31 @@ public class MenuManagerTests { } + private List<MenuCell> createDynamicMenu6_forUniqueNamesTest() { + MenuSelectionListener menuSelectionListenerA = mock(MenuSelectionListener.class); + MenuSelectionListener menuSelectionListenerB = mock(MenuSelectionListener.class); + MenuSelectionListener menuSelectionListenerC = mock(MenuSelectionListener.class); + MenuSelectionListener menuSelectionListenerD = mock(MenuSelectionListener.class); + + SdlArtwork icon1 = new SdlArtwork("livio", FileType.GRAPHIC_PNG, R.drawable.sdl_lockscreen_icon, false); + SdlArtwork icon2 = new SdlArtwork("livio2", FileType.GRAPHIC_PNG, R.drawable.ic_sdl, false); + SdlArtwork icon3 = new SdlArtwork("livio3", FileType.GRAPHIC_PNG, R.drawable.sdl_tray_icon, false); + SdlArtwork icon4 = new SdlArtwork("livio4", FileType.GRAPHIC_PNG, R.drawable.spp_error, false); + + MenuCell A = new MenuCell("A", icon1, null, menuSelectionListenerA); + + MenuCell B = new MenuCell("A", icon2, null, menuSelectionListenerB); + + MenuCell C = new MenuCell("A", icon3, null, menuSelectionListenerC); + + MenuCell subA = new MenuCell("A", icon1, null, menuSelectionListenerA); + MenuCell subB = new MenuCell("A", icon2, null, menuSelectionListenerB); + MenuCell subC = new MenuCell("A", icon3, null, menuSelectionListenerC); + MenuCell subD = new MenuCell("A", icon4, null, menuSelectionListenerD); + + MenuCell D = new MenuCell("A", MenuLayout.LIST, icon4, Arrays.asList(subA, subB, subC, subD)); + + 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 06b08c09c..f36ad2190 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 @@ -66,7 +66,9 @@ import com.smartdevicelink.proxy.rpc.listeners.OnRPCNotificationListener; import com.smartdevicelink.util.DebugTool; import java.lang.ref.WeakReference; +import java.util.HashMap; import java.util.HashSet; +import java.util.LinkedHashSet; import java.util.List; import java.util.Locale; @@ -196,9 +198,12 @@ abstract class BaseChoiceSetManager extends BaseSubManager { return; } - final HashSet<ChoiceCell> choicesToUpload = new HashSet<>(choices); - choicesToUpload.removeAll(preloadedChoices); - choicesToUpload.removeAll(pendingPreloadChoices); + LinkedHashSet<ChoiceCell> mutableChoicesToUpload = getChoicesToBeUploadedWithArray(choices); + + mutableChoicesToUpload.removeAll(preloadedChoices); + mutableChoicesToUpload.removeAll(pendingPreloadChoices); + + final LinkedHashSet<ChoiceCell> choicesToUpload = (LinkedHashSet<ChoiceCell>) mutableChoicesToUpload.clone(); if (choicesToUpload.size() == 0) { if (listener != null) { @@ -529,7 +534,40 @@ abstract class BaseChoiceSetManager extends BaseSubManager { return choicesSet; } - void updateIdsOnChoices(HashSet<ChoiceCell> choices) { + /** + * Checks if 2 or more cells have the same text/title. In case this condition is true, this function will handle the presented issue by adding "(count)". + * 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<ChoiceCell> choices) { + HashMap<String, Integer> dictCounter = new HashMap<>(); + + for (ChoiceCell cell : choices) { + String cellName = cell.getText(); + Integer counter = dictCounter.get(cellName); + + if (counter != null) { + dictCounter.put(cellName, ++counter); + cell.setUniqueText(cell.getText() + " (" + counter + ")"); + } else { + dictCounter.put(cellName, 1); + } + } + } + + private LinkedHashSet<ChoiceCell> getChoicesToBeUploadedWithArray(List<ChoiceCell> choices) { + LinkedHashSet<ChoiceCell> 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. + if (choices != null && internalInterface.getSdlMsgVersion() != null + && (internalInterface.getSdlMsgVersion().getMajorVersion() < 7 + || (internalInterface.getSdlMsgVersion().getMajorVersion() == 7 && internalInterface.getSdlMsgVersion().getMinorVersion() == 0))) { + addUniqueNamesToCells(choiceSet); + } + choiceSet.removeAll(preloadedChoices); + return choiceSet; + } + + void updateIdsOnChoices(LinkedHashSet<ChoiceCell> choices) { for (ChoiceCell cell : choices) { cell.setChoiceId(this.nextChoiceId); this.nextChoiceId++; @@ -650,14 +688,14 @@ abstract class BaseChoiceSetManager extends BaseSubManager { } } - HashSet<String> choiceTextSet = new HashSet<>(); + HashSet<ChoiceCell> uniqueChoiceCells = new HashSet<>(); HashSet<String> uniqueVoiceCommands = new HashSet<>(); int allVoiceCommandsCount = 0; int choiceCellWithVoiceCommandCount = 0; for (ChoiceCell cell : choices) { - choiceTextSet.add(cell.getText()); + uniqueChoiceCells.add(cell); if (cell.getVoiceCommands() != null) { uniqueVoiceCommands.addAll(cell.getVoiceCommands()); @@ -666,9 +704,8 @@ abstract class BaseChoiceSetManager extends BaseSubManager { } } - // Cell text MUST be unique - if (choiceTextSet.size() < choices.size()) { - DebugTool.logError(TAG, "Attempted to create a choice set with duplicate cell text. Cell text must be unique. The choice set will not be set."); + if (uniqueChoiceCells.size() != choices.size()) { + DebugTool.logError(TAG, "Attempted to create a choice set with a duplicate cell. Cell must have a unique value other than its primary text. The choice set will not be set."); return false; } diff --git a/base/src/main/java/com/smartdevicelink/managers/screen/choiceset/ChoiceCell.java b/base/src/main/java/com/smartdevicelink/managers/screen/choiceset/ChoiceCell.java index 5e690fe0a..7d59bf555 100644 --- a/base/src/main/java/com/smartdevicelink/managers/screen/choiceset/ChoiceCell.java +++ b/base/src/main/java/com/smartdevicelink/managers/screen/choiceset/ChoiceCell.java @@ -35,11 +35,13 @@ package com.smartdevicelink.managers.screen.choiceset; import androidx.annotation.NonNull; import com.smartdevicelink.managers.file.filetypes.SdlArtwork; +import com.smartdevicelink.util.DebugTool; +import java.util.ArrayList; import java.util.List; -public class ChoiceCell { - private String text, secondaryText, tertiaryText; +public class ChoiceCell implements Cloneable{ + private String text, secondaryText, tertiaryText, uniqueText; private List<String> voiceCommands; private SdlArtwork artwork, secondaryArtwork; private Integer choiceId; @@ -56,6 +58,7 @@ public class ChoiceCell { */ public ChoiceCell(@NonNull String text) { setText(text); + setUniqueText(text); setChoiceId(MAX_ID); } @@ -68,6 +71,7 @@ public class ChoiceCell { */ public ChoiceCell(@NonNull String text, List<String> voiceCommands, SdlArtwork artwork) { setText(text); + setUniqueText(text); setVoiceCommands(voiceCommands); setArtwork(artwork); setChoiceId(MAX_ID); @@ -87,6 +91,7 @@ public class ChoiceCell { setText(text); setSecondaryText(secondaryText); setTertiaryText(tertiaryText); + setUniqueText(text); setVoiceCommands(voiceCommands); setArtwork(artwork); setSecondaryArtwork(secondaryArtwork); @@ -226,6 +231,29 @@ public class ChoiceCell { return choiceId; } + /** + * NOTE: USED INTERNALLY + * Primary text of the cell to be displayed on the module. Used to distinguish cells with the + * same `text` but other fields are different. This is autogenerated by the screen manager. + * Attempting to use cells that are exactly the same (all text and artwork fields are the same) + * will not cause this to be used. This will not be used when connected to modules supporting RPC 7.1+. + * + * @param uniqueText - the uniqueText to be used in place of primaryText when core does not support identical names for ChoiceSets + */ + void setUniqueText(String uniqueText) { + this.uniqueText = uniqueText; + } + + /** + * NOTE: USED INTERNALLY + * Get the uniqueText that was used in place of primaryText + * + * @return the uniqueText for this Choice Cell + */ + String getUniqueText() { + return uniqueText; + } + @Override public int hashCode() { int result = 1; @@ -264,9 +292,37 @@ public class ChoiceCell { @NonNull public String toString() { return "ChoiceCell: ID: " + this.choiceId + " Text: " + text + " - Secondary Text: " + secondaryText + " - Tertiary Text: " + tertiaryText + " " + - "| Artwork Names: " + ((getArtwork() == null || getArtwork().getName() == null) ? "Primary Art null" : getArtwork().getName()) + (text.equals(uniqueText) ? "" : "| Unique Text: " + uniqueText) + " | Artwork Names: " + ((getArtwork() == null || getArtwork().getName() == null) ? "Primary Art null" : getArtwork().getName()) + " Secondary Art - " + ((getSecondaryArtwork() == null || getSecondaryArtwork().getName() == null) ? "Secondary Art null" : getSecondaryArtwork().getName()) + " | Voice Commands Size: " + ((getVoiceCommands() == null) ? 0 : getVoiceCommands().size()); } + /** + * Creates a deep copy of the object + * + * @return deep copy of the object, null if an exception occurred + */ + @Override + public ChoiceCell clone() { + try { + ChoiceCell clone = (ChoiceCell) super.clone(); + if (this.artwork != null) { + clone.artwork = this.artwork.clone(); + } + if (this.secondaryArtwork != null) { + clone.secondaryArtwork = this.secondaryArtwork.clone(); + } + if (this.voiceCommands != null) { + clone.voiceCommands = new ArrayList<>(voiceCommands); + } + + return clone; + } catch (CloneNotSupportedException e) { + if (DebugTool.isDebugEnabled()) { + throw new RuntimeException("Clone not supported by super class"); + } + } + + return null; + } } diff --git a/base/src/main/java/com/smartdevicelink/managers/screen/choiceset/ChoiceSet.java b/base/src/main/java/com/smartdevicelink/managers/screen/choiceset/ChoiceSet.java index d3d938c7b..b8d7412ee 100644 --- a/base/src/main/java/com/smartdevicelink/managers/screen/choiceset/ChoiceSet.java +++ b/base/src/main/java/com/smartdevicelink/managers/screen/choiceset/ChoiceSet.java @@ -66,6 +66,10 @@ public class ChoiceSet { * Initialize with a title, listener, and choices. It will use the default timeout and layout, * all other properties (such as prompts) will be `null`. * + * NOTE: If you display multiple cells with the same `text` with the only uniquing property between cells being different `vrCommands` or a feature that is not displayed on the head unit (e.g. if the head unit doesn't display `secondaryArtwork` and that's the only uniquing property between two cells) then the cells may appear to be the same to the user in `Manual` mode. This only applies to RPC connections >= 7.1.0. + * + * NOTE: On < 7.1.0 connections, the `text` cell will be automatically modified among cells that have the same `text` when they are preloaded, so they will always appear differently on-screen when they are displayed. `choiceCell.uniqueText` will be created by appending ` (2)`, ` (3)`, etc. + * * @param title - The choice set's title * @param listener - The choice set listener called after the user has interacted with your choice set * @param choices - The choices to be displayed to the user for interaction @@ -87,6 +91,10 @@ public class ChoiceSet { /** * Constructor with all possible properties. * + * NOTE: If you display multiple cells with the same `text` with the only uniquing property between cells being different `vrCommands` or a feature that is not displayed on the head unit (e.g. if the head unit doesn't display `secondaryArtwork` and that's the only uniquing property between two cells) then the cells may appear to be the same to the user in `Manual` mode. This only applies to RPC connections >= 7.1.0. + * + * NOTE: On < 7.1.0 connections, the `text` cell will be automatically modified among cells that have the same `text` when they are preloaded, so they will always appear differently on-screen when they are displayed. `choiceCell.uniqueText` will be created by appending ` (2)`, ` (3)`, etc. + * * @param title - The choice set's title * @param listener - The choice set listener called after the user has interacted with your choice set * @param layout - The layout of choice options (Manual/touch only) @@ -128,6 +136,10 @@ public class ChoiceSet { /** * Constructor with all possible properties. * + * NOTE: If you display multiple cells with the same `text` with the only uniquing property between cells being different `vrCommands` or a feature that is not displayed on the head unit (e.g. if the head unit doesn't display `secondaryArtwork` and that's the only uniquing property between two cells) then the cells may appear to be the same to the user in `Manual` mode. This only applies to RPC connections >= 7.1.0. + * + * NOTE: On < 7.1.0 connections, the `text` cell will be automatically modified among cells that have the same `text` when they are preloaded, so they will always appear differently on-screen when they are displayed. `choiceCell.uniqueText` will be created by appending ` (2)`, ` (3)`, etc. + * * @param title - The choice set's title * @param listener - The choice set listener called after the user has interacted with your choice set * @param layout - The layout of choice options (Manual/touch only) diff --git a/base/src/main/java/com/smartdevicelink/managers/screen/choiceset/PreloadChoicesOperation.java b/base/src/main/java/com/smartdevicelink/managers/screen/choiceset/PreloadChoicesOperation.java index c857f0878..8f47a2880 100644 --- a/base/src/main/java/com/smartdevicelink/managers/screen/choiceset/PreloadChoicesOperation.java +++ b/base/src/main/java/com/smartdevicelink/managers/screen/choiceset/PreloadChoicesOperation.java @@ -59,6 +59,7 @@ import java.lang.ref.WeakReference; import java.util.ArrayList; import java.util.Collections; import java.util.HashSet; +import java.util.LinkedHashSet; import java.util.List; import java.util.Map; @@ -75,7 +76,7 @@ class PreloadChoicesOperation extends Task { private boolean choiceError = false; PreloadChoicesOperation(ISdl internalInterface, FileManager fileManager, String displayName, WindowCapability defaultMainWindowCapability, - Boolean isVROptional, HashSet<ChoiceCell> cellsToPreload, CompletionListener listener) { + Boolean isVROptional, LinkedHashSet<ChoiceCell> cellsToPreload, CompletionListener listener) { super("PreloadChoicesOperation"); this.internalInterface = new WeakReference<>(internalInterface); this.fileManager = new WeakReference<>(fileManager); @@ -195,7 +196,7 @@ class PreloadChoicesOperation extends Task { vrCommands = cell.getVoiceCommands(); } - String menuName = shouldSendChoiceText() ? cell.getText() : null; + String menuName = shouldSendChoiceText() ? cell.getUniqueText() : null; if (menuName == null) { DebugTool.logError(TAG, "Could not convert Choice Cell to CreateInteractionChoiceSet. It will not be shown. Cell: " + cell.toString()); 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 28100a4c3..a65e44895 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 @@ -74,6 +74,7 @@ import org.json.JSONException; import java.lang.ref.WeakReference; import java.util.ArrayList; +import java.util.HashMap; import java.util.HashSet; import java.util.List; import java.util.Map; @@ -174,6 +175,13 @@ abstract class BaseMenuManager extends BaseSubManager { // Create a deep copy of the list so future changes by developers don't affect the algorithm logic List<MenuCell> 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); + } + if (currentHMILevel == null || currentHMILevel.equals(HMILevel.HMI_NONE) || currentSystemContext.equals(SystemContext.SYSCTXT_MENU)) { // We are in NONE or the menu is in use, bail out of here waitingOnHMIUpdate = true; @@ -196,27 +204,8 @@ abstract class BaseMenuManager extends BaseSubManager { menuCells.addAll(clonedCells); } - // HashSet order doesnt matter / does not allow duplicates - HashSet<String> titleCheckSet = new HashSet<>(); - HashSet<String> allMenuVoiceCommands = new HashSet<>(); - int voiceCommandCount = 0; - - for (MenuCell cell : menuCells) { - titleCheckSet.add(cell.getTitle()); - if (cell.getVoiceCommands() != null) { - allMenuVoiceCommands.addAll(cell.getVoiceCommands()); - voiceCommandCount += cell.getVoiceCommands().size(); - } - } - // Check for duplicate titles - if (titleCheckSet.size() != menuCells.size()) { - DebugTool.logError(TAG, "Not all cell titles are unique. The menu will not be set"); - return; - } - - // Check for duplicate voice commands - if (allMenuVoiceCommands.size() != voiceCommandCount) { - DebugTool.logError(TAG, "Attempted to create a menu with duplicate voice commands. Voice commands must be unique. The menu will not be set"); + // 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<String>())) { return; } @@ -972,7 +961,7 @@ abstract class BaseMenuManager extends BaseSubManager { private AddCommand commandForMenuCell(MenuCell cell, boolean shouldHaveArtwork, int position) { - MenuParams params = new MenuParams(cell.getTitle()); + MenuParams params = new MenuParams(cell.getUniqueTitle()); params.setSecondaryText(cell.getSecondaryText()); params.setTertiaryText(cell.getTertiaryText()); params.setParentID(cell.getParentCellId() != MAX_ID ? cell.getParentCellId() : null); @@ -992,7 +981,7 @@ abstract class BaseMenuManager extends BaseSubManager { } private AddSubMenu subMenuCommandForMenuCell(MenuCell cell, boolean shouldHaveArtwork, int position) { - AddSubMenu subMenu = new AddSubMenu(cell.getCellId(), cell.getTitle()); + AddSubMenu subMenu = new AddSubMenu(cell.getCellId(), cell.getUniqueTitle()); subMenu.setSecondaryText(cell.getSecondaryText()); subMenu.setTertiaryText(cell.getTertiaryText()); subMenu.setPosition(position); @@ -1369,4 +1358,71 @@ abstract class BaseMenuManager extends BaseSubManager { } return clone; } + + private void addUniqueNamesToCells(List<MenuCell> cells) { + HashMap<String, Integer> dictCounter = new HashMap<>(); + + for (MenuCell cell : cells) { + String cellName = cell.getTitle(); + Integer counter = dictCounter.get(cellName); + + if (counter != null) { + dictCounter.put(cellName, ++counter); + cell.setUniqueTitle(cellName + " (" + counter + ")"); + } else { + dictCounter.put(cellName, 1); + } + + if (cell.getSubCells() != null && cell.getSubCells().size() > 0) { + addUniqueNamesToCells(cell.getSubCells()); + } + } + } + + /** + Check for cell lists with completely duplicate information, or any duplicate voiceCommands + + @param cells The cells you will be adding + @return Boolean that indicates whether menuCells are unique or not + */ + private boolean menuCellsAreUnique(List<MenuCell> cells, ArrayList<String> allVoiceCommands) { + //Check all voice commands for identical items and check each list of cells for identical cells + HashSet<MenuCell> 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 (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."); + return false; + } + } + + // Voice commands have to be identical across all lists + if (cell.getVoiceCommands() == null) { + continue; + } + allVoiceCommands.addAll(cell.getVoiceCommands()); + } + + + // Check for duplicate cells + if (identicalCellsCheckSet.size() != cells.size()) { + DebugTool.logError(TAG, "Not all cells are unique. The menu will not be set."); + return false; + } + + // All the VR commands must be unique + HashSet<String> voiceCommandsSet = new HashSet<>(allVoiceCommands); + if (allVoiceCommands.size() != voiceCommandsSet.size()) { + DebugTool.logError(TAG, "Attempted to create a menu with duplicate voice commands. Voice commands must be unique. The menu will not be set"); + return false; + } + + return true; + } } diff --git a/base/src/main/java/com/smartdevicelink/managers/screen/menu/MenuCell.java b/base/src/main/java/com/smartdevicelink/managers/screen/menu/MenuCell.java index 7694399a8..652891181 100644 --- a/base/src/main/java/com/smartdevicelink/managers/screen/menu/MenuCell.java +++ b/base/src/main/java/com/smartdevicelink/managers/screen/menu/MenuCell.java @@ -100,6 +100,13 @@ public class MenuCell implements Cloneable { */ private SdlArtwork secondaryArtwork; + /** + * Primary text of the cell to be displayed on the module. Used to distinguish cells with the + * same `title` but other fields are different. This is autogenerated by the screen manager. + * This will not be used when connected to modules supporting RPC 7.1+. + */ + private String uniqueTitle; + // CONSTRUCTORS // SINGLE MENU ITEM CONSTRUCTORS @@ -115,6 +122,7 @@ public class MenuCell implements Cloneable { @Deprecated public MenuCell(@NonNull String title, @Nullable SdlArtwork icon, @Nullable List<String> voiceCommands, @Nullable MenuSelectionListener listener) { setTitle(title); // title is the only required param + setUniqueTitle(title); setIcon(icon); setVoiceCommands(voiceCommands); setMenuSelectionListener(listener); @@ -135,6 +143,7 @@ public class MenuCell implements Cloneable { */ public MenuCell(@NonNull String title, @Nullable String secondaryText, @Nullable String tertiaryText, @Nullable SdlArtwork icon, @Nullable SdlArtwork secondaryArtwork, @Nullable List<String> voiceCommands, @Nullable MenuSelectionListener listener) { setTitle(title); // title is the only required param + setUniqueTitle(title); setSecondaryText(secondaryText); setTertiaryText(tertiaryText); setIcon(icon); @@ -160,6 +169,7 @@ public class MenuCell implements Cloneable { @Deprecated public MenuCell(@NonNull String title, @Nullable MenuLayout subMenuLayout, @Nullable SdlArtwork icon, @Nullable List<MenuCell> subCells) { setTitle(title); // title is the only required param + setUniqueTitle(title); setSubMenuLayout(subMenuLayout); setIcon(icon); setSubCells(subCells); @@ -182,6 +192,7 @@ public class MenuCell implements Cloneable { */ public MenuCell(@NonNull String title, @Nullable String secondaryText, @Nullable String tertiaryText, @Nullable MenuLayout subMenuLayout, @Nullable SdlArtwork icon, @Nullable SdlArtwork secondaryArtwork, @Nullable List<MenuCell> subCells) { setTitle(title); // title is the only required param + setUniqueTitle(title); setSecondaryText(secondaryText); setTertiaryText(tertiaryText); setSubMenuLayout(subMenuLayout); @@ -399,6 +410,26 @@ public class MenuCell implements Cloneable { return secondaryArtwork; } + /** + * NOTE: USED INTERNALLY + * Set the uniqueTitle. + * + * @param uniqueTitle - the uniqueTitle to be used in place of primary title when core does not support identical names for MenuCells + */ + void setUniqueTitle(String uniqueTitle) { + this.uniqueTitle = uniqueTitle; + } + + /** + * NOTE: USED INTERNALLY + * Get the uniqueTitle that was used in place of the primary title + * + * @return the uniqueTitle for this MenuCell + */ + String getUniqueTitle() { + return uniqueTitle; + } + // HELPER /** |