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-0238] Keyboard Enhancements #366

Conversation

ymalovanyi
Copy link
Contributor

@ymalovanyi ymalovanyi commented Dec 14, 2020

Fixes #340

Risk

This PR makes minor API changes.

Testing Plan

  • I have verified that I have not introduced new warnings in this PR
  • I have run the unit tests with this PR
  • I have tested this PR against Core and verified behavior

Unit Tests

Added unit tests cover [SDL-0238] Keyboard Enhancements changes

Core Tests

  • each of the properties in the struct KeyboardCapabilities can be received from HMI
  • each of the values in the enum KeyboardInputMask can be received from HMI
  • KeyboardEvent enum contains two additional values "INPUT_KEY_MASK_ENABLED", and "INPUT_KEY_MASK_DISABLED"
  • each of the properties in the struct ConfigurableKeyboards can be received from HMI
  • keyboardCapabilities property is present in WindowCapability struct, and contains configurableKeys property
  • KeyboardLayout enum contains additional value "NUMERIC"
  • maskInputCharacters and customizeKeys properties in the struct KeyboardProperties can be received from HMI
  • in SetGlobalProperties message request keyboardProperties parameter can be sent with keyboardLayout property set to "NUMERIC", and defined maskInputCharacters and customizeKeys properties

Core version / branch / commit hash / module tested against: LuxoftSDL/sdl_core#sdl_0238_keyboard_enhancements
HMI name / version / branch / commit hash / module tested against: LuxoftSDL/sdl_hmi#sdl_0238_keyboard_enhancements

Summary

Applied [SDL-0238] Keyboard Enhancements changes

Changelog

Breaking Changes
  • N/A
Enhancements
  • New structs KeyboardCapabilities, ConfigurableKeyboards
  • New enum KeyboardInputMask
  • Added required getters/setters in related RPC classes
Bug Fixes
  • N/A

Tasks Remaining:

  • N/A

CLA

@ymalovanyi
Copy link
Contributor Author

@santhanamk the PR is ready for Ford review.

Copy link

@santhanamk santhanamk left a comment

Choose a reason for hiding this comment

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

@ymalovanyi Code looks good.

@ymalovanyi ymalovanyi marked this pull request as ready for review December 18, 2020 08:55
@jordynmackool
Copy link

@ymalovanyi is this ready for Livio Reivew?

@vladmu
Copy link
Contributor

vladmu commented Dec 18, 2020

@jordynmackool @crokita the description was fixed to include all sections from the template and the code was tested against the Core and HMI (corresponded links also included in the description). Please review. Also, please take into account that the JavaScript Suite doesn't contain Choice Set Manager (it seems it will be added in the implementation of https://github.com/smartdevicelink/sdl_evolution/blob/master/proposals/0272-sdl-javascript-manager-layer.md) therefore only RPC changes of the proposal included here and we have tested this against Core with cases defined in the description.

@jordynmackool
Copy link

@ymalovanyi / @vladmu the proposal markdown file has been updated per the revisions included in the accepted with revisions review issue: Revise SDL-0238 Keyboard Enhancements.

Please make the needed updates to this PR and then tag @renonick87 to review when ready. Thanks!

@BogMW
Copy link
Contributor

BogMW commented Feb 8, 2021

@renonick87 all required adjustments were made, the PR is ready for review.

Copy link
Contributor

@renonick87 renonick87 left a comment

Choose a reason for hiding this comment

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

@ymalovanyi From an initial review, the content of this PR looks good. But with the addition of the ChoiceSetManager in #371, there are additional updates needed for the manager to support this feature.

@vladmu
Copy link
Contributor

vladmu commented Feb 18, 2021

@renonick87, we've made additional changes to the ChoiceSetManager. Moreover, this aligned with the last state of Java PR smartdevicelink/sdl_java_suite#1587. We have tested the code with the Core/HMI. Please review.

Copy link
Contributor

@renonick87 renonick87 left a comment

Choose a reason for hiding this comment

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

@ymalovanyi See my feedback for a minor suggestion.

@@ -306,6 +306,8 @@ class _PresentChoiceSetOperation extends _Task {
} else if (onKeyboardInput.getEvent() === KeyboardEvent.ENTRY_ABORTED || onKeyboardInput.getEvent() === KeyboardEvent.ENTRY_CANCELLED) {
// notify of abort / cancelation
this._keyboardListener.onKeyboardDidAbortWithReason(onKeyboardInput.getEvent());
} else if (onKeyboardInput.getEvent().equals(KeyboardEvent.INPUT_KEY_MASK_ENABLED) || onKeyboardInput.getEvent().equals(KeyboardEvent.INPUT_KEY_MASK_DISABLED)) {
Copy link
Contributor

Choose a reason for hiding this comment

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

This addition should also be included in the _PresentKeyboardOperation.js file.

@BogMW
Copy link
Contributor

BogMW commented Feb 23, 2021

Hey @renonick87 , changes were added.

@renonick87 renonick87 merged commit 38969fe into smartdevicelink:develop Feb 24, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

[SDL 0238] Keyboard Enhancements
6 participants