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 0240 - WebEngine support for SDL JavaScript #767

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

Comments

@theresalech
Copy link
Contributor

theresalech commented Jun 19, 2019

Hello SDL community,

The review of the revised proposal "SDL 0240 - WebEngine support for SDL JavaScript" begins now and runs through December 10, 2019. Previous reviews of this proposal took place November 13 - 19, 2019, October 16 - 28, 2019, July 17 - October 8, 2019, and June 19 - 25, 2019. The proposal is available here:

https://github.com/smartdevicelink/sdl_evolution/blob/master/proposals/0240-sdl-js-pwa.md

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

#767

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

This comment has been minimized.

@theresalech

This comment has been minimized.

@theresalech theresalech changed the title [In Review] SDL 0240 - WebEngine support for SDL JavaScript [Returned for Revisions] SDL 0240 - WebEngine support for SDL JavaScript Jun 26, 2019
@theresalech
Copy link
Contributor Author

The Steering Committee voted to return this proposal for revisions, so that the author can update per the action items identified in the 2019-06-24 workshop, as listed in this comment.

@smartdevicelink smartdevicelink locked and limited conversation to collaborators Jun 26, 2019
@theresalech theresalech changed the title [Returned for Revisions] SDL 0240 - WebEngine support for SDL JavaScript [In Review] SDL 0240 - WebEngine support for SDL JavaScript Jul 17, 2019
@theresalech
Copy link
Contributor Author

The author has updated the proposal, and the revised proposal is now in review until 2019-07-23.

@smartdevicelink smartdevicelink unlocked this conversation Jul 17, 2019
@joeljfischer

This comment has been minimized.

@theresalech
Copy link
Contributor Author

The Steering Committee voted to defer this proposal, keeping it in review until our next meeting on 2019-07-30, to allow the author and all SDLC Members to review the OpenHMI Response from the project maintainer and provide input. It was reiterated that all SDLC Members are encouraged to review this proposal and project maintainer's feedback.

@theresalech
Copy link
Contributor Author

During the 2019-07-30 Steering Committee meeting, an SDLC Member asked for the general intention of this proposal. The author answered that it provides a new way of how applications are presented on the HMI, especially in relation to in-vehicle web applications, using JavaScript. The Steering Committee then voted to defer this proposal, keeping it in review until our next meeting on 2019-08-06, to allow for more review and discussion of the proposal. It was again reiterated that the project maintainer believes this proposal to be introducing a very impactful change, which should be carefully considered by all members.

@theresalech
Copy link
Contributor Author

The Steering Committee voted to defer this proposal, keeping it in review until our next meeting on 2019-08-20, to allow the author and members time to respond to the feedback from the Project Maintainer. It was agreed that if there was no activity on the review issue by the next meeting, the proposal would be deferred as inactive.

@kshala-ford

This comment has been minimized.

@theresalech
Copy link
Contributor Author

theresalech commented Aug 21, 2019

The Steering Committee voted to return this proposal for revisions, to remove the OpenHMI components and only focus on in-vehicle applications and the web engine. OpenHMI components will be discussed separately in a subsequent proposal, once in-vehicle applications and web engine are agreed upon. As the author noted revisions (removing OpenHMI) were ready (#810), the Steering Committee then voted to bring the revised proposal into review.

The revised proposal will now be in review until 2019-08-27.

It was also communicated during the Steering Committee Meeting that returning a proposal for revisions and bringing the revisions into review in the same week is not something that should happen moving forward, unless the proposal is critically time sensitive. The Project Maintainer is working to clarify the SDL Evolution Process in the repository’s process.md, and will share with the Steering Committee for review soon. Once the process is more defined, we can ensure it’s adhered to in all scenarios moving forward.

@theresalech
Copy link
Contributor Author

The Steering Committee voted to defer this proposal, keeping it in review until our next meeting on 2019-09-03, to allow for additional time to review.

@joeygrover
Copy link
Member

1.

While SDL Core is operating, the server should be permanently available and listen for connections on the specified port.

*if Core is built with DWEBSOCKET_SERVER_TRANSPORT_SUPPORT set to `true,

Should there be any other mechanism that would determine if Core should listen on this port? For example, if the policy table includes at least one entry for a local web engine app the websocket server transport should be activated? I would prefer if the websocket wasn't opened unless there was an app that was intended to use it, but this is not a blocker for me. I would imagine it would also be possible to know how many connections core should allow based on PT entries, but would likely cause more issues than it is worth.

2a.

Due to a new app platform, the hybrid app preference should be modified to track mobile, cloud and local apps.

Just as a clarification, the original decision was to treat local and cloud apps as the same type that would give preference to local apps over cloud.

2b.

App registration on the SDL Developer Portal (smartdevicelink.com) should allow a developer to specify an app as a local web app.

The developer portal currently allows for Android, iOS, Cloud, and Embedded as the registered app types. If a change is requested to that set I want to make sure it is very clearly stated and this statement alone isn't taken as the spec of what should be done.

3.

1.4 HMI API using App Properties RPCs

Should the PT entry be extended to include more information about the app and its transport in the case of needed a Websocket server vs Core being the ws client?

4. I think one aspect that could enhance this proposal would be the ability load the app as a javascript module from the HMI if the HMI were also using a webengine. Maybe it makes sense to create a new line in the manifest to include the javascript file only.

{
 "entrypoint_html": "index.html",
 "entrypoint_js": "js/hello_sdl.js",
 
 ...
 
}

This could be a separate proposal, but changing the entrypoint key should be down now to have it qualified with the specific type of entry point if this type of feature has support.

5.

vii. Min SDL version supported

Is this intended to be RPC spec version? Currently we support versions for both the protocol layer and the RPC messaging layer and they are different. So I would imagine we should specific this version to RPC spec version, and possibly add a new key/value pair for protocol.

6. The manifest has a lot of useful information included, but the previously implemented Cloud Transport Proposal used the policy table for some of this information to display to the HMI. (eg app icon). Creating this new packaging for webengine apps would create a division between how other information from other apps (Java SE, Java EE, Node.js) is gathered currently. Maybe it's possible to create a similar structure for the other non-mobile frameworks as well?

7.

Both are difficult to sandbox. Compared to a web engine the effort to sandbox a Node.js or java application and to protect the vehicle system are very high.

I'm not sure I agree with this point when using the Java Virtual Machine (JVM) has a few different options to increase security to a sandboxed environment. This includes simply running the JVM as different user or implementing the Java Security Manager. I would also venture that if apps have created Java implementations they would be familiar with these. However, the volume of apps that would use this framework might be less than node.js/javascript so the point is likely moot.

@nickschwab
Copy link
Contributor

@kshala-ford Hey Kujtim, some feedback/questions below. Keeping with the numbering that @joeljfischer started in his "technical notes" - some of which should probably still be addressed before bringing this proposal to a vote.

1. Does the SDL open-source project have any responsibility regarding app updates, or is the burden of handling app installation and updates intended to be placed entirely on the shoulders of OEMs? For example, if an app developer creates an app targeting sdlMinVersion: 5.0 but then later updates to sdlMinVersion: 6.0, whose responsibility is it ensure that only qualifying installations of Core receive the app update? Currently, SDL Server does not alter responses based on the version of SDL Core which triggered the Policy Table Update request.

9. How is the OEM supposed to receive the compressed app package from a developer? Are developers expected to upload their compressed app package on smartdevicelink.com, which would then be attached to SHAID app records in the form of a file URL and propagated to SDL Servers? The end-to-end process from the perspective of an app developer is a bit vague to me.

10. The mobile app properties HybridAppPreference change from BOTH to ALL provides more clarity around behavior, but it creates a potential backward-compatibility issue in SDL Server since ALL would not be recognized by older versions of Core. How would older versions of Core react to receiving an unrecognized value of ALL? If this would produce undesired results, how is an SDL Server supposed to know which ENUM value to send back in app policies when the Policy Table Update request does not indicate which version of Core it originated from? Perhaps @JackLivio or @jacobkeeler can provide insight.

11. The mobile app properties HybridAppPreference addition of LOCAL does not match the current platform option available to developers on smartdevicelink.com today. I suggest changing this new enum value to EMBEDDED to be as consistent as possible. This would also help avoid any misconceptions that a "LOCAL" app cannot retrieve data from external web APIs.

12. The proposal states that these apps would not be allowed to be modified without going through another round of SDLC review for security reasons, however, there is an inherent risk of an app performing maliciously post-certification by using an external web API with well-obfuscated requests and responses. While this isn't something that can be easily addressed, but I feel it should be recognized as a weakness and carefully considered when reviewing these kinds of apps.

13. When an app isn't recognized, the proposal states that a Policy Table update will occur and the app's name will be verified with the list of approved app nicknames. Is a Policy Server required for this kind of app to work, or can these apps be used through the use of Core's default Policy Table?

14. Can you please describe your reasoning for putting so much metadata in the manifest.json file which could be retrieved from a Policy Table's app_policies instead?

15. In the example manifest.json provided in the proposal, the example locales do not follow the current Policy Table locale format of [language]-[country]. For example, instead of de_DE, we should use de-de for consistency (note all-lowercased and uses a hyphen).

@theresalech
Copy link
Contributor Author

The Steering Committee voted to defer this proposal, keeping it in review until our next meeting on 2019-09-10, to allow for additional conversation on the review issue between the author and commenters.

@kshala-ford
Copy link
Contributor

@nickschwab, @joeljfischer, @joeygrover I will follow Joey's enumeration from 1 to 7 and reply to Nick's comments from 9 to 15. Technical notes from Joel will be appended with new numbers due to the revision that made a few notes obsolete.

To @joeygrover comments

|1

*if Core is built with DWEBSOCKET_SERVER_TRANSPORT_SUPPORT set to `true,

Of course, I think this can be implied as listen to a port is impossible without the code included.

Should there be any other mechanism that would determine if Core should listen on this port? For example, if the policy table includes at least one entry for a local web engine app

I like the idea but I have the same concerns around increase complexity. The query for the policy database is no rocket science but not sure if it delays Core initialization (might affect app resumption). I think the proposed ini settings to also specify an adapter address (e.g. localloop) it gives a fair amount of security to production vehicles. The benefit of this ini is that we could configure and listen to local network for test benches hence help developers to test their app from their computer.

|2a I'm not sure if I am understanding. I know cloud apps are independent of their location and could run on the local machine (though specified as cloud app). The new app preference should be independent of and should not interfere with the cloud app feature.

|2b I was reading SDL-0203 and the comments made during review but couldn't find any useful information. smartdevicelink.com doesn't say much about embedded apps. What does "Embedded" stand for in the developer portal? Isn't it what I mentioned in 2a? If you want us to spec out things properly we need the PM to do the same.

|3 Please have a look at bullet point 6

"endpoint" parameter will be omitted. Instead the HMI is responsible to launch local apps.

|4 Not sure if I can really follow what you're suggesting. I assume it is about loading an app into a web based HMI by pointing to a js file. I'm not a fan of this approach as it would require a monolithic, single process, single DOM, single domain where nothing is protecting the HMI from apps. If this feature is really something we would like to include (in future?) it needs to be fleshed out further. I believe "entrypoint" doesn't need to be changed to prepare this feature as a new key like "app_type" = [default|extension] where default is the app type we are proposing here and extension (or any other name) is regarding your suggested feature. Still this new parameter could be introduce in a future proposal.

|5 Yes this is the RPC spec version. It should be optional and allow developers to specify a RPC spec version required for the app to be operational. Same can be included for min Protocol version.

|6 Can you specify what information you are referring to? The only field I can see is the app icon. While I understand why an icon_url make sense for a cloud app I see benefits for a local app to embed the icon into the application package. The app installation routine of the IVI system will have all app information available when installing the app. For the developers and for the OEMs it's beneficial in the way that it is clear to how the app appears on the system. It's not possible to change the app appearance or reachability (voice commands etc.) from a different source. Last the driver/user could be confused if the app all of a sudden appears differently (different app icon) or if the app cannot be launched with the previously known voice command, all without an actual app update.

|7 One of the main features of JVM is to provide platform independency and of course this approach naturally includes security mechanisms. There are methods of Java to increase the security (as you mentioned the security manager) and practices to restrict access using the OS user roles. Using a sandbox with a web engine doesn't only protect the system from the app but also the app from the system (or other apps). Security and privacy are main goals of the sandbox. By default features are permission controlled (see chrome://settings/content) providing a very good protection service out of the box. As mentioned, there are many web apps available and we expect a high chance to reuse existing app code for in-vehicle apps.

To @nickschwab comment

|8 (belongs to @nickschwab notes to 1.)

Does the SDL open-source project have any responsibility regarding app updates ... whose responsibility is it ensure that only qualifying installations of Core receive the app update?

The burden is on the shoulders of the OEMs. An OEM will have many different versions of SDL Core on the road and these vehicles would request an app list from an OEM app store backend returning a list specific for the vehicle. The OEM backend would be based on data from the SDL server.

|9 This is one possible option to submit the compressed package to smartdevicelink.com. Imagining an app certification for such apps I believe it would be necessary. However, we're planning a developer to submit to our (OEM) developer platform (which can be referenced from smartdevicelink.com if such an API exist).

|10 I think this is a general question about SDL server and versioning. The first thing I have in mind to solve the problem for this proposal is to:

  1. Deprecate BOTH and don't add ALL
  2. Add a new parameter to (Cloud)AppProperties named hybridAppPreferences which is an array
  3. The old parameter remains for existing clients in the wild. The new is the successor and should be used in favor or the old one.
  4. ALL is realized by the array including all enum elements.
  5. Old parameter set to BOTH would match the new parameter set to [MOBILE, CLOUD, LOCAL] (or embedded... terminology yet to be decided)

|11 This is funny because in automotive using the word embedded is understood as "it is shipped with the vehicle" or "it's purely offline" or "it's fixed with the surrounding (software)". Local is describing a location instead. I would say "A local web application is running in an embedded WebEngine".

To the point that "EMBEDDED" already exist... I want to refer to 2b from Joey and defer the discussion to the next SC meeting. I believe there's need to discuss how EMBEDDED got introduced by the PM.

|12 You are right. The possibility of backend logic or data changes post-certification is a challenge. It's necessary to verify that the requests an app can send are known during certification and that they are all are properly tested. The app behavior might change due to backend sending different data hence triggering the app to behave differently. I want to note that WebEngines prevent apps to post-load executable code from a backend.

|13 The plan is when an app is installed into the vehicle, the HMI sends SetAppProperties to SDL Core with the app ID. SDL 0248 was designed for this case to allow SDL requesting a policy table update if the provided app ID is unknown.

|14 Can you please be more specific to "so much"?

|15 I want to make everyone aware of the RFC 5646 which we should follow everywhere where internationalization and localization is required. to your point: Hyphen is okay but I don't think lowercase region is a good idea. It should be uppercase.

To @joeljfischer comment (excluding OpenHMI questions)

|16 1. Will SDLServer have to change to accommodate these OEM store changes?

If this question is still valid but unanswered can you please be more specific to "these OEM Store changes"?

|17 3. I don't see the security concerns of running unknown JavaScript code on the head unit addressed...

Just to recap. With developers submitting a compressed app package to SDL/OEMs, all the code can and will be reviewed before it is made available to apps for installation. As I noted in |12 WebEngines prevent post-loading executable code therefore the app code is known with the app being installed. The question regarding the web browser/engine vulnerability (but also compatibility): It should be the OEMs goal (as it is our goal for this feature) is to keep the engine up-to-date throughout the vehicle lifespan. This means that regular software updates should be made available to the vehicle. At least the engine should really be at the latest version. I want to add that this is a general issue for any kind of runtime environment (node, JVM etc.). It's the host-owner responsibility to keep the host up-to-date For cloud apps, it's the developer's server. For local apps it's the vehicle/IVI.

|18

  1. There doesn't seem to be a mechanism to update an app bundle. I'm assuming that the OEM app store would need to know that its installed bundle is out of date and be able to update it. Is that up to the OEM app store backend?

Yes!

But I would assume this would need to be a policies issue to ensure they have the most up to date app bundle that supports the given head unit. (e.g. if v1.0 supports SDL 3.0-5.0 and v2.0 supports SDL 5.1+).

this is the purpose of minSDLVersion. The OEM store should know all apps and also different versions of the app. Depending on the requesting vehicle and the current version of the vehicle it may return only supported apps.

|19

  1. Ideally, a change like this would also include a way for a mobile app to transfer / update a PWA app and act as an internet relay for it. That way vehicles without built in modems (or users without modem subscriptions) can access this feature and the apps that use it.

We have done some analysis to how far phones can be used as a relay to the internet. Phone features like Hotspot or WiFi direct or using an app as a relay... The complexity and the risk of phone OS changes breaking the suggested feature caused us to not continue this route further.

|20

  1. In 3.1, you write: The app package should be a compressed zip file zip is not a great compression format. If OTA size is the main concern, why not consider something like tar + gzip? Though zip is very widely available, and would probably work on all systems in some manner.

I think any compression or packaging would work. With the OEM app store we will anyway need to unpackage and repackage the app compatible with the OEM specific OTA app installation procedure. This is planned to be completely transparent to the app and the partner and should not affect the behavior of the app.

|21

  1. In 3.5 you write: The entry point HTML file should refer to the manifest file (<script src="manifest.json" />) Where? In the head? Providing a sample HTML file may be helpful.

Any place is fine. Adding them in the <head> should be no issue (except performance wise). I think the common practice these days is at the bottom of the HTML file.

<!doctype html>
<html>
<head></head>
<body></body>
<script src="manifest.json"></script>
<script src="sdl.js"></script>
</html>

@theresalech
Copy link
Contributor Author

The author has revised this proposal per Steering Committee requested revisions. The revised proposal will now be in review until December 10, 2019. In addition to reviewing the revised proposal, Steering Committee Members should be aware of the following open items which will require vote at our next meeting:

15. Vote between the two presented options: Option 1: continue using existing patterns within SDL project Option 2: use standard proposed by Ford, and convert locale strings when necessary.

21. Vote on whether or not the location of the manifest's <script> tag placement should be specified within the proposal.

@smartdevicelink smartdevicelink unlocked this conversation Dec 4, 2019
@nickschwab
Copy link
Contributor

@kshala-ford Thank you for working with us to address several feedback items thus far. Additional items below:

  1. See quoted sentence below. This information should be read from a copy of the data stored in SDL Server, as SHAID is not guaranteed to know the OEM's individual approval status of an application version. Please change to: Instead the app information and assets should be read from SDL Server via a supporting API endpoint.

Instead the app information and assets should be read using SHAID with the additional Application APIs.

  1. Please add SHAID as an impacted platform.

  2. See quoted sentences below. Suggest revising for clarity and removal of conflicting statements: The initial submission of a WebEngine application will undergo SDLC app certification testing. Once the initial submission has passed certification, future submissions will not be required to undergo full certification testing but may be subject to additional certification tests/requirements.

App developers can request SDLC app certification performed by the SDLC on the initial submission of the application. Once the initial submission has passed the certification tests, the app should be marked as certified independent of future releases which won't be tested anymore. New versions of an app aren't required to undergo additional testing, but may be subject to additional certification tests/requirements.

  1. See quoted sentences below. Recommend removing mention of the "flexible html based user interface" and "driver distraction" since this proposal states that applications will use existing SDL templates rather than rendering custom HTML.

The upside of apps running with a WebEngine is that it comes with an extremely flexible html based user interface and with a very sandboxed runtime environment. This can also be seen as a downside as the responsibility of following driver distraction rules increases and a sandbox environment is limiting the functionality of apps.

  1. Impact on existing code needs additions: (4) SHAID changes to store and return the new data items displayed on the Developer Portal, (5) SDL Server changes to store copies of newly added SHAID data items and provide a customizable skeleton method for copying app packages from SHAID to an OEM-provided storage solution (e.g. Amazon S3).

  2. This proposal does not include functionality for the open-source version of SDL Core and HMI to be able to decompress and run WebEngine apps in a containerized environment. I believe the SDLC should take this into consideration when voting on this proposal, as app developers would not have a way to test their WebEngine apps without partnering directly with an OEM, negatively impacting the ease of SDL adoption by app developers.

@kshala-ford
Copy link
Contributor

Hi @nickschwab thanks for the feedback. I have a few questions:

|1. 👍 Do you agree to this suggestion? Instead the app information and assets should be read from the OEM policy server via a supporting API endpoint.. It means the OEM store should read app information from the SDL Server instance hosted on an OEM backend.

|2. 👍

|3. 👍

|4. 👍

|5. 👍 (Adding the bullet points but don't refer to external services)

|6. With my little knowledge about Manticore implementation I believe it could be possible to support the WebSocket-Server transport with Manticore. The only change required would be to expose the hostname and port of the Core transport. Also when building Core yourself with BUILD_WEBSOCKET_SERVER_SUPPORT you can run sdl_hmi on your computer for testing purposes.

In both variants the "WebEngine" would be the browser on your development computer, still I think app developers do have a way to test their WebEngine apps without partnering directly with an OEM. I want to note that SDL-0158 "Cloud App Transport Adapter" didn't mention the testing environment specifically either. What I'm saying is that even though the proposal doesn't clearly mention manticore, a test environment can be created and that there's no negative impact to the ease of SDL adoption.

|7. I was informed that the proposed AppProperties struct in the HMI API should rename the param appID to policyAppID.

<struct name="AppProperties">
- <param name="appID" ... />
+ <param name="policyAppID" type="String" maxlength="100" minlength="1" mandatory="true" />

This suggested change doesn't have a major impact to the feature. It's just to match the naming and attributes within the HMI_API.

|8. The proposal suggests that HMI should launch the app including SDL Core's hostname and port as GET parameters. The names should be sdl-host and sdl-port instead ws-host and ws-port. This suggested change doesn't have an impact to the feature. Just the naming should be more specific to SDL.

I've created a PR already that includes all the agreed suggestions. You can find the PR here #888

@nickschwab
Copy link
Contributor

@kshala-ford

1. 👍 Sounds good to me. Are you ok with the supporting endpoint(s) in SDL Server being an implementation detail determined by the PM?

6. I think the changes to support the testing of WebEngine apps "in the open" would be more involved due to the fact that app packages would need to be decompressed, stored, and executed from somewhere, but @JackLivio is probably best to address that effort from a Core/HMI perspective and @crokita from a Manticore perspective. I haven't looked back at the Cloud Transport Proposal, but regardless of what it contains, I think it is in the best interest of the SDLC (and SDL app developers) that new app-related features are supported by existing testing tools, and that such support is clearly documented in their respective proposals.

7. Will let @JackLivio confirm this.

8. 👍 I'm good with this.

@kshala-ford
Copy link
Contributor

  1. Regarding a test environment I was more focused on using the desktop browser to run the web application and connect it to manticore. This should not be a big deal with the content of this proposal as long as Manticore provides IP/host and port to the WebSocket Server transport. I do support a more advanced test environment where app packages can be uploaded, decompressed, stored and executed, however I believe this is a large extension to Manticore which I support in a separate proposal.

@nickschwab
Copy link
Contributor

@kshala-ford

6. Since there aren't strict system/runtime requirements at this time for WebEngine containers (other than supporting ECMA-2017 for the SDL JavaScript library), allowing Manticore to expose Core's WebSocket Server port should be a suitable way to allow developers to test their WebEngine apps. Can you please specify this in the proposal and add Manticore as an impacted platform, or mention the ability to test WebEngine apps as a prerequisite prior to releasing the feature (similar to what was written for WebEngine App Certification)?

@Jack-Byrne
Copy link
Contributor

Jack-Byrne commented Dec 9, 2019

a test environment can be created and that there's no negative impact to the ease of SDL adoption.

@kshala-ford Could we define some minimum capabilities for a test environment that would be delivered with the web engine feature? "Test environment" in the context of testing with a local build of core and a reference hmi.

  1. 👍 I agree with appID renaming to policyAppID in the �HMI API's App Properties struct.

@joeygrover
Copy link
Member

joeygrover commented Dec 9, 2019

6. I agree we have to have something for developers to test their production level bundle with at the time of this features release. I believe it is very important to being good stewards of our technology that we provide the proper tools at the time of launch for features if we can. These tools should also come from/be hosted by the SDLC, so that no specific OEM gains an advantage here.

7. ( I could be wrong here)The naming of the appID param matches the RPC spec initially, but that means the mapping will now be different between the RPC spec and the HMI spec. The RPC spec also does not have a min length required, therefore the parameter can't simply be passed from Core to HMI and could be rejected by the HMI even though the RPC spec doesn't state the same requirements. For those reasons I am against those changes.

8. Adding SDL as a prefix is good but could we still specify these with a transport type? websocket client vs server, or even TCP client/server? There's nothing worse than naming something vaguely and having to create a corner case around it because it's legacy.

@Jack-Byrne
Copy link
Contributor

but that means the mapping will now be different between the RPC spec and the HMI spec.

There is already a disconnect between RPC spec and HMI api regarding "appID". In the context of the hmi api, "appID" refers to an integer that is generated by Core and is not related to the actual app id of an app. The HMI API refers to an apps mobile/policy app id as "policyAppID".

Refer to the HMIApplication struct in the HMI_API.XML

<struct name="HMIApplication">
...
    <param name="policyAppID" type="String" maxlength="50" minlength="1" mandatory="true">
      <description>Policy ID(=the appID the application registers with) of registered application.</description>
    </param>
...
    <param name="appID" type="Integer" mandatory="true">
      <description>Unique (during ignition cycle) id of the application. To be used in all RPCs sent by both HU system and SDL</description>
    </param>
...
</struct>

Regarding the minlength, the don't believe the AppProperties struct is passed from any HMI RPC to any MOBILE RPC so the differing minlength requirement should not have an impact. However adding that minlength could cause a headache in the future if we come up with a feature that will pass that struct from HMI to Mobile, or vice versa.

@joeygrover
Copy link
Member

7. Ahh, that should be something likesessionID rather than appID because of other usages of that term. I understand now, and am fine with the change.

@kshala-ford
Copy link
Contributor

@nickschwab @joeygrover

  1. I've added Manticore as a platform. and added following to "impact of existing code" see here

  2. I am completely fine with a more specific name. Can we agree that finding a suitable name is a development detail? I don't think we need to be that specific for naming at this point. I can mention the names to be an example.

@joeygrover
Copy link
Member

  1. This is something that could easily be missed so I'm defining it as such :
    sdl-transport-role-idType
    sdl-{ws|wss|tcp}-{server|client}-{ip|port}

For this proposal we only need:
sdl-ws-server-ip
sdl-ws-server-port

Possibly:

sdl-wss-server-ip
sdl-wss-server-port

@kshala-ford
Copy link
Contributor

Thanks @joeygrover. Instead of ip I would use host which would be a valid naming for ip addresses or host names. I have a question though to the perspective of the role. When you say sdl-wss-server-ip what does it mean exactly? Does it mean the app should connect to a WSS server? or does it mean the app should be a server?

@joeygrover
Copy link
Member

joeygrover commented Dec 10, 2019

I'm fine with host.

In this case the HMI is passing these parameters to the app right? So the context would be this is the host name for a secure websocket connection server. So the parameters would be describing the Core/HMI information rather than an action for the client to take.

Edit: The other option would be including additional params instead of naming them this way.
sdl-transport-role:{ws-server, ws-client, wss-server, wss-client, tcp-server, tcp-client}

@kshala-ford
Copy link
Contributor

Thank you for the clarification. So for WebEngine the role name should be server as the IVI hosts the "server".

I like the transport role as it avoids mixing roles in host and port. I'll add it to the PR

@kshala-ford
Copy link
Contributor

The PR #888 contains the suggested changes. https://github.com/smartdevicelink/sdl_evolution/pull/888/files

The actual changes are rather minor making corrections to the text, being more specific to the GET params and enabling Manticore as a basic test environment. Therefore if there are no other issues to address I would suggest to accept the proposal with revisions.

@nickschwab
Copy link
Contributor

@kshala-ford

1. Are you ok with the supporting endpoint(s) in SDL Server being an implementation detail determined by the PM? If so, can you please note this in the proposal?

@theresalech
Copy link
Contributor Author

The Steering Committee voted to keep this proposal in review, as members were not yet ready to vote. It will be very important that members are prepared to vote on the following during the 2019-12-17 Steering Committee meeting:

  • Item 15 in comments: How should locales in manifest.json file be formatted?
    • Option 1: continue using existing patterns within SDL project
    • Option 2: use standard (RFC 5646), and convert locale strings when necessary
  • Item 21 in comments: Should the location of the manifest's <script> tag placement be specified in the proposal?
    • Option 1: Yes
    • Option 2: No
  • Accept the proposal with the revisions decided upon for items 15 and 21 above, as well as those included in this PR: https://github.com/smartdevicelink/sdl_evolution/pull/888/files

@theresalech theresalech changed the title [In Review] SDL 0240 - WebEngine support for SDL JavaScript [Accepted with Revisions] SDL 0240 - WebEngine support for SDL JavaScript Dec 18, 2019
@theresalech
Copy link
Contributor Author

theresalech commented Dec 18, 2019

The Steering Committee voted to accept this proposal with the following revisions:

  • When formatting locales in the manifest.json file, use standard (RFC 5646), and convert locale strings when necessary.
  • Do not specify the location of the manifest's <script> tag placement within the proposal.
  • All revisions included in this PR SDL-0240 WebEngine Support Revisions #888

@smartdevicelink smartdevicelink locked and limited conversation to collaborators Dec 18, 2019
@theresalech
Copy link
Contributor Author

Proposal updated to reflect accepted revisions, and implementation issues have been 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

7 participants