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 0188 - Interior Vehicle Data resumption #554

Closed
theresalech opened this issue Jul 18, 2018 · 13 comments
Closed

Comments

@theresalech
Copy link
Contributor

Hello SDL community,

The review of "SDL 0188 - Interior Vehicle Data resumption" begins now and runs through July 24, 2018. The proposal is available here:

https://github.com/smartdevicelink/sdl_evolution/blob/master/proposals/0188-get-interior-data-resumption.md

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

#554

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
theresa@livio.io

@joeljfischer
Copy link
Contributor

What is the need for this? The main reason for data resumption up to this point is to speed up the app being ready for the user to use because of some calls that take a long time (anything that involves VR, for example). I don't have a lot of experience with RC, but does it take a long time to re-subscribe on a reconnection? Is it worth the added complexity? If so, why for RC and not for vehicle data?

@Jack-Byrne
Copy link
Contributor

Is there a reason that we don't already include all subscription types in the resumption process? (Buttons, RC, Vehicle Data, Way Points). Trade off for having subscription resumption could be the apps don't know for sure if they are still subscribed after resumption and the mobile will probably send a subscribe request anyways on reconnection.

Maybe we could add a getSubscriptions request that returns all subscribed data to the phone.

However I am not sure all of these changes are necessary.

@joeygrover
Copy link
Member

This change seems extremely arbitrary and too thinly focused. I agree with Joel's comments that this would need to be a global change for subscriptions if included. It can't only be used for a singular subscription model.

I'm wondering if this change is meant more for strictly between Core and the HMI and not including the apps' subscriptions. Basically if an app had previously subscribed to RC, there was an unexpected crash on the module side, Core should send the HMI the subscription resumption information even though Core does not reestablish the subscription with the client apps until they actually send their subscription requests through the normal means as it is today. This would essentially be a call to ready the RC data caches.

@theresalech
Copy link
Contributor Author

The Steering Committee voted to defer this proposal, keeping it in review until our next meeting on 2018-07-31, to allow the author time to respond to the comments left on the review issue.

@LuxoftAKutsan
Copy link
Contributor

Is there a reason that we don't already include all subscription types in the resumption process? (Buttons, RC, Vehicle Data, Way Points).

The original proposal of interior vehicle data subscription does not include resumption process. So now if the application was unexpected disconnected, all interior vehicle data subscriptions will be lost.

@LuxoftAKutsan
Copy link
Contributor

LuxoftAKutsan commented Jul 31, 2018

Is there a reason that we don't already include all subscription types in the resumption process?

Actually, By current requirements SDL should restore al subscriptions except for interior vehicle data:

In case hashID of the application registered equals stored value for the application, SDL must register the application according to standard process and:

1. Return RegisterAppInterface response of (success: true, resultCode: SUCCESS, info "Resume succeeded") after successful registering
2. Notify HMI by OnAppRegistered (resumeVrGrammars: true) to restore VRgrammars persisted on HMI

3.Restore application related data:
• AddCommand (Menu +VR)
• AddSubMenu
• CreateInteractionChoiceSet
• SetGlobalProperties
• SubscribeButton
• SubscribeVehicleData
- SubscribeWayPoints

4. Send the following restored data to HMI right after OnAppRegistered notification sent to HMI:
- AddCommand (Menu only)
- AddSubMenu
- CreateInteractionChoiceSet (AddCommands of the ChoiceSets)
- SetGlobalProperties
- OnButtonSubscription (isSubscribed=true)
- SubscribeVehicleData
- SubscribeWayPoints

Full data resumption requirements attached as PDF.
APPLINK-DataResumption-310718-1326-148.pdf

Trade off for having subscription resumption could be the apps don't know for sure if they are still subscribed after resumption and the mobile will probably send a subscribe request anyways on reconnection.

Application subscriptions are the same persistent data as AddCommands or SubMenus, so if the application was successfully resumed it should assume that all subscriptions still present.

Maybe we could add a getSubscriptions request that returns all subscribed data to the phone.

Maybe it could be useful, but not required

@LuxoftAKutsan
Copy link
Contributor

This change seems extremely arbitrary and too thinly focused. I agree with Joel's comments that this would need to be a global change for subscriptions if included. It can't only be used for a singular subscription model.

Global change of subscription model is good initiative, but it certainly requires major version change, isn't it?

I'm wondering if this change is meant more for strictly between Core and the HMI and not including the apps' subscriptions. Basically if an app had previously subscribed to RC, there was an unexpected crash on the module side, Core should send the HMI the subscription resumption information even though Core does not reestablish the subscription with the client apps until they actually send their subscription requests through the normal means as it is today. This would essentially be a call to ready the RC data caches.

SDL will restore subscription only in case if during registration application will send correct hash id. So if mobile know the correct hash ID , it should know list of subscriptions that are actual for this mobile

@joeygrover
Copy link
Member

I'm still a little confused by your comments @LuxoftAKutsan. So I want to make sure everyone is clear.

  • Currently, all other subscription models(vehicle data, soft buttons, etc) are resumed if the correct hash id is sent to core. Correct?
  • If Interior vehicle data subscriptions were originally not resumed, then including this change will be a break change.
  • Where was this a requirement? The SDLC can't honor an arbitrary spec doc. Does this live on the developer portal somewhere? When was this requirement put into place?

@LuxoftAKutsan
Copy link
Contributor

@joeygrover

Currently, all other subscription models(vehicle data, soft buttons, etc) are resumed if the correct hash id is sent to core. Correct?

Correct, except interior data subscriptions.

If Interior vehicle data subscriptions were originally not resumed, then including this change will be a break change.

There are no changes in API, only in SDL behavior changes. Backward compatibility should not be broken. The only changes that applications that after reconnect will try to restore subscriptions (via GetInteriorVehicleData {subscribe=true}) will be notified that it is already subscribed via (GetInteriorVehicleData {isSubscribed=true})

Where was this a requirement? The SDLC can't honor an arbitrary spec doc. Does this live on the developer portal somewhere? When was this requirement put into place?

This document just describes current SDL behavior. There are plenty of requirements about existing SDL functionality that are not delivered to open source developer portal.

@yang1070
Copy link
Contributor

yang1070 commented Aug 1, 2018

here is the link to the existing document about app resumption
https://smartdevicelink.com/en/docs/hmi/master/basiccommunication/onappregistered/

@theresalech
Copy link
Contributor Author

The Steering Committee voted to accept this proposal with revisions. The revisions are outlined below:

  • The resultCode should be changed to a warning for double subscription, providing information if an app is already subscribed.
  • Documentation has to be added to hashID in registerAppInterface in the RPC spec including the MOBILE_API.xml file in Core
  • Hash ID Resumption information should be added to the Best Practices guide.

It will be determined upon implementation if the existing documentation about app resumption (https://smartdevicelink.com/en/docs/hmi/master/basiccommunication/onappregistered/) can be used as a Best Practice Guide, or if new documentation is needed to ensure clarity for developers.

@theresalech
Copy link
Contributor Author

@LuxoftAKutsan and @yang1070 - 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 the respective repositories for implementation. Thanks!

@theresalech theresalech changed the title [In Review] SDL 0188 - Interior Vehicle Data resumption [Accepted with Revisions] SDL 0188 - Interior Vehicle Data resumption Aug 1, 2018
@smartdevicelink smartdevicelink locked and limited conversation to collaborators Aug 1, 2018
@jordynmackool
Copy link
Contributor

jordynmackool commented Oct 9, 2018

Proposal has been revised, and issues have been entered:
Core
SDL HMI

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

No branches or pull requests

7 participants