Skip to content
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.

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

Dynamically Update Menu Manager Cells #1282

Merged
merged 23 commits into from Jun 10, 2019

Conversation

justingluck93
Copy link
Contributor

Fixes #1144

This PR is [ready] for review.

Risk

This PR makes [minor] API changes.

Testing Plan

Unit tests have been created to test dynamic and non dynamic menu manger updates.

Summary

This PR makes the menu manager more efficient when creating a new menu. The menu manger will now compare the old and new menu to find cells to keep. This will reduce the number of Add and delete RPCs. A flag has been added to either use Dynamic menu manger or not. 'dynamicMenuUpdatesMode` this flag can be set using the screen manger.

CLA

@joeljfischer joeljfischer self-requested a review May 29, 2019 13:15
@joeljfischer joeljfischer added enhancement proposal Accepted SDL Evolution Proposal labels May 29, 2019
@joeljfischer joeljfischer added this to In progress in v6.3 via automation May 29, 2019
@joeljfischer joeljfischer added this to the 6.3.0 milestone May 29, 2019
@joeljfischer joeljfischer changed the title Feature/issue 1144 dynamic menu manager Dynamically Update Menu Manager Cells May 29, 2019
SmartDeviceLink/SDLMenuManager.m Outdated Show resolved Hide resolved
SmartDeviceLink/SDLMenuManager.h Outdated Show resolved Hide resolved
SmartDeviceLink/SDLScreenManager.h Outdated Show resolved Hide resolved
SmartDeviceLink/SDLScreenManager.h Outdated Show resolved Hide resolved
SmartDeviceLink/SDLMenuRunScore.h Outdated Show resolved Hide resolved
SmartDeviceLinkTests/DevAPISpecs/SDLMenuManagerSpec.m Outdated Show resolved Hide resolved
SmartDeviceLink/SDLMenuManager.m Outdated Show resolved Hide resolved
SmartDeviceLink/SDLMenuManager.m Outdated Show resolved Hide resolved
v6.3 automation moved this from In progress to Review in progress May 29, 2019
Justin Gluck added 7 commits May 30, 2019 12:40
Rename some files
added more unit tests
fixed syntax
Moved enums to a public file
changed function names to align more with the standards
removed setter
updated unit tests removed fit
Copy link
Contributor

@joeljfischer joeljfischer left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Please retest against Sync 3 and Generic HMI before resubmitting.

  1. Delete all but one submenu
  2. Delete all but one command
  3. Rearrange submenus and commands
  4. Add one submenu
  5. Add one command

SmartDeviceLinkTests/DevAPISpecs/SDLMenuCellSpec.m Outdated Show resolved Hide resolved
SmartDeviceLink/SDLMenuManager.m Outdated Show resolved Hide resolved
SmartDeviceLink/SDLScreenManager.h Outdated Show resolved Hide resolved
SmartDeviceLink/SDLDynamicMenuUpdateAlgorithm.m Outdated Show resolved Hide resolved
SmartDeviceLink/SDLMenuManager.m Outdated Show resolved Hide resolved
SmartDeviceLink/SDLMenuManager.m Outdated Show resolved Hide resolved
SmartDeviceLink/SDLMenuManager.m Outdated Show resolved Hide resolved
SmartDeviceLink/SDLMenuManager.m Outdated Show resolved Hide resolved
SmartDeviceLink/SDLMenuManager.m Outdated Show resolved Hide resolved
Copy link
Contributor

@joeljfischer joeljfischer left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Just a few more small items!

SmartDeviceLink/SDLDynamicMenuUpdateAlgorithm.m Outdated Show resolved Hide resolved
SmartDeviceLink/SDLMenuManager.m Outdated Show resolved Hide resolved
Copy link
Contributor

@joeljfischer joeljfischer left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Sorry! Also a few more items, and be sure to check the previous items for any that weren't resolved.

SmartDeviceLink/SDLMenuManager.m Outdated Show resolved Hide resolved
e.g. If the new menu array is [A, B, C, D] but only [C, D] are new we need to pass [A, B , C , D] so C and D can be added to index 2 and 3 respectively.
@return list of SDLRPCRequest addCommands
*/
- (NSArray<SDLRPCRequest *> *)sdl_mainMenuCommandsForCells:(NSArray<SDLMenuCell *> *)cells withArtwork:(BOOL)shouldHaveArtwork usingIndexOf:(NSArray<SDLMenuCell *> *)menu {
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Change usingIndexOf: to usingIndexesFrom:? Is that more accurate?

SmartDeviceLink/SDLScreenManager.h Outdated Show resolved Hide resolved
Copy link
Contributor

@joeljfischer joeljfischer left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

One more

SmartDeviceLink/SDLMenuManager.m Outdated Show resolved Hide resolved
v6.3 automation moved this from Review in progress to Reviewer approved Jun 10, 2019
@joeljfischer joeljfischer merged commit 63f9648 into develop Jun 10, 2019
v6.3 automation moved this from Reviewer approved to Done Jun 10, 2019
@joeljfischer joeljfischer deleted the feature/issue-1144-Dynamic-Menu-Manager branch June 10, 2019 18:50
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
proposal Accepted SDL Evolution Proposal
Projects
No open projects
v6.3
  
Done
Development

Successfully merging this pull request may close these issues.

None yet

2 participants