victronenergy / gui-v2

Other
28 stars 10 forks source link

WASM: improve how reloading of UI happens after GX update #686

Open mpvader opened 9 months ago

mpvader commented 9 months ago

text updated to status per 2024-10-01.

Many improvements have already been done within scope of this issue; and how it behaved good enough for release of v1.0.0.

How it works now: when a GX update is detected, local WASM gui-v2 will be reloaded correctly. Also, VRM WASM gui-v2 will reload the entire VRM page. For VRM, that has the effect of closing the currently-open gui-v2 frame, not perfect.

The remaining work for this issue is to, on VRM side, somehow figure out when the "new" gui-v2 is available for launch from VRM (by detecting updated gui-v2 hash?) and then automatically re-launch the frame which contains gui-v2 (or something? that might be a bit strange for the user, to have a frame pop up automatically?)

mpvader commented 9 months ago

Meanwhile, I was also able to reproduce it.

blammit commented 9 months ago

Maybe every 10 minutes, gui-v2 can check the currently installed version, and if it doesn’t match the one that was loaded when the app started, then force the app to be reloaded in the browser.

chriadam commented 9 months ago

There are a variety of things we can do here.

The first issue is that we need to detect the version update (as suggested, either via polling the firmware version value, or via listening to a change signal for it, and handling specifically).

Once we have detected that version update, we need to trigger a browser page reload. Some preliminary investigation suggests that we can modify the qtloader.js to handle an application exit code and then trigger reloading the page, which should pull down the new blob from the device.

kwindrem commented 9 months ago

If the reload takes more than a second or two, interrupting the GUI could get in the way of a critical operation. After years of "stealing" the computer out from under a mission critical task, Microsoft now asks if you want to update now. You should ask before triggering an operation that could take a while on a slow internet connection.

chriadam commented 9 months ago

We could potentially show a modal dialog "Version update detected, reload required" which requires the user to click "OK" which will then trigger the reload, rather than automatically triggering the reload.

Note that the modal would be "inside" the web browser frame (i.e. part of gui-v2 webassembly application) not an OS modal dialog.

kwindrem commented 9 months ago

And a "Cancel" button also. Otherwise the effect is the same: a forced reload which could take time. Remember the WASM image is 15+ MB.

mpvader commented 9 months ago

In the end it will need to be forced @kwindrem , that is just how it is.

So, if there has to be a cancel button, ok, but then popup should come back 10 minutes later again.

The weirdest things may happen when using an outdated gui versus the running Venus OS version. Ofcourse it will be fine 99% of the time, but not always.

So @chriadam, please implement this:

Text: Version updated detected, reload required

Two buttons:

  1. Remind me in 10 minutes
  2. OK

And the OK one is the larger one or highlighted one.

kwindrem commented 9 months ago

I've given this issue some additional thought and believe the GUI reload should happen as soon as possible and does not need any user warning.

The WASM will update only when the Venus OS firmware updates so the GX device will go down at this time. This should be detected and reported by the remote GUI ("GX device not responding"). Then when the GX device is dented again a test can be made in the GUI and the new WASM version detected, triggering an immediate download.

If you wait 10 minutes to update the local GUI code, then you could be in a situation where critical operations are in progress and the need for notifications and user choices to delay the update become necessary again. That's why I suggest an GUI update as soon as possible.

During development, the WASM image on the GX device could be updated without a GX device firmware update or reboot. This too should probably result in an immediate refresh of the WASM image on the display device.

chriadam commented 9 months ago

Did some investigation into this yesterday.

With Qt 6.5.x builds, the qtloader API has e.g. showExit function hook, so that you can choose what to display when the application exits. With Qt 6.6.x builds, the qtloader API has onExit function hook, so that you can choose what to display when the application exits. Both version support config options to define whether the application should be restarted or not upon exit, and the method by which it should be restarted (i.e. just restarting the wasm module, or reloading the entire page which is what we'd want to do).

Unfortunately, none of it works. I've created https://bugreports.qt.io/browse/QTBUG-121765 to track the issue in Qt 6.6.x (since that's the version we will shortly be targeting with WebAssembly builds).

We may have to directly invoke a JS method (i.e. one which just calls location.reload()) from C++ via the emscripten API. I need to figure out how to expose such a method directly to the C++ side. Once the machinery is in place, it will be a simple matter of performing a Qt.quit() in the QML when required, and then in main.cpp listen for the app.aboutToQuit() signal and invoke the location.reload() method.

chriadam commented 9 months ago

Turns out it's even simpler, can just do emscripten_run_script("location.reload()"); directly from within C++, without needing to expose any JavaScript function to C++ as an extern function etc.

blammit commented 9 months ago

Assigned to @chriadam , he is looking into this already.

DanielMcInnes commented 9 months ago

already on dbus - 'firmware installed version'. Should already be on mqtt

DanielMcInnes commented 9 months ago

Check slack for screenshot

DanielMcInnes commented 9 months ago

image

mpvader commented 8 months ago

Please use installed build rather than installed version @DanielMcInnes / @chriadam

chriadam commented 8 months ago

https://github.com/victronenergy/gui-v2/pull/791

Note that VRM doesn't seem to send the message straight away, so I added a ten minute poll timer to force fetch the value.

For local devices, QtMqtt takes up to two minutes to notice that the connection is lost before reconnecting, which is a bit slow, but we can look into that separately. I did a bit of digging into that today, and managed to force state change to disconnected when the websocket is disconnected, but then I ran into a bunch of other issues with QtMqtt with WebSockets after a reconnection (specifically: subscriptions not sending message-received signals after reconnect). So I gave up for now, since it's lower prio than other issues.

chriadam commented 8 months ago

https://github.com/victronenergy/veutil/pull/3/files fixes the websocket issue noted above. (once that one is merged, we still need to update the submodule ref in gui-v2, but that will be a separate PR)

chriadam commented 8 months ago

Merged to develop. Closing as done.

philipptrenz commented 3 months ago

I performed a firmware update from v3.50~3 to v3.50~4 via local network WASM version. According to the JS console, it correctly registered the changed build version, but the browser reload actually never happens.

Bildschirmfoto 2024-07-23 um 13 57 11
mpvader commented 3 months ago

we'll need to make sure that the reloading works both when running the wasm locally as well as via VRM.

chriadam commented 3 months ago

@mpvader with this PR ^ local reload is fixed again. However, for VRM, it seems to reload the entire page (e.g. https://vrm.victronenergy.com/installation/133622/dashboard) rather than the popup which contains the gui-v2 view. Who should I ask for help with this?

chriadam commented 1 month ago

As discussed in the meeting: in the case where the firmware version on device is updated and there is an associated change to the wasm blob, it will be pushed up to VRM. This can take some time, so simply reloading immediately upon detecting the firmware version value update, will not have the desired effect. Jeroen mentioned that for release builds, the prebuilt wasm blob with the correct hash should be prepopulated into the VRM cache, so it won't be a big issue.

So, I still need to fix the "just reload the iframe, and not the entire page" case (talk to Alex about this), but it's low priority task (since the gui-v2 currently isn't getting stuck in a half-working / incorrect state).

Marking low prio.

mpvader commented 1 month ago

@chriadam - afaik this is mostly solved; right?

Can you change the title of the issue and add a summary above of what works and what remains to be done?

chriadam commented 1 month ago

What works: when a GX update is detected, local WASM gui-v2 will be reloaded correctly. Also, VRM WASM gui-v2 will reload the entire VRM page (which has the effect of closing the currently-open gui-v2 frame).

What remains to be done: on VRM side, somehow figure out when the "new" gui-v2 is available for launch from VRM (by detecting updated gui-v2 hash?) and then automatically re-launch the frame which contains gui-v2 (or something? that might be a bit strange for the user, to have a frame pop up automatically?)

mpvader commented 1 month ago

thanks clear, I've updated to top comment & title a bit further. it will come up later and then we can discuss with el nino.