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.

Sign up for GitHub

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

Jump to bottom

SDL 0192 Button Subscription response from HMI #580

Merged
merged 18 commits into from
Aug 12, 2021
Merged

SDL 0192 Button Subscription response from HMI #580

merged 18 commits into from
Aug 12, 2021

Conversation

ValeriiMalkov
Copy link
Contributor

@ValeriiMalkov ValeriiMalkov commented Jul 20, 2021 •
edited by GetmanetsIrina
Loading

Implements #0192

This PR is ready for review.

Testing Plan

Manual testing

Instruction for the feature 0192_Button_Subscription_response_from_HMI_instruction_Web_HMI.xlsx

Summary

Implementation of Button Subscription response from HMI

CLA

@ashwink11
Copy link

@jordynmackool this PR is ready for review and is approved by Ford.

@theresalech theresalech linked an issue Jul 26, 2021 that may be closed by this pull request
[SDL 0192] Button Subscription response from HMI #107
Closed
jacobkeeler
jacobkeeler requested changes Jul 26, 2021
View reviewed changes
app/controller/SettingsController.js Outdated Show resolved Hide resolved
app/view/settings/RPCControls/RPCControlConfigView.js Outdated Show resolved Hide resolved
app/view/settings/RPCControls/RPCControlConfigView.js Outdated Show resolved Hide resolved
css/settings.css Outdated Show resolved Hide resolved
ffw/RPCHelper.js Outdated Show resolved Hide resolved
ffw/ButtonsRPC.js Show resolved Hide resolved
index.html Show resolved Hide resolved
ffw/RPCHelper.js Outdated Show resolved Hide resolved
LitvinenkoIra and others added 2 commits July 28, 2021 15:06
jacobkeeler
jacobkeeler requested changes Jul 28, 2021
View reviewed changes
ffw/ButtonsRPC.js Show resolved Hide resolved
ffw/ButtonsRPC.js Outdated Show resolved Hide resolved
ffw/ButtonsRPC.js Outdated Show resolved Hide resolved
@theresalech
Copy link
Contributor

@ValeriiMalkov just checking in to make sure you've received Jacob's review. Please let us know if you have any questions or concerns with addressing his feedback. If you could also provide us with an anticipated date for when this PR will be ready for Livio's re-review, we'd appreciate it! Thank you!

@ValeriiMalkov
Copy link
Contributor Author

@ValeriiMalkov just checking in to make sure you've received Jacob's review. Please let us know if you have any questions or concerns with addressing his feedback. If you could also provide us with an anticipated date for when this PR will be ready for Livio's re-review, we'd appreciate it! Thank you!

@theresalech Thanks for the reminder. Comments done and ready for review.

jacobkeeler
jacobkeeler requested changes Aug 5, 2021
View reviewed changes
ffw/ButtonsRPC.js Outdated Show resolved Hide resolved
ffw/ButtonsRPC.js Outdated Show resolved Hide resolved
capabilities/display_capabilities.js Outdated Show resolved Hide resolved
capabilities/display_capabilities.js Outdated Show resolved Hide resolved
capabilities/display_capabilities.js Outdated Show resolved Hide resolved
app/view/settings/RPCControls/RPCControlConfigView.js Outdated Show resolved Hide resolved
app/view/settings/RPCControls/RPCControlConfigView.js Outdated Show resolved Hide resolved
app/view/settings/RPCControls/RPCControlConfigView.js Outdated Show resolved Hide resolved
iCollin
iCollin reviewed Aug 6, 2021
View reviewed changes
ffw/ButtonsRPC.js Outdated Show resolved Hide resolved
jacobkeeler
jacobkeeler approved these changes Aug 10, 2021
View reviewed changes
Copy link
Contributor

@jacobkeeler jacobkeeler left a comment

Choose a reason for hiding this comment

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

This PR looks good, although it looks like there is an issue where the capabilities do not update when switching templates. That is out of the scope of this feature though, so that can be fixed separately

jacobkeeler
jacobkeeler requested changes Aug 10, 2021
View reviewed changes
capabilities/display_capabilities.js Outdated
"longPressAvailable": true,
"upDownAvailable": true
}],
"buttonCapabilities": SDL.ButtonCapability,
Copy link
Contributor

Choose a reason for hiding this comment

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

Suggested change
"buttonCapabilities": SDL.ButtonCapability,
"buttonCapabilities": SDL.ButtonCapability.filter(button => button.name !== 'PLAY_PAUSE'),

Ah, looks like PLAY_PAUSE was not present in this template's capabilities originally

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Done bc91ea3

@jacobkeeler
Copy link
Contributor

Apologies, one more minor suggestion

@ValeriiMalkov
Copy link
Contributor Author

Apologies, one more minor suggestion

Done bc91ea3

@jacobkeeler jacobkeeler merged commit c2152a9 into smartdevicelink:develop Aug 12, 2021
@jacobkeeler jacobkeeler mentioned this pull request Aug 12, 2021
Closed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@LitvinenkoIra LitvinenkoIra LitvinenkoIra left review comments

@iCollin iCollin iCollin left review comments

@jacobkeeler jacobkeeler jacobkeeler approved these changes

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

[SDL 0192] Button Subscription response from HMI
6 participants
@ValeriiMalkov @ashwink11 @theresalech @jacobkeeler @iCollin @LitvinenkoIra