Skip to content

Menu Manager won't send submenu cell images on RPC versions >= 5.0 && < 7.0 #1759

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
bilal-alsharifi merged 8 commits into from
Nov 22, 2021
Merged

Menu Manager won't send submenu cell images on RPC versions >= 5.0 && < 7.0 #1759

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
8 commits
Select commit Hold shift + click to select a range
445c576
Fix issue #1756
bilal-alsharifi Oct 25, 2021
cdadf4e
Update unit tests to include tests for WindowCapabilitySupportsPrimar…
bilal-alsharifi Oct 25, 2021
4593a88
Updte findAllArtworksToBeUploadedFromCells() to check if images are s…
bilal-alsharifi Oct 25, 2021
4cb8e19
Merge branch 'develop' into bugfix/issue_1756
bilal-alsharifi Oct 29, 2021
4626dca
Extract isRPCVersionBetween5And7() into a seaparate method
bilal-alsharifi Nov 8, 2021
28a8bfe
Fix never upload AddSubmenu RPC (menu cell w/ subcells) primary image…
bilal-alsharifi Nov 8, 2021
9237659
Simplify cellsWithRemovedPropertiesFromCells()
bilal-alsharifi Nov 8, 2021
a627e9e
Update MenuReplaceUtilitiesTests
bilal-alsharifi Nov 8, 2021
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
5 changes: 4 additions & 1 deletion ...droid/src/androidTest/java/com/smartdevicelink/managers/screen/menu/MenuManagerTests.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -261,12 +261,15 @@ public void testSettingNonUniqueCells() {

@Test
public void testUpdatingOldWay() {
ISdl internalInterface = mock(ISdl.class);
when(internalInterface.getSdlMsgVersion()).thenReturn(new SdlMsgVersion(new Version(8, 0, 0)));

// Force Menu Manager to use the old way of deleting / sending all
menuManager.setDynamicUpdatesMode(DynamicMenuUpdatesMode.FORCE_OFF);
assertEquals(menuManager.dynamicMenuUpdatesMode, DynamicMenuUpdatesMode.FORCE_OFF);
// when we only send one command to update, we should only be returned one add command
List<MenuCell> newArray = Arrays.asList(mainCell1, mainCell4);
assertEquals(MenuReplaceUtilities.allCommandsForCells(newArray, menuManager.fileManager.get(), menuManager.windowCapability, MenuLayout.LIST).size(), 4); // 1 root cells, 1 sub menu root cell, 2 sub menu cells
assertEquals(MenuReplaceUtilities.allCommandsForCells(internalInterface, newArray, menuManager.fileManager.get(), menuManager.windowCapability, MenuLayout.LIST).size(), 4); // 1 root cells, 1 sub menu root cell, 2 sub menu cells
menuManager.currentHMILevel = HMILevel.HMI_FULL;
menuManager.setMenuCells(newArray);

Expand Down
94 changes: 89 additions & 5 deletions .../androidTest/java/com/smartdevicelink/managers/screen/menu/MenuReplaceUtilitiesTests.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -34,13 +34,16 @@

import androidx.test.ext.junit.runners.AndroidJUnit4;

import com.smartdevicelink.managers.ISdl;
import com.smartdevicelink.managers.file.FileManager;
import com.smartdevicelink.managers.file.filetypes.SdlArtwork;
import com.smartdevicelink.proxy.rpc.ImageField;
import com.smartdevicelink.proxy.rpc.SdlMsgVersion;
import com.smartdevicelink.proxy.rpc.WindowCapability;
import com.smartdevicelink.proxy.rpc.enums.ImageFieldName;
import com.smartdevicelink.proxy.rpc.enums.MenuLayout;
import com.smartdevicelink.test.TestValues;
import com.smartdevicelink.util.Version;

import org.junit.Before;
import org.junit.Test;
Expand Down Expand Up @@ -209,8 +212,89 @@ public void testAddMenuRequestWithCommandId() {
assertEquals(1, actualMenuCellList.get(4).getSubCells().get(1).getSubCells().size());
}

@Test
public void testWindowCapabilitySupportsPrimaryImage() {
WindowCapability windowCapability;
ISdl internalInterface = mock(ISdl.class);
MenuCell menuCell = mock(MenuCell.class);

// Test case 0
windowCapability = createWindowCapability(false, true);
when(menuCell.isSubMenuCell()).thenReturn(true);
when(internalInterface.getSdlMsgVersion()).thenReturn(new SdlMsgVersion(new Version(4, 9, 0)));
assertTrue(MenuReplaceUtilities.windowCapabilitySupportsPrimaryImage(internalInterface, windowCapability, menuCell));

// Test case 1
windowCapability = createWindowCapability(false, false);
when(menuCell.isSubMenuCell()).thenReturn(true);
when(internalInterface.getSdlMsgVersion()).thenReturn(new SdlMsgVersion(new Version(4, 9, 0)));
assertFalse(MenuReplaceUtilities.windowCapabilitySupportsPrimaryImage(internalInterface, windowCapability, menuCell));

// Test case 2
windowCapability = createWindowCapability(false, false);
when(menuCell.isSubMenuCell()).thenReturn(true);
when(internalInterface.getSdlMsgVersion()).thenReturn(new SdlMsgVersion(new Version(5, 0, 0)));
assertFalse(MenuReplaceUtilities.windowCapabilitySupportsPrimaryImage(internalInterface, windowCapability, menuCell));

// Test case 3
windowCapability = createWindowCapability(true, false);
when(menuCell.isSubMenuCell()).thenReturn(true);
when(internalInterface.getSdlMsgVersion()).thenReturn(new SdlMsgVersion(new Version(5, 0, 0)));
assertTrue(MenuReplaceUtilities.windowCapabilitySupportsPrimaryImage(internalInterface, windowCapability, menuCell));

// Test case 4
windowCapability = createWindowCapability(false, false);
when(menuCell.isSubMenuCell()).thenReturn(true);
when(internalInterface.getSdlMsgVersion()).thenReturn(new SdlMsgVersion(new Version(6, 0, 0)));
assertFalse(MenuReplaceUtilities.windowCapabilitySupportsPrimaryImage(internalInterface, windowCapability, menuCell));

// Test case 5
windowCapability = createWindowCapability(true, false);
when(menuCell.isSubMenuCell()).thenReturn(true);
when(internalInterface.getSdlMsgVersion()).thenReturn(new SdlMsgVersion(new Version(6, 0, 0)));
assertTrue(MenuReplaceUtilities.windowCapabilitySupportsPrimaryImage(internalInterface, windowCapability, menuCell));

// Test case 6
windowCapability = createWindowCapability(false, false);
when(menuCell.isSubMenuCell()).thenReturn(true);
when(internalInterface.getSdlMsgVersion()).thenReturn(new SdlMsgVersion(new Version(7, 0, 0)));
assertFalse(MenuReplaceUtilities.windowCapabilitySupportsPrimaryImage(internalInterface, windowCapability, menuCell));

// Test case 7
windowCapability = createWindowCapability(false, false);
when(menuCell.isSubMenuCell()).thenReturn(true);
when(internalInterface.getSdlMsgVersion()).thenReturn(new SdlMsgVersion(new Version(7, 1, 0)));
assertFalse(MenuReplaceUtilities.windowCapabilitySupportsPrimaryImage(internalInterface, windowCapability, menuCell));

// Test case 8
windowCapability = createWindowCapability(false, false);
when(menuCell.isSubMenuCell()).thenReturn(true);
when(internalInterface.getSdlMsgVersion()).thenReturn(new SdlMsgVersion(new Version(8, 0, 0)));
assertFalse(MenuReplaceUtilities.windowCapabilitySupportsPrimaryImage(internalInterface, windowCapability, menuCell));

// Test case 9
windowCapability = createWindowCapability(false, true);
when(menuCell.isSubMenuCell()).thenReturn(true);
when(internalInterface.getSdlMsgVersion()).thenReturn(new SdlMsgVersion(new Version(8, 0, 0)));
assertTrue(MenuReplaceUtilities.windowCapabilitySupportsPrimaryImage(internalInterface, windowCapability, menuCell));

// Test case 10
windowCapability = createWindowCapability(false, false);
when(menuCell.isSubMenuCell()).thenReturn(false);
when(internalInterface.getSdlMsgVersion()).thenReturn(new SdlMsgVersion(new Version(8, 0, 0)));
assertFalse(MenuReplaceUtilities.windowCapabilitySupportsPrimaryImage(internalInterface, windowCapability, menuCell));

// Test case 11
windowCapability = createWindowCapability(true, false);
when(menuCell.isSubMenuCell()).thenReturn(false);
when(internalInterface.getSdlMsgVersion()).thenReturn(new SdlMsgVersion(new Version(8, 0, 0)));
assertTrue(MenuReplaceUtilities.windowCapabilitySupportsPrimaryImage(internalInterface, windowCapability, menuCell));
}

@Test
public void testShouldCellIncludeImage() {
ISdl internalInterface = mock(ISdl.class);
when(internalInterface.getSdlMsgVersion()).thenReturn(new SdlMsgVersion(new Version(8, 0, 0)));
MenuCell menuCell;
WindowCapability windowCapability;
FileManager fileManager;
Expand All @@ -220,31 +304,31 @@ public void testShouldCellIncludeImage() {
menuCell = new MenuCell(TestValues.GENERAL_STRING, TestValues.GENERAL_ARTWORK, voiceCommands, null);
windowCapability = createWindowCapability(true, true);
fileManager = createMockFileManager(true);
assertTrue(MenuReplaceUtilities.shouldCellIncludePrimaryImageFromCell(menuCell, fileManager, windowCapability));
assertTrue(MenuReplaceUtilities.shouldCellIncludePrimaryImageFromCell(internalInterface, menuCell, fileManager, windowCapability));

// Case 2 - Image are not supported
menuCell = new MenuCell(TestValues.GENERAL_STRING, TestValues.GENERAL_ARTWORK, voiceCommands, null);
windowCapability = createWindowCapability(false, false);
fileManager = createMockFileManager(true);
assertFalse(MenuReplaceUtilities.shouldCellIncludePrimaryImageFromCell(menuCell, fileManager, windowCapability));
assertFalse(MenuReplaceUtilities.shouldCellIncludePrimaryImageFromCell(internalInterface, menuCell, fileManager, windowCapability));

// Case 3 - Artwork is null
menuCell = new MenuCell(TestValues.GENERAL_STRING, null, voiceCommands, null);
windowCapability = createWindowCapability(true, true);
fileManager = createMockFileManager(true);
assertFalse(MenuReplaceUtilities.shouldCellIncludePrimaryImageFromCell(menuCell, fileManager, windowCapability));
assertFalse(MenuReplaceUtilities.shouldCellIncludePrimaryImageFromCell(internalInterface, menuCell, fileManager, windowCapability));

// Case 4 - Artwork has not been uploaded
menuCell = new MenuCell(TestValues.GENERAL_STRING, TestValues.GENERAL_ARTWORK, voiceCommands, null);
windowCapability = createWindowCapability(true, true);
fileManager = createMockFileManager(false);
assertFalse(MenuReplaceUtilities.shouldCellIncludePrimaryImageFromCell(menuCell, fileManager, windowCapability));
assertFalse(MenuReplaceUtilities.shouldCellIncludePrimaryImageFromCell(internalInterface, menuCell, fileManager, windowCapability));

// Case 5 - Artwork is static icon
menuCell = new MenuCell(TestValues.GENERAL_STRING, TestValues.GENERAL_ARTWORK_STATIC, voiceCommands, null);
windowCapability = createWindowCapability(true, true);
fileManager = createMockFileManager(false);
assertTrue(MenuReplaceUtilities.shouldCellIncludePrimaryImageFromCell(menuCell, fileManager, windowCapability));
assertTrue(MenuReplaceUtilities.shouldCellIncludePrimaryImageFromCell(internalInterface, menuCell, fileManager, windowCapability));
}

private WindowCapability createWindowCapability (boolean supportsCmdIcon, boolean supportsSubMenuIcon) {
Expand Down
22 changes: 10 additions & 12 deletions base/src/main/java/com/smartdevicelink/managers/screen/menu/MenuReplaceOperation.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -48,6 +48,8 @@
import static com.smartdevicelink.managers.screen.menu.MenuReplaceUtilities.subMenuCommandsForCells;
import static com.smartdevicelink.managers.screen.menu.MenuReplaceUtilities.transferCellIDsFromCells;
import static com.smartdevicelink.managers.screen.menu.MenuReplaceUtilities.transferCellListenersFromCells;
import static com.smartdevicelink.managers.screen.menu.MenuReplaceUtilities.windowCapabilitySupportsPrimaryImage;
import static com.smartdevicelink.managers.screen.menu.MenuReplaceUtilities.windowCapabilitySupportsSecondaryImage;

import com.livio.taskmaster.Task;
import com.smartdevicelink.managers.CompletionListener;
Expand Down Expand Up @@ -198,7 +200,7 @@ public void onComplete(boolean success) {
}

private void uploadMenuArtworks(final CompletionListener listener) {
List<SdlArtwork> artworksToBeUploaded = new ArrayList<>(findAllArtworksToBeUploadedFromCells(updatedMenu, fileManager.get(), windowCapability));
List<SdlArtwork> artworksToBeUploaded = new ArrayList<>(findAllArtworksToBeUploadedFromCells(internalInterface.get(), updatedMenu, fileManager.get(), windowCapability));
if (artworksToBeUploaded.isEmpty()) {
listener.onComplete(true);
return;
Expand Down Expand Up @@ -373,10 +375,10 @@ private void sendAddMenuCells(final List<MenuCell> addMenuCells, final List<Menu
MenuLayout defaultSubmenuLayout = menuConfiguration != null ? menuConfiguration.getSubMenuLayout() : null;

// RPCs for cells on the main menu level. They could be AddCommands or AddSubMenus depending on whether the cell has child cells or not.
final List<RPCRequest> mainMenuCommands = mainMenuCommandsForCells(addMenuCells, fileManager.get(), fullMenu, windowCapability, defaultSubmenuLayout);
final List<RPCRequest> mainMenuCommands = mainMenuCommandsForCells(internalInterface.get(), addMenuCells, fileManager.get(), fullMenu, windowCapability, defaultSubmenuLayout);

// RPCs for cells on the second menu level (one level deep). They could be AddCommands or AddSubMenus.
final List<RPCRequest> subMenuCommands = subMenuCommandsForCells(addMenuCells, fileManager.get(), windowCapability, defaultSubmenuLayout);
final List<RPCRequest> subMenuCommands = subMenuCommandsForCells(internalInterface.get(), addMenuCells, fileManager.get(), windowCapability, defaultSubmenuLayout);

sendRPCs(mainMenuCommands, internalInterface.get(), new SendingRPCsCompletionListener() {
@Override
Expand Down Expand Up @@ -465,12 +467,14 @@ List<MenuCell> cellsWithRemovedPropertiesFromCells(List<MenuCell> cells, WindowC
// 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.
// Just assume that if cmdIcon is not available, the submenu icon is not either.
if (!hasImageFieldOfName(windowCapability, ImageFieldName.cmdIcon)) {
if (!windowCapabilitySupportsPrimaryImage(internalInterface.get(), windowCapability, cell)) {
cell.setIcon(null);
}

if (!windowCapabilitySupportsSecondaryImage(windowCapability, cell)) {
cell.setSecondaryArtwork(null);
}

// Check for subMenu fields supported
if (cell.isSubMenuCell()) {
if (!hasTextFieldOfName(windowCapability, TextFieldName.menuSubMenuSecondaryText)) {
Expand All @@ -479,9 +483,6 @@ List<MenuCell> cellsWithRemovedPropertiesFromCells(List<MenuCell> cells, WindowC
if (!hasTextFieldOfName(windowCapability, TextFieldName.menuSubMenuTertiaryText)) {
cell.setTertiaryText(null);
}
if (!hasImageFieldOfName(windowCapability, ImageFieldName.menuSubMenuSecondaryImage)) {
cell.setSecondaryArtwork(null);
}
cell.setSubCells(cellsWithRemovedPropertiesFromCells(cell.getSubCells(), windowCapability));
} else {
if (!hasTextFieldOfName(windowCapability, TextFieldName.menuCommandSecondaryText)) {
Expand All @@ -490,9 +491,6 @@ List<MenuCell> cellsWithRemovedPropertiesFromCells(List<MenuCell> cells, WindowC
if (!hasTextFieldOfName(windowCapability, TextFieldName.menuCommandTertiaryText)) {
cell.setTertiaryText(null);
}
if (!hasImageFieldOfName(windowCapability, ImageFieldName.menuCommandSecondaryImage)) {
cell.setSecondaryArtwork(null);
}
}
}
return removePropertiesClone;
Expand Down
Loading