Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Android: All defined notifications run on device reboot #571

Closed
mix3d opened this issue May 27, 2015 · 19 comments
Closed

Android: All defined notifications run on device reboot #571

mix3d opened this issue May 27, 2015 · 19 comments

Comments

@mix3d
Copy link

mix3d commented May 27, 2015

I only have my one device, but all the notifications I specified, in an extremely simple cut-down version of the kitchen-sink demo, are firing immediately after the device reboots. Even ones that I define but are orphaned because there is no JS code to fire them.

What could cause this?

(LG g3 Lollipop)

EDIT: this also is happening on the device emulators, as low as 2.3 Gingerbread, so I don't think it is an isolated issue.

@slikk66
Copy link

slikk66 commented May 29, 2015

Same problem here, was coming to search and see if there were any comments about it.

My app always just fires a single notification that gets updated with the same ID (it's a music app, so it's the currently playing song) - If I stop the music, then clear the notification, then reboot the phone.. the last song that was playing notification pops up again on reboot.

Has to be an easy fix! Thanks

@pavei
Copy link

pavei commented Jun 1, 2015

1+

1 similar comment
@acianti
Copy link

acianti commented Jun 5, 2015

1+

@alessandrodamore
Copy link

i solved this issue in my scenario....Go to Notification.java --> showNotification() method --> insert at the end of the method a call to "unpersist()".......Bye bye katzer

@dansanti
Copy link

dansanti commented Jun 8, 2015

hi, alessandrodamore , this solution cancel repeater notifications too.. is only applicable for one time notification

@alessandrodamore
Copy link

exactly!! I talked about a solution that was fine in my scenario...I don't need that notifications be repeated......... The problem depends on how it is managed persistence options!! When you reboot your device/app AbstractRestoreReceiver is triggered and it try to recover all persisted options. Among these options you can see your old options because nobody has ever canceled.

@dansanti
Copy link

Well for me now making this replace on options.java, i can fix repeat for daily notification (i don't know about others notification) . if notification was in past, move it to tomorrow
/**
* Trigger date in milliseconds.
*/
public long getTriggerTime() {
long triggerTime=options.optLong("at", 0) * 1000;
while(System.currentTimeMillis() > triggerTime && triggerTime > 0)
triggerTime += (AlarmManager.INTERVAL_DAY);
return Math.max(
System.currentTimeMillis(),
triggerTime
);
}

@dansanti
Copy link

i think reeplacing AlarmManager.INTERVAL_DAY by interval can fix others repeats

@djdmbrwsk
Copy link

+1

@mix3d
Copy link
Author

mix3d commented Jun 24, 2015

@dansanti I don't quite follow what you mean by replacing the alarm manager, can you elaborate?

@dansanti
Copy link

HI,! i've change these solution, because some time cancel alert now, work fine (for repetitions alerts) :

  • on Notification.java around line 176 add:
triggerTime=options.getTriggerTimeUp();

after

 if (isRepeating()) {

then:

 if (isRepeating()) {
  triggerTime=options.getTriggerTimeUp();

complete method:

public void schedule() {
        long triggerTime = options.getTriggerTime();

        persist();

        // Intent gets called when the Notification gets fired
        Intent intent = new Intent(context, receiver)
                .setAction(options.getIdStr())
                .putExtra(Options.EXTRA, options.toString());

        PendingIntent pi = PendingIntent.getBroadcast(
                context, 0, intent, PendingIntent.FLAG_CANCEL_CURRENT);

        if (isRepeating()) {
            triggerTime=options.getTriggerTimeUp();
            getAlarmMgr().setRepeating(AlarmManager.RTC_WAKEUP,
                    triggerTime, options.getRepeatInterval(), pi);
        } else {
            getAlarmMgr().set(AlarmManager.RTC_WAKEUP, triggerTime, pi);
        }
    }
  • now in Options.java add the follow method, inside and at the end of file and before of last }:
 public long getTriggerTimeUp() {
        long triggerTime=options.optLong("at", 0) * 1000;
        while(System.currentTimeMillis() > triggerTime )
            triggerTime += (interval);

        return  triggerTime;
    }

@mix3d
Copy link
Author

mix3d commented Aug 14, 2015

This change definitely works. I recommend adopting this change.

@lauxjpn
Copy link

lauxjpn commented Sep 8, 2015

The Problem of the recurring notifications comes down to RestoreReceiver.java:9, where it is decided wether to add a notification again after the device has rebooted or not.

    if (notification.isScheduled()) {
        notification.schedule();
    }

notification.isScheduled() refers to Notification.java:128

    public boolean isScheduled () {
        return isRepeating() || !wasInThePast();
    }

which refers to Notification.java:121

    public boolean wasInThePast () {
        return new Date().after(options.getTriggerDate());
    }

which uses options.getTriggerDate() from Options.java:208 to decide, if the notification already happened or not.

    public Date getTriggerDate() {
        return new Date(getTriggerTime());
    }

getTriggerTime() defined at Options.java:2215 is the root cause of the problem, because it will always return the current time as the trigger time for notifications that already happened.

    public long getTriggerTime() {
        return Math.max(
                System.currentTimeMillis(),
                options.optLong("at", 0) * 1000
        );
    }

Because of that, wasInThePast() will then return true since it is compared to a time value retrieved before getTriggerTime() was called.

An easy solution would be to explicitly call getTriggerTime() first:

    public boolean wasInThePast () {
        var triggerDate = options.getTriggerDate();
        return new Date().after(triggerDate);
    }

It ensures that the call order of the two dates which will be compared is as it should be.

Of couse there are other solutions which might be better to maintain in the long run:

    public long getTriggerTime() {
        return Math.max(
                System.currentTimeMillis(),
                getOriginalTriggerTime()
        );
    }

    public long getOriginalTriggerTime() {
        return options.optLong("at", 0) * 1000;
    }

    public Date getOriginalTriggerDate() {
        return new Date(getOriginalTriggerTime());
    }

    public boolean wasInThePast () {
        return new Date().after(options.getOriginalTriggerDate());
    }

This solution makes it more explicit what is going on.

In the end, the plugin would really benefit from an additional (optional) scheduling parameter validFor, which would specify a duration in which the notification is valid to be shown and after it has passed should definitly not be triggered anymore.

vkeepe added a commit to vkeepe/cordova-plugin-local-notifications that referenced this issue Sep 29, 2015
@giriprashaad
Copy link

@dansanti fix works. Just did as mentioned. Repeated notifications work correctly. Tested on android 4.3 .

@MichelReij
Copy link
Contributor

What is the status of this thread? Are the proposed changes submitted as a pull request to Sebastian @katzer?
I'm asking because I received some complaints from a few Android users about receiving multiple notiifications immediately after booting up their phone.

@dansanti
Copy link

Hi! i testesd latest commit, in a clean installation and seems to works fine, bug fixed.

@MichelReij
Copy link
Contributor

I can confirm too that it is fixed.

@askxp
Copy link

askxp commented Jan 27, 2016

I can confirm too that is NOT fixed

@mix3d
Copy link
Author

mix3d commented Feb 1, 2016

@AbstractH - care to elaborate?

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests