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.

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

Revise SDL-0099 - Add a keepContextAvailable parameter and fix typos #601

Merged
merged 3 commits into from
Oct 3, 2018

Conversation

yang1070
Copy link
Contributor

@yang1070 yang1070 commented Oct 1, 2018

Introduction

Proposal SDL-0099 has some issues:

  1. Parameter keepContextAvailable is missing in the AudioControlCapabilities. It is used to tell the mobile applications that the vehicle can support parameter keepContext or not. Usually, for each controllable parameter, there is an available parameter in the capabilities. And keepContextAvailable is missing.
  2. It has some typos in the description of parameter keepContext.

Motivation

Fix the issues.

Proposed solution

Add a new keepContextAvailable parameter to AudioControlCapabilities.
Update the description of parameter keepContext.

Potential downsides

None the author could identify.

Impact on existing code

RPC (mobile_api and hmi_api) spec shall be updated.
Android and iOS implementations will also need to be updated to include this new parameter.
Since this is an update to the accepted but not released proposal, the impact shall be minimum.

Alternatives considered

No alternatives were considered.

@yang1070
Copy link
Contributor Author

yang1070 commented Oct 1, 2018

@theresalech it is ready for review. thank you.

Copy link
Contributor

@theresalech theresalech left a comment

Choose a reason for hiding this comment

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

@yang1070 I found one typo in the proposal file that should be fixed. Additionally, can you please update the Impact on Existing Code section of the PR description to call out that both the Android and iOS implementations of this feature will also need to be updated to include this new parameter, and update the reference to the proposal number in the PR title and Introduction section of the description to SDL-0099?

@@ -240,6 +240,9 @@ New AUDIO data types.
<param name="sourceAvailable" type="Boolean" mandatory="false">
<description>Availability of the control of audio source. </description>
</param>
<param name="keepContextAvailable" type="Boolean" mandatory="false">
<description>Availability of the keepContext paramter. </description>
Copy link
Contributor

Choose a reason for hiding this comment

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

Please fix typo "parameter"

Copy link
Contributor Author

Choose a reason for hiding this comment

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

@theresalech updated, thanks

@theresalech theresalech changed the title Revise SDL-099 - Add a keepContextAvailable parameter and fix typos Revise SDL-0099 - Add a keepContextAvailable parameter and fix typos Oct 1, 2018
@theresalech
Copy link
Contributor

theresalech commented Oct 3, 2018

The Steering Committee voted to accept this PR with the following revision: include default value (defValue) in parameter line for keepContext.

@theresalech
Copy link
Contributor

@yang1070 please advise when PR has been updated per agreed upon revision. We'll then merge so that SDL 0099 is up to date.

@yang1070
Copy link
Contributor Author

yang1070 commented Oct 3, 2018

@theresalech
I have added defvalue="false" to keepContext parameter. Thanks for you reminder.

@jordynmackool jordynmackool merged commit 3b8803f into smartdevicelink:master Oct 3, 2018
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants