Feature/web engine support #274

AKalinich-Luxoft · 2020-01-27T15:10:57Z

Implements SDL-0240
Related to #269

This PR is ready for review.

Testing Plan

Tested manually
Manual check-list can be found here

Summary

PR contains sdl_hmi implementation of [SDL 0240] WebEngine support for SDL JavaScript
PR.

Changelog

Implemented Apps Store view
Added web apps properties editor
Implemented new RPCs logic
Implemented web apps activation logic
Included new JS files into index HTML

CLA

I have signed the CLA

mked-luxoft · 2020-01-27T15:15:40Z

app/view/info/appsStoreView.js

@@ -0,0 +1,106 @@
+/*
+ * Copyright (c) 2019, Ford Motor Company All rights reserved.


2020 here and in similar places

@mked-luxoft fixed in a72a3d6

mked-luxoft · 2020-01-27T15:22:05Z

app/controller/InfoController.js

+        }
+        catch (err) {
+          SDL.PopUp.create().appendTo('body').popupActivate(
+            "Unable to load manifest file #" + index + ": " + err.message


i think here and in similar places it's better to use templated string if possible

@mked-luxoft makes sense. Replaced in a72a3d6

mked-luxoft · 2020-01-27T15:25:08Z

app/controller/InfoController.js

@@ -44,6 +62,214 @@ SDL.InfoController = Em.Object.create(
        event.goToState
      );
    },
+
+    loadManifestFile(input, index) {


i think function descriptions should be added in this file

@mked-luxoft description has been added to all new functions and elements in this PR. See a72a3d6

mked-luxoft · 2020-01-27T15:28:21Z

app/controller/sdl/RPCController.js

+         *
+         * @param {Object}
+         *            params
+         */


i think return should be added to description

@mked-luxoft fixed in a72a3d6

mked-luxoft · 2020-01-27T15:44:54Z

app/view/sdl/RunWebEngineAppView.js

+        {
+            elementId: 'runWebEngineSDLPortInput',
+            classNames: 'runWebEngineSDLPortInput',
+            value: '2020'


maybe this value should be stored in config file?

@mked-luxoft good idea. I moved host, port and transport params to a separate config structure in config file. See a72a3d6

mked-luxoft · 2020-01-27T15:45:14Z

app/view/sdl/RunWebEngineAppView.js

+        {
+            elementId: 'runWebEngineSDLHostInput',
+            classNames: 'runWebEngineSDLHostInput',
+            value: 'localhost'


maybe this value should be stored in config file?

@mked-luxoft good idea. I moved host, port and transport params to a separate config structure in config file. See a72a3d6

mkorniichuk · 2020-01-27T15:51:35Z

app/controller/InfoController.js

+
+      reader.onload = function() {
+        var file_content = reader.result;
+        var json_content;


@AKalinich-Luxoft do we need this declaration outside of try?

@mkorniichuk Actually, not :) But anyway that was a dead code which I removed in a72a3d6

mrapitis · 2020-02-05T15:44:13Z

@theresalech Ford has approved of this PR.

Added a new button to apps view to open apps store. Added controls, styles and logic for this new view.

Added a new view where user can set new properties or edit existing one for already installed applications. View is available from Apps Store view.

Implemented logic for get/set web app properties

Added a new view for specifying web app params Also added all required styles and displaying web apps on UI. Also updated styles to highlight with yellow enabled web apps and highlight with green running web apps.

All new files were added to the main HTML file Also was updated popup logic to hide disabled buttons

app/controller/InfoController.js

app/controller/sdl/Abstract/Controller.js

css/info.css

app/controlls/CodeEditor.js

app/model/sdl/RModel.js

app/view/info/webAppSettingsView.js

app/controller/InfoController.js

AStasiuk · 2020-02-11T11:10:28Z

@iCollin, @theresalech all comments were processed, could you please let me know when it’s planned to finish the review of this PR?

iCollin · 2020-02-11T16:32:04Z

@AStasiuk I gave a more detailed response to your question on the sdl_hmi_integration_guidelines PR but to summarize, we plan to be fully completed by February 26.

@AKalinich-Luxoft how did you test these HMI changes? Do you have an example web engine app you would be willing to share with me for testing purposes?

AKalinich-Luxoft · 2020-02-11T16:39:40Z

@iCollin yes, you can use web app from smartdevicelink/sdl_javascript_suite#107

You can also find manual testing guides attached to PR description

iCollin · 2020-02-11T22:07:12Z

@AKalinich-Luxoft right now, the web engine app is focused when it is opened. I believe the HMI should stay in focus at all times, and the web engine app should just be opened in the background.

AKalinich-Luxoft · 2020-02-11T23:52:40Z

@iCollin nowadays, it's not possible to implement pop under windows in the latest versions of Chrome and similar browsers due to not implemented window.focus() function (however it should work in another browsers). So the only thing I can propose here, is to launch web application in the small popup window but keep showing HMI window. Please check 195d468 for details.
If such approach is not the best, I can revert these changes.

iCollin · 2020-02-12T13:18:15Z

@AKalinich-Luxoft With some research I think that the best approach will be an invisible iframe.

AKalinich-Luxoft · 2020-02-12T17:18:13Z

@iCollin this approach seems work fine. Please check for updated implementation in b7e65b0

iCollin · 2020-02-13T16:46:52Z

Hello @AKalinich-Luxoft,

The actual downloading of the app package still needs to be implemented in this PR.

We have come up with two different ways this could be implemented.

In either implementation,

The sdl_hmi shall hit the app store server for a list of available apps (url can be hard coded)
These applications found in the app store should populate the "Available apps" column in the app store of the sdl_hmi

Completely In Browser

It would be possible to download the app bundle and launch web engine apps completely without the use of a separate component.
This could be done by:

fetching the app bundle in the browser from the app store
using a javascript zip library to unzip it
saving the file contents into the browsers local storage

If you were to use this approach, when an app is loaded, the 'src' tags in the html wouldn't work as the files are not saved on disk. To circumvent this problem, any element with a src tag would need to have its content inlined. For example, the manifest script that is included in the entry point html file would need to have its content placed between the script tags and the src attribute removed.

Python Helper Script

With the HMI PTU feature, a separate server (I will call it hmi_server) component was added to the sdl_hmi in order to get around the limitations of the browser. You could just pass the package_url that you receive from the list of available apps to the hmi_server, and add logic in the hmi_server to download the zip file, unzip it, and save the contents on disk, perhaps in a folder identified by the policyAppId so that the sdl_hmi could find the package on its own.

Other Changes

The example app json includes two properties, endpoint and authToken, that do not apply to web engine apps. These can be removed from the app properties editor.
The RunWebEngineAppView runWebEngineHTMLPathLabel and Input can be removed, but we can keep the GET params here.

Please reach out if you have any questions! I am happy to help.

AKalinich-Luxoft · 2020-02-17T02:52:41Z

Hello @iCollin

I tried "Python helper" way to implement all necessary changes according to your comment above.
I was able to get the list of available applications from the App Store server, download bundles, and unzip them to required folder. However, I was not able to read json content from manifest file of unzipped bundle. I am getting the following error as JS is working in the sandbox and does not have access to the file system:

Looks like HMI JS is not able to read the entrypoint from the manifest file. So we can either:

Hardcode index.html as the entrypoint, so HMI JS does not need to parse manifest file
Use python helper to read manifest file right after bundle unzipping and provide its content to HMI JS in the response. In that case HMI JS can work with app manifest read by python helper

Please let me know what is the best solution from your point of view

Implementation were done in a Python way - HMI sends request to Python script to download app bundle, unzip it and get the content of the manifest.js HMI is using entrypoint from manifest to generate path to HTML page to launch. Corresponding input has been replaced with label showing generated path. Python script was updated to support new requests from HMI App Store was updated to filter non API properties and display app icons

AKalinich-Luxoft · 2020-02-18T21:10:14Z

@iCollin please check for the updated implementation provided in ce14023

Implementation were done in a Python way - HMI sends request to Python script and request it to download app bundle, unzip it and get the content of the manifest.js
HMI JS is using entrypoint from manifest to generate path to HTML page to launch. Corresponding input has been replaced with label showing generated path.
Python script was updated to support new intenral requests from HMI.
App Store was updated to filter non API properties and to display app icons.

Please let me know if any other updates required

Jack-Byrne · 2020-02-19T16:05:07Z

app/controller/InfoController.js

+
+        let on_installation_failed = function() {
+          SDL.PopUp.create().appendTo('body').popupActivate(
+            `Can't install ${policyAppID} app from applications store...`, null, false


Is it possible to display the first nickname in the array (if available) instead of the policy app id?

@JackLivio no problems, fixed in f824da1

Jack-Byrne · 2020-02-19T18:26:45Z

app/view/sdl/RunWebEngineAppView.js

@@ -42,7 +42,7 @@ SDL.RunWebEngineAppView = Em.ContainerView.create(
      childViews: [
        'runWebEngineAppLabel',
        'runWebEngineHTMLPathLabel',
-        'runWebEngineHTMLPathInput',
+        'runWebEngineEntrypointLabel',


runWebEngineEntryPointLabel

I think the P in Point should be capitalized for all entry point mentions

@JackLivio renamed in f824da1

Jack-Byrne · 2020-02-19T18:36:38Z

app/controller/InfoController.js

+     * @param {String} response string containing server response
+     */
+    processAppsStoreResponse: function(response) {
+      var json_content = JSON.parse(response);


This lie is throwing an error:

VM314:1 Uncaught SyntaxError: Unexpected token o in JSON at position 1

response is already a json typed object, no need parse again.

@JackLivio yeah, previously server was responding with String so I have to parse it. Now it returns JSON object. Fixed in f824da1

Jack-Byrne · 2020-02-19T18:40:07Z

I apologize, I did not realize the PR was not ready for review yet. I wont leave any more comments until it is review ready.

AKalinich-Luxoft · 2020-02-19T19:38:56Z

I apologize, I did not realize the PR was not ready for review yet. I wont leave any more comments until it is review ready.

@JackLivio no worries, I processed your comments anyway :)
I also did some minor changes after the manual testing. I will let you know when PR is ready for review

Jack-Byrne · 2020-02-19T20:12:40Z

Note that ctrl+c does not kill the script after running the deploy_server.sh script

iCollin · 2020-02-21T17:05:57Z

In order for Web Engine apps to use PutFile RPCs and read contents of local files, a static web server has been added to the generic_hmi to host the app bundles. This enables the web engine apps to use fetch (or similar) and relative paths to read files from their bundle. A similar static HTTP server could be added to the sdl_hmi in the python script that would enable web engine apps to read their local files.

Python script has been updated to run static file server on start. This allows HMI to start web applications over HTTP protocol instead of file which has more limitations. Also script was updated to handle Ctrl+C signal instead of not obvious Q. Default transport role params has been changed to server

AKalinich-Luxoft · 2020-02-24T16:58:26Z

In order for Web Engine apps to use PutFile RPCs and read contents of local files, a static web server has been added to the generic_hmi to host the app bundles. This enables the web engine apps to use fetch (or similar) and relative paths to read files from their bundle. A similar static HTTP server could be added to the sdl_hmi in the python script that would enable web engine apps to read their local files.

@iCollin static HTTP server has been added to sdl_hmi. Please check 299ff0a

Note that ctrl+c does not kill the script after running the deploy_server.sh script

@JackLivio I also updated script to handle Ctrl+C as expected. Please check 299ff0a and proceed with your review

app/controller/InfoController.js

tools/start_server.py

AKalinich-Luxoft requested review from mked-luxoft and mkorniichuk January 27, 2020 15:10

mked-luxoft reviewed Jan 27, 2020

View reviewed changes

mkorniichuk reviewed Jan 27, 2020

View reviewed changes

mked-luxoft approved these changes Jan 27, 2020

View reviewed changes

LuxoftAKutsan mentioned this pull request Jan 27, 2020

Feature/Web Engine support smartdevicelink/sdl_core#3221

Merged

2 tasks

AStasiuk requested review from a user and mrapitis January 29, 2020 09:44

mrapitis approved these changes Feb 3, 2020

View reviewed changes

AKalinich-Luxoft added 6 commits February 6, 2020 12:16

Implemented Apps Store view

682217a

Added a new button to apps view to open apps store. Added controls, styles and logic for this new view.

Added web apps properties editor

a6e3941

Added a new view where user can set new properties or edit existing one for already installed applications. View is available from Apps Store view.

Implement new RPCs logic

8727d03

Implemented logic for get/set web app properties

Implement web app activation

c084884

Added a new view for specifying web app params Also added all required styles and displaying web apps on UI. Also updated styles to highlight with yellow enabled web apps and highlight with green running web apps.

Include new files in index HTML

0d6c1c9

All new files were added to the main HTML file Also was updated popup logic to hide disabled buttons

Answer review comments

6f45c9d

mked-luxoft force-pushed the feature/web_engine_support branch from a72a3d6 to 6f45c9d Compare February 6, 2020 10:18

iCollin requested changes Feb 6, 2020

View reviewed changes

Minor renamings and typo fixes

6d922a5

Run app in the pop under window

195d468

Run web apps in invisible frames

b7e65b0

Make apps installation from the apps store

bf55e3a

AKalinich-Luxoft added 2 commits February 18, 2020 13:50

Fix sending of empty message over RPC client

1e95e5f

Jack-Byrne reviewed Feb 19, 2020

View reviewed changes

Minor changes after manual testing

f824da1

iCollin approved these changes Feb 24, 2020

View reviewed changes

iCollin reviewed Feb 25, 2020

View reviewed changes

Minor changes according to comments

b506afc

iCollin approved these changes Feb 25, 2020

View reviewed changes

Jack-Byrne approved these changes Feb 27, 2020

View reviewed changes

Jack-Byrne merged commit 10954f7 into develop Feb 27, 2020

iCollin deleted the feature/web_engine_support branch February 27, 2020 15:57

		@@ -0,0 +1,106 @@
		/*
		* Copyright (c) 2019, Ford Motor Company All rights reserved.

Feature/web engine support #274

Feature/web engine support #274

Conversation

AKalinich-Luxoft commented Jan 27, 2020 • edited by VjKlepikov Loading

Testing Plan

Summary

Changelog

CLA

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

mrapitis commented Feb 5, 2020

AStasiuk commented Feb 11, 2020

iCollin commented Feb 11, 2020 • edited Loading

AKalinich-Luxoft commented Feb 11, 2020

iCollin commented Feb 11, 2020

AKalinich-Luxoft commented Feb 11, 2020

iCollin commented Feb 12, 2020

AKalinich-Luxoft commented Feb 12, 2020

iCollin commented Feb 13, 2020 • edited Loading

Completely In Browser

Python Helper Script

Other Changes

AKalinich-Luxoft commented Feb 17, 2020

AKalinich-Luxoft commented Feb 18, 2020

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Jack-Byrne commented Feb 19, 2020

AKalinich-Luxoft commented Feb 19, 2020

Jack-Byrne commented Feb 19, 2020

iCollin commented Feb 21, 2020

AKalinich-Luxoft commented Feb 24, 2020

AKalinich-Luxoft commented Jan 27, 2020 •

edited by VjKlepikov

Loading

iCollin commented Feb 11, 2020 •

edited

Loading

iCollin commented Feb 13, 2020 •

edited

Loading