-
Notifications
You must be signed in to change notification settings - Fork 1.4k
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
[flutter_local_notifications] Add audioAttributesUsage to NotificationDetails #1649
Conversation
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thanks for the PR. It looks like the code is missing some logic to handle how there could be scheduled notification that would have values for the new fields added on the Java/Android side. This means changes need to be added to fallback to say the audio usage is for notifications. Without this, what would happen is a scheduled notification is deserialised from shared preferences and a NPE would occur
Allows users to change AudioAttributes (https://developer.android.com/reference/android/media/AudioAttributes) to the notification sound Closes: MaikuB#1519
2df4fa1
to
4726ae1
Compare
if (notificationDetails.audioAttributesUsage == null) { | ||
notificationDetails.audioAttributesUsage = AudioAttributes.USAGE_NOTIFICATION; | ||
} |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Not sure if this should null checks or not, can revert if it is unnecessary
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The null checks you've added aren't necessary as the Dart/Flutter API won't allow this to happen. The issue I mentioned in my previous message can still occur so this can be reverted (i.e. this isn't the location where the problem can occur). I'll point out exactly where the issue I mentioned can be resolved in a separate comment
@@ -972,7 +972,7 @@ private static void setupNotificationChannel( | |||
notificationChannel.setGroup(notificationChannelDetails.groupId); | |||
if (notificationChannelDetails.playSound) { | |||
AudioAttributes audioAttributes = | |||
new AudioAttributes.Builder().setUsage(AudioAttributes.USAGE_NOTIFICATION).build(); | |||
new AudioAttributes.Builder().setUsage(notificationChannelDetails.audioAttributesUsage).build(); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This is the place to coalesce a null value from a scheduled notification to AudioAttributes.USAGE_NOTIFICATION
…tificationsPlugin
e9d4d2e
to
a139479
Compare
...d/src/main/java/com/dexterous/flutterlocalnotifications/FlutterLocalNotificationsPlugin.java
Show resolved
Hide resolved
flutter_local_notifications/lib/src/platform_specifics/android/enums.dart
Show resolved
Hide resolved
…property, 'values'
…nDetails (MaikuB#1649) * Add audioAttributesUsage to NotificationDetails Allows users to change AudioAttributes (https://developer.android.com/reference/android/media/AudioAttributes) to the notification sound Closes: MaikuB#1519 * Adding null check when reading audioAttributesUsage in FlutterLocalNotificationsPlugin * Adding missing enum 'notificationRingtone' in AudioAttributesUsage's property, 'values'
…nDetails (MaikuB#1649) * Add audioAttributesUsage to NotificationDetails Allows users to change AudioAttributes (https://developer.android.com/reference/android/media/AudioAttributes) to the notification sound Closes: MaikuB#1519 * Adding null check when reading audioAttributesUsage in FlutterLocalNotificationsPlugin * Adding missing enum 'notificationRingtone' in AudioAttributesUsage's property, 'values'
Allows users to change AudioAttributes (https://developer.android.com/reference/android/media/AudioAttributes) to the notification sound
Closes: #1519