Skip to content

Update remote control radio parameter - #93

Merged
jacobkeeler merged 8 commits into
developfrom
feature/rc_radio_parameter_update
Aug 28, 2018
Merged

Update remote control radio parameter #93
jacobkeeler merged 8 commits into
developfrom
feature/rc_radio_parameter_update

Conversation

@ghost

@ghost ghost commented Jun 15, 2018

Copy link
Copy Markdown

Technical task: smartdevicelink/sdl_core#2162
Add new parameter to RadioControlData:

  • hdRadioEnable
    Update max value from 3 to 7 for the next parameters:
  • availableHDs
  • hdChannel
    Add new radio control capabilities:
  • hdRadioenableAvailable
  • siriusxmRadioAvailable

false
],

hdChannelAvailableCurrent:[

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

why it is an array?

Copy link
Copy Markdown
Author

Choose a reason for hiding this comment

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

@yang1070 This is array, because we can access the array to index which that the matches with available channel. And that's why we can to take off channel on the radio view. If it was a structure, we doesn't can to do that.

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

@ValeriiMalkov thank you

@yang1070 yang1070 left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

There need a change in the case that when hdRadioEnabled changes from false to true, the OnInteriorVehicleData notifications shall include all parameters related to HD radio, not just the hdRadioEnabled parameter only. Similarly, for the response of SetInteriorVehicleData that turns on the hdRadio, all HD related parameters shall be included.

@yang1070

Copy link
Copy Markdown

Another requirement is that the XM radio channel number shall be the same as in the frequency integer. When click a XM radio station name in HMI, the frequency integer shall show the XM channel number.
When set a XM with channel number, HMI shall select the corresponding station number.

@ghost

ghost commented Jun 15, 2018

Copy link
Copy Markdown
Author

@yang1070 You can see fix hdRadioEnabled with related parameters at the commit 17dc458

@yang1070

Copy link
Copy Markdown

@ValeriiMalkov I haven't see any update for the XM radio channel number update so far.

@LitvinenkoIra

Copy link
Copy Markdown
Contributor

@yang1070 Sorry, updates for XM radio channel number are still in progress

@BSolonenko

Copy link
Copy Markdown

@yang1070 In commit a199fb7, I made the necessary changes to the XM radio.
Also, in b4ad697 I added the file with RC capabilities.

@BSolonenko
BSolonenko force-pushed the feature/rc_radio_parameter_update branch 7 times, most recently from 490daba to cb828c0 Compare June 21, 2018 11:01
Comment thread app/model/media/RadioModel.js Outdated
}
break;
default:
properties.push('signalStrength');

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

these three properties shall be in AM and FM too

@BSolonenko
BSolonenko force-pushed the feature/rc_radio_parameter_update branch 2 times, most recently from 12fa09d to 2d71ea6 Compare June 22, 2018 16:13

@yang1070 yang1070 left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

The changes look good to me.

@yang1070

Copy link
Copy Markdown

@theresalech Ford reviewed, tested and approves this change.

@BSolonenko
BSolonenko force-pushed the feature/rc_radio_parameter_update branch 2 times, most recently from e0d4cd1 to c80210a Compare June 23, 2018 14:58
@jacobkeeler

Copy link
Copy Markdown
Contributor

@ValeriiMalkov please fix merge conflicts

@AKalinich-Luxoft
AKalinich-Luxoft force-pushed the feature/rc_radio_parameter_update branch 2 times, most recently from e2f2180 to 6c6514a Compare August 20, 2018 17:15
@AKalinich-Luxoft
AKalinich-Luxoft force-pushed the feature/rc_radio_parameter_update branch 3 times, most recently from cdd76ac to 33ca053 Compare August 20, 2018 18:24
@AKalinich-Luxoft

Copy link
Copy Markdown
Contributor

@jacobkeeler all merge conflicts have been resolved

@ghost
ghost force-pushed the feature/rc_radio_parameter_update branch from ac8cd93 to 984c991 Compare August 27, 2018 13:54
@ghost

ghost commented Aug 27, 2018

Copy link
Copy Markdown
Author

@jacobkeeler, PR was rebased to develop branch

Comment thread capabilities/rc_capabilities.js Outdated
longPressAvailable: false,
upDownAvailable: false
}],
audioControlCapabilities: [{

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

audioControlCapabilities already exists in this file

Copy link
Copy Markdown
Author

Choose a reason for hiding this comment

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

@jacobkeeler This issue was resolved after rebase

Comment thread capabilities/rc_capabilities.js Outdated
frontVerticalPositionAvailable: true,
headSupportHorizontalPositionAvailable: true
}],
buttonCapabilities: [{

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

buttonCapabilities already exists in this file

Copy link
Copy Markdown
Author

Choose a reason for hiding this comment

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

@jacobkeeler This issue was resolved after rebase

Comment thread index.html Outdated
src="app/view/navigationApp/baseNavigationView.js"></script>
<script type="text/javascript" src="app/view/navigationAppView.js"></script>

<!-- SDL Capabilities -->

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Copy link
Copy Markdown
Author

Choose a reason for hiding this comment

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

@jacobkeeler This issue was resolved after rebase

Comment thread capabilities/rc_capabilities.js Outdated
upDownAvailable: false
}],
climateControlCapabilities: [{
moduleName: 'Climate Control Module',

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

moduleName already exists for this capability object

Copy link
Copy Markdown
Author

Choose a reason for hiding this comment

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

@jacobkeeler This issue was resolved after rebase

@jacobkeeler jacobkeeler Aug 27, 2018

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Most of the issues were fixed, but this wasn't:

moduleName: 'primary_climate',

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

@jacobkeeler we leave rc_capabilities.js version from develop branch and add two new parameters. All other changes were reverted. See 9de70a2

@GetmanetsIrina

Copy link
Copy Markdown

@jacobkeeler, HMI is tested after rebase, works according to proposal.

@ghost
ghost force-pushed the feature/rc_radio_parameter_update branch 3 times, most recently from f076885 to b90f1a3 Compare August 27, 2018 16:10
Valerii and others added 3 commits August 27, 2018 19:24
Add new parameter to RadioControlData:
- `hdRadioEnable`
Update max value from 3 to 7 for the next parameters:
- `availableHDs`
- `hdChannel`
Add new radio control capabilities:
- `hdRadioenableAvailable`
- `siriusxmRadioAvailable`
Add parameters to notification in case
when switching hdRadio enable
@ghost
ghost force-pushed the feature/rc_radio_parameter_update branch from b90f1a3 to e0f4d08 Compare August 27, 2018 16:25
@AStasiuk

Copy link
Copy Markdown
Contributor

@jacobkeeler, all comments are processed, please have a look.

Comment thread capabilities/rc_capabilities.js Outdated
ventilationModeAvailable: true
}],
radioControlCapabilities: [{
moduleName: 'Radio Control Module',

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

missing hdRadioEnableAvailable from radioControlCapabilities

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

@JackLivio new parameters hdRadioEnableAvailable and siriusxmRadioAvailable was added. See 9de70a2

Comment thread capabilities/rc_capabilities.js Outdated
upDownAvailable: false
}],
climateControlCapabilities: [{
moduleName: 'Climate Control Module',

@jacobkeeler jacobkeeler Aug 27, 2018

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Most of the issues were fixed, but this wasn't:

moduleName: 'primary_climate',

Comment thread capabilities/rc_capabilities.js Outdated
ventilationModeAvailable: true
}],
radioControlCapabilities: [{
moduleName: 'Radio Control Module',

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Now this one is a duplicate key (see line 118)

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

@AKalinich-Luxoft
AKalinich-Luxoft force-pushed the feature/rc_radio_parameter_update branch from e0f4d08 to 0f69786 Compare August 28, 2018 07:44
@GetmanetsIrina

Copy link
Copy Markdown

@jacobkeeler , regarding the issue with the registration of several apps from different devices - bug was created #112 .
Also there is fix for this issue in #111 .

@jacobkeeler
jacobkeeler merged commit 3e81a3f into develop Aug 28, 2018
@Jack-Byrne Jack-Byrne mentioned this pull request Oct 19, 2018
@jacobkeeler
jacobkeeler deleted the feature/rc_radio_parameter_update branch March 18, 2020 13:19
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.

9 participants