-
And I think that's good. I'm not really sure I understand the reasoning behind making the weather app behave differently to every other app (going to launcher instead of the clock) but my preference would be to make 'back' go back to the clock.
The idea was less changing the way the app behaves and more removing the need to manually remove
Bangle.CLOCK
after setting the UI to clock. An "clocklike" UI that does not set that marker var would be enough for that. The clock UI messes with some widgets if users forget to removeBangle.CLOCK
and it is not easy to find if none of those widgets are randomly installed during testing.I think we'd already discussed this in a PR?
Yeah, I think we did :) Actually
fastload
already includes some optional code to redirect close to every load to launcher so always going to clock is fine with me. -
-
I think the back button and stuff like
widclose
/widcloselaunch
have slightly different purposes. The widgets get me back to clock/launcher in one touch from everywhere. The back button is/should be more dependent on context. It was initially created for things like menus, to be able to navigate the different layers there without having to create an explicit menu entry for going back. So essentially back navigates inside the current app and on reaching the "root" level of the app it currently either disappears or the next touch goes to launcher or clock. What is actually happening is of course implementation dependant.
This is the reason for https://espruino.github.io/BangleApps/?id=fastload having the redirect option which can redirect every call toBangle.load
(and thus indirectlyBangle.showClock
) toBangle.showLauncher
.Maybe we need some kind of UX guideline in the docs regarding the use of the back button? Or a default implementation going back to clock or launcher (configurable?).
Regarding the weather app, I think the use of the clock UI is a workaround for not having to copy over the clock UI implementation. Since the only thing that UI actually does is setting a button for going back to launcher, replacing that for weather with a custom UI would probably be ok.
-
-
The improvements are pretty much the same as between clock and launcher minus the CRC32 check for changes.
Checking forloadWidgets
everytime would take between around 200ms and 1000ms, so essentially eating all improvements. Caching the results and just checking the CRC takes 30ms-150ms.
Assuming a 300ms boot there is a 2x to 5x decrease in loading time from launcher. My bloated bangle install takes a lot longer than 300ms to boot ;)I would not install this by default or integrate in firmware at this point in time either. Since it has no dependencies and "only" applies what is already done in
.bootcde
to other loads it should not have that many side effects (there are probably some with other apps redefiningload
). After leaving an app there is still a full reload every time as long as apps do not widely implementremove
. -
I have done a new version of my
fastload
app. This can fastload any app when it contains widgets and the calling app implements a remove method. It is essentially the same thing.bootcde
does, but for any app. Thewidgetupdate
app from my apploader should be removed, if you have it installed. It is not yet modified to work correctly with the slide out widgets and has fallen behind the changes tofastload
.It works by overloading
Bangle.load
andload
to catch all load attempts and then tries usingBangle.load
for everything withBangle.loadWidgets()
in it. That causes a fast load if possible or a normal load if the calling app does not have a removal method. Currently this means fastloading everything with widgets from launchers with removal method and normal loading on the way back. @Gordon do you think something like this would be ok for the normal apploader? -
There is already a link to the apps folder on github and one click further (History) gets you all commits for that app. Usually it is possible to make out creator and main contributors for an app at a glance that way.
Automatically detecting that by amount of changes etc. is difficult. Someone reformatting the code for example would appear to having rewritten the complete app as close to every line would have changes. Another problem would be apploader forks.I think there could be made an argument for saying @Gordon is the maintainer of the espruino/BangleApps repository and practically for all apps there, since he merges the pull requests and has an eye on the changes. Similarly everybody forking that repo and publishing a custom apploader essentially becomes maintainer for all apps on that fork. A fork not keeping the apps current becomes a fork with changes against the espruino repo by default after some time.
Manually tracking one person as a maintainer in the metadata would mean this person being shown as maintainer in each an every of the currently over 800 forks.
Maybe the problem of forked apps could be a bit alleviated by defining those as
variant
of the same thing and show only that in the apploader. For example the whole crop of bluetooth widgets (widbt, widbthide, widbt_notify, widminbt) could all be marked as widbt variants and the apploader could show only that one variant as app and ask the user which one to actually install in a second step. That would essentially be the same as is done with theprovides
tag in metadata for modules.
I initially implementedgpstrek
as a duplicate ofgpsnav
. While it never got on the official apploader in the initial stages, it would have been a prime candidate for thisvariant
mechanic. It could have lived as a variant and then after some development it got different enough for not being a variant anymore (the route feature, completely new gui, background running). I had decided to not make a pull request as long it was to similar to the original, but essentially it already had some improvements that could have been useful in the apploader as a variant.A second step could be deprecating apps that way and not show them any longer in the apploader by default. Forks could be created for a specific and if they are functionally fully supersede the original app at some point, both could be marked as
deprecated
anddeprecatedBy
to filter those out during display as well. A newwidclosebutton
with settings for what to load could deprecatewidclose
andwidcloselaunch
this way while keeping those available for people still using them. There could also be hints in the apploader to change fromwidclose
towidclosebutton
. Something like this could be a way to implement a grace period before actually removing apps from the official apploader. -
Those are some really great numbers, definitely easy to see in daily use.
The not yet fast load compatible clocks could be changed to loading widgets and then directly hiding them using
widget_utils
without ever drawing. That would be enough for fast loading them and would not disturb the full screen watch face. But it comes with the caveat that then the widgets are loaded to RAM and active. Especially their registered listeners do their thing and could for example react to touches or do whatever they usually do when visible. -
Regarding another minifier than esprima, have you tried uglifyjs? It is build for Node.js and there are projects using it wrapped for a browser like https://github.com/Skalman/UglifyJS-online. I know close to nothing about the whole NPM/Node world, it's just what I had seen used for minification sometime before.
As for the minification problem in iconlaunch, I currently can not reproduce that because of changes I have done to iconlaunch since then. Probably because the problematic code now just uses other variable names after minification. I will have an eye on that and try to build a minimum sample if I see it happening again.
-
Continuation of: original post
In custom.html, merge all of imageclock.draw.js into imageclock.app.js - and then it'll all get minified together and should end up faster.
I tried this and got 5-10% faster time to first complete draw by pretokenization alone. Additionally using minification got about 5% on top of that. So pretty worthwhile I think.
for (let a of a.getPoints())
It looks like a minifier bug? a = [3,4,5]; for (let a of a) console.log(a); fails in Node.js too so it's not a problem with Espruino not being spec compliant.That could well be. I could not find similar problem descriptions, but redeclaring a variable existing in an outside scope sure seems fishy.
Edit: I have tried running the closure compiler on the same code and it createsfor (var n of a.getPoints())
insteadthe minifier generated a constant named g into a block of code calling Bangle.load which in turn calls g.reset()
That feels like another minifier bug?I don't know. Does the minifier actually know that
Bangle.load
can callg.whatever()
? This is probably a quite seldom problem, since you need to have code using enough variables that the minifier gets to "g" as a variable name. Maybe it would be enough to prohibit the minifier from using "g" as a name? -
I have found two apps breaking with the new minification switch: imageclock and gpstrek.
Imageclock breaks because the code generated by the customizer (and evaled in the app) references things that have been renamed by the minifier. What is the best way around this? Defining those globally and deleting them on remove?
In gpstrek the minifier generates code like
for (let a of a.getPoints()) {
which not surprisingly results inUncaught Error: Cannot read property 'getPoints' of undefined
.a
is a function parameter in this case before getting redefined in the for loop.Edit: During experimenting with iconlaunch the minifier generated a constant named
g
into a block of code callingBangle.load
which in turn callsg.reset()
. That then fails because of the minifier-generated constant. -
-
What are your settings for BTHRM? Are you using recording during run? It seems to work fine on 2v15.140 for me (Default Mode, Debug->Bonding, Recording HRM in Run). Since the last couple updates 0 bpm events are filtered, so coming out of the fallback internal HRM takes a little longer if the BT belt delivers 0 bpm events at the start. My Tickr X2 needs about 15-20 seconds after connect before sending usable heartrate data after a cold start.
As for UX I had thought about creating an optional widget for showing current connection state to be able to see problems while using another app. Since apps are usually not aware of the BTHRM and just use HRM events that could be helpful. -
Edit: Removed duplication with @Ganblejs earlier post.
- Check for any watches, timeouts, intervals that might survive the removal of the app and clear them in
remove
. - If code running in timeouts or intervals needs access to variables you might need to move them out of the block into the global scope. Those must then explicitly be deleted during
remove
.
I use the following uploaded to RAM to do a smoke test for memory leaks:
let app = "launch.app.js"; print("Initally load app", process.memory().free); Bangle.load(app) setInterval(()=>{ print("Reload app with free", process.memory().free); Bangle.load(app); },1000);
This code loads the app over and over. It usually settles in after 1 or 2 iterations and does not change a lot after that with most apps. This will not cover all cases, but it should give you a start. As usual, do changes piece by piece and test between steps.
- Check for any watches, timeouts, intervals that might survive the removal of the app and clear them in
-
Maybe the clock was manually set before the changes to the boot code and there is no value for settings.clockHasWidgets? In that case setting the clock again in the settings app should help. It seems to work for me.
Edit: Actually works only once, it seems some more cleanup is needed. The app does not run in a scope and actively only cleans up the timeouts/intervals. So all variables stay globally available and that clashes on reloading the app during
const SETTINGSFILE
assignment as already existing. -
With the newest additions to the cutting edge firmware I could reduce down the code in my experimental
fastload
andwidgetupdate
apps.fastload
does close to nothing now, it essentially only shows the "Fastloading..." screen as a visual aid to differentiate between fast and normal loading.widgetupdate
still does some trickery while loading widgets to wrap their display input related handlers to prevent them registering interactions while not visible and to reduce the number ofdrawWidgets
calls to at most two per call from an app.I think the state in the normal development apploader is now plenty fast for daily use and we are currently at a point of diminishing returns for experiments like mine. That does not mean I will stop playing around with this but I think there have been some really great improvements the last few weeks.
-
If you want to use a block scope (
{ code here ... }
) to let the garbage collector do it's job, you need to use let/const and not function/var, because function/var would be defined in the global scope. Everything defined in a block scope with var/function would need to be cleaned up manually before switching. -
I suspect both the example as well as swipe menu work as expected and override whatever exists before :) So probably my wrapping the methods to apply the remove methods are just overriden. Since those would probably most useful in firmware instead of boot code that would not be catastrophic once mine or similar changes are in firmware. There is however the problem that stuff overriding functions like this breaks all new features implemented later in those system functions. Wrapping/Enhancing should probably be preferred to hard replacing existing functions.
-
Yes, the launcher currently needs to load the clock app using
setTimeout(eval,0,s.read(".bootcde"));
and additionally it needs to undo all changes with a global impact other thanBangle.loadWidgets()
to prevent memory/resource leaks. The default launcher for example does the cleanup in lines 48-50 in thereturnToClock
-method.The clock needs to use
Bangle.setUI
and give it an options object containing aremove
method to clean up everything it changed in the global scope.If clock and launcher are implemented this way, fast switching between those works. If only one of launcher and clock use this way fast switching only works while leaving that app.
-
The first 4 of your pictures are most probably caused by the fast loading and widget handling and should be fixed by updating to the current versions on the development apploader https://espruino.github.io/BangleApps.
The fifth picture is caused by having debug output enabled, that moves the screen up and the clock draws parts of its graphics over that moved image.
The white box issue is probably somehow caused by a bug in the watchface app and as you assumed unrelated to the problem and fixes regarding the widgets. -
Nice low impact solution for this. There are edge cases like some clock storing the Bangle object in another variable and calling loadWidgets on that, but that probably never happens.
I think a new style of widgets could be implemented without breaking backwards compatibility to the old widgets style. Only generic fastloading would not work with classic widgets present. The other way around (old firmware and new widgets) would be problematic since the widgets could not be correctly loaded then. I will probably toy around with the idea to see if it leads anywhere.
-
I guess actually my change to the launcher would apply to all builds - it's just anyone that has launcher 0.15 or later
Yes, that is probably correct since it evals directly.
I guess we've got your changes to loadWidgets (with hiding) that could be used?
The current state of my widgetupdate branch probably solves this independent of the build. It does however do some things that probably need some thought before implementing directly in firmware:
- Keeping state of widget visibility in a global way, currently that are variables in block scope of the boot.js code
- Currently it overrides the load method to reset the state, but we could probably do a appload event or something to handle that outside of load.
- Hiding currently only catches the display handlers(touch,swipe,drag) if they are set directly during load by the widget. Handlers set during the livetime of the widget are not caught. Some widgets do replace their handlers regularly. Maybe some documentation on restrictions for widgets would suffice.
- The first dummy draw using
FAKE_G
is not necessary, just a try to save a little bit of drawing time. It could be replaced by a standard call todrawWidgets
.
Checking for loadWidgets in the code seems expensive, but maybe we could mark apps in their info files as using widgets.
Another idea would be defining a new style of widgets, where they are not arbitrary code somehow modifying
WIDGETS
but an object like{ load: function, remove: function, draw: function, update: function, //Must handle hiding and reaction to Bangle.CLOCK, e.g. removing handlers area: function, //Allowed to be modified by load/update/remove but not draw width: function //Allowed to be modified by load/update/remove but not draw }
and then only fastload if no classic style widgets are loaded. That would feel less workaroundy.
- Keeping state of widget visibility in a global way, currently that are variables in block scope of the boot.js code
-
No, I meant the espruino pull you linked since that does not change the access order. I would suspect that it was broken even before that for the no widgets loaded situation.