thyttan commented 2 months ago

Thanks!

The intended function seems to work well. But activating repeatable timers is a bit wonky.

Bug with repeatable timers

If a newly set timer has run out and is turned off, if I start it via the clock info entry it will run out again immediately. If I start it after a minute or two it will instead say it will run out in "24h".

Reproduce

Set a new timer in the alarm app for 15 seconds. Uncheck "Delete After Expiration".
Navigate to a clock face with clock infos on it (I used 'bwclk').
Wait for the timer to run out.
Click "Stop" on the timer alert screen.
Click the now inactive 15 s timer on the clock info to restart it.
The clock info says "0m" (or "-1m") and the timer runs out immediately.
Click "Stop" on the timer alert screen.
Wait a minute or two.
Click the inactive timer on the clock info again.
The clock info will say it will run out in "24h" and the timer will indeed not run out in the specified 15 seconds.

Other considerations

Events set with "Delete After Expiration" unchecked will be toggleable via the clock info entry.
- This may be confusing. A user may see the entry on the clock info and toggle it on and expect an alarm to go off at that time the next day but it will not go off until the next year (depending on configuration of the event).
- I think the icon for events should be different from the one for standard alarms.
- Instead of clock time, maybe the date of the event should be displayed?
Similar to the situation with events, maybe we want to differentiate between standard alarms that will not fire soon (the next time the clock is the time displayed on the clock info entry) and those that will not go off as soon?
- Maybe writing a letter on the alarm icon corresponding to the weekday it will fire on next?

bobrippling commented 2 months ago

Nice finds! For events, perhaps we ignore these for now so the user can't toggle them, to avoid any confusion. For timers, I've made it so we restart the timer. What do you think?

thyttan commented 2 months ago

I think that's a good idea to make events not toggleable for now - longer term maybe they shouldn't even be displayed in the same clock info? I don't know.

Restarting the timer makes sense!

Some bugs

Toggling a timer or alarm on through the clock info removed all other alarms, timers and events completely!
Toggling off an alarm through the clock info didn't actually disable it.

bobrippling commented 1 month ago

I think that's a good idea to make events not toggleable for now - longer term maybe they shouldn't even be displayed in the same clock info? I don't know.

Yeah I don't know either - I don't use them enough to have a good opinion.

Restarting the timer makes sense!

Sweet!

Some bugs

Toggling a timer or alarm on through the clock info removed all other alarms, timers and events completely!

I can't reproduce this - I had the IDE running, tapped on a disabled alarm and noticed that it became enabled - widalarmeta also activated, showing a countdown. It then fired at the time - this was created (but disabled) via the alarm app, is that what you're using?

Toggling off an alarm through the clock info didn't actually disable it.

Similarly, I created an alarm set for 1 minute's time, switched back to the clock and disabled it via clkinfo. The alarm didn't fire and I confirmed (with require("sched").getAlarms()) that it was off. Perhaps I have an odd setup? Do you have a sched.json that has some repros?

thyttan commented 1 month ago

Perhaps I have an odd setup?

Or perhaps I do 🤔🙃 I can do more testing in a day or two.

bobrippling commented 1 month ago

Thanks, and thanks for your testing so far too :)

If it helps, I've deployed to my gh-pages too

thyttan commented 1 month ago

I did some more testing - the two problems seem to arise:

independent of the used clock face.
- Tried with bwclk and pebblepp.
independent of firmware.
- Tried with firmwares stable 2v24 and 2v24.61.
independent of using my setup or doing a factory reset before trying.
independent of pretokising apps or not.

I don't see errors in the Web IDE.

EDITS:

test without pretokenized apps
Could it be something with timezones? That could be one difference between us two maybe? I'm at GMT+2.

bobrippling commented 1 month ago

Interesting - I'm GMT+0, shall take a look at whether that could be doing anything. These are the steps I'm following - perhaps I've missed something that brings up the bug?

Disabling an alarm

Create an alarm for 2 minute's time (via the alarm app)
Return to clock face
Confirm alarm is active (via both widalarmeta and seeing the alarm in clkinfo as active)
Tap alarm in clkinfo

Outcomes:

Observe it transitions to the inactive icon
Wait 2 minutes, confirm alarm doesn't fire

Enabling an alarm

Create a disabled alarm for 2 minute's time (via the alarm app)
Return to clock face
Confirm alarm is disabled (no widalarmeta shown, icon is disabled / has the little 'x')
Tap alarm in clkinfo

Outcomes:

Active icon is shown
widalarmeta appears, showing time to alarm (~1 min)
Wait 2 mins, alarm fires

thyttan commented 1 month ago

Disabling an alarm

1. Create an alarm for 2 minute's time (via the alarm app)

2. Return to clock face

3. Confirm alarm is active (via both `widalarmeta` and seeing the alarm in clkinfo as active)

4. Tap alarm in clkinfo

Outcomes:

1. Observe it transitions to the inactive icon

2. Wait 2 minutes, confirm alarm doesn't fire

My outcomes for that disabling flow on factory reset, non-pretokenized apps, fw 2v24, bwclklite differs from yours:

The clock info transitions to the inactive icon.
widalarmeta does NOT stop displaying the eta.
Wait 2 minutes - the alarm fires anyway.

Enabling an alarm

1. Create a disabled alarm for 2 minute's time (via the alarm app)

2. Return to clock face

3. Confirm alarm is disabled (no `widalarmeta` shown, icon is disabled / has the little 'x')

4. Tap alarm in clkinfo

Outcomes:

1. Active icon is shown

2. `widalarmeta` appears, showing time to alarm (~1 min)

3. Wait 2 mins, alarm fires

For this enabeling flow I get the same result as you.

thyttan commented 1 month ago

Test for "Toggling a timer or alarm on through the clock info removed all other alarms, timers and events completely!"

Connect to Web IDE.
Create two alarms and leave them enabled, I choose 12:00 and 14:00.
Return to clock face
Run require("sched").getAlarms() to see both alarms in the Web IDE console field. Both have on:true.
Toggle off the 12:00 alarm through the clock info.
- notice widalarmeta does not change.
Run require("sched").getAlarms() to see both alarms in the Web IDE console field. Both have on:true.
Toggle on the 12:00 alarm through the clock info.
Run require("sched").getAlarms() to see ONLY THE 12:00 alarm in the Web IDE console field. The 14:00 alarm has been removed.
- Also verifiable by opening the alarm app, then only showing the 12:00 alarm.

bobrippling commented 1 month ago

My outcomes for that disabling flow on factory reset, non-pretokenized apps, fw 2v24, bwclklite differs from yours:
1. The clock info transitions to the inactive icon.

2. `widalarmeta` does NOT stop displaying the eta.

3. Wait 2 minutes - the alarm fires anyway.

Interesting, I'll have a go at that - thanks for the IDE route as well!

bobrippling commented 1 month ago

I believe I have this fixed now, back to the original code but with timers working - thanks for your patience, thorough testing and advice, I appreciate it :)

To go back to your original questions:

Events set with "Delete After Expiration" unchecked will be toggleable via the clock info entry.

This may be confusing. A user may see the entry on the clock info and toggle it on and expect an alarm to go off at that time the next day but it will not go off until the next year (depending on configuration of the event).

Agreed - I think our two options are to hide the "Delete After Expiration" ones from clkinfo, or leave for now and see?

I think the icon for events should be different from the one for standard alarms.

Yes I think so, I'm no good with icons but I could have a whirl

Instead of clock time, maybe the date of the event should be displayed?

Sounds good!

Similar to the situation with events, maybe we want to differentiate between standard alarms that will not fire soon (the next time the clock is the time displayed on the clock info entry) and those that will not go off as soon?

Maybe writing a letter on the alarm icon corresponding to the weekday it will fire on next?

I like that idea, I could give that try too - maybe as a separate PR?

thyttan commented 1 month ago

Yes - it seems to work as expected on my end as well! Thanks :smiley:

Regarding events I think we go a real long way by changing the icon (both in the app and clock info) to something like or from https://icons8.com/icons/set/event .

So maybe we could bring that into this PR and then merge? Or we could do that + change to displaying date in a later PR.

What do you think?

I also think "day letter" on alarms fits better in a separate PR! :)

bobrippling commented 1 month ago

Sweet - I like the look of those icons, gone with the latter which seems to fit in with the others but don't mind changing. I can't seem to get it to show though, I've encoded it as a 1-bit black/white Image String but it doesn't show up for me 🔍

(sorted the text too - left off the year for breverity, let me know what you think)

thyttan commented 1 month ago

I also don't see the icon :thinking:

Otherwise - looking good!

thyttan commented 1 month ago

What do you say we merge this?

bobrippling commented 1 month ago

Sounds good, yes please!

thyttan commented 1 month ago

Thanks 🌸☺️

bobrippling commented 1 month ago

My pleasure - out of interest, do you know how to go from an image string to the image itself? (For getting at some of these images)

thyttan commented 1 month ago

I didn't test this myself but seems like a solution: https://onlinepngtools.com/convert-base64-to-png#tool

bobrippling commented 1 month ago

Thanks - it seems I might need a subscription but I've found another method (guessing at the dimensions - I've tried multiple):

$ base64 -D < alarm_on.b64 > alarm_on.raw
$ magick -depth 1 -size 8x8 rgb:alarm_on.raw alarm_on.png

Except I get back:

magick: unexpected end-of-file 'alarm_on.raw': No such file or directory @ error/rgb.c/ReadRGBImage/249.

Despite:

$ ls alarm_on.raw 
alarm_on.raw

@gfwilliams how do you go from the base64 strings to an image?

gfwilliams commented 1 month ago

The image format itself is covered in https://www.espruino.com/Graphics#strings - but it's unique to Espruino. You could use imagemagick after skipping the few bytes of header...

But what I do is just use Espruino. In the simplest case just draw the image to the screen and dump the contents of the screen (which you can then copy/paste from the IDE console), then manually crop it out:

let img = atob("GBiBAAAAAAAAAAAAAA//8B//+BgAGBgAGBgAGB//+B//+B//+B/++B/8+B/5+B8z+B+H+B/P+B//+B//+B//+A//8AAAAAAAAAAAAA==")
g.clear(1).drawImage(img).dump();

Or you can create a Graphics instance of the right size and dump that:

let img = atob("GBiBAAAAAAAAAAAAAA//8B//+BgAGBgAGBgAGB//+B//+B//+B/++B/8+B/5+B8z+B+H+B/P+B//+B//+B//+A//8AAAAAAAAAAAAA==")
let m = g.imageMetrics(img);
let t = Graphics.createArrayBuffer(m.width,m.height,m.bpp,{msb:true});
t.drawImage(img).dump();

bobrippling commented 1 month ago

Ah I see, so the first four bytes are a header in the string-only version, got it. Thanks!

gfwilliams commented 1 month ago

Ah I see, so the first four bytes are a header in the string-only version, got it. Thanks!

Yes, but it can vary. If it's not transparent it's only 3 bytes

espruino / BangleApps

Allow toggling an alarm via clkinfo #3579

Bug with repeatable timers

Reproduce

Other considerations

Some bugs

Some bugs

Disabling an alarm

Enabling an alarm

Disabling an alarm

Enabling an alarm

Test for "Toggling a timer or alarm on through the clock info removed all other alarms, timers and events completely!"