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

SDL 0180 - Fix to support broaden choice cell uniqueness #1886

Merged
Merged
Show file tree
Hide file tree
Changes from 22 commits
Commits
Show all changes
36 commits
Select commit Hold shift + click to select a range
52dc8b3
uniqueText property added in SDLChoiceCell
FrankElias77 Jan 8, 2021
76eaf29
SDLChoiceSet and SDLChoiceSetManager update
FrankElias77 Jan 8, 2021
f97ef9a
update PreloadChoicesOperation
FrankElias77 Jan 8, 2021
19dfe89
test update for equivalent cell text
FrankElias77 Jan 8, 2021
f87d61d
encapsulation and comment fixes
FrankElias77 Jan 11, 2021
df8f8e9
Function time enhancement
FrankElias77 Jan 12, 2021
163091e
ChoiceSet updates
FrankElias77 Jan 13, 2021
1cc4736
MenuCell updates
FrankElias77 Jan 13, 2021
5af989e
spec file updates
FrankElias77 Jan 13, 2021
94a24ea
Comments review
FrankElias77 Jan 13, 2021
f84eede
menuCell updates
FrankElias77 Jan 14, 2021
b4c28b9
Merge branch 'develop' into feature/issue-1024-sdl-0180-broaden-choic…
joeljfischer Feb 2, 2021
821dfd6
comments review
FrankElias77 Feb 3, 2021
f74c128
Comments review
FrankElias77 Feb 4, 2021
13b66fd
comments review
FrankElias77 Feb 9, 2021
2873483
comments review
FrankElias77 Feb 10, 2021
ebfdd35
Comments review
FrankElias77 Feb 10, 2021
5071953
Update RPC_Spec
joeljfischer Feb 15, 2021
648bfad
comments review
FrankElias77 Feb 18, 2021
1a05fc6
comments review
FrankElias77 Feb 18, 2021
7a17f0d
Merge branch 'develop' into feature/issue-1024-sdl-0180-broaden-choic…
FrankElias77 Feb 19, 2021
0d29e70
Fixing warnings
FrankElias77 Feb 19, 2021
1350438
comments review
FrankElias77 Feb 19, 2021
7112c0a
Fixes for 0180
joeljfischer Feb 22, 2021
1f62149
Fixes to menu manager checks for unique voice commands
joeljfischer Feb 22, 2021
7a762fb
Style and other slight fixes
joeljfischer Feb 22, 2021
a3a6995
Style and other slight fixes
joeljfischer Feb 22, 2021
9c38eae
Choice set fixes
joeljfischer Feb 22, 2021
65f1281
Fix choice set spec
joeljfischer Feb 22, 2021
9d598d0
Clarify a log
joeljfischer Feb 22, 2021
3076f62
Fix crashing tests
joeljfischer Feb 25, 2021
af32086
Fix menu manager not properly uploading menu when there are not dupli…
joeljfischer Feb 25, 2021
61100f1
Add additional comments about uniquing choice cells
joeljfischer Feb 25, 2021
78342fc
Additional documentation
joeljfischer Feb 25, 2021
3561365
Merge branch 'develop' into feature/issue-1024-sdl-0180-broaden-choic…
joeljfischer Mar 4, 2021
35e3168
Fix tests
joeljfischer Mar 4, 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
210 changes: 108 additions & 102 deletions SmartDeviceLink-iOS.xcodeproj/project.pbxproj

Large diffs are not rendered by default.

19 changes: 19 additions & 0 deletions SmartDeviceLink/private/NSArray+NSObject.h
Original file line number Diff line number Diff line change
@@ -0,0 +1,19 @@
//
// NSArray+NSObject.h
FrankElias77 marked this conversation as resolved.
Show resolved Hide resolved
// SmartDeviceLink
//
// Created by Frank Elias on 2/18/21.
// Copyright © 2021 smartdevicelink. All rights reserved.
//

#import <Foundation/Foundation.h>

NS_ASSUME_NONNULL_BEGIN

@interface NSArray (NSObject)
FrankElias77 marked this conversation as resolved.
Show resolved Hide resolved

-(NSUInteger)dynamicHash;
FrankElias77 marked this conversation as resolved.
Show resolved Hide resolved

@end

NS_ASSUME_NONNULL_END
27 changes: 27 additions & 0 deletions SmartDeviceLink/private/NSArray+NSObject.m
Original file line number Diff line number Diff line change
@@ -0,0 +1,27 @@
//
// NSArray+NSObject.m
// SmartDeviceLink
//
// Created by Frank Elias on 2/18/21.
// Copyright © 2021 smartdevicelink. All rights reserved.
//

#import "NSArray+NSObject.h"

@implementation NSArray (NSObject)

NSUInteger const NSUIntBitCellDynamicHash = (CHAR_BIT * sizeof(NSUInteger));
NSUInteger NSUIntRotateCellDynamicHash(NSUInteger val, NSUInteger howMuch) {
return ((((NSUInteger)val) << howMuch) | (((NSUInteger)val) >> (NSUIntBitCellDynamicHash - howMuch)));
}
FrankElias77 marked this conversation as resolved.
Show resolved Hide resolved

- (NSUInteger)dynamicHash {
NSUInteger returnValue = 0;
for (NSObject *object in self) {
returnValue += NSUIntRotateCellDynamicHash(object.hash, NSUIntBitCellDynamicHash/2);
}
return returnValue;
FrankElias77 marked this conversation as resolved.
Show resolved Hide resolved
}


@end
29 changes: 29 additions & 0 deletions SmartDeviceLink/private/SDLChoiceSetManager.m
Original file line number Diff line number Diff line change
Expand Up @@ -39,6 +39,7 @@
#import "SDLSystemCapability.h"
#import "SDLSystemCapabilityManager.h"
#import "SDLWindowCapability.h"
#import "SDLVersion.h"
#import "SDLWindowCapability+ScreenManagerExtensions.h"

NS_ASSUME_NONNULL_BEGIN
Expand All @@ -53,6 +54,7 @@
@interface SDLChoiceCell()

@property (assign, nonatomic) UInt16 choiceId;
@property (strong, nonatomic, readwrite) NSString *uniqueText;

@end

Expand Down Expand Up @@ -446,11 +448,38 @@ - (void)dismissKeyboardWithCancelID:(NSNumber<SDLInt> *)cancelID {
/// @return The choices that have not yet been uploaded to the module
- (NSSet<SDLChoiceCell *> *)sdl_choicesToBeUploadedWithArray:(NSArray<SDLChoiceCell *> *)choices {
NSMutableSet<SDLChoiceCell *> *choicesSet = [NSMutableSet setWithArray:choices];
joeljfischer marked this conversation as resolved.
Show resolved Hide resolved

// 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.
SDLVersion *choiceUniquenessSupportedVersion = [[SDLVersion alloc] initWithMajor:7 minor:1 patch:0];
FrankElias77 marked this conversation as resolved.
Show resolved Hide resolved
if ([[SDLGlobals sharedGlobals].rpcVersion isLessThanVersion:choiceUniquenessSupportedVersion]) {
[self sdl_addUniqueNamesToCells:choicesSet];
}
[choicesSet minusSet:self.preloadedChoices];

return [choicesSet copy];
}

/// 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 choices to be uploaded.
- (void)sdl_addUniqueNamesToCells:(NSMutableSet<SDLChoiceCell *> *)choices {
FrankElias77 marked this conversation as resolved.
Show resolved Hide resolved
// Tracks how many of each cell primary text there are so that we can append numbers to make each unique as necessary
NSMutableDictionary<NSString *, NSNumber *> *dictCounter = [[NSMutableDictionary alloc] init];
FrankElias77 marked this conversation as resolved.
Show resolved Hide resolved
for (SDLChoiceCell *cell in choices) {
NSString *cellName = cell.text;
NSNumber *counter = dictCounter[cellName];
if (counter != nil) {
counter = @(counter.intValue + 1);
dictCounter[cellName] = counter;
} else {
dictCounter[cellName] = @1;
}
if (counter.intValue > 1) {
cell.uniqueText = [NSString stringWithFormat: @"%@ (%d)", cell.text, counter.intValue];
}
}
}

/// Checks the passed list of choices to be deleted and returns the items that have been uploaded to the module.
/// @param choices The choices to be deleted
/// @return The choices that have been uploaded to the module
Expand Down
107 changes: 83 additions & 24 deletions SmartDeviceLink/private/SDLMenuManager.m
Original file line number Diff line number Diff line change
Expand Up @@ -48,6 +48,7 @@ @interface SDLMenuCell()

@property (assign, nonatomic) UInt32 parentCellId;
@property (assign, nonatomic) UInt32 cellId;
@property (strong, nonatomic, readwrite) NSString *uniqueTitle;

@end

Expand All @@ -74,6 +75,8 @@ @interface SDLMenuManager()

UInt32 const ParentIdNotFound = UINT32_MAX;
UInt32 const MenuCellIdMin = 1;
NSMutableSet<NSString *> *identicalVoiceCommandsCheckSet;
NSUInteger voiceCommandCount;
FrankElias77 marked this conversation as resolved.
Show resolved Hide resolved

@implementation SDLMenuManager

Expand All @@ -90,6 +93,9 @@ - (instancetype)init {
[[NSNotificationCenter defaultCenter] addObserver:self selector:@selector(sdl_hmiStatusNotification:) name:SDLDidChangeHMIStatusNotification object:nil];
[[NSNotificationCenter defaultCenter] addObserver:self selector:@selector(sdl_commandNotification:) name:SDLDidReceiveCommandNotification object:nil];

identicalVoiceCommandsCheckSet = [NSMutableSet set];
voiceCommandCount = 0;

return self;
}

Expand Down Expand Up @@ -160,6 +166,21 @@ - (void)setMenuConfiguration:(SDLMenuConfiguration *)menuConfiguration {
}

- (void)setMenuCells:(NSArray<SDLMenuCell *> *)menuCells {
FrankElias77 marked this conversation as resolved.
Show resolved Hide resolved
///Check for cell lists with completely duplicate information, or any duplicate voiceCommands
FrankElias77 marked this conversation as resolved.
Show resolved Hide resolved
if (![self sdl_menuCellsAreUnique:menuCells]) {
return;
FrankElias77 marked this conversation as resolved.
Show resolved Hide resolved
} else if (identicalVoiceCommandsCheckSet.count != voiceCommandCount) {
FrankElias77 marked this conversation as resolved.
Show resolved Hide resolved
SDLLogE(@"Attempted to create a menu with duplicate voice commands. Voice commands must be unique. The menu will not be set.");
identicalVoiceCommandsCheckSet = [NSMutableSet set];
voiceCommandCount = 0;
return;
}

SDLVersion *menuUniquenessSupportedVersion = [[SDLVersion alloc] initWithMajor:7 minor:1 patch:0];
if ([[SDLGlobals sharedGlobals].rpcVersion isLessThanVersion:menuUniquenessSupportedVersion]) {
[self sdl_addUniqueNamesToCells:menuCells];
}
FrankElias77 marked this conversation as resolved.
Show resolved Hide resolved

if (self.currentHMILevel == nil
|| [self.currentHMILevel isEqualToEnum:SDLHMILevelNone]
|| [self.currentSystemContext isEqualToEnum:SDLSystemContextMenu]) {
Expand All @@ -171,28 +192,6 @@ - (void)setMenuCells:(NSArray<SDLMenuCell *> *)menuCells {

self.waitingOnHMIUpdate = NO;

NSMutableSet *titleCheckSet = [NSMutableSet set];
NSMutableSet<NSString *> *allMenuVoiceCommands = [NSMutableSet set];
NSUInteger voiceCommandCount = 0;
for (SDLMenuCell *cell in menuCells) {
[titleCheckSet addObject:cell.title];
if (cell.voiceCommands == nil) { continue; }
[allMenuVoiceCommands addObjectsFromArray:cell.voiceCommands];
voiceCommandCount += cell.voiceCommands.count;
}

// Check for duplicate titles
if (titleCheckSet.count != menuCells.count) {
SDLLogE(@"Not all cell titles are unique. The menu will not be set.");
return;
}

// Check for duplicate voice recognition commands
if (allMenuVoiceCommands.count != voiceCommandCount) {
SDLLogE(@"Attempted to create a menu with duplicate voice commands. Voice commands must be unique. The menu will not be set.");
return;
}

_oldMenuCells = _menuCells;
_menuCells = menuCells;

Expand Down Expand Up @@ -530,6 +529,62 @@ - (BOOL)sdl_isDynamicMenuUpdateActive:(SDLDynamicMenuUpdatesMode)dynamicMenuUpda
}
}

/// 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 choices to be uploaded.
- (void)sdl_addUniqueNamesToCells:(nullable NSArray<SDLMenuCell *> *)choices {
FrankElias77 marked this conversation as resolved.
Show resolved Hide resolved
// Tracks how many of each cell primary text there are so that we can append numbers to make each unique as necessary
NSMutableDictionary<NSString *, NSNumber *> *dictCounter = [[NSMutableDictionary alloc] init];
for (SDLMenuCell *cell in choices) {
NSString *cellName = cell.title;
NSNumber *counter = dictCounter[cellName];
if (counter != nil) {
counter = @(counter.intValue + 1);
dictCounter[cellName] = counter;
} else {
dictCounter[cellName] = @1;
}

counter = dictCounter[cellName];
FrankElias77 marked this conversation as resolved.
Show resolved Hide resolved
if (counter.intValue > 1) {
cell.uniqueTitle = [NSString stringWithFormat: @"%@ (%d)", cell.title, counter.intValue];
}

if (cell.subCells.count > 0) {
[self sdl_addUniqueNamesToCells:cell.subCells];
}
}
}

/**
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
*/
-(BOOL)sdl_menuCellsAreUnique:(NSArray<SDLMenuCell *> *)cells {
FrankElias77 marked this conversation as resolved.
Show resolved Hide resolved
///Check all voice commands for identical items and check each list of cells for identical cells
NSMutableSet *identicalCellsCheckSet = [NSMutableSet set];
for (SDLMenuCell *cell in cells) {
[identicalCellsCheckSet addObject:cell];
if (cell.subCells.count > 0 && ![self sdl_menuCellsAreUnique:cell.subCells]) {
return NO;
}

if (cell.voiceCommands == nil) { continue; }
[identicalVoiceCommandsCheckSet addObjectsFromArray:cell.voiceCommands];
voiceCommandCount += cell.voiceCommands.count;
FrankElias77 marked this conversation as resolved.
Show resolved Hide resolved
}

// Check for duplicate cells
if (identicalCellsCheckSet.count != cells.count) {
SDLLogE(@"Not all cells are unique. The menu will not be set.");
return NO;
}

return YES;
FrankElias77 marked this conversation as resolved.
Show resolved Hide resolved
}

#pragma mark Artworks

- (NSArray<SDLArtwork *> *)sdl_findAllArtworksToBeUploadedFromCells:(NSArray<SDLMenuCell *> *)cells {
Expand Down Expand Up @@ -670,7 +725,7 @@ - (SDLAddCommand *)sdl_commandForMenuCell:(SDLMenuCell *)cell withArtwork:(BOOL)
SDLAddCommand *command = [[SDLAddCommand alloc] init];

SDLMenuParams *params = [[SDLMenuParams alloc] init];
params.menuName = cell.title;
params.menuName = cell.uniqueTitle;
params.parentID = cell.parentCellId != UINT32_MAX ? @(cell.parentCellId) : nil;
params.position = @(position);
params.tertiaryText = cell.tertiaryText;
Expand All @@ -695,7 +750,7 @@ - (SDLAddSubMenu *)sdl_subMenuCommandForMenuCell:(SDLMenuCell *)cell withArtwork
} else {
submenuLayout = self.menuConfiguration.defaultSubmenuLayout;
}
return [[SDLAddSubMenu alloc] initWithMenuID:cell.cellId menuName:cell.title position:@(position) menuIcon:icon menuLayout:submenuLayout parentID:nil secondaryText:cell.secondaryText tertiaryText:cell.tertiaryText secondaryImage:secondaryImage];
return [[SDLAddSubMenu alloc] initWithMenuID:cell.cellId menuName:cell.uniqueTitle position:@(position) menuIcon:icon menuLayout:submenuLayout parentID:nil secondaryText:cell.secondaryText tertiaryText:cell.tertiaryText secondaryImage:secondaryImage];
}

#pragma mark - Calling handlers
Expand Down Expand Up @@ -744,6 +799,8 @@ - (void)sdl_hmiStatusNotification:(SDLRPCNotificationNotification *)notification
![self.currentSystemContext isEqualToEnum:SDLSystemContextMenu]) {
if (self.waitingOnHMIUpdate) {
[self setMenuCells:self.waitingUpdateMenuCells];
identicalVoiceCommandsCheckSet = [NSMutableSet set];
voiceCommandCount = 0;
self.waitingUpdateMenuCells = @[];
return;
}
Expand All @@ -758,6 +815,8 @@ - (void)sdl_hmiStatusNotification:(SDLRPCNotificationNotification *)notification
&& ![self.currentHMILevel isEqualToEnum:SDLHMILevelNone]) {
if (self.waitingOnHMIUpdate) {
[self setMenuCells:self.waitingUpdateMenuCells];
identicalVoiceCommandsCheckSet = [NSMutableSet set];
voiceCommandCount = 0;
self.waitingUpdateMenuCells = @[];
}
}
Expand Down
2 changes: 1 addition & 1 deletion SmartDeviceLink/private/SDLPreloadChoicesOperation.m
Original file line number Diff line number Diff line change
Expand Up @@ -160,7 +160,7 @@ - (nullable SDLCreateInteractionChoiceSet *)sdl_choiceFromCell:(SDLChoiceCell *)

NSString *menuName = nil;
if ([self sdl_shouldSendChoiceText]) {
menuName = cell.text;
menuName = cell.uniqueText;
}

if(!menuName) {
Expand Down
4 changes: 4 additions & 0 deletions SmartDeviceLink/public/SDLChoiceCell.h
Original file line number Diff line number Diff line change
Expand Up @@ -45,6 +45,10 @@ NS_ASSUME_NONNULL_BEGIN
*/
@property (strong, nonatomic, readonly, nullable) SDLArtwork *secondaryArtwork;

/**
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+.
*/
@property (strong, nonatomic, readonly) NSString *uniqueText;
/**
Initialize the cell with nothing. This is unavailable

Expand Down
7 changes: 5 additions & 2 deletions SmartDeviceLink/public/SDLChoiceCell.m
Original file line number Diff line number Diff line change
Expand Up @@ -9,12 +9,14 @@
#import "SDLChoiceCell.h"

#import "SDLArtwork.h"
#import "NSArray+NSObject.h"

NS_ASSUME_NONNULL_BEGIN

@interface SDLChoiceCell()

@property (assign, nonatomic) UInt16 choiceId;
@property (nonatomic, readwrite) NSString *uniqueText;

@end

Expand All @@ -40,6 +42,7 @@ - (instancetype)initWithText:(NSString *)text secondaryText:(nullable NSString *
_voiceCommands = voiceCommands;
_artwork = artwork;
_secondaryArtwork = secondaryArtwork;
_uniqueText = text;

_choiceId = UINT16_MAX;

Expand All @@ -65,7 +68,7 @@ - (NSUInteger)hash {
^ NSUIntRotate(self.tertiaryText.hash, NSUIntBit / 4)
^ NSUIntRotate(self.artwork.name.hash, NSUIntBit / 5)
^ NSUIntRotate(self.secondaryArtwork.name.hash, NSUIntBit / 6)
^ NSUIntRotate(self.voiceCommands.hash, NSUIntBit / 7);
^ NSUIntRotate([self.voiceCommands dynamicHash], NSUIntBit / 7);
}

- (BOOL)isEqual:(id)object {
Expand All @@ -84,7 +87,7 @@ - (BOOL)isEqualToChoice:(SDLChoiceCell *)choice {
#pragma mark - Etc.

- (NSString *)description {
return [NSString stringWithFormat:@"SDLChoiceCell: %u-\"%@ - %@ - %@\", artworkNames: %@ - %@, voice commands: %lu", _choiceId, _text, _secondaryText, _tertiaryText, _artwork.name, _secondaryArtwork.name, (unsigned long)_voiceCommands.count];
return [NSString stringWithFormat:@"SDLChoiceCell: %u-\"%@ - %@ - %@\", artworkNames: %@ - %@, voice commands: %lu, uniqueText: %@", _choiceId, _text, _secondaryText, _tertiaryText, _artwork.name, _secondaryArtwork.name, (unsigned long)_voiceCommands.count, ([_text isEqualToString:_uniqueText] ? @"NO" : _uniqueText)];
}

@end
Expand Down
Loading