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

On Android, for repeating notifications, time interval added on every reboot pushing notification further into future. #251

Closed
c0ff33-b34n opened this issue Feb 3, 2022 · 4 comments
Assignees
Labels
Android Android only issue bug Something isn't working

Comments

@c0ff33-b34n
Copy link
Contributor

c0ff33-b34n commented Feb 3, 2022

Describe the bug
After rebooting the device repeating notifications have their notification interval added to the notify time, without first checking that the current notification time is in the future. This means that if the device is rebooted multiple times before the notification time is reached, the new notification time is pushed further and further into the future.

For example, if you turn on your device at 7am and had a notification set for 8am, that notification is rescheduled to 8am the next day. Turn the device off and on again and it will move even further into the future and you will only receive the notification it if you leave the device on until it catches up.

To Reproduce
Steps to reproduce the behavior:

  1. Create a NotificationRequest with a repeating notification for some time in the future, say in 5 minutes time, with a repeat time interval of 5 minutes.
...
Schedule =
	{
		NotifyTime = DateTime.Now.AddMinutes(5),
		RepeatType=NotificationRepeat.TimeInterval,
		NotifyRepeatInterval=TimeSpan.FromMinutes(5),
	}
};
await NotificationCenter.Current.Show(notification);
  1. Reboot the device before the 5 minutes is up. (It will have added an extra 5 minutes to the notification time, i.e. the notification will now be 10 minutes since you first set it.
  2. Reboot again before the 10 minutes is reached and the notification will be pushed a further 5 minutes in the future. I'm just using 5 minutes as an example but it's the same with whatever repeat interval you have chosen.

Expected behavior
I would expect that if a notification time has not yet been reached then it would show that notification at the original set time, and then only add the repeat time interval when that notification is received.

Platform:

  • OS: Android
  • Plugin.LocalNotification Version 9.1.3 and prior.

Smartphone:

  • Device: Samsung Galaxy A32 Phone
  • Android 11.0
  • API 30.
  • But will occur with any Android device.

Additional context

The issue seems to be caused when the BootReciever class calls request.Schedule.NotifyTime = request.GetNextNotifyTime();
When request.GetNextNotifyTime(); is called it adds on the repeat interval regardless.

foreach (var request in activeForReScheduleRequestList)
	{
		request.Schedule.NotifyTime = request.GetNextNotifyTime();

		// re schedule again.
		NotificationCenter.Log($"ReScheduled Notification Request {request.NotificationId}");
		notificationService.ShowLater(request);
	}

In NotificationRequest:

internal DateTime? GetNextNotifyTime()
        {
            if (IsStillActiveForReSchedule() == false)
            {
                return null;
            }

            var repeatInterval = GetNotifyRepeatInterval();
            if (repeatInterval is null)
            {
                return null;
            }

            if (Schedule.NotifyTime.HasValue == false)
            {
                return null;
            }

            var newNotifyTime = Schedule.NotifyTime.Value.Add(repeatInterval.Value);
            var nowTime = DateTime.Now.AddSeconds(10);
            while (newNotifyTime <= nowTime)
            {
                newNotifyTime = newNotifyTime.Add(repeatInterval.Value);
            }
            return newNotifyTime;
        }

With a downloaded copy of the plugin I wrapped the above code from BootReceiver with a check to see if the alarm time had already passed first.

if (request.Schedule.NotifyTime < DateTime.Now)
{
	request.Schedule.NotifyTime = request.GetNextNotifyTime();
} 

With the update above it seems to fix the problem.

As an aside I noticed that if I set the repeat interval to only 2 minutes the phone tried to optimize power usage, as did having power save mode switched-on on my device. Even with power saving turned off the device was performing other optimizations, so I had to turn off Battery Optimization for my specific app on the device too. All these Android automatic optimizations / phone manufacturer's 3rd party settings make testing of the underlying code a bit trickier with local notifications, I think something everyone should pay close attention to when using local notifications on Android.

Setting notification repeats to 3 minutes and turning power saving and battery optimization off allowed each repetition of the notification to come through after reboot once changes to the BootReciever code were made in my local copy.

@thudugala
Copy link
Owner

@c0ff33-b34n without changing

if (request.Schedule.NotifyTime < DateTime.Now)
{
	request.Schedule.NotifyTime = request.GetNextNotifyTime();
} 

I was think we should change GetNextNotifyTime from var newNotifyTime = Schedule.NotifyTime.Value.Add(repeatInterval.Value); to var newNotifyTime = Schedule.NotifyTime.Value;

internal DateTime? GetNextNotifyTime()
        {
            if (IsStillActiveForReSchedule() == false)
            {
                return null;
            }

            var repeatInterval = GetNotifyRepeatInterval();
            if (repeatInterval is null)
            {
                return null;
            }

            if (Schedule.NotifyTime.HasValue == false)
            {
                return null;
            }

            var newNotifyTime = Schedule.NotifyTime.Value;
            var nowTime = DateTime.Now.AddSeconds(10);
            while (newNotifyTime <= nowTime)
            {
                newNotifyTime = newNotifyTime.Add(repeatInterval.Value);
            }
            return newNotifyTime;
        }

@thudugala thudugala added the Android Android only issue label Feb 4, 2022
thudugala pushed a commit that referenced this issue Feb 4, 2022
…ndroid

#251 On Android, for repeating notifications, time interval added on every reboot pushing notification further into future.
@thudugala
Copy link
Owner

@c0ff33-b34n Can you please try version 9.2.0-preview01

@thudugala
Copy link
Owner

@c0ff33-b34n Can you please try version 9.2.0-preview02

@c0ff33-b34n
Copy link
Contributor Author

This is now working and the correct time interval is being added in 9.2.0-preview03.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Android Android only issue bug Something isn't working
Projects
None yet
Development

No branches or pull requests

2 participants