[sched] Setting an alarm for an earlier time of day makes the alarm fire immediately.

[thyttan]

Affected hardware version

Bangle 2

Your firmware version

2v29.75

The bug

Setting an alarm using the sheduling library, for a time that already passed today makes the alarm fire immediately.

I am not sure if this is actually a bug. It may be the intended behavior.

My expectation would have been for the alarm to fire at the set time the next day instead.

Setting an alarm at such a time through the alarms app does indeed set the alarm for the next day instead. I am unsure of how this is achieved.

Here're the lines responsible for the behavior:

BangleApps/apps/sched/boot.js

Lines 19 to 21 in b9038c8

	active = active.sort((a,b)=>a.t-b.t); // sort by time
	var t = active[0].t-currentTime;
	if (t<1000) t=1000; // start alarm minimum 1 sec from now

Here's a diff with a fix:
https://github.com/espruino/BangleApps/compare/master...thyttan:sched?expand=1

However that fix seems to go against recent changes to fix other bugs in issues #2712, #4220.

I wonder if you @ticalc-travis have any immediate thoughts regarding this since you seem to have thought about the scheduling library quite a bit not that long ago.

Reproduce

Use the Web IDE to upload the following script to RAM.

{
  S = require("sched");
  const TIME_AT_NEXT_BUZZ = 10 * 1000; //10 seconds after midnight.
  S.setAlarm("test", {
    on: true,
    t: TIME_AT_NEXT_BUZZ,
    //dow: 0b1111111, // all days of the week.
    //last: Date().getDate(), // Don't fire until tomorrow.
  });
  S.reload();
}

Background

I'm working on making the twenties app use the scheduling library instead of setting it's own timeouts. Draft PR at #4231. That's when I happened upon this behavior. Notably I set and make use of the .js attribute when scheduling the alarm.

[ticalc-travis]

Taking a quick look, the code for handling adding alarms earlier than the current time appears to be the second line of the function prepareAlarmForSave in the Alarms app:

function prepareAlarmForSave(alarm, alarmIndex, time, date, temp) {
  alarm.t = require("time_utils").encodeTime(time);
  alarm.last = alarm.t < require("time_utils").getCurrentTimeMillis() ? new Date().getDate() : 0;
  if(date) alarm.date = date.toLocalISOString().slice(0,10);
  …

If the alarm is before the current time of day, it sets alarm.last to today's date to tell the system that the alarm has already been taken care of for today, preventing it from firing until tomorrow. It doesn't look like the sched lib itself currently handles this.

Perhaps it would be reasonable to move this check to somewhere in the sched app's setAlarm() code? If the new alarm being set is a time-of-day alarm and alarm.last has not already been defined, it could apply the same logic to set alarm.last so that other apps could set alarms at any time of day and have it behave as one would expect.

[thyttan]

Thanks for taking a look! That sounds reasonable to me - would you mind me doing a PR for that or do you want to? And then I ask Gordon to look at it when he comes back from vacation.