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] SDL 0249 - Persisting HMI Capabilities specific to headunit #816

Closed
theresalech opened this issue Aug 28, 2019 · 13 comments
Closed

Comments

@theresalech
Copy link
Contributor

theresalech commented Aug 28, 2019

Hello SDL community,

The review of "SDL 0249 - Persisting HMI Capabilities specific to headunit" begins now and runs through September 3, 2019. The proposal is available here:

https://github.com/smartdevicelink/sdl_evolution/blob/master/proposals/0249-Persisting-HMI-Capabilities-specific-to-headunit.md

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

#816

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

I'm not super familiar with HMI development, so forgive me if any of my comments are ill-informed.

  1. Assuming that the HMI is able to have a software update change its capabilities, how would it be able to force Core to overwrite its cache?
  2. (a) Would Buttons.GetCapabilities return different results for different templates? (b) Do RC capabilities change based on the connected application (whether or not it has permissions, for example)? (c) Wouldn't GetLanguage change between ignition cycles if the user changed the language of the head unit (i.e. they change it, then do a new ignition cycle)?

@atiwari9
Copy link
Contributor

atiwari9 commented Sep 3, 2019

@joeljfischer

  1. Core can simply delete the file after loading up data from cache file. That'd trigger file re-creation and re-persistence of capabilities data for next ignition cycle. It depends on OEM implementation, but most SW updates usually restart headunits anyways.

  2. a. Button.GetCapabilities sent all supported buttons. Templates may utilize a subset of this, details on that are sent as response for SetDisplayLayout rpc.

    b. No, RC capabilities from HMI(System) remain same. App's access to RC modules is controlled by policies.

    c. Yes, language change would trigger UI.OnLanguageChange , VR.OnLanguageChange, TTS.OnLanguageChange and that is why we treat those as Dynamic ones since this is a User triggered event. Core would always overwrite the data in these notifications to cache file. So even if user did an ign cycle after that, cache file would already be updated with new language data.

@Jack-Byrne
Copy link
Contributor

Jack-Byrne commented Sep 3, 2019

  1. If you want Core to handle deleting the cache on a HMI software update then I think there needs to be a new enum added to ApplicationsCloseReason in the HMI API.
<enum name="ApplicationsCloseReason">
  <description>Describes the reasons for exiting all of applications.</description>
  <element name="IGNITION_OFF" />
  <element name="MASTER_RESET" />
  <element name="FACTORY_DEFAULTS" />
  <element name="SUSPEND" />
+ <element name="SOFTWARE_UPDATE" />
</enum>

Unless you had an alternate method to handle this part?

SDL Core should regenerate HMICapabilitiesCacheFile file when system SW version changes.

@atiwari9
Copy link
Contributor

atiwari9 commented Sep 3, 2019

@JackLivio

  1. So for SW update scenario, SDL Core would know about it only after system has restarted. That is when SDL sends GetSystemInfo to HMI. It is only now that SDL Core would know that SW has changed. SDL Core can then:
  • Keep the current capabilities cached and delete the cache file. Upon next ign cycle, SDL Core would check for this file as per flow defined in proposal and trigger all the capabilities requests to HMI and persist the response. (Proposed flow)
  • Trigger all the capabilities requests to HMI and wait for the response(and overwrite cache). Once response is received, unregister all registered apps with a new ApplicationsCloseReason so that apps can request a fresh registration. (Alternate flow)

Cons of Proposed flow:

  • Apps won't get the latest Capabilities changes for the ignition cycle right after SW update. For subsequent ignition cycles app would get updated capabilities.

Cons of Alternate flow:

  • Apps would unregister abruptly during usage. That might be undesired user experience if user is interacting with application or if application is providing some service etc.
  • Additional logic needs to be added to core to trigger capabilities requests upon HMI SW version check(ccpu_version)

In my opinion, it is more work if we follow Alternate flow while providing interruption to user. Proposed flow could also affect app functionality but assuming HMI would make changes which are backwards compatible, impact should be low here.

cc: @joeljfischer

PS: Noticed that the sequence diagram is incorrect in implementation guidelines

@Jack-Byrne
Copy link
Contributor

Jack-Byrne commented Sep 3, 2019

@atiwari9 OK thank you for the explanation. Unfortunately the Con listed for the Proposed flow is kind of a deal breaker for me. I believe an app that has incorrect capabilities could cause other issues that might be worse for the user experience compared to an app re-registering.

If the alternate solution is still not acceptable, then maybe there is a way we can improve it. Right now in Cores RAI request run function, Core will make an app wait for the HMI to be connected and cooperating with core. The flag (IsHMICooperating) that allows to Core to continue processing the RAI request is set to true after the HMI sends its first OnReady notification.

I suggest we delay setting core's flag IsHMICooperating to true until after the HMI responds to Core's GetSystemInfo request to check the HMIs software version (best case), or delayed until the HMI responds to all of the interfaces' GetCapabilities requests after a software update (worst case). This will allow core to collect the capability information (cached, or from HMI) before responding to the RAI request.

Con for this approach: After a software update if one of the interfaces' GetCapabilities responses fails, Core might have to keep an app waiting to register for up to the defined MaxTimeout before proceeding with the default hmi_capabilities.json.

@atiwari9
Copy link
Contributor

atiwari9 commented Sep 3, 2019

@JackLivio

I am ok with alternate flow then along with your suggestion to hold up RAI till hmi responds to all capabilities requests when data is missing in cache file . This is to ensure that we do not break app resumption use case across ignition cycles

@atiwari9
Copy link
Contributor

atiwari9 commented Sep 4, 2019

@JackLivio

I created a draft flow for what we discussed, let me know if you have same understanding.

image

@theresalech theresalech changed the title [In Review] SDL 0249 - Persisting HMI Capabilities specific to headunit [Returned for Revisions] SDL 0249 - Persisting HMI Capabilities specific to headunit Sep 4, 2019
@theresalech
Copy link
Contributor Author

The Steering Committee discussed either returning this proposal for revisions or accepting this proposal with revisions, and it was determined the proposal should be returned. The author will revise the proposal to use the alternate approach as agreed to in this comment, and include defined flow charts for what this connection scheme will look like. The project maintainer may be able to assist with this offline as well. Returning the proposal for revisions will allow all SDLC Members with the chance to review the new SDL Core to HMI connection scheme, and check to make sure it works within their implementations.

@smartdevicelink smartdevicelink locked and limited conversation to collaborators Sep 4, 2019
@jordynmackool
Copy link
Contributor

The author has updated this proposal as requested in this comment .

The re-review of "SDL 0249 - Persisting HMI Capabilities specific to headunit" begins now and runs through September 17, 2019. The proposal is available here:

https://github.com/smartdevicelink/sdl_evolution/blob/master/proposals/0249-Persisting-HMI-Capabilities-specific-to-headunit.md

@smartdevicelink smartdevicelink unlocked this conversation Sep 11, 2019
@jordynmackool jordynmackool changed the title [Returned for Revisions] SDL 0249 - Persisting HMI Capabilities specific to headunit [In Review] SDL 0249 - Persisting HMI Capabilities specific to headunit Sep 11, 2019
@Jack-Byrne
Copy link
Contributor

👍

@theresalech theresalech changed the title [In Review] SDL 0249 - Persisting HMI Capabilities specific to headunit [Accepted] SDL 0249 - Persisting HMI Capabilities specific to headunit Sep 18, 2019
@theresalech
Copy link
Contributor Author

The Steering Committee fully agreed to accept this proposal.

@theresalech
Copy link
Contributor Author

Implementation issue entered: smartdevicelink/sdl_core#3037.

@theresalech
Copy link
Contributor Author

SDL HMI issue: smartdevicelink/sdl_hmi#387

@theresalech theresalech added the hmi label Jul 7, 2020
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

5 participants