Add base RPCs and supporting classes #24

joeygrover · 2019-11-13T20:04:31Z

This PR is ready for review.

Risk

This PR makes minor API changes.

Summary

Adding RPCs that are necessary for base connection and registration

Tasks Remaining:

CLA

I have signed the CLA and this pull request adheres to the Contributing Guide.

…script_suite into feature/mvp_rpcs # Conflicts: # lib/js/rpc/enums/FunctionID.js

…script_suite into feature/mvp_rpcs

lib/rpc_spec/MOBILE_API.xml

vladmu · 2019-11-19T12:53:51Z

Please take into attention that the RPC generator will cover almost all 'enums/structs/messages' included in this PR, as a part of the generated code. Also, I noticed the difference between currently existed RPC classes and the new ones, at least in the properties declaration, static in-class vs. added after the class declaration. Should we follow the last principle in our generator instead of the old?

lib/js/src/rpc/enums/BitsPerSample.js

joeygrover · 2019-11-19T14:45:45Z

We are assuming the generator will be able to cover all the RPC related classes in this PR. However, since we do not have timing on when that will be submitted, approved, merged, we must create the base RPCs we need to test other pieces in the library.

If you check the develop branch you will see that the static members were moved because of JavaScript requirements.

nickschwab

Some comments/suggestions/requested changes in-line.

Additional comments:

MAP should be prefixed with an underscore to represent a private property. Can be addressed in a later passthrough.
Recommend naming boolean parameters/variables with the following prefixes: "is", "has", or "can".

lib/js/src/rpc/enums/BitsPerSample.js

lib/js/src/rpc/enums/DisplayType.js

lib/js/src/rpc/enums/HMILevel.js

lib/js/src/rpc/messages/Show.js

lib/js/src/rpc/messages/RegisterAppInterface.js

lib/js/src/rpc/messages/RegisterAppInterfaceResponse.js

lib/js/src/rpc/messages/Show.js

lib/js/src/rpc/structs/ButtonCapabilities.js

lib/js/src/rpc/structs/SoftButton.js

Co-Authored-By: Nick Schwab <nick.schwab@livio.io>

joeygrover

Some comments/suggestions/requested changes in-line.

Additional comments:

MAP should be prefixed with an underscore to represent a private property. Can be addressed in a later passthrough.

Recommend naming boolean parameters/variables with the following prefixes: "is", "has", or "can".

Updated
While I agree it should be this way, the RPC classes should reflect the RPC Spec directly for ease of use with the generator.

nickschwab

@joeygrover Just a few small-ish things; sorry I didn't catch them all on the first review!

lib/js/src/rpc/enums/FunctionID.js

lib/js/src/rpc/enums/TextFieldName.js

lib/js/src/rpc/messages/RegisterAppInterfaceResponse.js

lib/js/src/rpc/structs/ButtonCapabilities.js

lib/js/src/rpc/structs/SoftButton.js

lib/js/src/rpc/messages/AddCommand.js

Co-Authored-By: Nick Schwab <nick.schwab@livio.io>

lib/js/src/rpc/messages/AddCommand.js

o-mishch · 2019-11-21T16:15:43Z

lib/js/src/rpc/structs/AppInfo.js

+    * @return {AppInfo}
+    */
+    setAppDisplayName(appDisplayName) {
+        this.setParameter(AppInfo.KEY_APP_DISPLAY_NAME, appDisplayName);


Don't we need to validate Strings? like this.validateType(String, name);

o-mishch · 2019-11-21T16:27:17Z

lib/js/src/rpc/structs/AudioPassThruCapabilities.js

+    setSamplingRate(samplingRate) {
+        this.validateType(SamplingRate, samplingRate)
+
+        this.setParameter(AudioPassThruCapabilities.KEY_SAMPLING_RATE, samplingRate);


the param for setSamplingRate is object, so shouldn't we use this.setValue(AudioPassThruCapabilities.KEY_SAMPLING_RATE, samplingRate); instead of this.setParameter(AudioPassThruCapabilities.KEY_SAMPLING_RATE, samplingRate);?

o-mishch · 2019-11-21T16:31:25Z

lib/js/src/rpc/structs/ButtonCapabilities.js

+    */
+    setShortPressAvailable(shortPressAvailable) {
+
+        this.setValue(ButtonCapabilities.KEY_SHORT_PRESS_AVAILABLE, shortPressAvailable);


shortPressAvailable param is simple Boolean, shouldn't we use this.setParameter(ButtonCapabilities.KEY_SHORT_PRESS_AVAILABLE, shortPressAvailable); instead of setValue?

o-mishch · 2019-11-21T16:32:04Z

lib/js/src/rpc/structs/ButtonCapabilities.js

+    */
+    setLongPressAvailable(longPressAvailable) {
+
+        this.setValue(ButtonCapabilities.KEY_LONG_PRESS_AVAILABLE, longPressAvailable);


same as above

joeygrover added 20 commits November 8, 2019 12:45

Add RAI and supporting classes

13790e2

Fix jsdoc

7f32e4c

Add RAI Response and supporting RPCs

5a6452c

Merge branch 'develop' of https://github.com/smartdevicelink/sdl_java…

767cd07

…script_suite into feature/mvp_rpcs # Conflicts: # lib/js/rpc/enums/FunctionID.js

Update enums static members

4e83fb8

Update static members for messages

ffa6e98

Update static members in structs

5f488e5

Add jsDoc back to FunctionId

73d944f

Add Show and response

22be335

Fix messages static member access

d258b2d

Fix struct static member access

e9c066e

Finish TextField

75d8451

Merge branch 'develop' of https://github.com/smartdevicelink/sdl_java…

1561b03

…script_suite into feature/mvp_rpcs

Move added RPCs to new structure

ef57894

Fix AddCommand jsdoc

e22461d

Add PutFile

feb4ce8

Fix incorrect set for bulkData

f3eca31

Add fileData setter in putFile

93f8d52

Add SetAppIcon

92234ef

Add OnHMIStatus

db70750

russjohnson09 approved these changes Nov 14, 2019

View reviewed changes

vladmu reviewed Nov 19, 2019

View reviewed changes

lib/rpc_spec/MOBILE_API.xml Outdated Show resolved Hide resolved

vladmu reviewed Nov 19, 2019

View reviewed changes

lib/js/src/rpc/enums/BitsPerSample.js Outdated Show resolved Hide resolved

nickschwab suggested changes Nov 19, 2019

View reviewed changes

o-mishch reviewed Nov 20, 2019

View reviewed changes

lib/js/src/rpc/messages/RegisterAppInterface.js Show resolved Hide resolved

o-mishch reviewed Nov 20, 2019

View reviewed changes

lib/js/src/rpc/messages/RegisterAppInterface.js Show resolved Hide resolved

o-mishch reviewed Nov 20, 2019

View reviewed changes

lib/js/src/rpc/messages/RegisterAppInterface.js Outdated Show resolved Hide resolved

o-mishch reviewed Nov 20, 2019

View reviewed changes

lib/js/src/rpc/messages/RegisterAppInterface.js Outdated Show resolved Hide resolved

joeygrover commented Nov 20, 2019

View reviewed changes

lib/js/src/rpc/messages/RegisterAppInterfaceResponse.js Outdated Show resolved Hide resolved

joeygrover commented Nov 20, 2019

View reviewed changes

lib/js/src/rpc/messages/RegisterAppInterfaceResponse.js Outdated Show resolved Hide resolved

joeygrover commented Nov 20, 2019

View reviewed changes

lib/js/src/rpc/messages/RegisterAppInterfaceResponse.js Outdated Show resolved Hide resolved

joeygrover commented Nov 20, 2019

View reviewed changes

lib/js/src/rpc/messages/Show.js Show resolved Hide resolved

joeygrover commented Nov 20, 2019

View reviewed changes

lib/js/src/rpc/messages/Show.js Show resolved Hide resolved

joeygrover commented Nov 20, 2019

View reviewed changes

lib/js/src/rpc/structs/ButtonCapabilities.js Show resolved Hide resolved

joeygrover commented Nov 20, 2019

View reviewed changes

lib/js/src/rpc/structs/SoftButton.js Show resolved Hide resolved

joeygrover and others added 6 commits November 20, 2019 11:38

Apply suggestions from code review

b5e4b86

Co-Authored-By: Nick Schwab <nick.schwab@livio.io>

Update lib/js/src/rpc/messages/RegisterAppInterface.js

cf1220a

Update lib/js/src/rpc/messages/RegisterAppInterfaceResponse.js

0603f45

Add validateType for TODOs

1c1a09b

Remove rpc spec xml

0b421fc

Make MAP member private

ef5af44

joeygrover commented Nov 20, 2019

View reviewed changes

nickschwab suggested changes Nov 20, 2019

View reviewed changes

Enums updated functions to match name

aad9c45

smartdevicelink deleted a comment from nickschwab Nov 21, 2019

o-mishch reviewed Nov 21, 2019

View reviewed changes

lib/js/src/rpc/messages/AddCommand.js Show resolved Hide resolved

Apply suggestions from code review 2

65ceefb

Co-Authored-By: Nick Schwab <nick.schwab@livio.io>

o-mishch reviewed Nov 21, 2019

View reviewed changes

lib/js/src/rpc/messages/AddCommand.js Show resolved Hide resolved

nickschwab approved these changes Nov 21, 2019

View reviewed changes

joeygrover merged commit c3dadc2 into develop Nov 21, 2019

joeygrover deleted the feature/mvp_rpcs branch November 21, 2019 16:09

o-mishch reviewed Nov 21, 2019

View reviewed changes

smartdevicelink locked as off-topic and limited conversation to collaborators Nov 21, 2019

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Add base RPCs and supporting classes #24

Add base RPCs and supporting classes #24

joeygrover commented Nov 13, 2019 •

edited

Loading

vladmu commented Nov 19, 2019 •

edited

Loading

joeygrover commented Nov 19, 2019

nickschwab left a comment

joeygrover left a comment

nickschwab left a comment

o-mishch Nov 21, 2019

o-mishch Nov 21, 2019

o-mishch Nov 21, 2019

o-mishch Nov 21, 2019

Add base RPCs and supporting classes #24

Add base RPCs and supporting classes #24

Conversation

joeygrover commented Nov 13, 2019 • edited Loading

Risk

Summary

Tasks Remaining:

CLA

vladmu commented Nov 19, 2019 • edited Loading

joeygrover commented Nov 19, 2019

nickschwab left a comment

Choose a reason for hiding this comment

joeygrover left a comment

Choose a reason for hiding this comment

nickschwab left a comment

Choose a reason for hiding this comment

o-mishch Nov 21, 2019

Choose a reason for hiding this comment

o-mishch Nov 21, 2019

Choose a reason for hiding this comment

o-mishch Nov 21, 2019

Choose a reason for hiding this comment

o-mishch Nov 21, 2019

Choose a reason for hiding this comment

joeygrover commented Nov 13, 2019 •

edited

Loading

vladmu commented Nov 19, 2019 •

edited

Loading