[Accepted] SDL 0341 - Add Generic HMI Plugin Support

[theresalech]

Hello SDL community,

The review of the revised proposal "SDL 0341 - Add Generic HMI Plugin Support" begins now and runs through January 18, 2022. Previous reviews of the proposal took place October 20 - November 9, 2021, and July 21 - August 16, 2021. The proposal is available here:

https://github.com/smartdevicelink/sdl_evolution/blob/master/proposals/0341-add-generic-hmi-plugin-support.md

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

#1168

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]

[joeljfischer]

1. I'm a little confused by the "Example Plugins/features" sections. Is this defining what the example plugins will support in terms of HMI JSON RPC messages?
2. Based on (1), I don't see in the proposal how the Generic HMI will communicate with these plugins. Are the example plugins features defining the messages of the plugins and the available plugins? In other words, does the example plugin "Voice Recognition Plugin" define what messages are available? Does it pass those JSON RPC HMI messages along the Flags.js Url parameter? Through a websocket? I think there's a lack of definition about the actual plugin interface.
3. How does a plugin provide an iframe or UI? Because these are plugins and not modifications to the Generic HMI, how does the plugin know the size of the UI it will provide? A lot of the UI definition seems to be geared for test interfaces (like Manticore) and there's not much example given about how this would work in a production setting. I see the "menuEnabled" example setting, but then there's no definition about how it would provide UI. I think that less "possibilities" and more concrete definition needs to exist around UI in this proposal.

[jacobkeeler]

1. This section defines any HMI RPC messages that a given plugin would handle as well as any other additional functionality that the plugin would include. Generally any functionality that doesn't have an RPC attached to it would involve some form of communication with other plugins or the main HMI.
2. Ah, I realize that I might've just skimmed over how the message broker works in the proposal:

Each of these plugins would maintain their own connection to SDL Core using a router service, which can be borrowed from the Manticore project (see here).

I can definitely add a section which describes this is more detail. The message broker would first connect to Core and all of the plugins (plus the main HMI) would separately connect to this application. The message broker would then direct any messages from Core to the appropriate plugins based on the interfaces they registered (VR, UI, etc.). The messages which are sent between plugins will need their own interface, but we should be able to route these messages in a similar way, since all of the plugins are connected to the message broker:

These messages can be routed using the message broker as well.
Each message would include the Plugin prefix and can include a destination parameter if targeting a specific component.

3. It seems there may be some confusion with the language in the proposal here, so I'll try to clear that up. There are two possible "UI" portions to each plugin, both are optional and can be enabled/disabled separately on a plugin-by-plugin basis:
   1. Sidebar
      • This is meant only for test plugins, and is described in the Running test plugins in-browser section.
      • The sidebar would be displayed beside the Generic HMI, and would not be visible in a production case.
      • This is the portion of the plugin that would be retrieved using the Url parameter and displayed via iframes or something similar.
      • The main purpose of this would be to edit test data and trigger events, using Manticore's sidebar as a model.
      • The size of this sidebar would likely be of a fixed width, and the plugins would need to be written to accommodate this.
   2. Menu
      • This is meant mainly for production plugins, and is described in the Integrated Plugin Controls section.
      • This UI would be accessible via a menu in the Generic HMI.
      • This portion is not part of the plugin itself, but is a part of the Generic HMI that communicates with the plugin.
      • This portion would be custom-built by integrators for production cases, using stub components included in the project as a base.
      • This is where the integrator is meant to include any controls for the plugin (such as an option to start voice recognition) or any plugin-related information it wants to display in-vehicle (such as a navigation map). Since this could be quite varied, the feature's description is left fairly open-ended.

I can try to update the description for these features make this more clear. I think better terminology would definitely help with this, so I'm open to any suggestions on how to differentiate them better.

[theresalech]

The Steering Committee voted to keep this proposal in review, to allow for further discussion on the review issue.

[joeljfischer]

1. Okay

2. Okay, I think this needs to be fleshed out in the proposal. We also need a list of all plugin "types," and their supported HMI RPCs. Please let me know if that doesn't make sense in the context of the proposal.

3. For part 2, production plugins, how flexible is this? Given that it's for production plugins, I would think it would need to be extremely flexible. For example, having a slide-out hamburger menu is probably not good UX design for a vehicle, as it has a small tap-target and hidden UI is generally not something that should be used. So I think (1) we have to make sure anything like this for production plugins is very customizable, (2) have a better default than a slide-out menu.

[jacobkeeler]

We also need a list of all plugin "types," and their supported HMI RPCs

I believe this is already covered by the Example Plugins section. Although I was leaving this list open to expansion, I could just make this the list of officially supported plugin types.

3. Agreed that this needs to be rather flexible. It's left fairly open-ended as of now, basically just providing an entry point for whatever custom UI a developer wants to include, but if we need to make the entry point itself more flexible, we can.

I believe there were plans to add this slide-out menu to the Generic HMI already in the works, which is why I used that example, but if we want to pivot the design for this we can. Would something like the TILES menu layout be a better default entry point? Obviously I'm open to any other suggestions as well.

[theresalech]

The Steering Committee voted to keep this proposal in review, to allow for further discussion on the review issue.

[theresalech]

Based on the discussion here, the Project Maintainer recommends the Steering Committee return this proposal for the following revisions:

• (Item 2): add a section to describe the plugin interface and how the message broker works in more detail. Additionally, make list of plugin types in the Example Plugins section the list of officially supported plugin types.
• (Item 3): update description for Sidebar and Menu features, consider alternatives to the slide-out menu for a navigation interface, and clarify that the menu interface is only an entry point to a customized portion of the Generic HMI.

[theresalech]

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

[theresalech]

Closing as inactive. The issue will remain in a returned for revisions state and unlocked so the author can notify the Project Maintainer when revisions have been submitted.

[theresalech]

The author has revised this proposal based on the Steering Committee's requested revisions, and the revised proposal is now in review until October 26, 2021.

[joeljfischer]

Unfortunately, with the releases tomorrow I have not been able to re-review this proposal. I would like to request an additional week to review.

[theresalech]

The Steering Committee voted to keep this proposal in review for an additional week.

[joeljfischer]

1. Under "Plugin Configuration", you state:

It should be possible to add, remove, disable, and replace these plugins with minimal configuration changes where feasible. Ideally, this configuration could be handled using the existing Flags.js file, which is currently used by the Generic HMI for any runtime configurations. A potential format for this configuration could be:...Note: This format is tentative and could change depending on the final implementation.

This language does a lot of hedging here, and because this is the proposed solution of a public API, I don't think you can hedge like this. The language needs to be changed to "will be" instead of "could be" for example. We can't have a "potential" format, we need "the format." If needed during development, we can change them through proposal revisions, but proposals are the spec. So either updates need to be made to make a spec, or the language needs to be updated.

2. Under "Web-based plugins":

In order to run these applications within the browser, the Generic HMI could open the provided Url in the plugin configuration within a hidden iframe or something similar, allowing the plugin to run alongside the HMI.

Similar here, I think this needs to be changed to, "...the Generic HMI will open the provided Url in the plugin configuration within a hidden iframe or something similar, allowing the plugin to run alongside the HMI."

3. While I'm happy to see the plugin types changed from examples to the concrete types, I think they're still under-specced. You've listed the API names, but nothing about the API details. I think you need to include information like "how are these called" (JSON-RPC? You mentioned multiple language support so I assume it's not just JavaScript functions), the API input and output types, etc.

4. Under "App Service Types", you write:

This could potentially be split into several plugins (by service type) depending on the system.

Again, I think this "could potentially" language needs to be taken out unless it's under the "Alternatives" section. Give a firm spec that is implementable.

5. I'm a bit confused about the section "Messages between components". Is this intended to describe APIs that developer plugins would need to support for these different features to work? What if a developer implements the module but not these methods? How do they set up to respond to these methods? What do they respond with? You say these are "a few of the potential" ones, but that leaves the public API surface up in the air again. I would rather see us do what we have in the past, which is set up a public API surface in the proposal and adjust it through revisions later if needed.

You also write, "In addition, customized messages will likely need to be created for production systems to accommodate differences in plugin design."

What differences in plugin design are you think of?

6. Again in "Integrated Plugin Controls" there a lot of "possibility" and "could" language. Could you please change that into a firm proposal with "will" language?

7. In "Impact to existing code" you mention: "Overall, a majority of the changes needed for this proposal will be in the form of test plugins, which will be isolated from any existing code." Is it a part of this proposal to create example versions of some / all of the plugins mentioned? Will they be released to the generic_hmi repo? Other repos? Will there be documentation on how to use them?

[jacobkeeler]

@joeljfischer

1. Alright, unfortunately I do think a decent portion of this proposal could be subject to change in that case, perhaps that's just unavoidable. I made an attempt to identify most of the possible cases we needed to cover by making a POC for this proposal, but I suspect some details just won't be possible to iron out until development is underway. I tried to account for this by leaving parts of the proposal as implementation details where it made sense, but if a hard specification is needed, I could update the wording accordingly.

I might still specifically argue that it makes more sense for the configuration file to be left as an implementation detail, as it is relatively internal to the HMI, but I could also see this factoring into points 2 and 6, since they use this configuration.

2. I think some of this "maybe" language is probably bleeding over from the configuration section. I can update the wording to be more concrete.

3. Sorry, I don't think I'm understanding where the confusion is here. These messages are defined in the HMI API, and the plugins would handle and/or send them the same way that the HMI does currently, they would just process a specific subset of RPCs. The programming language for the plugin would not factor into this.

For example, the VehicleInfo plugin could receive a VehicleInfo.GetVehicleData request from Core:

{
   "id":11,
   "jsonrpc":"2.0",
   "method":"VehicleInfo.GetVehicleData",
   "params":{
      "vin":true
   }
}

And the plugin would respond to the message, just as the HMI would normally:

{
   "jsonrpc":"2.0",
   "id":11,
   "result":{
      "vin":"52-452-52-752",
      "code":0,
      "method":"VehicleInfo.GetVehicleData"
   }
}

4. I disagree in this case, as this is meant to be a potential design decision for the plugin creator. The Generic HMI should be able to handle both cases (combined or separate app service plugins). If you think it is unclear, I could perhaps reword it to say that the HMI supports both of these designs.

5. Alright, I'll agree that this is probably the section most in need of a concrete specification. I'm fairly certain that this list will need to be expanded once development starts, which is why I'd originally included the open-ended language, but at this point it would likely be better to just update the proposal if there are any significant changes to this section.

As for the comment about "differences in plugin design", I didn't have anything specific in mind, but I wanted to make sure that the design was kept flexible enough so that an HMI developer could implement custom interactions if they needed them.

6. Same as 2

7. Yes, this is stated in the proposal

As part of this proposal, an example plugin of each type should be created for testing purposes, and these examples can then be used as a reference for production implementations.

My initial thought was that these would be included directly in the project, as this would allow users to have a fully functioning test HMI with minimal configuration, but they could be maintained in separate repositories if that would be preferred. Of course, the documentation would be updated accordingly (most likely this would be included in the README or the Core Guides section of the developer portal).

[theresalech]

We did not have a quorum present during our meeting on 2021-11-02, and so were unable to vote on this proposal. It will therefore remain in review until our next meeting on 2021-11-09.

[joeljfischer]

1. Yeah, I understand that a lot of this is hard to figure out ahead of time without doing all the work, but the rule has generally been that implementation details should be explained in detail if they're important and anything developer-facing must be set and changed through revisions.

The configuration does seem to be developer-facing. I can't see an implementation of this feature that doesn't require the developer to update and change the configuration. I think that would require us to call this a public API.

2. 👍

3. That's my mistake based on my unfamiliarity with the HMI API.

4. I understand now, thank you for clarifying. As long as the configuration allows for this decision, would the developer just put the same IP/Port combo for multiple plugin configurations?

5. 👍

6. 👍

7. I think including them in the repo is reasonable, or perhaps a separate monorepo included as a submodule. Either way should be specified though.

[jacobkeeler]

1. Alright, I can try to solidify the configuration file format.

4. Ah, another consequence of not having this defined in the configuration description. In this case I was thinking that the both the AppService and RemoteControl plugin sections of the config file could each accept an array of plugin configurations, with each array element having their own URL. I can add this to the configuration description.

7. 👍

[jacobkeeler]

To summarize the changes requested:

• Finalize plugin configuration file format (including examples of interfaces which can handle multiple plugins, such as AppService).
• Add details and examples for the "Messages between components" section. Finalize the specification for these messages as much as possible.
• Update wording throughout the proposal to avoid hedging language ("should be", "could be", etc.)
• Add note that example plugins will be included directly in the Generic HMI repository

[theresalech]

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

[theresalech]

Closing as inactive. The issue will remain in a returned for revisions state and unlocked so the author can notify the Project Maintainer when revisions have been submitted.

[theresalech]

The author has revised this proposal based on the Steering Committee's requested revisions, and the revised proposal is now in review until January 18, 2022.

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

[joeljfischer]

I apologize, but I would like more time to review this.

[theresalech]

The Steering Committee voted to keep this proposal in review until our next meeting to allow for additional time for member and project maintainer review.

[joeljfischer]

I believe that all of my concerns have been addressed.

[theresalech]

The Steering Committee fully agreed to accept this proposal.

[theresalech]

Implementation issue: smartdevicelink/generic_hmi#496