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 0216 - Widget support #664
Comments
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
The Steering Committee voted to reject this proposal on 2019-01-29 based on the amount of feedback given in the comments. The author plans to rework the proposal in a new format and resubmit the updated proposal for review. |
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
I think it might be simpler to only allow one widget per app (per display) for now, and expand later in a separate proposal when we can think through it in a more focused manner (we'd probably have to remove the system capability for sending back max screens / windows for now so that OEMs don't just change the value and we end up with a Wild West of OEM UX). Alternatives: a. Adding a new RPC to change the primary window / screen for a display per window / screen type. I don't think anything else would work at all, and (b) would suck UX-wise pretty bad (e.g. the currently displayed primary widget is removed from its slot on the head unit and the new one isn't added back into that slot). I think (a) is the only viable alternative. So, my suggestion is this: <function name="CreateScreen" messagetype="request" since="5.x">
<!-- Everything that's already there -->
<param name="appServiceAssociation" type="AppServiceType" mandatory="false">
<description>If provided, this widget should be used in dynamic situations when this app is the active app for the service provided. e.g. If the app is the active "media" app service, this is the widget you want displayed on the system if the system only picks one for a "media" widget slot. This overrides the `MakePrimaryScreen` RPC, which is used in dynamic situations in which the system wants only one widget representing your app. If the app has not provided a manifest for this service type already, this RPC will be rejected with resultCode `DISALLOWED`. If this parameter is not provided, the widget will not be associated with a service.</description>
</param>
</function>
<function name="MakePrimaryWidget" messageType="request" since="5.x">
<param name="screenID" type="Integer" mandatory="true">
<description>When sent, the widget associated with the screenID provided will become the primary widget for this app. If the system wishes to show only one widget from this app, and this request doesn't involve an active app service, or the app does not provide a widget associated with an app service (see `appServiceAssociation` in `CreateScreen`, then the widget specified here will be used.</description>
</param>
</function> While it's not ideal to use a separate RPC for this functionality, all other considerations I could think of had large downsides. The managers could hide away this RPC and make it easy for app developers to use. New Issue (17): It appears that a widget cannot use the |
The Steering Committee voted to return this proposal for revisions, as the author has been working on updating the proposal to include the agreed upon items (1, 2, 3, 4, 5, 6, 7, 9, 10, 11, 12, 13, 14) in PR #698. The Steering Committee will continue to discuss and work through the open items (8, 15, 16, 17) once the revised proposal is back in review. |
The author has revised this proposal to include the Steering Committee's requested revisions, as listed in this comment. The revised proposal is now in review until 2019-05-14. |
|8. I remain in favor of "mirror" terminology. |16. I'm in favor of your It solves a lot of problems, though it won't solve the problem of pulling one widget per app if, for example, you wanted to use a "recent apps" screen using widgets. Some possible solutions I thought of:
I think I lean to (3). If they |17. I didn't see any response on this one. What do you think about widget button subscriptions for media apps? |18. As a note, it appears that mandatory parameters are added to existing |19. Adding |20. I don't see it mentioned in the proposal under the |21. I don't believe there's a way to see if soft buttons support text, and it appears that your example widget designs don't support images (and this may be common). I think this is probably a very good time to add the ability to add a new |22.
Can you explain this more? I assume you mean the |
|8 All I can offer is to rename the parameter to |16 my biggest concern with using the service enum is the lack of permission management. I could not find any reference to policies with app services. To my understanding a media app could easily publish weather service data? I believe we anyway need to keep
Can you explain this deeper integration?
Same here? More explanation please. Are you planning to auto-create widgets from the libraries which get filled with service data?
I don't think a primary widget is needed. Also I don't want to add a widget specific parameter to |17 I saw your note but I had no chance to respond to it outside of the proposal. (Hard) button subscription is a system RPC. As a media app when you subscribe to PLAY_PAUSE you will receive hard button press on |18 that's a mistake. The description mentions "if this param is not included...". Consider the parameters for |19 Why would they? I don't see any need to do so? It could be easily abused by setting the same template. Why add a barrier if it's so easy to work around it? |20 I'm not sure what you mean. Right now only the window that changes should be reported. See here |21
I think until now we expect soft buttons to always support text.
Or course they do support graphics. I don't see why widgets shouldn't support graphics.
I have focused on restructuring the existing capabilities without adding new elements. I don't see the need or importance for your proposed addition but you can feel free to create another proposal for it. The current widget proposal has a lot of content which is not in scope of widgets and I don't want to add more things to it which are not related to widgets. |22 RAI response and SetDisplayLayout response. The OEM has to provide display capabilities and window capabilities in two different ways at least until core 6.0. I think this is more effort. Of course it's not too much work but I wanted everyone to understand that the old and new structure must be supported by OEMs. |
|16
I just double checked the developer portal, and there creating app services does require requesting permissions via policies.
Sure, I just meant a more formalized way to describe the "current system" media, navigation, weather, etc. app. This is similar to what currently exists in Sync 3 with the current media app data being presented on the Home and Audio screens, but more formalized via service data, which prevents the possible case of the media app going to a different template and then the media data is no longer
I think that would certainly be possible and even relatively easy if there were a proposal to do so in the future.
The main downside I see to this, as opposed to my other suggestion of using the "first" created, and if the "first" is deleted then falling back to the "second" created, is that if the |17. The concern that I have here is the ability for a widget to display the "soft" subscription buttons that e.g. Sync displays (this usually happens on the main window when the subscription happens). I'm fine with not doing that and simply having the developer make soft buttons that do the same thing. |18. 👍 |19. The barrier can't be worked around because Core prevents it from changing the color scheme if you set the same template that is already in use. |20. I see that note now. 👍 |21.
I typed "images" when I meant "text" 🤦♂ I do think that this is an important thing for good widget support. If developers have no way of knowing that a soft button doesn't support text then they may end up with creating soft buttons that work on some systems but not others. These features are important for widgets to work well on a wide variety of systems, and not just one OEM's. If OEM 1 supports text in a widget soft button and OEM 2 does not, and the developer tests only on OEM 2's system, then the app's support for OEM 1's system will be poor or broken. Because widgets makes an implicit change to have soft buttons that don't support text, there should be a corresponding capabilities change to describe whether a soft button supports text. This is a small change but I believe it is important. |22. 👍 |
i) Core will validate against policies the app service type an app is allowed to publish. After this, if a published media service app sends weather data in the OnAppServiceData notification, it will be up to the consumer app to know that data is wrong. ii)
Edit: misunderstood the question. iii) CreateWindow RPC has a param The reason for this in app services was that Core could not validate enums for future app service types that an older module does not know about. That part of app services does not really apply to widgets so if AppServiceType is kept as the type for widgetType, then the Documentation tag should be removed from AppServiceType in the spec. |
Just wanted to say thanks @kshala-ford for all the hard work you've been putting into this proposal. For me at least, the latest revision has a very understandable and comprehensive description of the solution. | 8. I think this works. The argument we had was that the param originally sounded like it was duplicating the window not the update to the window; that's why mirror made more sense with that param name. But the suggestion helps clear that up. Other suggestions (in case you like one better, I'm indifferent at this point with at least the suggested renaming) | 16. I think something like | 19. I believe in the proposal for template colors we specifically stated that the updates should only occur when a template is being changed. I would have to find the specific call out, but I know it was a discussion point. Another option would be creating a smaller struct that held both params to specifically call out that it should only happen during a layout change. (Ignore the name, it's not great) <function name="Show" functionID="ShowID" messagetype="request" since="1.0">
:
:
<param name="layoutUpdate" type="LayoutUpdate" mandatory="false">
<description>
If included the window should update the supplied layout configurations (layout type and color scheme)
</description>
</param>
</function>
<struct name="LayoutUpdate">
<param name="layout" type="String" maxlength="500" mandatory="true">
<description>
Predefined or dynamically created window layout.
Currently only predefined window layouts are defined.
</description>
</param>
<param name="dayColorScheme" type="TemplateColorScheme" mandatory="false" />
<param name="nightColorScheme" type="TemplateColorScheme" mandatory="false" />
</struct> | 21. So we have to answer are widget SoftButtons required to support text? If yes, then they follow the same guidelines as other soft buttons on the system and we could supply a proposal later to have a system wide change. However, if there is a case we believe that will only support images/graphics, then we HAVE to include a new param in the <struct name="SoftButtonCapabilities" since="2.0">
:
:
<param name="imageSupported" type="Boolean" mandatory="true">
<description>The button supports referencing a static or dynamic image.</description>
</param>
+ <param name="textSupported" type="Boolean" mandatory="false" since="X.X">
+ <description>
+ The button supports the use of text.
+ If not included, the default value should be considered true that the button will support text.
+ </description>
+ </param>
<struct> |
I'm leaving out numbers we have agreed on. (17. 18. 20. 22.) @JackLivio Regarding 16 iii) We can make it of type String, though enum would be handy for the app developer to know what to put into the parameter. Do we know about service types supported by the HMI? Is there some capabilities with a list of supported services? |8. Can we agree on "duplicateUpdatesFromWindowID"? |16 I think the actual use case is to have recent apps with widgets. Therefore I would suggest a new "PredefinedWindows" called |19. In the color scheme proposal I found:
Either we do Joeys suggestion or we make "layoutUpdate" parameter optional. | 21 Joeys suggestion of the parameter looks reasonable. If that's all we need we can easily accept this one for the revision. |
|19.
That is true, however it then says in the next sentence:
So there may have been a little confusion there. The last sentence I quoted is what is implemented. I think we're in agreement that we'd have to do the same thing with the color scheme in |
Yes. I'm sorry I haven't read and understand this sentence in the first place. |19 still: I think I just found another issue with color scheme and the fact that it requires a template change. In the widget proposal there is this:
saying that color scheme should be applied to the app and to all windows, but the new layout will only be applied to the specified window (not to all windows). Here are my suggestions (in hope we can agree to a solution for today): |
16 iii) If the enum is preferred then we could just remove the documentation tag from AppServiceType. Also there are no capabilities for what AppServiceTypes the HMI supports but it could be inferred from the rpc spec version core is using. |
Thank you @JackLivio but I believe your original concern is valid, therefore in the RPC spec the parameter type should be changed to String. In the library implementation we can offer the enum though. This would ensure modern apps to allow use new enum values on older vehicles (e.g. VOIP might be added in future and an app uses it but some vehicles won't support VOIP). |
Sorry for the spam but I wanted to inform you about the PR #729 which includes revisions as we have found an agreement to all items: |8 Rename |16 Add |16 iii) Change "widgetType" type from |17 No action. |18 Made |19 Templates and color schemes are set with a struct (as Joey's suggestion). The struct is called |20 no action |21 |22 no action |
The Steering Committee voted to accept this proposal with the agreed upon revisions, described in this comment and included in PR #729. |
Proposal updated to reflect agreed upon revisions, issues entered: |
Hello SDL community,
The review of the revised "SDL 0216 - Widget Support" begins now and runs through May 14, 2019. Previous reviews of this proposal took place March 20 - April 9, 2019 and January 23 - February 26, 2019. The proposal is available here:
https://github.com/smartdevicelink/sdl_evolution/blob/master/proposals/0216-widget-support.md
Reviews are an important part of the SDL evolution process. All reviews should be sent to the associated Github issue at:
#664
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:
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,
Jordyn Mackool
The text was updated successfully, but these errors were encountered: