search

espruino / BangleApps

Bangle.js App Loader (and Apps)
https://banglejs.com/apps
MIT License
496 stars 1.17k forks source link

Add power usage module #3498

Closed bobrippling closed 4 months ago

bobrippling commented 4 months ago

A follow-up to #3477, this adds require("power_usage") support

Also updates types (based on espruino/Espruino#2533), specifically for startsWith()

bobrippling commented 4 months ago

Blocked on a firmware bug - espruino/Espruino#2529

bobrippling commented 4 months ago

Firmware bug closed - will hold off on merging until 2v24's released

gfwilliams commented 4 months ago

That fix reverted because it breaks parsing (1)/2 - I think it would be best to modify this code so it can be merged as fixing the issue properly is going to take a while.

What about changing if(!/^(LCD|LED)/.test(key)) to just if (!key.startsWith("LCD")) - I could be wrong but as Bangle.js doesn't have an LED, it seems like there's no need to check it? And this code is likely faster and definitely more readable.

bobrippling commented 4 months ago

Sounds good, sorted - I'll test this for a little while and merge in a few days. Bangle.js has both LED1 and LED2, but I doubt anyone's using them since they're just shown on the LCD, so it's probably not important

Poolitzer commented 4 months ago

I will join the testing in a couple hours, thanks for making the module

bobrippling commented 4 months ago

@Poolitzer I'll merge later today unless you've seen any issues?

Poolitzer commented 4 months ago

Yes looks good to me. For the end user it might be better to have it not that jumpy between screen on/off, but I feel like this is either a bigger addition to the module or smth the apps should take care of since they know in which intervals this gets called.

But thanks for putting it in a module and in the daisy app way better.

bobrippling commented 4 months ago

Cool - yes the jump isn't as extreme as when the LCD was on, but smoothing or improving the firmware to give a better estimate is what would be done next.

But yes - no problems on my end either, shall merge