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 0116 - Open Menu RPC #352

Closed
theresalech opened this issue Nov 22, 2017 · 8 comments
Closed

[Accepted with Revisions] SDL 0116 - Open Menu RPC #352

theresalech opened this issue Nov 22, 2017 · 8 comments

Comments

@theresalech
Copy link
Contributor

Hello SDL community,

The review of "Open Menu RPC" begins now and runs through November 28, 2017. The proposal is available here:

https://github.com/smartdevicelink/sdl_evolution/blob/master/proposals/0116-open-menu.md

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

#352

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 generally support this proposal. I'm a bit torn on if we should append this functionality to the already overloaded Show RPC or add a new ShowMenu RPC. I think I lean toward the latter, just due to how overloaded Show already is. I would appreciate input from others, however.

@NickFromAmazon
Copy link

I agree with @joeljfischer - the existing parameters on the 'show' request have primarily corresponded to applicable the fields on the selected layout. I feel opening the menu is a separate action, and so the API would remain more cohesive if treated as such.

@Jack-Byrne
Copy link
Contributor

Jack-Byrne commented Nov 27, 2017

I think this proposal is is similar to SDL 0115 - CloseApplication RPC and that both cases could be solved by the use of a single RPC instead of each case having their own.

We could maybe create an RPC called SetHMIStatus and use enums already defined in Core

    <function name="SetHMIStatus" functionID="OnHMIStatusID" messagetype="request">
        <param name="hmiLevel" type="HMILevel" mandatory="false">
            <description>Enumeration that describes current levels of HMI</description>
        </param>
        
        <param name="systemContext" type="SystemContext" mandatory="false">
            <description>Enumeration that describes possible contexts an app's HMI might be in. Communicated to whichever app is in HMI FULL, except Alert.</description>
        </param>
    </function>

To Close app
Proxy -> Core
SetHMIStatus : { appID : 1234, hmiLevel: NONE, systemContext : MAIN }

To Show app menu
Proxy -> Core
SetHMIStatus : { appID : 1234, hmiLevel: FULL, systemContext : MENU }

@kshala-ford
Copy link
Contributor

(Copied from CloseApplication RPC)

Generally this proposal does not conflict with CloseApplication RPC. Both proposals could coexist and solve problems with apps who want to use the built-in menu and those who don't (we have both kinds of apps).

  • CloseApplication is for nav apps who don't want to use the built-in menu
  • OpenMenu is supposed to be used for nav apps using AddCommand

@kshala-ford
Copy link
Contributor

kshala-ford commented Nov 28, 2017

I have no issues with using a separate RPC rather than Show. I'm not sure if using SystemContext is the right approach. There are many values unused which might be confusing. See Comment from Nick

Instead I would propose the following:

<function name="ShowAppMenu" messagetype="Request">
  <param name="menuID" type="Integer" mandatory="false">
    <description>
      If omitted the HMI opens the apps menu.
      If set to a sub-menu ID the HMI opens the corresponding sub-menu
      previously added using `AddSubMenu`.
  </description>
</param>
</function>

It's more specific to the issue of opening the built-in app menu.

@theresalech theresalech changed the title [In Review] SDL 0116 - Open Menu RPC [Accepted with Revisions] SDL 0116 - Open Menu RPC Nov 29, 2017
@theresalech
Copy link
Contributor Author

The Steering Committee voted to accept this proposal, with the revision outlined in kshala-ford's last comment. There was also discussion about allowing an RPC to close the menu, but it was determined that this proposal works to address when there isn't a system button on the screen, and there would always be a system button giving the user the option to close.

@theresalech
Copy link
Contributor Author

@kshala-ford 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 an issue in the respective repositories for implementation. Thanks!

@theresalech
Copy link
Contributor Author

Proposal has been revised to include agreed upon changes and issues have been entered:
Android
Core
iOS
RPC

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

5 participants