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

[Accepted with Revisions] SDL 0280 - Adding new parameter of requiresAudioSupport and BluetoothDeviceAddress #941

Closed
theresalech opened this issue Feb 19, 2020 · 65 comments

Comments

@theresalech
Copy link
Contributor

theresalech commented Feb 19, 2020

Hello SDL community,

The review of the revised proposal "SDL 0280 - Adding new parameter of requiresAudioSupport and BluetoothDeviceAddress" begins now and runs through September 29, 2020. Previous reviews of this proposal took place February 19 - March 17, 2020, June 2 - 23, 2020, and July 22 - August 18, 2020. The proposal is available here:

https://github.com/smartdevicelink/sdl_evolution/blob/master/proposals/0280-Adding-new-parameter-of-requiresAudioSupport-and-BluetoothDeviceAddress.md

Reviews are an important part of the SDL evolution process. All reviews should be sent to the associated Github issue at:

#941

What goes into a review?

The goal of the review process is to improve the proposal under review through constructive criticism and, eventually, determine the direction of SDL. When writing your review, here are some questions you might want to answer in your review:

  • Is the problem being addressed significant enough to warrant a change to SDL?
  • Does this proposal fit well with the feel and direction of SDL?
  • If you have used competitors with a similar feature, how do you feel that this proposal compares to those?
  • How much effort did you put into your review? A glance, a quick reading, or an in-depth study?
    Please state explicitly whether you believe that the proposal should be accepted into SDL.

More information about the SDL evolution process is available at

https://github.com/smartdevicelink/sdl_evolution/blob/master/process.md

Thank you,
Theresa Lech

Program Manager - Livio
[email protected]

@joeygrover
Copy link
Member

1. For clarification, Google deprecated audio streaming through AOA 2.0 in Android 8.0. See here. I'm honestly not sure any OEM has implemented the audio profiles for AOA anyways.

2. bluetoothDeviceAddress is mandatory in the DeviceInfo struct, this is not necessary and causes a breaking change. In the case of iOS devices, there is no need to provide this information anyways and therefore adds extra data. This should be changed to `mandatory="false". Any place in Core/HMI that need this data should assume false if no data is provided.

3.

The current SDL Java Suite library cancels SDL launch if the requiresAudioSupport setting is TRUE and BT A2DP is not connected.
However, with this proposal, the SDL app is always launched without depending on the connection status of BT A2DP.

This completely changes how apps work today. There is no mention of this beyond "apps will launch regardless of audio state". If you go back and review the issue and PR you will see a lot of information about the issue and why the fix was put in place. I don't think this solution adequately addresses how older systems that can't be updated to support this logic will be handled. The author must take this into account as it will break OEMs implementations and create worse user experiences, ie apps showing up but playing audio through the phone speaker is worse than the app not showing up.

Issue for reference: Audio over AOA Issues
Pull request for solution: Add audio requirement ability

@stefanek22 Please pay attention to this proposal. This will break Ford's implementation, you need to review this.

4. What if bluetooth fails to connect? How will the app know? What should happen if the device is currently connected to another bluetooth device, will it disconnect from that device to await the connection from the head unit? Will the app stay registered even though there is no audio support?

5. What happens when the bluetooth adapter is off on the phone? Should the app automatically turn it on?

6. What if the device has never been paired with the IVI system before? Will the user be required to interact with their phone to accept the pair request? Is this an acceptable UX for driver distraction?

7. If a user selects the app before A2DP is connected the audio will start from the phone and transition to the IVI once connected. Does the app need to wait until the bluetooth connection is made before it starts streaming audio?

Overall I think we do need to do something better than what is there today but I don't think this is the right way to go about it.

@Kazuki-Sugimoto
Copy link
Contributor

Kazuki-Sugimoto commented Feb 25, 2020

@joeygrover -san
Thank you for your opinion.
I will reply on behalf of @Shohei-Kawano.

  1. bluetoothDeviceAddress is mandatory in the DeviceInfo struct, this is not necessary and causes a breaking change. In the case of iOS devices, there is no need to provide this information anyways and therefore adds extra data. This should be changed to `mandatory="false". Any place in Core/HMI that need this data should assume false if no data is provided.

I agree with you.

  1. This completely changes how apps work today. There is no mention of this beyond "apps will launch regardless of audio state". If you go back and review the issue and PR you will see a lot of information about the issue and why the fix was put in place. I don't think this solution adequately addresses how older systems that can't be updated to support this logic will be handled. The author must take this into account as it will break OEMs implementations and create worse user experiences, ie apps showing up but playing audio through the phone speaker is worse than the app not showing up.

After launching the app, display a message prompting automatic connection or BT connection.
I think that UX is much better than before.

  1. What if bluetooth fails to connect? How will the app know? What should happen if the device is currently connected to another bluetooth device, will it disconnect from that device to await the connection from the head unit? Will the app stay registered even though there is no audio support?

For these, a message is displayed to the user.

  1. What happens when the bluetooth adapter is off on the phone? Should the app automatically turn it on?

If the bluetooth adapter is off, the bluetooth address cannot be obtained, so just display a message.

  1. What if the device has never been paired with the IVI system before? Will the user be required to interact with their phone to accept the pair request? Is this an acceptable UX for driver distraction?

The implementation depends on the HMI, but I think it prompt the user with a message or voice requesting pairing later, not immediately while driving.

  1. If a user selects the app before A2DP is connected the audio will start from the phone and transition to the IVI once connected. Does the app need to wait until the bluetooth connection is made before it starts streaming audio?

I think the app might be better to wait.

Overall I think we do need to do something better than what is there today but I don't think this is the right way to go about it.

I don't think the current UX that silently rejects connection at least if BT is not connected is not good.
The user should be prompted for a BT connection or, if possible, automatically.
What if you have a better idea?

@joeygrover
Copy link
Member

@Kazuki-Sugimoto

3. I am going to have to ask for more explanation to this and some other answers. For example, just stating a message will be displayed to the user is not a satisfactory response. What does the message say? Where will be the message be displayed? What triggers the display of this message specifically? You also never addressed what happens to current systems.

4. See 3.

5. See 3.

6. So the app will be displayed on the HMI, and the user can activate the app but the sound will not be streamed to the IVI. How will the app ever know if the IVI system is still trying to connect bluetooth or if the feature has failed?

7. This is asking a lot from the developer and we would have to define specific requirements for SDL to make sure all apps follow the same behavior. Would the library have this ability or sample code for them? What does that look like?

Currently app developers can change this flag in their settings and display their own messages on the IVI that would tell the user to connect bluetooth and that requires no change to SDL.
For other solutions, why not just ask the user to connect bluetooth before displaying their apps on screen? Make it a requirement for AOA to work that a bluetooth connection must also be present. Or the HMI can display a message that includes info "For media/music applications to use SDL you must connect bluetooth" after an AOA connection? Have you looked into the phone requesting to pair and connect instead of the IVI system?

@theresalech
Copy link
Contributor Author

The Steering Committee voted to keep this proposal in review, to allow the author time to respond to comments and for additional conversation on the review issue.

@Kazuki-Sugimoto
Copy link
Contributor

@joeygrover -san

  1. I am going to have to ask for more explanation to this and some other answers. For example, just stating a message will be displayed to the user is not a satisfactory response. What does the message say?

"BT connection required to launch Media app."

Where will be the message be displayed?

Display on the HU.

What triggers the display of this message specifically?

When an app is selected in the HMI app list (when BT is not connected and BT address is unknown).

You also never addressed what happens to current systems.

I think that there is no effect on cases other than media applications.

  1. So the app will be displayed on the HMI, and the user can activate the app but the sound will not be streamed to the IVI. How will the app ever know if the IVI system is still trying to connect bluetooth or if the feature has failed?

It is assumed that the app will not be launched unless a BT connection is established.

This is asking a lot from the developer and we would have to define specific requirements for SDL to make sure all apps follow the same behavior. Would the library have this ability or sample code for them? What does that look like?

The expression "the app might be better to wait" in my answer was incorrect.
It does not do anything on the app side. I wanted to explain that the app does nothing until launched.

The BT connection is determined by the HMI when the app is selected from the app list.
Then the app is launched or the message is displayed.

Currently app developers can change this flag in their settings and display their own messages on the IVI that would tell the user to connect bluetooth and that requires no change to SDL.

I do not know this feature. Is it listed somewhere?

For other solutions, why not just ask the user to connect bluetooth before displaying their apps on screen?

Sorry for making you misunderstand. Same assumption.

Make it a requirement for AOA to work that a bluetooth connection must also be present.

Does that mean that USB and BT connections are required?
It might also be good, but it can be bad UX for users who don't need it.

Or the HMI can display a message that includes info "For media/music applications to use SDL you must connect bluetooth" after an AOA connection?

If this is displayed every time, it is also likely to be bad UX for users.

Have you looked into the phone requesting to pair and connect instead of the IVI system?

I don't understand the meaning of the question, how does it relate to this proposal?

@joeygrover
Copy link
Member

3.

I think that there is no effect on cases other than media applications.

That's the entire issue though. Media apps need a way to work both with older implementations and this feature. If they only use this feature, the issue will regress that was fixed previously. So older systems, these media apps will connect over AOA with no way to connect bluetooth automatically; then the media would simply play out of the mobile device again. This has to be addressed.

8. I just noticed DeviceInfo was set to mandatory="true" as well. This is not necessary and should be changed back to mandatory="false".

Have you looked into the phone requesting to pair and connect instead of the IVI system?

I don't understand the meaning of the question, how does it relate to this proposal?

I brought this as a point because Android can programmatically pair/connect with the IVI, so if driver distraction is an issue, it might be possible to do this in the background without user interaction. This would make the feature better and less distracting.


I think it's important that all these details are provided in the proposal so OEMs know how this feature might be implemented. While the idea of the proposal is simple, the implications it has are much larger. So it would be helpful if you could provide a complete flow chart of what messages are sent when with the previously discussed situations.

From a high level I understand the feature to work as:

  1. IVI connects with mobile device over AOA.
  2. Application registers with requiresAudioSupport param set to true and bluetoothDeviceAddress attached to the DeviceInfo struct in the RAI.
  3. If phone has previously paired with the IVI, IVI will attempt to automatically connect bluetooth. If it has not, the HMI will prompt the user to pair the mobile device when it is safe to do so.
  4. If the device is not paired or connected over bluetooth to the IVI at the time of activating an app that had the requiresAudioSupport set true, the IVI will not put the app into HMI_FULL until that happens (paired and connected).

@Kazuki-Sugimoto
Copy link
Contributor

@joeygrover -san

That's the entire issue though. Media apps need a way to work both with older implementations and this feature. If they only use this feature, the issue will regress that was fixed previously. So older systems, these media apps will connect over AOA with no way to connect bluetooth automatically; then the media would simply play out of the mobile device again. This has to be addressed.

Can we address this problem by adding capability?
If there is no capability that indicates whether there is a BT automatic connection function, perform the conventional movement (do not send RAI).

  1. I just noticed DeviceInfo was set to mandatory="true" as well. This is not necessary and should be changed back to mandatory="false".

You're right.

I brought this as a point because Android can programmatically pair/connect with the IVI, so if driver distraction is an issue, it might be possible to do this in the background without user interaction. This would make the feature better and less distracting.

Does the pairing / connection include the first time (never connected)?
Is it an app that does it? OS?
I looked into it and found no information that seemed relevant. Is there a URL that can be helpful?

From a high level I understand the feature to work as:

There is no difference in your understanding of this feature.
As you suggested, I want to add a message flowchart (sequence diagram).

@mrapitis
Copy link
Contributor

mrapitis commented Mar 3, 2020

From the Ford perspective we have concerns regarding breaking fixes put in place for the resolved issue referenced below:
smartdevicelink/sdl_java_suite#1056

In general, audio over AOA is not well supported across handsets with many implementations behaving differently.

The statement below would break backwards compatibility with existing production head units in the field:

However, with this proposal, the SDL app is always launched without depending on the connection status of BT A2DP

@joeygrover
Copy link
Member

3. The issue is that the feature currently ignores the transport connection notification if there is no audio streaming (A2DP, Audio over APA, etc) available. This means the app does not register at all on the IVI system and no messages are sent. This works best for current production head units given the current specs.

The problem with adding a capability is that the app won't be notified of the IVI's capabilities until after the app has registered. This is due to the face the IVI system would have to know the app is a MEDIA app and then include the capability into the RAI response. This means on an a current production system the app will appear connected and then disappear without informing the user of why. App developers currently have the option to allow the MEDIA app to connect through AOA with no audio streaming available and prompt the user to connect such methods through their UI (Show, Alert, etc). I agree this solution is not great as it puts the burden on app developers and there is no standard messaging put in place for all MEDIA apps in this situation.

I believe current production IVI system owners will have to chime in on how they want to handle this feature and what they find acceptable in terms of UX.

9. (Numbering this for reference)
As of API level 19 the createBond() method is available on a BluetoothDevice object. See the Android documentation here. That should allow the phone to at least prompt the user to pair with the IVI programmatically. I might have been mistaken on this point that it could happen without user interaction, but I do believe now that it is still required so it likely does not make the situation much better as the feature currently stands.

10. (Numbering this for reference)
Ok, yes I think having the flow chart will help. Also, in the case a MEDIA app does not register with a bluetooth MAC address or the requiresAudioSupport flag set to true, would you expect Core to reject the RAI request?

@theresalech
Copy link
Contributor Author

During the Steering Committee meeting, the Ford team expressed their concern about this proposal breaking their current implementation and suggested continuing the behavior currently in place. The author requested that the proposal be kept in review for another week, so they could review the last few comments on the issue and respond. The Steering Committee agreed to keep this proposal in review until the 2020-03-10 meeting.

@Kazuki-Sugimoto
Copy link
Contributor

Kazuki-Sugimoto commented Mar 5, 2020

@mrapitis -san

However, with this proposal, the SDL app is always launched without depending on the connection status of BT A2DP

Sorry. The sentence we wrote was wrong, therefore misleading. It should be, "the SDL app is always registered", not launched.

By adding the capability that indicates the BT automatic connection function, the existing operation is maintained. Therefore, we assume that there will be no problem regarding that matter.


@joeygrover -san

  1. The issue is that the feature currently ignores the transport connection notification if there is no audio streaming (A2DP, Audio over APA, etc) available. This means the app does not register at all on the IVI system and no messages are sent. This works best for current production head units given the current specs.

If the transport connection is ignored, users will not know which apps and for what reason those apps were not registered. We want to improve this as we have mentioned in our proposal.

The problem with adding a capability is that the app won't be notified of the IVI's capabilities until after the app has registered. This is due to the face the IVI system would have to know the app is a MEDIA app and then include the capability into the RAI response. This means on an a current production system the app will appear connected and then disappear without informing the user of why.

The core will refer to these following three items. First, the requiresAudioSupport that is to be added to the RAI request. Second, the BT connection status. And third, the capability that indicates BT automatic connection function.

So, when requiresAudioSupport is true, but there is no BT connection and the capability is false, the RAI request will be rejected. What do you think about this kind of behavior?

I believe current production IVI system owners will have to chime in on how they want to handle this feature and what they find acceptable in terms of UX

Does this mean that users can choose whether or not to automatically connect to BT?

  1. As of API level 19 the createBond() method is available on a BluetoothDevice object. See the Android documentation here. That should allow the phone to at least prompt the user to pair with the IVI programmatically. I might have been mistaken on this point that it could happen without user interaction, but I do believe now that it is still required so it likely does not make the situation much better as the feature currently stands.

Thank you for providing information. I would like to refer to it as needed.

  1. Ok, yes I think having the flow chart will help. Also, in the case a MEDIA app does not register with a bluetooth MAC address or the requiresAudioSupport flag set to true, would you expect Core to reject the RAI request?

In order to protect the existing operation, when the capability of BT automatic connection function (for now, we'll refer to it as AutoBTCapability) is false, under applicable conditions, it is expected to reject the RAI request, just like in the existing one.

The following cases are assumed:

  • In case of RAI (Core's behavior)
  1. When requiresAudioSupport is true and there is a BT connection, RAI is successful and the app is registered.
  2. When both requiresAudioSupport and AutoBTCapability are true, but there is no BT connection, RAI is successful and the app is registered.
  3. When requiresAudioSupport is true, but AutoBTCapability is false and there is no BTconnection, RAI is rejected.
  4. When requiresAudioSupport is false, RAI is successful and the app is registered.
  • In case of MEDIA app launch (HMI's behavior)
  1. When there is BT connection, the app is launched.
  2. When there is no BT connection but BT address is registered, automatically connect BT.
    2-1. When the BT automatic connection is successful, the app is launched.
    2-2. When the BT automatic connection fails, a message that prompts BT connection is displayed.
  3. When there is no BT connection and BT address is not registered, a message that prompts BT connection is displayed.

@Shohei-Kawano
Copy link
Contributor

@mrapitis -san
We believe it very important to consider the impact on already released HUs.
Nexty team @Kazuki-Sugimoto has responded with comments on how to mitigate the impact.
We think this is a very important discussion, so please answer as soon as possible.
If we have an impact that we are unaware of, then giving us a specific impact will help us figure out how to avoid it.

Thank you.

@mrapitis
Copy link
Contributor

mrapitis commented Mar 10, 2020

@Shohei-Kawano @Kazuki-Sugimoto one concern around the AutoBTCapability flag would be that the app would need to set / maintain the value based on the head unit that its connected to. An existing head unit implementation would require AutoBTCapability set to false while potential future implementations could accept true. If this work could automatically be managed at the proxy level it may take some burden away from the app developer.

@joeygrover
Copy link
Member

If the transport connection is ignored, users will not know which apps and for what reason those apps were not registered. We want to improve this as we have mentioned in our proposal.

For older IVI systems, if the apps connect over USB with no BT connection, they will register and then disappear, users will not know for what reason those apps were registered but then disappeared.

For older IVI systems, if the apps connect over USB, register, stay connected and then the user selects the app to be launched, the app will then play music out of the mobile phone speaker, users will not know for what reason SDL is not handling this.

These items MUST be addressed in this proposal. There needs to be a specific section of what the use case is when the new version of the proxy that includes this feature connects to an older version of Core that does not support this feature.

... one concern around the AutoBTCapability flag would be that the app would need to set / maintain the value based on the head unit that its connected to. An existing head unit implementation would require AutoBTCapability set to false while potential future implementations could accept true. If this work could automatically be managed at the proxy level it may take some burden away from the app developer.

The only way an app knows what head unit it is connected to is after the RAI response. This will cause the app to flash on the screen before being removed. This is worse than not showing the app in my opinion.

@theresalech
Copy link
Contributor Author

Per the author's request, the Steering Committee voted to keep this proposal in review for an additional week, to allow the author time to respond to the questions posted to the review issue. It was emphasized that the author will need to respond to the major concerns of this feature breaking current implementations (see: this comment and this comment) before the next Steering Committee meeting. It was noted that it will be important to take action on this proposal next week, and if the reviewers' concerns cannot be addressed by the author, we may need to move to reject this proposal.

@Kazuki-Sugimoto
Copy link
Contributor

For older IVI systems, if the apps connect over USB with no BT connection, they will register and then disappear, users will not know for what reason those apps were registered but then disappeared.
For older IVI systems, if the apps connect over USB, register, stay connected and then the user selects the app to be launched, the app will then play music out of the mobile phone speaker, users will not know for what reason SDL is not handling this.

The core version is determined based on the protocol version obtained in StartSession response. The audio requirement judgment that was performed before StartSession will be moved to before RAI, and the core that do not support this feature will not send RAI instead of ignoring the transport connection. I suggest this, what do you think?

These items MUST be addressed in this proposal. There needs to be a specific section of what the use case is when the new version of the proxy that includes this feature connects to an older version of Core that does not support this feature.

I will add the use case and flowcharts for this case as well.

@Jack-Byrne
Copy link
Contributor

Jack-Byrne commented Mar 16, 2020

The core version is determined based on the protocol version obtained in StartSession response.

Note that the version sent in the StartServiceAck is the negotiated protocol version, not the version of Core or the RPC spec. I believe tying this use case to a specific protocol version would be a major version change. This approach could work, but it means all headunits implementing SDL Core would have to support this feature for apps negotiating with the new protocol version.

@joeygrover
Copy link
Member

joeygrover commented Mar 16, 2020

Well Core version is directly tied to protocol version nor the other way around. While it can be mostly assumed, since they are individual projects with their own specific versioning using one to determine the other is not something that should be done. Instead, features themselves can be tied directly to the protocol version.

With that in mind, moving the check into the StartService protocol message for the RPC service is a potential path forward but does result in a very complicated flow.

Prerequisites:

  • The protocol version will need to be incremented for a major change.
  • Using the current library apps will not register at all without audio support. So it is assumed in this flow the app will only be using a compatible library.
  1. The developer will set their requiresAudioSupport to true.
  2. The library will send a StartService for the RPC service with a new param in the payload, requiresAudioSupport.
  3. Core will receive the StartService for the RPC service:
    1. Core will check it's current audio connects (BT A2DP), if it has a supported audio connect it will continue in flow.
    2. If there is no Audio support, core will check if it supports the auto connect BT feature. If it does it will continue in flow.
    3. If there is no supported audio methods and Core does not support the auto connect BT feature, it will send a StartServiceNAK with the reason No audio support available.
  4. If Core has continued, it will send a StartServiceACK with a new param autoBTCapability set to true.
  5. The app receives the response to its StartService for the RPC service:
    1.If the response was a StartServiceNAK the app will shutdown.
    2. If the response was a StartServiceACK, requiresAudioSupport was set to true, but the protocol version of the ACK is less than the major version of this feature, the app will shutdown.
    3. If the response was a StartServiceACK, requiresAudioSupport was set to true, the protocol version of the ACK is equal or greater than the major version of this feature, and the autoBTCapability flag is set to false, the app will check if audio support is available using the MediaStreamingStatus class. If it is available it will continue, if not it will shutdown.
    4. If the response was a StartServiceACK, requiresAudioSupport was set to true, the protocol version of the ACK is equal or greater than the major version of this feature, and the autoBTCapability flag is set to true, the app will continue.
  6. The app will send its RegisterAppInterface which will include the hmiTypes and isMediaApplication flag.
  7. Core will receive the RegisterAppInterface:
    1. If the requiresAudioSupport flag was not included in the StartService, thehmiType of MEDIA isMediaApplication is set to true in the RAI and the protocol version for the session is less than the major version of the version that included this feature it will send a RegisterAppInterface response with success=false and deny the app's registration.
    2. If the requiresAudioSupport flag was not included in the StartService, the hmiType of MEDIA is included isMediaApplication is set to true in the RAI and the protocol version for the session is equal to or greater than the major version of the version that included this feature it will send a RegisterAppInterface response with success=true but not move forward with this proposals feature.
    3. If the requiresAudioSupport flag was set to true in the StartService, the hmiType of MEDIA is included isMediaApplication is set to true in the RAI and the protocol version for the session is equal to or greater than the major version of the version that included this feature it will send a RegisterAppInterface response with success=true and move forward with this proposals feature.

I might have made an error or two in the flow, but I believe that would be the requirements for a check that ensured old IVI systems still worked as well as preventing flashing of the app.

EDIT: Changed from using hmiType to using isMediaApplication flag.

@Jack-Byrne
Copy link
Contributor

@joeygrover Is there an advantage to requiring the MEDIA appHMIType instead of using the isMediaApplication in the RAI request?

@joeygrover
Copy link
Member

@JackLivio actually no. that's a good point. I will update.

@Kazuki-Sugimoto
Copy link
Contributor

@joeygrover -san
Thanks for the supplement.
Will the proposal be accepted if it is revised based on that plan?

@joeygrover
Copy link
Member

@Kazuki-Sugimoto I can't guarantee it will be accepted. Other members will need to review the updated proposal to make sure it works for them. I would ask that @mrapitis review my comment and hopefully we can vote to "Return for Revisions". If that is the result of the vote and once the proposal is updated, it will be brought back into review again to make sure the feature as a whole works for all members and is agreed upon.

@mrapitis
Copy link
Contributor

I have also reviewed the comment with the suggested flow, it does make sense and may be an adequate solution for the scenarios that were previously discussed. The flow seem fairly complicated, however I do not believe their is much of a way to simply. It would be beneficial if the proposal was updated to include a state diagram with the suggested flow as it may make it easier for reviewers to follow.

@theresalech theresalech changed the title [In Review] SDL 0280 - Adding new parameter of requiresAudioSupport and BluetoothDeviceAddress [Returned for Revisions] SDL 0280 - Adding new parameter of requiresAudioSupport and BluetoothDeviceAddress Mar 18, 2020
@theresalech
Copy link
Contributor Author

The Steering Committee voted to return this proposal for the revisions included in this comment.

@theresalech
Copy link
Contributor Author

theresalech commented Sep 23, 2020

The author has updated this proposal based on the Steering Committee's agreed upon revisions, and the revised proposal is now in review until September 29, 2020.

The specific items that were updated since the last review can be found in this merged pull request: #1075.

@smartdevicelink smartdevicelink unlocked this conversation Sep 23, 2020
@theresalech theresalech changed the title [Returned for Revisions] SDL 0280 - Adding new parameter of requiresAudioSupport and BluetoothDeviceAddress [In Review] SDL 0280 - Adding new parameter of requiresAudioSupport and BluetoothDeviceAddress Sep 23, 2020
@joeygrover
Copy link
Member

Note: Starting numbering over for the revised proposal.

1.

If the response was a StartServiceACK, requiresAudioSupport was set to true, but the protocol version of the ACK is less than the major version of this feature, the app will shut down.

This needs to include the note about the MediaStreamingStatus class and its usage. For example, if the protocol version is lower, the library could and should check if audio is already connected. Otherwise, apps that require audio support wouldn't connect to anything currently in production. The flow chart will likely also have to be updated to reflect this change.

2.

By using the value of isAudioOutputAvailable() method on MediaStreamingStatus class, the Java Suite library can make a decision whether BT A2DP is connected like below:

The MediaStreamingStatus class actually checks for multiple Audio Output options. I just want to make sure that's clear.

3.

With the changes of the flow, it is necessary to do refactoring of the Java Suite library to move the logic of the MediaStreamingStatus class from the preprocessing of the StartService to the preprocessing of the RegisterAppInterface.

I think this is better stated as post-processing of StartService ACK or NAK.

4. I don't see any description of how the code will be refactored as mentioned, but with the upcoming release a lot of these classes are not accessible to developers so it can be considered implementation details that the project maintainer will have have discretion over unless any other members would like to weigh in beforehand.

All these points are fairly minor and I think overall we could move to accept with those revisions if everything is agreed upon here.

@Akihiro-Miyazaki
Copy link
Contributor

@joeygrover -san,
1.
We understood.
We will modify the sequence like below.
Does this modification match your advice?
Modify_Sequence

That's right.
In this proposal, we will use the value of isAudioOutputAvailable() method which is the one of functions on MediaStreamingStatus class.

OK.
We will modify to replace the sentense from "from the preprocessing of the StartService to the preprocessing of the RegisterAppInterface" to "to the post-processing of StartService ACK or NAK".
Does this modification match your advice?

@theresalech
Copy link
Contributor Author

@Akihiro-Miyazaki, thanks for your responses! It appears we are in agreement on items 1 - 3. If you could please confirm your agreement with item 4 as well, we can ask the Steering Committee to vote on accepting this proposal with these 4 revisions during tomorrow's meeting.

@Akihiro-Miyazaki
Copy link
Contributor

@joeygrover -san, @theresalech -san,
4.
Since there is no requirement about the implementation, we agree your suggestion.
We would like to depend on the project maintainer about the implementation.
Thank you in advance for your cooperation.

@theresalech theresalech changed the title [In Review] SDL 0280 - Adding new parameter of requiresAudioSupport and BluetoothDeviceAddress [Accepted with Revisions] SDL 0280 - Adding new parameter of requiresAudioSupport and BluetoothDeviceAddress Sep 30, 2020
@theresalech
Copy link
Contributor Author

The Steering Committee voted to accept this proposal with the following revisions:

  1. Modify flow chart to incorporate sequence in this comment and revise the section below to include the note about the MediaStreamingStatus class and its usage. For example, if the protocol version is lower, the library could and should check if audio is already connected. Otherwise, apps that require audio support wouldn't connect to anything currently in production.

If the response was a StartServiceACK, requiresAudioSupport was set to true, but the protocol version of the ACK is less than the major version of this feature, the app will shut down.

  1. Specify in MediaStreamingStatus class section of proposal, that feature will use value of isAudioOutputAvailable() method, since the MediaStreamingStatus class actually checks for multiple Audio Output options.
  2. In MediaStreamingStatus class section, change “…to the preprocessing of the RegisterAppInterface" to "…to the post-processing of StartService ACK or NAK".
  3. Specify in proposal that since there aren't any public code changes listed in the proposal, the SDLC Project Maintainer will have discretion over implementation details, including changes to classes that are not accessible to developers, especially given changes to Java Suite library in 5.0 release.

@theresalech
Copy link
Contributor Author

@Akihiro-Miyazaki , please advise when a new PR has been entered to update the proposal to reflect the agreed upon revisions. I'll then merge the PR so the proposal is up to date, and enter issues in impacted repositories for implementation. Thanks!

@theresalech
Copy link
Contributor Author

Proposal has been updated and implementation issues have been entered:

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

8 participants