-
Notifications
You must be signed in to change notification settings - Fork 43
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
Implementation of Expandable design for proprietary data exchange feature #73
Implementation of Expandable design for proprietary data exchange feature #73
Conversation
change name systemRequest Label Bugfix error at the ffw/BasicCommunication OnSystemRequest method with subType parameter which was null and which was calling. Change name systemRequest Label from 'System request' to 'requestSubType'.
…oprietary_data_exchange Bugfix error at the OnSystemRequest method and
…roprietary_data_exchange Implementation of Expandable design for proprietary data exchange feature
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The change looks good to me.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@theresalech this has been reviewed and approved by Ford.
ffw/BasicCommunicationRPC.js
Outdated
@@ -1149,6 +1149,14 @@ FFW.BasicCommunication = FFW.RPCObserver | |||
if (appID) { | |||
JSONMessage.params.appID = appID; | |||
} | |||
if(subType && subType.length > 0){ |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Fix indentation
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@jacobkeeler fixed in 4428727
css/sdl.css
Outdated
@@ -827,6 +834,11 @@ | |||
top: 180px; | |||
} | |||
|
|||
#systemRequestView .requestSubTypeInput { | |||
top: 217px; | |||
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Extra newline
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@jacobkeeler fixed in 4428727
ffw/BasicCommunicationRPC.js
Outdated
if (subType && subType.length > 0) { | ||
if (type == 'OEM_SPECIFIC'|| | ||
type == 'HTTP'|| | ||
type == 'PROPRIETARY') { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Shouldn't need this restriction. The only cases where subType shouldn't be used are existing PTU Requests (HTTP
request w/ BINARY
fileType or PROPRIETARY
request w/ JSON
fileType): https://github.com/smartdevicelink/sdl_evolution/blob/master/proposals/0083-Expandable-design-for-proprietary-data-exchange.md#impact-on-existing-code
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@jacobkeeler good catch, thanks.
Fixed in 3003881
Fixes smartdevicelink/sdl_core#1734
This PR is ready for review.
Risk
This PR makes minor API changes.
Testing Plan
HMI behavior should be tested manually
Summary
Made the following changes:
CLA