[flutter_local_notifications] Fixes #1526: Add the ability to schedule inexact notifications on android

[mirland]

It fixes issue #1526, it adds the ability to schedule inexact notifications on android.

The documentation is missing. If you think the change is appropriated, I can include the documentation

[mirland]

. Was thinking if it's worth deprecating the old allowWhileIdle and having a new enum but I reckon it would cause more of a mess when it comes to maintaining the code on the Java side

The problem is that you can combine both, you can use allowWhileIdle/not allow idle with exact and inexact mode. Check this code:

 if (BooleanUtils.getValue(notificationDetails.allowWhileIdle)) {
      if (BooleanUtils.getValue(notificationDetails.useInexactMode)){
        AlarmManagerCompat.setAndAllowWhileIdle(
                alarmManager,
                AlarmManager.RTC_WAKEUP,
                epochMilli,
                pendingIntent);
      } else {
        AlarmManagerCompat.setExactAndAllowWhileIdle(
                alarmManager,
                AlarmManager.RTC_WAKEUP,
                epochMilli,
                pendingIntent);
      }
    } else {
      if (BooleanUtils.getValue(notificationDetails.useInexactMode)) {
        alarmManager.set(
                AlarmManager.RTC_WAKEUP,
                epochMilli,
                pendingIntent);
      } else {
        AlarmManagerCompat.setExact(
                alarmManager,
                AlarmManager.RTC_WAKEUP,
                epochMilli,
                pendingIntent);
      }
    }

So if you want to use an enum, you should use two enums (or use an enum with the combination of the 4 options, but I think it's not clear)

[MaikuB]

So if you want to use an enum, you should use two enums (or use an enum with the combination of the 4 options, but I think it's not clear)

I was referring to an enum with at four values. These all relate to the AlarmManager APIs so there be a way to make it "clear" via naming e.g. scheduleType, scheduleAccuracy

Note: I'm replying to say to share my thoughts on that comment. I'm not doing so to say it should switch to an enum

[mirland]

Makes sense! I'll do the change tomorrow!

[mirland]

I checked it, and if we do that we have to deprecate a lot of methods and the enum will have only two values (and they will not have more in the future, because it like a flag, true or false).

I added:

enum ScheduleType {
  /// This type is used to schedule a regular alarm.
  /// This alarm may not be executed when the system is in low-power idle modes.
  regular,

  /// This type of alarm must only be used for situations where it is actually
  /// required that the alarm go off while in idle -- a reasonable example would
  /// be for a calendar notification that should make a sound so the user is aware
  /// of it.
  allowIdle,
}
enum ScheduleAccuracy {
  /// Setup an alarm with regular accuracy.
  /// When in low-power idle modes this duration may be significantly longer,
  /// such as 15 minutes.
  regular,

  /// Schedule an alarm to be delivered precisely at the stated time.
  /// Only alarms for which there is a strong demand for exact-time delivery
  /// (such as an alarm clock ringing at the requested time) should be scheduled
  /// as exact.
  /// Note that this accuracy require to declare more permissions on Android.
  exact,
}

However, n my opinion the library code will be worse because we will have to duplicate the methods, think of new names (that will be less clear), and deprecate the old ones.

So If you want I can do that but I want confirmation before making the change. Thanks!

[MaikuB]

I understand what you mean and relates to my original concern. Do you think the same issue would also happen if it was a enum of four options /values though? To clarify, what I mean is the enum would have values for the following

• inexact
• inexact and allow while idle
• exact
• exact and allow while idle

[mirland]

Yes, because we will have to duplicate a lot of methods to keep the compatibility.

On the other hand inexact and allow while idle are different things, and if android decides add a new value in the future we will have in troubles, I mean, we will have the double of values

Again, if you think it's the best approach I can do that. However, you need know we will need to create a lot of 'duplicate' methods

[mirland]

Hi @MaikuB , I made a refactor in the schedule method, can you take a look t it?

In my opinion, zonedSchedule has a lot of parameters, so I added a new ScheduleNotificationRequest class to handle all request parameters.

the new method is:

Future<void> scheduleNotification({
    required int id,
    required String? title,
    required String? body,
    required NotificationDetails notificationDetails,
    required ScheduleNotificationRequest scheduleNotificationRequest,
    String? payload,
  })

I think now is more clean. If you like the impl, I can add the docs of the new method.

[MaikuB]

Yes, because we will have to duplicate a lot of methods to keep the compatibility.

There are ways that won't involve duplication and for the purpose of this discussion bear in the mind the following points

• assume the scheduleType property that matches the enum values I mentioned at https://github.com/MaikuB/flutter_local_notifications/pull/1653#issuecomment-1204969056 is introduced
• currently androidAllowWhileIdle being true equates to a scheduleType value of that matches the behaviour for "exact and allow while idle". If it's false or null, then it just means exact

This now means there are ways between mapping between the old and new property. A breaking change could be done to remove the androidAllowWhileIdle property on the Flutter/Dart side but keep the Java property. The plugin's logic in the Java side could then be updated to do something like the following

1. check the value of the notificationDetails.scheduleType and save it against a local variable called scheduleType that scoped to the method that would've called the AlarmManager API. For example, this could be done at https://github.com/MaikuB/flutter_local_notifications/blob/b707190d752a69976f71b8a8610896fba54389c7/flutter_local_notifications/android/src/main/java/com/dexterous/flutterlocalnotifications/FlutterLocalNotificationsPlugin.java#L451
2. Check if scheduleType is null or not. If it's null, this means notifications created before the new scheduleType enum existed, check the value of the allowWhileIdle property associated with NotificationDetails Java class. The plugin can map this to one of the values for the new scheduleType enum values and save the result to the scheduleType local variable
3. The scheduleType value should no longer be null at this point, check the value and call the appropriate AlarmManager API

By doing this, there shouldn't be any duplication and it handles backwards compatibility. It is also more clean as switch statement can be used to determine AlarmManager API to call whilst the approach shown in your PR has an additional level of nesting given nested if statements are involved

On the other hand inexact and allow while idle are different things, and if android decides add a new value in the future we will have in troubles, I mean, we will have the double of values

While, they are different things, the fact that you have nested if statements to still call an AlarmManager API shows that they are treated as being related. This is because they're not values as you were calling them, rather they are to do with the available AlarmManager APIs/methods. Using the enum approach I mentioned shows the mapping more directly. If anything, I would say using the enum approach means it can handle new AlarmManager APIs being added more cleanly as the enum already maps to specific AlarmManager APIs. For the purpose of demonstrating this, let's assume Android adds a setFoo() API that this plugin could use. Presumably, what you're proposing is to add foo property but you then end up with a problem where any value passed to androidAllowWhileIdle and androidScheduleAccuracy doesn't do anything. Using the enum approach, only foo needs to be added and don't run into this problem.

Hi @MaikuB , I made a refactor in the schedule method, can you take a look t it?

Please take a look at what I stated above first. Regarding the changes you have done, what concerns is that there'd be two APIs exposed by the plugin with almost identical names and none of the APIs have notification as a suffix so there's inconsistency being introduced. Rather than introduce another API with a name that could cause confusion, I would say it's better to do it as a breaking change.

Note: i have plans to publish 10.0 as a stable release soon so you'd then see changes merged to master

[mirland]

Hi @MaikuB

This now means there are ways between mapping between the old and new property. A breaking change could be done to remove the androidAllowWhileIdle property on the Flutter/Dart side but keep the Java property. The plugin's logic in the Java side could then be updated to do something like the following

Ok, so you are proposing add a new braking change in dart code, right?

androidAllowWhileIdle property on the Flutter/Dart side but keep the Java property.

In this case, we have to add a new bool property to handle the inexact mode, right?

While, they are different things, the fact that you have nested if statements to still call an AlarmManager API shows that they are treated as being related. This is because they're not values as you were calling them, rather they are to do with the available AlarmManager APIs/methods.

Ok, i mean, if you know that in the future android adds a new 'property', we will have 8 enum values, that I think it will not be ok. I splitted it in two properties thinking in the future. However, I can use just an enum

Regarding the changes you have done, what concerns is that there'd be two APIs exposed by the plugin with almost identical names and none of the APIs have notification as a suffix so there's inconsistency being introduced. Rather than introduce another API with a name that could cause confusion, I would say it's better to do it as a breaking change.

I did that because multiple factors:

1. if you want to include an enum in the dart api, we have to create a new method or break compatibility. (I think we should't break compatibility if we dont deprecate the method before)
2. zonedSchedule has a lot of parameters, it's an antipattern, so I created a class to hold the request
3. schedule, zonedSchedule and scheduleNotification it's currently a bit strange, if you are planning to create a major version I suggest you to remove all deprecated methods.

---

To sum up, what you're saying is that I should break compatibility in the dart code, adding a new enum in the zonedSchedule method. Then, the java module API should add a new boolean, that internally it will be mapped to the enum. In that case we have to convert the enum to booleans (when we call java api) and then in the java api we transform the booleans to the enim.

[MaikuB]

To sum up, what you're saying is that I should break compatibility in the dart code, adding a new enum in the zonedSchedule method. Then, the java module API should add a new boolean, that internally it will be mapped to the enum. In that case we have to convert the enum to booleans (when we call java api) and then in the java api we transform the booleans to the enim.

Yep except I didn't talk about introducing a new boolean on the Java side. There should be a property on the Java side to match the enum on the Dart side. There shouldn't be a need for a boolean if you were to do a breaking change and followed the logic I explained earlier on handling. Even if you were to deprecate first and then do breaking change later then the logic I explained provides ways to allow for a relatively easy transition to do the breaking change later

[MaikuB]

Closing as this is now superseded by #1815