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 0119 Revisions - SDL-passenger-mode #769

Closed
theresalech opened this issue Jun 19, 2019 · 14 comments
Closed

[Accepted with Revisions] SDL 0119 Revisions - SDL-passenger-mode #769

theresalech opened this issue Jun 19, 2019 · 14 comments

Comments

@theresalech
Copy link
Contributor

Hello SDL community,

The review of "Revise 0119-SDL-passenger-mode.md" begins now and runs through June 25, 2019. This will be a review of proposed revisions to a previously accepted but not yet implemented proposal, SDL 0119.

The pull request outlining the revisions under review is available here:

#766

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

#769

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

@LuxoftAKutsan
Copy link
Contributor

Hi I have several questions :

  • Should lock_screen_dismissal_warning section present in preloaded PT, and sdl snapshot?
  • Can list of languages be empty in PTU, or in that case it will be invalid PTU?
  • Should pt contain all languages ( from enum name="Language") by default, or only english?
  • SDL should use language or hmiDisplayLanguage for this oarameter in notification to mobile?
  • If required language does not exist in policy, should SDL use english by default?

@jacobkeeler
Copy link
Contributor

jacobkeeler commented Jun 20, 2019

@LuxoftAKutsan

  • This should be possible to preload or overwrite in a PTU, just like the lock_screen_dismissal_enabled parameter.
  • Empty: invalid PTU, Omitted: valid PTU
  • I would imagine this work like the consumer_friendly_messages section. Best practice says that messages should be included for every language supported by the HMI.
  • I might suggest copying the behavior of consumer_friendly_messages.
  • Again, I would look at the behavior for consumer_friendly_messages in this case.

@joeljfischer
Copy link
Contributor

joeljfischer commented Jun 20, 2019

This parameter must be present if "lockScreenDismissalEnabled" is set to true.

Would it be better to wrap lockScreenDismissalWarning and lockScreenDismissalEnabled in a struct that can be mandatory false while the two parameters are mandatory true?

@LuxoftAKutsan
Copy link
Contributor

@jacobkeeler Thanks for the answers.
Probably it is more related to implementation details.
But where can we get appropriate messages for lock_screen_dismissal_warning for other languages?
Not sure that using automatic translation is a good idea.

@joeljfischer
Copy link
Contributor

But where can we get appropriate messages for lock_screen_dismissal_warning for other languages Not sure that using automatic translation is a good idea.

This is dependent on the OEM. Wherever they get translations for their head unit display they could get translations for this text as well.

@LuxoftAKutsan
Copy link
Contributor

LuxoftAKutsan commented Jun 25, 2019

Copying "consumer_friendly_messages" section behavior does not resolve all issues because of 2 reasons:

  1. Consumer Friendly messages send to HMI by request with explicitly specified language.
  2. We have no clear documentation for Consumer-Friendly messages, we can only analyze existing SDL behavior.
    I propose to add the following explanation notes to the proposal within accepting this proposal :

1 lock_screen_dismissal_warning may persist in PolicyTableUpdate as not mandatory parameter,
2 lock_screen_dismissal_warning should not persist in policy snapshot
3 lock_screen_dismissal_warning must persist in preloaded_pt as mandatory, and it must contain at least "en-us" language. (behavior copied from consumer-friendly messages)
4 If required language for lock_screen_dismissal_warning does not exist in policy table SDL will use "en-us" language.

@LuxoftAKutsan
Copy link
Contributor

@joeljfischer
My understanding that SDLC will vote for open source translations should be provided by project maintainers.

@joeygrover
Copy link
Member

@LuxoftAKutsan Only english will be provided for the purpose of this proposal. OEMs will be responsible for their own translations and messages beyond that. As stated in the PR Swipe up to dismiss, acknowledging that you are not the driver will be provided to the open, and no additional translations will be provided in the open.

@Jack-Byrne
Copy link
Contributor

Jack-Byrne commented Jun 25, 2019

I might suggest copying the behavior of consumer_friendly_messages.

I like this idea. We could extend this behavior for other strings that mobile should display to the user. There is already an existing case that could be useful, which is the string prompting the user to open an app to begin a video stream.

I believe consumer friendly messages could be expanded instead of using module config. In the future there might be a string that is useful to display to the user on both the mobile device and the HMI.

Also the consumer friendly messages infrastructure is in place which would make implementing this feature "convenient".

@jacobkeeler
Copy link
Contributor

jacobkeeler commented Jun 25, 2019

@LuxoftAKutsan

  1. In this case, it would follow the behavior of the case where language is not provided in GetUserFriendlyMessage, which would be to default to the active UI language.

@JackLivio
I'm not against this idea, though I would like to note that multiple strings can be included in a UserFriendlyMessage entry:

                        "en-us": {
                            "tts": "An app can access the following vehicle information: Fuel Level, Fuel Economy, Engine RPMs, Odometer, VIN, External Temperature, Gear Position, Tire Pressure.",
                            "label": "Vehicle information",
                            "textBody": "An app can access the following vehicle information: Fuel Level, Fuel Economy, Engine RPMs, Odometer, VIN, External Temperature, Gear Position, Tire Pressure."
                        }

We would need to specify a single field that should be read for the lockScreenDismissalWarning field, my choice would be textBody. Documentation would have to be clear that the rest of the potential fields would be ignored, since this would be a non-standard use of this policy structure.

@LuxoftAKutsan
Copy link
Contributor

@JackLivio @jacobkeeler regarding keeping lock_screen_dismissal_warning in consumer_friendly_messages.
I like this idea it removed most of the corner cases and issues and just applies existing behavior to new strings.

@jacobkeeler consumer_friendly_messages section in Policy Table have the following structure :

 "consumer_friendly_messages": {
  "version": "001.001.021",
  "messages": {
    "AppPermissions": {
       "languages": {
          "de-de": {
             "tts": ...
           }, 
          "en-us": {
             "tts": ...
           }
       },
     "AppPermissionsHelp" : {"languages" : {...}}, 
     "AnotherOneLable : {"languages" : {...}}, 

We can follow your suggestion and extend "consumer_friendly_messages" with another one label : LockScreenDismissalWarning and using "textBody" as text placement.

@theresalech
Copy link
Contributor Author

The Steering Committee voted to accept this change with the revisions noted in this comment.

@theresalech
Copy link
Contributor Author

@jacobkeeler please advise when the PR has been updated to reflect the agreed upon revisions. I'll then merge so that the proposal file is up to date, and reference the change in the associated implementation issues.

@theresalech theresalech changed the title [In Review] SDL 0119 Revisions - SDL-passenger-mode [Accepted with Revisions] SDL 0119 Revisions - SDL-passenger-mode Jun 26, 2019
@smartdevicelink smartdevicelink locked and limited conversation to collaborators Jun 26, 2019
@theresalech
Copy link
Contributor Author

Proposal file has been updated and implementation issues have been updated to reflect accepted revisions:
RPC
Core
iOS
Android
Policy Server

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

6 participants