emekalites / react-native-alarm-notification

schedule alarm and local notification in react-native
https://www.npmjs.com/package/react-native-alarm-notification
MIT License
225 stars 95 forks source link

Alarm is sounded in the order of id, not the fire_date. #63

Closed PhuocHoangMinhNguyen closed 3 years ago

PhuocHoangMinhNguyen commented 4 years ago

For example, I have one alarm with id (not alarm_id) as 1, and fire at 12:40. And another alarm with id 2, and fire at 12:35. For some reason, it skips alarm with id 2, and only fire alarm with id one. I guess it is because the alarm only checks the time of the alarm with the lowest id, instead of the entire alarm list.

emekalites commented 4 years ago

I have checked and couldnt reproduce this issue.

sebastian-sanchez-variv commented 4 years ago

I have the same issue in Android, check following code for more details:

        const fireDate = ReactNativeAN.parseDate(new Date(Date.now() + 15000));
        const fireDate2 = ReactNativeAN.parseDate(new Date(Date.now() + 60000));
        const alarmNotifData = {
            title: "Test Marketing",
            message: "Marketing worked",
            small_icon: "ic_launcher",
            channel: "marketing",
            vibration:500,
            play_sound:false,
            has_button:false,
            vibrate:true,
            data: { foo: "bar" },
        };
        const alarmNotifData2 = {
            title: "Test Reminders",
            message: "Reminders worked",
            small_icon: "ic_launcher",
            channel: "reminders",
            vibration:500,
            play_sound:false,
            has_button:false,
            vibrate:true,
            data: { foo: "bar" },
        };
        const alarm = await ReactNativeAN.scheduleAlarm({ ...alarmNotifData, fire_date: fireDate });
        const alarm2 = await ReactNativeAN.scheduleAlarm({ ...alarmNotifData2, fire_date: fireDate2 });
        console.log(alarm); // { id: x }  <--- does show
        console.log(alarm2); // { id: x + 1 } <--- does not show
sebastian-sanchez-variv commented 4 years ago

Actually it is very weird what happens.... alarm is fired at alarm2's time.... alarm's time is overridden and alarm's 2 content is overridden....

emekalites commented 4 years ago

Thanks for the feedback, I'll look into it again

laurent22 commented 4 years ago

Isn't it a race condition in here https://github.com/emekalites/react-native-alarm-notification/blob/65c1af8ae0d3b830c5c85bc50b328a54eae0b764/android/src/main/java/com/emekalites/react/alarm/notification/ANModule.java ? Maybe if you configure multiple alarms simultaneously, as in the examples above, the code gets confused somewhere and overwrite the alarm from one call with the alarm from another.

laurent22 commented 4 years ago

I'm guessing it's fine at DB level because the module correctly return {id: x}, and then {id: x+1} so IDs and DB rows are set correctly. It could be in alarmUtil.setAlarm(alarm) that one call overwrites another.

esfxra commented 4 years ago

I am experiencing a similar issue on Android. My app is a timer that schedules up to 7 alarms at the same time. However, in my case, none of the alarms get fired. These are all meant to occur at different times in the future.

Scheduling a single alarm at a time works, but I require scheduling multiple ones because of how the timer is supposed to work.

This is a snippet of the JS I schedule alarms with for reference:

      // Schedule for every remaining period
      for (let i = currentPeriod; i < periods; i += 1) {
        const fireDate = ReactNativeAN.parseDate(new Date(dateTargets[i]));
        const {title, message, channel} = defineNotification(i);
        const alarmNotifData = {
          title: title,
          message: message,
          channel: channel,
        };

        const alarm = await ReactNativeAN.scheduleAlarm({
          ...alarmNotifData,
          ...notifSettings,
          fire_date: fireDate,
        });
        identifiers.push(alarm.id);
      }

Where:

laurent22 commented 4 years ago

@diegoserranor, does it work if you leave a small delay between each scheduling? Because if it does it could confirm there's some race condition when multiple alarms are created at the same time.

With something like this:

async function msleep(ms) {
  return new Promise((resolve) => {
    setTimeout(() => {
      resolve();
    }, ms);
  });
}

// Schedule for every remaining period
for (let i = currentPeriod; i < periods; i += 1) {
  const fireDate = ReactNativeAN.parseDate(new Date(dateTargets[i]));
  const {title, message, channel} = defineNotification(i);
  const alarmNotifData = {
    title: title,
    message: message,
    channel: channel,
  };

  const alarm = await ReactNativeAN.scheduleAlarm({
    ...alarmNotifData,
    ...notifSettings,
    fire_date: fireDate,
  });
  identifiers.push(alarm.id);

  await msleep(500); // Wait for 100ms
}
esfxra commented 4 years ago

@laurent22 The delay helped! It seems that alarmId was set to the same value for all scheduled alarms. Here is also part of a log from Android Studio which confirms it:

Screen Shot 2020-10-19 at 12 29 48 PM

Had to use a 1000ms delay to make sure the values were all different. This might be because the value is losing some precision when converted from a long to an int (see below).

A bit tricky though since it is also used as the requestCode for the PendingIntent, and that has to be an integer. The fact that there were PendingIntents with the same code was probably why the notifications were not sent.

Not sure what the better approach would be.

ANModule.java (line 58)

long time = System.currentTimeMillis() / 1000;

alarm.setAlarmId((int) time);
esfxra commented 4 years ago

Also, regarding the alarm delivery at different times that @PhuocHoangMinhNguyen was facing: This might have to do with setAndAllowWhileIdle.

In my case, the notifications did come in order, but I ran into this bit of documentation:

Unlike other alarms, the system is free to reschedule this type of alarm to happen out of order with any other alarms, even those from the same app. This will clearly happen when the device is idle (since this alarm can go off while idle, when any other alarms from the app will be held until later), but may also happen even when not idle.

Might be one of those hurdles with Android battery management. It probably happens very rarely when not in idle. However having battery saver enabled, being on idle, or a similar state maybe opens up the possibility.

laurent22 commented 4 years ago

Here is also part of a log from Android Studio which confirms it:

Screen Shot 2020-10-19 at 12 29 48 PM

That makes sense, it seems the alarm ID is the Unix timestamp in seconds so if many alarms are created within a second they'll all get the same ID, and indeed using a 1 second delay would fix the issue.

I'm not familiar with that part of Android. Is the ID set by the operating system? In which case, I guess a simple sleep(1) somewhere in the Java code would be enough. Or if it's possible to set the ID manually, it should be some unique random number rather than the time in seconds.

esfxra commented 4 years ago

That would do the trick. It would be good to check if alarmId has another purpose somewhere for which it is important it matches the creation time of the alarm.

Also, not too familiar with how the operating system handles it either. That is the gist that I got from looking at the code below.

AlarmUtil.java, line 141

PendingIntent alarmIntent = PendingIntent.getBroadcast(mContext, alarmId, intent, 0);
stale[bot] commented 3 years ago

Hola! So here's the deal, between open source and my day job and life and what not, I have a lot to manage, so I use a GitHub bot to automate a few things here and there. This particular GitHub bot is going to mark this as stale because it has not had recent activity for a while. It will be closed if no further activity occurs in a few days. Do not take this personally--seriously--this is a completely automated action. If this is a mistake, just make a comment, DM me, send a carrier pidgeon, or a smoke signal.

laurent22 commented 3 years ago

Still happens, and still a critical issue. @emekalites, I'm curious if you're still planning to maintain this project? I've moved to it because I preferred its simplicity to Firebase, but if critical bugs are going to be auto-closed, I guess I'll need to move back to Firebase. In any case, it would be good to know what are the plans for this project.

emekalites commented 3 years ago

I'd really love to continue supporting the library but I am stuck with how much time i can spare because of the number of projects i am working on currently. I can should create some time in a few weeks to look into the issues, but in the meantime, anyone is free to create PRs and I'll merge them.

stale[bot] commented 3 years ago

Hola! So here's the deal, between open source and my day job and life and what not, I have a lot to manage, so I use a GitHub bot to automate a few things here and there. This particular GitHub bot is going to mark this as stale because it has not had recent activity for a while. It will be closed if no further activity occurs in a few days. Do not take this personally--seriously--this is a completely automated action. If this is a mistake, just make a comment, DM me, send a carrier pidgeon, or a smoke signal.

laurent22 commented 3 years ago

still happens

vishva-shukla commented 3 years ago

I'm facing the same issue

stale[bot] commented 3 years ago

Hola! So here's the deal, between open source and my day job and life and what not, I have a lot to manage, so I use a GitHub bot to automate a few things here and there. This particular GitHub bot is going to mark this as stale because it has not had recent activity for a while. It will be closed if no further activity occurs in a few days. Do not take this personally--seriously--this is a completely automated action. If this is a mistake, just make a comment, DM me, send a carrier pidgeon, or a smoke signal.

laurent22 commented 3 years ago

Hola! So here's the deal, between open source and my day job and life and what not, I have a lot to manage, so I use a GitHub bot to automate a few things here and there. This particular GitHub bot is going to mark this as stale because it has not had recent activity for a while. It will be closed if no further activity occurs in a few days. Do not take this personally--seriously--this is a completely automated action. If this is a mistake, just make a comment, DM me, send a carrier pidgeon, or a smoke signal.

stale[bot] commented 3 years ago

Hola! So here's the deal, between open source and my day job and life and what not, I have a lot to manage, so I use a GitHub bot to automate a few things here and there. This particular GitHub bot is going to mark this as stale because it has not had recent activity for a while. It will be closed if no further activity occurs in a few days. Do not take this personally--seriously--this is a completely automated action. If this is a mistake, just make a comment, DM me, send a carrier pidgeon, or a smoke signal.

stale[bot] commented 3 years ago

Closed due to inactivity. Holler if this is a mistake, and we'll re-open it.