sched: Fix readStorage when files do not exist

[nxdefiant]

see #2985

[bobrippling]

I'd noticed a similar issue with rep and forgot to fix it - what do you think of promoting this functionaliy to Util for other apps?

[bobrippling]

Looks good to me - @thyttan, @halemmerich any comments before I merge this? (mainly asking it's a "core" app, and the first non-Gordon merge)

[thyttan]

Looks good to me - @thyttan, @halemmerich any comments before I merge this? (mainly asking it's a "core" app, and the first non-Gordon merge)

• Without this fix I can't access interface.html from App Loader ever (checked to see that I had sched.json on my watch which was there).
• With the fix I can access it on Chromium browser on my laptop and in Bromite browser on my phone and set alarms. 👍 :)
• But on the App Loader inside Gadgetbridge I can't use the interface before or after the fix. It gets stuck on "Please wait Loading..." (EDIT: I think this may be a general problem with such interfaces in Gadgetbridge, it's similar with at least Power Manager interface). If I am connected to the Web IDE I get:

>["sched.json"]692
W3sidCI6MTczNDAwMDAsImRlbCI6ZmFsc2UsIm9uIjp0cnVlLCJycCI6dHJ1ZSwiYXMiOnRydWUsImRvdyI6MTI3LCJsYXN0IjowLCJ2aWJyYXRlIjoiOjoiLCJkYXRlIjoiMjAyMy0wOC0wNCIsIm1zZyI6IkJva2Ega2Fub3Rlcm5hIiwiZGF0YSI6eyJobSI6ZmFsc2V9fSx7InQiOjI1MTQwMDAwLCJkZWwiOmZhbHNlLCJvbiI6ZmFsc2UsInJwIjpmYWxzZSwiYXMiOmZhbHNlLCJkb3ciOjEyNywidmlicmF0ZSI6Ijo6IiwibXNnIjoiU2FuZ2tsYWRlciIsImRhdGEiOnsiaG0iOmZhbHNlfSwibGFzdCI6MjF9LHsidCI6MzkzMDAwMDAsImRlbCI6ZmFsc2UsIm9uIjpmYWxzZSwicnAiOmZhbHNlLCJhcyI6ZmFsc2UsImRvdyI6MTI3LCJsYXN0IjoyMCwidmlicmF0ZSI6Ijo6IiwiZGF0YSI6eyJobSI6ZmFsc2V9LCJtc2ciOiIifSx7InQiOjQ3OTQwMDAwLCJkZWwiOmZhbHNlLCJvbiI6ZmFsc2UsInJwIjpmYWxzZSwiYXMiOmZhbHNlLCJkb3ciOjEyNywibGFzdCI6MjgsInZpYnJhdGUiOiI6OiIsIm1zZyI6IiJ9XQ==?

• I suggest bumping the version and ChangeLog so users that tried to but couldn't access the feature before get notified that there was a fix done.
• @nxdefiant, @halemmerich and you are probably better at judging the technicalities.

[nxdefiant]

• I suggest bumping the version and ChangeLog so users that tried to but couldn't access the feature before get notified that there was a fix done.

I don't see the point in increasing the version when the app itself is unchanged. Also have never done that, e.g. #2552 #2979

I can't say anything about Gadgetbridge

[thyttan]

I can't say anything about Gadgetbridge

It seems to me like the problem inside Gadgetbridge affects all similar interfaces. So lets not stop this PR from being merged because of it. I'll open a separate issue for that.

I don't see the point in increasing the version when the app itself is unchanged. Also have never done that, e.g. #2552 #2979

Fair enough :) I still think it's a good idea for the reason I wrote. But OK with me to merge also without. 👍

[gfwilliams]

So the issue is that Util.readStorage actually fails to return if the file doesn't exist?

As @bobrippling says I think we should make sure Util.readStorage works even if the file doesn't exist - basically making it behave in the way it does in this PR, but for everyone (just return undefined).

It should be easy enough to modify without adding list() since .read(filename) just returns undefined if the file doesn't exist. It's a bit odd though as it looks like it should work that way already: https://github.com/espruino/EspruinoAppLoaderCore/blob/431a3fb743da5c370729ab748cb2c177e70a345b/js/comms.js#L479

You can test on the console of the web browser with Comms.readFile("foo", x => console.log("f",x)) - I think it's probably readTextBlock that is broken?

[bobrippling]

Yes, I believe commit b8813ab solved the missing-file problem, and means we see a callback fired and receive an empty string for the file. When I run:

Util.readStorage("hi.txt", x => console.log("cb", JSON.stringify(x)))

I get a callback with this log:

cb "\u0000\u0000\u0000"

And JSON.parse("\x00\x00\x00") throws, which is probably what's stopping interface.html from parsing.

---

The three bytes come through because in atobSafe we have this do-while, which I think would be safe to change to a while(i < input.length){...} ?

With the fix from b8813ab and the atobSafe fix, I believe that'll let us receive an empty string when the file doesn't exist, and interface.htmls will then be ok with missing files.

[gfwilliams]

That sounds great to me! I've just added that change - also the code was in the IDE too so I've now changed that as well

[bobrippling]

Ah great, thank you!