SDL0238 'Keyboard Enhancements/r3': apply review comments (refine the logic to sort outgoing capabilities)

4 files changed, 36 insertions, 28 deletions

diff --git a/SmartDeviceLink/private/SDLPresentChoiceSetOperation.m b/SmartDeviceLink/private/SDLPresentChoiceSetOperation.m
index a4e2b5c9f..2b487121e 100644
--- a/SmartDeviceLink/private/SDLPresentChoiceSetOperation.m
+++ b/SmartDeviceLink/private/SDLPresentChoiceSetOperation.m
@@ -124,15 +124,16 @@ NS_ASSUME_NONNULL_BEGIN
 #pragma mark - Sending Requests
 
 - (void)sdl_updateKeyboardPropertiesWithCompletionHandler:(nullable void(^)(void))completionHandler {
-    SDLKeyboardProperties *outProperties = [self.windowCapability filterValidKeyboardProperties:self.keyboardProperties];
-    if (!outProperties) {
+    // Create the keyboard configuration based on the window capability's keyboard capabilities
+    SDLKeyboardProperties *modifiedKeyboardConfig = [self.windowCapability createValidKeyboardConfigurationBasedOnKeyboardCapabilitiesFromConfiguration:self.keyboardProperties];
+    if (modifiedKeyboardConfig == nil) {
         if (completionHandler != nil) {
             completionHandler();
         }
         return;
     }
     SDLSetGlobalProperties *setProperties = [[SDLSetGlobalProperties alloc] init];
-    setProperties.keyboardProperties = outProperties;
+    setProperties.keyboardProperties = modifiedKeyboardConfig;
 
     __weak typeof(self) weakself = self;
     [self.connectionManager sendConnectionRequest:setProperties withResponseHandler:^(__kindof SDLRPCRequest * _Nullable request, __kindof SDLRPCResponse * _Nullable response, NSError * _Nullable error) {
diff --git a/SmartDeviceLink/private/SDLPresentKeyboardOperation.m b/SmartDeviceLink/private/SDLPresentKeyboardOperation.m
index 58499ff5f..939869680 100644
--- a/SmartDeviceLink/private/SDLPresentKeyboardOperation.m
+++ b/SmartDeviceLink/private/SDLPresentKeyboardOperation.m
@@ -90,15 +90,16 @@ NS_ASSUME_NONNULL_BEGIN
 #pragma mark - Sending Requests
 
 - (void)sdl_updateKeyboardPropertiesWithCompletionHandler:(nullable void(^)(void))completionHandler {
-    SDLKeyboardProperties *outProperties = [self.windowCapability filterValidKeyboardProperties:self.keyboardProperties];
-    if (!outProperties) {
-        if (completionHandler) {
+    // Create the keyboard configuration based on the window capability's keyboard capabilities
+    SDLKeyboardProperties *keyboardConfiguration = [self.windowCapability createValidKeyboardConfigurationBasedOnKeyboardCapabilitiesFromConfiguration:self.keyboardProperties];
+    if (keyboardConfiguration == nil) {
+        if (completionHandler != nil) {
             completionHandler();
         }
         return;
     }
     SDLSetGlobalProperties *setProperties = [[SDLSetGlobalProperties alloc] init];
-    setProperties.keyboardProperties = outProperties;
+    setProperties.keyboardProperties = keyboardConfiguration;
 
     [self.connectionManager sendConnectionRequest:setProperties withResponseHandler:^(__kindof SDLRPCRequest * _Nullable request, __kindof SDLRPCResponse * _Nullable response, NSError * _Nullable error) {
         if (error != nil) {
diff --git a/SmartDeviceLink/private/SDLWindowCapability+ScreenManagerExtensions.h b/SmartDeviceLink/private/SDLWindowCapability+ScreenManagerExtensions.h
index 689c4cf05..ca095da24 100644
--- a/SmartDeviceLink/private/SDLWindowCapability+ScreenManagerExtensions.h
+++ b/SmartDeviceLink/private/SDLWindowCapability+ScreenManagerExtensions.h
@@ -23,7 +23,12 @@ NS_ASSUME_NONNULL_BEGIN
 - (BOOL)hasTextFieldOfName:(SDLTextFieldName)name;
 - (BOOL)hasImageFieldOfName:(SDLImageFieldName)name;
 
-- (SDLKeyboardProperties *__nullable)filterValidKeyboardProperties:(SDLKeyboardProperties *__nullable)keyboardProperties;
+/// Takes a keyboard configuration (SDLKeyboardProperties) and creates a valid version of it, if possible, based on this object's internal keyboardCapabilities (SDLKeyboardCapabilities).
+/// If there is no internal keyboardCapabilities, it will just return the passed configuration as-is.
+/// If no valid configuration can be determined based on the internal keyboard capabilities, it will return nil.
+/// @param keyboardConfiguration The configuration to use to determine a valid configuration
+/// @return The passed keyboardConfiguration if there are no changes needed or possible, a modified keyboardConfiguration if a valid version of the configuration could be determined, or nil if a valid configuration could not be created
+- (nullable SDLKeyboardProperties *)createValidKeyboardConfigurationBasedOnKeyboardCapabilitiesFromConfiguration:(nullable SDLKeyboardProperties *)keyboardConfiguration;
 
 @end
 
diff --git a/SmartDeviceLink/private/SDLWindowCapability+ScreenManagerExtensions.m b/SmartDeviceLink/private/SDLWindowCapability+ScreenManagerExtensions.m
index a09e70da5..0006daa4c 100644
--- a/SmartDeviceLink/private/SDLWindowCapability+ScreenManagerExtensions.m
+++ b/SmartDeviceLink/private/SDLWindowCapability+ScreenManagerExtensions.m
@@ -56,41 +56,42 @@
     return NO;
 }
 
-- (SDLKeyboardProperties *__nullable)filterValidKeyboardProperties:(SDLKeyboardProperties *__nullable)inKeyboardProperties {
-    if (!self.keyboardCapabilities || !inKeyboardProperties || !inKeyboardProperties.keyboardLayout) {
+- (nullable SDLKeyboardProperties *)createValidKeyboardConfigurationBasedOnKeyboardCapabilitiesFromConfiguration:(nullable SDLKeyboardProperties *)inKeyboardProperties {
+    // If there are no keyboard capabilities, if there is no passed keyboard configuration, or if there is no layout to the passed keyboard configuration, just pass back the passed in configuration
+    if ((self.keyboardCapabilities == nil) || (inKeyboardProperties == nil) || (inKeyboardProperties.keyboardLayout == nil)) {
         return inKeyboardProperties;
     }
+
     SDLKeyboardLayoutCapability *selectedLayoutCapability = nil;
-    if (inKeyboardProperties.keyboardLayout) {
-        for (SDLKeyboardLayoutCapability *layoutCapability in self.keyboardCapabilities.supportedKeyboards) {
-            if ([layoutCapability.keyboardLayout isEqualToEnum:inKeyboardProperties.keyboardLayout]) {
-                selectedLayoutCapability = layoutCapability;
-                break;
-            }
+    for (SDLKeyboardLayoutCapability *layoutCapability in self.keyboardCapabilities.supportedKeyboards) {
+        if ([layoutCapability.keyboardLayout isEqualToEnum:inKeyboardProperties.keyboardLayout]) {
+            selectedLayoutCapability = layoutCapability;
+            break;
         }
     }
-    if (!selectedLayoutCapability) {
-        SDLLogD(@"Keyboard layout is not supported: %@", inKeyboardProperties.keyboardLayout);
+    if (selectedLayoutCapability == nil) {
+        SDLLogE(@"Configured keyboard layout is not supported: %@", inKeyboardProperties.keyboardLayout);
         return nil;
     }
 
-    SDLKeyboardProperties *outKeyboardProperties = [inKeyboardProperties copy];
-
-    if (!inKeyboardProperties.customKeys.count) {
-        outKeyboardProperties.customKeys = nil;
+    SDLKeyboardProperties *modifiedKeyboardConfiguration = [inKeyboardProperties copy];
+    if (inKeyboardProperties.customKeys.count == 0) {
+        modifiedKeyboardConfiguration.customKeys = nil;
     } else {
-        const NSUInteger numConfigurableKeys = (NSUInteger)selectedLayoutCapability.numConfigurableKeys.integerValue;
-        if (inKeyboardProperties.customKeys.count > numConfigurableKeys) {
-            outKeyboardProperties.customKeys = [inKeyboardProperties.customKeys subarrayWithRange:NSMakeRange(0, numConfigurableKeys)];
-            SDLLogD(@"Too many custom keys %d, only the first %d will be used and the rest will get dropped.", (int)inKeyboardProperties.customKeys.count, (int)numConfigurableKeys);
+        // If there are more custom keys than are allowed for the selected keyboard layout, we need to trim the number of keys to only use the first n number of custom keys, where n is the number of allowed custom keys for that layout.
+        NSUInteger numConfigurableKeys = selectedLayoutCapability.numConfigurableKeys.unsignedIntegerValue;
+        if (modifiedKeyboardConfiguration.customKeys.count > numConfigurableKeys) {
+            modifiedKeyboardConfiguration.customKeys = [modifiedKeyboardConfiguration.customKeys subarrayWithRange:NSMakeRange(0, numConfigurableKeys)];
+            SDLLogW(@"%lu custom keys set, but the selected layout: %@ only supports %lu. Dropping the rest.", (unsigned long)modifiedKeyboardConfiguration.customKeys.count, modifiedKeyboardConfiguration.keyboardLayout, (unsigned long)numConfigurableKeys);
         }
     }
 
+    // If the keyboard does not support masking input characters, we will remove it from the keyboard configuration
     if (!self.keyboardCapabilities.maskInputCharactersSupported.boolValue) {
-        outKeyboardProperties.maskInputCharacters = nil;
+        modifiedKeyboardConfiguration.maskInputCharacters = nil;
     }
 
-    return outKeyboardProperties;
+    return modifiedKeyboardConfiguration;
 }
 
 @end