Skip to content
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

Feature/RC SEAT #2208

Merged
merged 4 commits into from
Jul 13, 2018
Merged

Feature/RC SEAT #2208

merged 4 commits into from
Jul 13, 2018

Conversation

LitvinenkoIra
Copy link
Contributor

@LitvinenkoIra LitvinenkoIra commented May 23, 2018

This implementation is based on RPC design refactoring, so this PR should be merged after #2250

Technical task : #1860
This PR is ready for review.

This pull request contains the implementation of new remote control module - SEAT
(proposal: https://github.com/smartdevicelink/sdl_evolution/blob/master/proposals/0105-remote-control-seat.md)
For testing use the following test set:
https://github.com/smartdevicelink/sdl_atf_test_scripts/blob/feature/rc_seat/test_sets/rc_SEAT.txt

Pull request with ATF scripts: smartdevicelink/sdl_atf_test_scripts#1910

For manual testing use:
HMI from : smartdevicelink/sdl_hmi#52
mobile application SPT from HockeyApp 20180213-Nightly-Android

Risk

This PR makes [major] API changes.

CLA

@BSolonenko BSolonenko force-pushed the feature/rc_refactoring branch 2 times, most recently from 4a97805 to 4ce2e58 Compare May 23, 2018 13:58
@LitvinenkoIra LitvinenkoIra force-pushed the feature/rc_refactoring branch from 4ce2e58 to 379e61b Compare May 24, 2018 11:05
@LitvinenkoIra LitvinenkoIra mentioned this pull request May 29, 2018
1 task
for (const auto& param : params) {
if (param.first == module_type) {
if (rc_capabilities.keyExists(param.second)) {
is_module_type_valid = true;

Choose a reason for hiding this comment

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

add a break; after set is_module_type_valid = true?

bool module_type_and_data_match = false;
for (const auto& data : data_mapping) {
if (data.first == module_type) {
module_type_and_data_match = module_data.keyExists(data.second);

Choose a reason for hiding this comment

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

break after it is found

@@ -1224,6 +1224,9 @@
</element>
<element name="MOBILE_APP">
</element>
<element name="RADIO_TUNER">

Choose a reason for hiding this comment

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

this does not relate to SEAT, please remove from this PR

@@ -469,6 +469,25 @@
"signalStrengthAvailable": true,
"stateAvailable": true
}
],
"seatControlCapabilities": [

Choose a reason for hiding this comment

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

missing moduleName
"moduleName" : "driver seat"
or "moduleName" : "DRIVER"

@LitvinenkoIra
Copy link
Contributor Author

@yang1070 Fixed all comments c92e0f6

Copy link

@yang1070 yang1070 left a comment

Choose a reason for hiding this comment

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

Looks good.

@yang1070
Copy link

yang1070 commented Jun 4, 2018

@theresalech Ford has reviewed, tested and approved this PR, It is ready for Livio to review.

@yang1070
Copy link

@theresalech @LuxoftAKutsan
This pull request contains the implementation of new remote control module - SEAT.
Ford approved this PR.
It shall be rebased to merge into develop branch after RPC refactoring work.

@LitvinenkoIra LitvinenkoIra changed the base branch from feature/rc_refactoring to feature/vehicle_info_plugin June 26, 2018 09:11
@LitvinenkoIra LitvinenkoIra force-pushed the feature/vehicle_info_plugin branch from b558a9f to 22d3d2e Compare June 26, 2018 09:53
This was referenced Jun 27, 2018
@LuxoftAKutsan LuxoftAKutsan changed the base branch from feature/vehicle_info_plugin to develop June 30, 2018 13:14
@Jack-Byrne Jack-Byrne merged commit 6c9ad77 into develop Jul 13, 2018
@Jack-Byrne Jack-Byrne deleted the feature/rc_seat branch July 13, 2018 19:06
# for free to join this conversation on GitHub. Already have an account? # to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants