Skip to content
New issue

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

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

Already on GitHub? # to your account

Fix for voice command that contains no string should be removed #1965

2 changes: 2 additions & 0 deletions SmartDeviceLink-iOS.xcodeproj/project.pbxproj
Original file line number Diff line number Diff line change
Expand Up @@ -1738,6 +1738,7 @@
C9707D1925DEE786009D00F2 /* NSArray+Extensions.m in Sources */ = {isa = PBXBuildFile; fileRef = C9707D1725DEE786009D00F2 /* NSArray+Extensions.m */; };
C9707D3025E0444D009D00F2 /* SDLMacros.h in Headers */ = {isa = PBXBuildFile; fileRef = C9707D2E25E0444D009D00F2 /* SDLMacros.h */; };
C9707D3125E0444D009D00F2 /* SDLMacros.m in Sources */ = {isa = PBXBuildFile; fileRef = C9707D2F25E0444D009D00F2 /* SDLMacros.m */; };
C972094126287D12007378D1 /* SDLVoiceCommandManagerSpec.m in Sources */ = {isa = PBXBuildFile; fileRef = 5DF40B27208FDA9700DD6FDA /* SDLVoiceCommandManagerSpec.m */; };
C975877F257AEFDB0066F271 /* SDLSeekIndicatorTypeSpec.m in Sources */ = {isa = PBXBuildFile; fileRef = C975877E257AEFDB0066F271 /* SDLSeekIndicatorTypeSpec.m */; };
C9758785257F4C570066F271 /* SDLSeekStreamingIndicatorSpec.m in Sources */ = {isa = PBXBuildFile; fileRef = C9758784257F4C570066F271 /* SDLSeekStreamingIndicatorSpec.m */; };
C9DFFE78257ACE0000F7D57A /* SDLSeekStreamingIndicator.h in Headers */ = {isa = PBXBuildFile; fileRef = C9DFFE76257ACE0000F7D57A /* SDLSeekStreamingIndicator.h */; settings = {ATTRIBUTES = (Public, ); }; };
Expand Down Expand Up @@ -8863,6 +8864,7 @@
DA9F7EA21DCC05E100ACAE48 /* SDLGetWaypointsSpec.m in Sources */,
162E82D71A9BDE8A00906325 /* SDLDeviceLevelStatusSpec.m in Sources */,
162E83841A9BDE8B00906325 /* SDLMenuParamsSpec.m in Sources */,
C972094126287D12007378D1 /* SDLVoiceCommandManagerSpec.m in Sources */,
5DB1BCE11D243DDE002FFC37 /* SDLConfigurationSpec.m in Sources */,
B3DF19ED251225300090D7BA /* TestSmartConnectionManager.m in Sources */,
162E83071A9BDE8B00906325 /* SDLVehicleDataNotificationStatusSpec.m in Sources */,
Expand Down
42 changes: 36 additions & 6 deletions SmartDeviceLink/private/SDLVoiceCommandManager.m
Original file line number Diff line number Diff line change
Expand Up @@ -28,6 +28,7 @@
@interface SDLVoiceCommand()

@property (assign, nonatomic) UInt32 commandId;
@property (copy, nonatomic, readwrite) NSArray<NSString *> *voiceCommands;

@end

Expand All @@ -40,6 +41,7 @@ @interface SDLVoiceCommandManager()

@property (assign, nonatomic) UInt32 lastVoiceCommandId;
@property (copy, nonatomic) NSArray<SDLVoiceCommand *> *currentVoiceCommands;
@property (copy, nonatomic) NSArray<SDLVoiceCommand *> *originalVoiceCommands;

@end

Expand Down Expand Up @@ -104,20 +106,26 @@ - (void)sdl_updateTransactionQueueSuspended {
#pragma mark - Setters

- (void)setVoiceCommands:(NSArray<SDLVoiceCommand *> *)voiceCommands {
if (voiceCommands == self.voiceCommands) {
if (voiceCommands == self.originalVoiceCommands) {
SDLLogD(@"New voice commands are equal to the existing voice commands, skipping...");
return;
}

// Set the ids
[self sdl_updateIdsOnVoiceCommands:voiceCommands];

// Set the new voice commands internally
_voiceCommands = voiceCommands;
// Validate the voiceCommand's strings. In the rare case that the user has set only empty whitespace strings, abort the update operation.
NSArray<SDLVoiceCommand *> *validatedVoiceCommands = [self sdl_removeEmptyVoiceCommands:voiceCommands];
if (validatedVoiceCommands.count == 0 && voiceCommands.count > 0) {
SDLLogE(@"New voice commands are invalid, skipping...");
return;
}
_originalVoiceCommands = voiceCommands;
_voiceCommands = validatedVoiceCommands;
// Set the ids
[self sdl_updateIdsOnVoiceCommands:self.voiceCommands];

// Create the operation, cancel previous ones and set this one
__weak typeof(self) weakSelf = self;
SDLVoiceCommandUpdateOperation *updateOperation = [[SDLVoiceCommandUpdateOperation alloc] initWithConnectionManager:self.connectionManager pendingVoiceCommands:voiceCommands oldVoiceCommands:_currentVoiceCommands updateCompletionHandler:^(NSArray<SDLVoiceCommand *> *newCurrentVoiceCommands, NSError * _Nullable error) {
SDLVoiceCommandUpdateOperation *updateOperation = [[SDLVoiceCommandUpdateOperation alloc] initWithConnectionManager:self.connectionManager pendingVoiceCommands:self.voiceCommands oldVoiceCommands:_currentVoiceCommands updateCompletionHandler:^(NSArray<SDLVoiceCommand *> *newCurrentVoiceCommands, NSError * _Nullable error) {
weakSelf.currentVoiceCommands = newCurrentVoiceCommands;
[weakSelf sdl_updatePendingOperationsWithNewCurrentVoiceCommands:newCurrentVoiceCommands];
}];
Expand Down Expand Up @@ -146,6 +154,28 @@ - (void)sdl_updateIdsOnVoiceCommands:(NSArray<SDLVoiceCommand *> *)voiceCommands
}
}

/// Remove all voice command strings consisting of just whitespace characters as the module will reject any "empty" strings.
/// @param voiceCommands - array of SDLVoiceCommands that are to be uploaded
/// @return A list of voice commands with the empty voice commands removed
- (NSArray<SDLVoiceCommand *> *)sdl_removeEmptyVoiceCommands:(NSArray<SDLVoiceCommand *> *)voiceCommands {
NSMutableArray<SDLVoiceCommand *> *validatedVoiceCommands = [[NSMutableArray alloc] init];
for (SDLVoiceCommand *voiceCommand in voiceCommands) {
NSMutableArray<NSString *> *voiceCommandStrings = [[NSMutableArray alloc] init];
for (NSString *voiceCommandString in voiceCommand.voiceCommands) {
NSString *trimmedString = [voiceCommandString stringByTrimmingCharactersInSet:[NSCharacterSet whitespaceAndNewlineCharacterSet]];
// Updates voice command strings array by only adding ones that are not empty(e.g. "", " ", ...)
if (trimmedString.length > 0) {
[voiceCommandStrings addObject:voiceCommandString];
}
}
if (voiceCommandStrings.count > 0) {
voiceCommand.voiceCommands = voiceCommandStrings;
[validatedVoiceCommands addObject:voiceCommand];
}
}
return [validatedVoiceCommands copy];
}

#pragma mark - Observers

- (void)sdl_commandNotification:(SDLRPCNotificationNotification *)notification {
Expand Down
1 change: 1 addition & 0 deletions SmartDeviceLink/public/SDLVoiceCommand.m
Original file line number Diff line number Diff line change
Expand Up @@ -13,6 +13,7 @@
@interface SDLVoiceCommand()

@property (assign, nonatomic) UInt32 commandId;
@property (copy, nonatomic, readwrite) NSArray<NSString *> *voiceCommands;

@end

Expand Down
49 changes: 49 additions & 0 deletions SmartDeviceLinkTests/DevAPISpecs/SDLVoiceCommandManagerSpec.m
Original file line number Diff line number Diff line change
Expand Up @@ -49,6 +49,10 @@ @interface SDLVoiceCommandManager()

__block SDLVoiceCommand *testVoiceCommand = [[SDLVoiceCommand alloc] initWithVoiceCommands:@[@"Test 1"] handler:^{}];
__block SDLVoiceCommand *testVoiceCommand2 = [[SDLVoiceCommand alloc] initWithVoiceCommands:@[@"Test 2"] handler:^{}];
__block SDLVoiceCommand *testVoiceCommand3 = [[SDLVoiceCommand alloc] initWithVoiceCommands:@[@"Test 3", @" ", @"Test 4", @"\t"] handler:^{}];
__block SDLVoiceCommand *testVoiceCommand4 = [[SDLVoiceCommand alloc] initWithVoiceCommands:@[@"\t"] handler:^{}];
__block SDLVoiceCommand *testVoiceCommand5 = [[SDLVoiceCommand alloc] initWithVoiceCommands:@[@""] handler:^{}];
__block SDLVoiceCommand *testVoiceCommand6 = [[SDLVoiceCommand alloc] init];
__block SDLOnHMIStatus *newHMIStatus = [[SDLOnHMIStatus alloc] init];
__block NSArray<SDLVoiceCommand *> *testVCArray = nil;

Expand Down Expand Up @@ -155,6 +159,51 @@ @interface SDLVoiceCommandManager()
});
});
});

context(@"if it has voice commands to upload where one voice command strings contains an empty string", ^{
beforeEach(^{
testManager.voiceCommands = @[testVoiceCommand3, testVoiceCommand4, testVoiceCommand5, testVoiceCommand6];
});

// should queue another operation
it(@"should queue another operation", ^{
expect(testManager.transactionQueue.operations).to(haveCount(2));
expect(testManager.voiceCommands).to(haveCount(1));
expect(testManager.voiceCommands.firstObject.voiceCommands).to(haveCount(2));
expect(testManager.voiceCommands.firstObject.voiceCommands).to(equal(@[@"Test 3", @"Test 4"]));
});

// when the first operation finishes and updates the current voice commands
describe(@"when the first operation finishes and updates the current voice commands", ^{
context(@"if it has voice commands to upload with empty voice command strings", ^{
beforeEach(^{
SDLVoiceCommandUpdateOperation *firstOp = testManager.transactionQueue.operations[0];
firstOp.currentVoiceCommands = [@[testVoiceCommand3] mutableCopy];
[firstOp finishOperation];

[NSThread sleepForTimeInterval:0.5];

testManager.voiceCommands = @[testVoiceCommand5];
});

// should queue another operation
it(@"should queue another operation", ^{
expect(testManager.transactionQueue.operations).to(haveCount(1));
});
});

context(@"if it has voice commands to upload with empty string voice command strings", ^{
beforeEach(^{
testManager.voiceCommands = @[testVoiceCommand5];
});

// should not queue another operation
it(@"should not queue another operation", ^{
expect(testManager.transactionQueue.operations).to(haveCount(1));
});
});
});
});
});

// on disconnect
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -19,6 +19,12 @@ @interface SDLVoiceCommand()

@end

@interface SDLVoiceCommandUpdateOperation()

@property (copy, nonatomic) NSArray<SDLVoiceCommand *> *pendingVoiceCommands;

@end

QuickSpecBegin(SDLVoiceCommandUpdateOperationSpec)

__block SDLDeleteCommandResponse *successDelete = nil;
Expand Down