Update upload and uninstall messages for clarity

[RKBoss6]

This PR just enhances messages shown while uploading and removing, and is state sensitive for firmware updates, providing a smoother feel of the bangle.js ecosystem. (Fixes #56)

[Copilot]

Copilot encountered an error and was unable to review this pull request. You can try again by re-requesting a review.

[RKBoss6]

I'm working on a few more subtle changes, I'll convert to a draft for now.

[gfwilliams]

That looks good, thanks - although if we're being pedantic, fwupdate just uploads the firmware image, and then Bangle.js restarts and actually installs it. I don't know if that changes what you want to display...

[RKBoss6]

Ok, I think this is good and it's been tested.

[gfwilliams]

I see you also changed

to

But this has almost doubled (158 bytes -> 288 bytes) the amount of code that has to be send before every app is uploaded, which is going to make every app upload that bit slower.

Plus on Bangle.js 1 your double-fill would cause a flicker (but I guess not many people use that now).

Any thoughts @thyttan @bobrippling ?

[gfwilliams]

The whole thing with uploads on Bangle.js is not ideal really, since now we upload with packets we don't update the progress bar as often. It used to be smooth and now it's very jumpy - I wonder whether we should actually look at changing the way it works.

What if:

• E.showMessage took an option uploadProgress:no_of_bytes
• When set, it would draw the progress bar, and upon packet upload, Espruino would automatically update the progress based on the number of bytes it had uploaded
• We could even upload the app icon first, and then do E.showMessage(...,{img:require("Storage").read("app.img"), ...}) (which it supports already) and get it to display an icon

Since then we're not having to upload a bunch of extra code (or even the p(..) commands) it should also be faster. We might even be able to add code to make showMessage animate if it was needed.

[RKBoss6]

That actually does sound a lot nicer than the current implementation, as you said, we are having to resend the progress bar code each time anyway, so having it bundled into showMessage will make it a lot faster and cleaner down the line. I also really like the app icon idea, it'll look really nice when tied together, maybe something like this:

[gfwilliams]

Ok, I'll take a look at this - I'm already mostly there with the Espruino bit, it's just making the app loader reasonably backwards compatible

[RKBoss6]

Cool! Also - not sure if this has anything to do with what we were discussing, but on my Bangle, firmware updates are taking increasingly long times, up to 30 minutes to upload. I'm wondering whether it's just cutting edge fw, as that's what I've been installing, or if there's something else. Before it used to be ~10 minutes maybe, and now the time has almost tripled... Have you noticed this when updating fw recently?

[gfwilliams]

I haven't noticed it getting slower personally. You're an iPhone user aren't you? So it's not that you're updating via Gadgetbridge (which can be slower)

[gfwilliams]

On recent builds you can now do E.showMessage( , {uploadProgress:no_of_bytes})

And you can make this change in the app loader to enable it:

diff --git a/js/comms.js b/js/comms.js
index f119985..1903924 100644
--- a/js/comms.js
+++ b/js/comms.js
@@ -121,27 +121,17 @@ const Comms = {
   espruinoDevice : undefined,
   // ================================================================================
   // Show a message on the screen (if available)
-  showMessage : (txt) => {
+  showMessage : (txt, options) => {
     console.log(`<COMMS> showMessage ${JSON.stringify(txt)}`);
     if (!Const.HAS_E_SHOWMESSAGE) return Promise.resolve();
-    return Comms.write(`\x10E.showMessage(${JSON.stringify(txt)})\n`, {noWait:true});
+    let opts = options!==undefined ? `,${JSON.stringify(options)}` : "";
+    return Comms.write(`\x10E.showMessage(${JSON.stringify(txt)}${opts})\n`, {noWait:true});
   },
   // When upload is finished, show a message (or reload)
   showUploadFinished : () => {
     if (SETTINGS.autoReload || Const.LOAD_APP_AFTER_UPLOAD || Const.SINGLE_APP_ONLY) return Comms.write("\x10load()\n");
     else return Comms.showMessage(Const.MESSAGE_RELOAD);
   },
-  // Gets a text command to append to what's being sent to show progress. If progress==undefined, it's the first command, otherwise it's 0..1
-  getProgressCmd : (progress) => {
-    console.log(`<COMMS> getProgressCmd ${progress!==undefined?`${Math.round(progress*100)}%`:"START"}`);
-    if (!Const.HAS_E_SHOWMESSAGE) {
-      if (progress===undefined) return "p=x=>digitalPulse(LED1,1,10);";
-      return "p();";
-    } else {
-      if (progress===undefined) return Const.CODE_PROGRESSBAR;
-      return `p(${Math.round(progress*100)});`
-    }
-  },
   // Reset the device, if opt=="wipe" erase any saved code
   reset : (opt) => {
     let tries = 8;
@@ -170,7 +160,6 @@ const Comms = {
     return Comms.write(`\x03\x10reset(${opt=="wipe"?"1":""});\n`).then(rstHandler);
   },
   // Upload a list of newline-separated commands that start with \x10
-  // You should call Comms.write("\x10"+Comms.getProgressCmd()+"\n")) first
   uploadCommandList : (cmds, currentBytes, maxBytes) => {
     // Chould check CRC here if needed instead of returning 'OK'...
     // E.CRC32(require("Storage").read(${JSON.stringify(app.name)}))
@@ -235,7 +224,7 @@ const Comms = {
           }
           // Actually write the command with a 'print OK' at the end, and use responseHandler
           // to deal with the response. If OK we call uploadCmd to upload the next block
-          return Comms.write(`${cmd};${Comms.getProgressCmd(currentBytes / maxBytes)}${Comms.espruinoDevice}.println("OK")\n`,{waitNewLine:true}).then(responseHandler);
+          return Comms.write(`${cmd};${Comms.espruinoDevice}.println("OK")\n`,{waitNewLine:true}).then(responseHandler);
         }
 
         uploadCmd()
@@ -289,14 +278,11 @@ const Comms = {
             if (uploadPacket) {
               let progressMin = 0.1 + (0.9*currentBytes / maxBytes);
               let progressMax = 0.1 + (0.9*(currentBytes+f.content.length) / maxBytes);
-              Progress.show({ percent: 0, min: progressMin,  max: progressMin }); // Don't show progress for sending the status update
-              return Comms.write(`\x10${Comms.getProgressCmd(currentBytes / maxBytes)}\n`, {noWait:true}).then(() => { // update percent bar on Bangle.js screen
-                Progress.show({ percent: 0, min: progressMin,  max: progressMax }); // Show progress for the actual packet upload
-                return Comms.getConnection().espruinoSendFile(f.name, f.content, { // send the file
-                  fs: Const.FILES_IN_FS,
-                  chunkSize: Const.PACKET_UPLOAD_CHUNKSIZE,
-                  noACK: Const.PACKET_UPLOAD_NOACK
-                });
+              Progress.show({ percent: 0, min: progressMin,  max: progressMax }); // Show progress for the actual packet upload
+              return Comms.getConnection().espruinoSendFile(f.name, f.content, { // send the file
+                fs: Const.FILES_IN_FS,
+                chunkSize: Const.PACKET_UPLOAD_CHUNKSIZE,
+                noACK: Const.PACKET_UPLOAD_NOACK
               });
             } else {
               return Comms.uploadCommandList(f.cmd, currentBytes, maxBytes);
@@ -323,8 +309,8 @@ const Comms = {
         // Start the upload
         function doUpload() {
           Progress.show({min:0.05, max:0.10}); // 5-10% for progress writing
-          Comms.showMessage(`Installing\n${app.id}...`).
-            then(() => Comms.write("\x10"+Comms.getProgressCmd()+"\n", {noWait:true})).
+          let bytes = fileContents.reduce((b,f)=>b+f.content.length, 0);
+          Comms.showMessage(`Installing\n${app.id}...`,{title:"App Loader", uploadProgress:bytes}).
             then(() => {
               doUploadFiles();
             }).catch((err) => {
@@ -718,9 +704,7 @@ ${Comms.espruinoDevice}.print("\\xFF");
       });
     } else {
       let cmds = AppInfo.getFileUploadCommands(filename, data);
-      return Comms.write("\x10"+Comms.getProgressCmd()+"\n").then(() =>
-        Comms.uploadCommandList(cmds, 0, cmds.length)
-      );
+      return Comms.uploadCommandList(cmds, 0, cmds.length);
     }
   },
 };

Note: you need to remove the progress call from backup.js too

So really it tidies stuff up a lot. Downside is that on firmwares without it (2v28 and before) you no longer get a progress bar - but then updates will be a bit faster.

... so I think before merging this we should wait until the 2v29 release

[RKBoss6]

Just putting this in a draft for right now, until 2v29 is released and we can get to work. The reason I don't want to close this just yet is to keep this PR as a reminder for when the time is right.