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 0294 - App Service - Messaging #973

Closed
theresalech opened this issue Mar 18, 2020 · 20 comments
Closed

[Accepted] SDL 0294 - App Service - Messaging #973

theresalech opened this issue Mar 18, 2020 · 20 comments

Comments

@theresalech
Copy link
Contributor

theresalech commented Mar 18, 2020

Hello SDL community,

The review of the revised "SDL 0294 - App Service - Messaging" begins now and runs through April 7, 2020. The review of the original proposal took place March 18 - March 24, 2020. The proposal is available here:

https://github.com/smartdevicelink/sdl_evolution/blob/master/proposals/0294-app-service-messaging.md

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

#973

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]

@Jack-Byrne
Copy link
Contributor

Should this be an array?

<param name="recentMessageThreads" type="MessageContact" mandatory="false" />
  1. Another downside is that any app with app service consumer permissions will be able to read the users messages.

  2. Maybe the messaging service type should have an exception for allowing more than one active service at a time.

  3. Do we want to allow consumer apps to request the message provider send messages? That capability could be an option added to the MessagingServiceManifest.

@joeljfischer
Copy link
Contributor

joeljfischer commented Mar 23, 2020

1. Yep 🤦‍♂

2. This is true. I'm going through different options in my head, but the only one I can think of that would work is to say that if the app wishes to keep this text protected, to not send messageText. If we want to ensure that all text is protected, we could remove the messageText parameter entirely. I can't think of any alternatives that are viable. This could be added as a potential downside.

3. I think that would get complicated in Core. I'm not sure of a good solution to this other than that, though, and it's probably the "best" solution for this particular service. Alternatively, we could make it so that non-active service's data can be polled (correct me if that's already possible). Only the active service would get data pushed to subscribers though. Or open up a way for non-active services to be subscribed to (correct me if that's already possible).

4. That's an interesting idea. I think it would look something like this:

Based on a forthcoming proposal from Joey, we would add the following to PerformAppServiceInteraction:

<function name="PerformAppServiceInteraction" functionID="PerformAppServiceInteractionID" messagetype="request">
    <!-- Existing params -->
    <param name="rpcRequest" type="String" mandatory="false">
        <description>A stringified version of an RPC request. Only RPCs defined in the RPC spec should be used.</description>
    </param>
</function>

<function name="PerformAppServiceInteraction" functionID="PerformAppServiceInteractionID" messagetype="response">
    <!-- Existing params -->
    <param name="rpcResponse" type="String" mandatory="false">
        <description>A stringified version of an RPC response. Only RPCs defined in the RPC spec should be used.</description>
    </param>
</function>

Then we would add a new RPC that can be send via PerformAppServiceInteraction.

<function name="SendMessage" functionID="SendMessageID" messagetype="request" since="X.X">
    <description>Request for sending a message to a contact</description>
    <param name="messageContactID" type="String" mandatory="true">
        <description>The contact ID retrieved from the app service's MessageContact struct</description>
    </param>
    <param name="messageText" type="String" mandatory="true"/>
</function>

<!-- Add a response without any special parameters, TBD -->

We would extend MessageContact with a UUID:

<struct name="MessageContact">
    <!-- Other parameters -->
    <param name = "contactID" type="String" mandatory="false" />
</struct>

If no contactID is provided, other apps cannot send messages using this app.

The SendMessage RPC request would otherwise be sent to the active app. There are privacy implications with permitting any app to send messages through the active messaging service.

@Sohei-Suzuki-Nexty
Copy link

Should the use case using this "Messaging" be discussed here?
If yes, please tell us about the use case for receiving messages.

@Jack-Byrne
Copy link
Contributor

Jack-Byrne commented Mar 24, 2020

Or open up a way for non-active services to be subscribed to (correct me if that's already possible).

Not possible right now but it would be easy to implement.

We could add a serviceID parameter to GetAppServiceData request. Consumers could choose to subscribe to any service, even if not the active service.

    <function name="GetAppServiceData" functionID="GetAppServiceDataID" messagetype="request" since="5.1">
        <description> This request asks the module for current data related to the specific service. It also includes an option to subscribe to that service for future updates</description>
        
        <param name="serviceType" type="String" mandatory="true">
            <description>The type of service that is to be offered by this app. See AppServiceType for known enum equivalent types. Parameter is a string to allow for new service types to be used by apps on older versions of SDL Core.</description>
        </param>

+       <param name="serviceID" type="String" mandatory="false"/>    
        
        <param name="subscribe" type="Boolean" mandatory="false">
            <description> If true, the consumer is requesting to subscribe to all future updates from the service publisher. If false, the consumer doesn't wish to subscribe and should be unsubscribed if it was previously subscribed.</description>
        </param>
    </function>

Sorry I don't follow why the PerformAppServiceInteraction is used with SendMessage?

@joeljfischer
Copy link
Contributor

4. Because that allows the receiving app to know what the originApp is and to make the app the active service. For example, the messaging app may want to only allow certain apps to send a message and reject others.

@joeljfischer
Copy link
Contributor

3.

We could add a serviceID parameter to GetAppServiceData request.

I like that solution.

@joeljfischer
Copy link
Contributor

Should the use case using this "Messaging" be discussed here? If yes, please tell us about the use case for receiving messages.

Hi @Sohei-Suzuki, this would be an extension to the app services system, so it would add the ability for chat apps to expose to the head unit their recent messages. The Head Unit could then surface those however they wish (e.g. show a status icon on system UI, etc.). The messaging app also exposes this data to other app consumers, so that a navigation app could show new messages in their own UI, for example.

@theresalech theresalech changed the title [In Review] SDL 0294 - App Service - Messaging [Returned for Revisions] SDL 0294 - App Service - Messaging Mar 25, 2020
@theresalech
Copy link
Contributor Author

The Steering Committee voted to return this proposal for revisions. The author will make the following changes to the proposal:

  1. Update recentMessageThreads to an array
  2. Include details of item 2 from comments in the "Potential Downsides" section of the proposal
  3. Add a serviceID parameter to GetAppServiceData request
  4. Update PerformAppServiceInteraction as described in this comment

It was also mentioned on the Steering Committee call that any additional questions about the messaging use case could be posed during the review of the revised proposal.

@smartdevicelink smartdevicelink locked and limited conversation to collaborators Mar 25, 2020
@theresalech theresalech changed the title [Returned for Revisions] SDL 0294 - App Service - Messaging [In Review] SDL 0294 - App Service - Messaging Apr 1, 2020
@theresalech
Copy link
Contributor Author

The author has updated the proposal to reflect the agreed upon revisions, and the revised proposal is now in review until April 7, 2020.

@smartdevicelink smartdevicelink unlocked this conversation Apr 1, 2020
@Sohei-Suzuki-Nexty
Copy link

@joeljfischer -san

Thank you for replying .

The messaging app also exposes this data to other app consumers, so that a navigation app could show new messages in their own UI, for example.

In this case, the behavior will be different depending on each App, so I think it may confuse the user.
Is this a problem?

@joeljfischer
Copy link
Contributor

@Sohei-Suzuki-Nexty I think that's a good question. This could introduce variability between apps' capabilities. However, I think that the capability of apps that don't expose the message text is still good because the app consumer can still open the message thread in the messaging app itself, and the user can read the text there.

The only alternative would be to remove the ability for app producers to show message text at all, which I don't think is something we'd like to do.

@Sohei-Suzuki-Nexty
Copy link

@joeljfischer -san

I think UX that varies between apps is not a good idea. Is it possible of you to add a message box for overlay and display it without depending on the app?

@joeljfischer
Copy link
Contributor

Is it possible of you to add a message box for overlay and display it without depending on the app?

I'm not sure what you're asking here, could you please clarify.

@Shohei-Kawano
Copy link
Contributor

@joeljfischer -san
It means that it is better to provide a mechanism to display Message on Native(Embedded) side.

Your design:
App1 (ex.SNS App)-(Message Text)-> AppService-(Message Text)-> App2 (ex.Navi App)

If App2 has a function to display a message, a message is displayed. If App2 do not have one, it will not be displayed.

Our idea:
App1 (ex.SNS App)-(Message Text)-> AppService-(Message Text)-> Native(Embedded) side.

The Native (Embedded) side displays a message on a layer above the App (Overlay layer), taking into account the status of the App.
By doing so, we believe that the message function can be realized without depending on the App.
Of course, I think the changes between Core and HMI will be bigger.

@joeljfischer
Copy link
Contributor

joeljfischer commented Apr 8, 2020

@Shohei-Kawano I think that this is a misunderstanding of the app services feature. Both "my design" and "your idea" occur in this proposal. App Service data goes to all app service consumers, both the head unit ("your idea") and to other apps ("my design"). There is currently no way to mark that app service data only goes to the head unit and not to other apps. However, in order for apps to receive app service data they do have to be approved by the OEM to use those RPCs, so there is some protection there.

The only other alternative I could think of would be to add a new parameter like:

<struct name="Message">
    <param name="isProtected" type="Boolean" mandatory="false">
        <description>If true, this data should only be sent to the HMI service consumer, and not to any app consumers.</param>
    </param>
</struct>

Alternatively, we could use an attribute:

<struct name="MessageContact">
    <param name="mostRecentMessage" type="Message" app-service-protected="true" mandatory="false">
        <description>Details about the most recent message in the chat for previewing purposes.</description>
    </param>
</struct>

Where if this attribute was true, the data would not be sent to app consumers and only to the HMI consumer. I'm not sure how I feel about those alternatives at the moment. @joeygrover @JackLivio what do you think?

EDIT: Github ate some of my comment, added it back in.

@theresalech
Copy link
Contributor Author

A quorum was not present during the 2020-04-07 Steering Committee Meeting, so this proposal will remain in review until our meeting on 2020-04-14.

@Shohei-Kawano
Copy link
Contributor

@joeljfischer -san
Thank you for the explanation.
I understand that it will be routed to both Native(HMI) and App.

If multiple app service consumers are registered, will the registered consumers be notified without priorities or conditions?

In short, what I mean is that there is a conflict between Native(HMI) and App in displaying messages.
So I think that conflicts can be avoided by having a mechanism that only Native(HMI) displays messages.
From the user's point of view, the message is displayed in the same UI, so there may be less confusion.
I imagine the mechanism of Android Notification.

@joeljfischer
Copy link
Contributor

If multiple app service consumers are registered, will the registered consumers be notified without priorities or conditions?

I understand your point now. You are looking to ensure that a head unit doesn't display a "native" notification and another app displaying the same notification. Due to the variety of HMIs and implementations we can't make any guarantees about the HMI implementation or UI. I think that it's important that apps have the capability of showing notifications and using their own features to be able to consume and respond to messages if possible. AFAIK, there's no current way with app services for the app to know if the HMI will handle the service nor how it would handle it in terms of UI.

@theresalech theresalech changed the title [In Review] SDL 0294 - App Service - Messaging [Accepted] SDL 0294 - App Service - Messaging Apr 15, 2020
@theresalech
Copy link
Contributor Author

During the 2020-04-14 Steering Committee Meeting, the author and Nexty team agreed that the questions/concerns posed in Nexty's comments on this review issue are out of scope for this proposal, and should be discussed separately with regards to the larger app services feature as a whole. The Steering Committee then fully agreed to accept this proposal.

@smartdevicelink smartdevicelink locked as resolved and limited conversation to collaborators Apr 15, 2020
@theresalech
Copy link
Contributor Author

Implementation Issues Entered:

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