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

[flutter_local_notifications] Add audioAttributesUsage to NotificationDetails #1649

Merged
merged 3 commits into from
Aug 29, 2022

Conversation

bornold
Copy link
Contributor

@bornold bornold commented Jul 15, 2022

Allows users to change AudioAttributes (https://developer.android.com/reference/android/media/AudioAttributes) to the notification sound

Closes: #1519

@bornold bornold changed the title Add audioAttributesUsage to NotificationDetails [flutter_local_notifications] Add audioAttributesUsage to NotificationDetails Jul 15, 2022
Copy link
Owner

@MaikuB MaikuB left a 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

@bornold bornold force-pushed the android-audio-attributes-usage branch from 2df4fa1 to 4726ae1 Compare August 25, 2022 17:54
Comment on lines 275 to 277
if (notificationDetails.audioAttributesUsage == null) {
notificationDetails.audioAttributesUsage = AudioAttributes.USAGE_NOTIFICATION;
}
Copy link
Contributor Author

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

Copy link
Owner

@MaikuB MaikuB Aug 28, 2022

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();
Copy link
Owner

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

@bornold bornold force-pushed the android-audio-attributes-usage branch from e9d4d2e to a139479 Compare August 28, 2022 10:03
@MaikuB MaikuB merged commit 801b6ca into MaikuB:master Aug 29, 2022
Kavantix pushed a commit to MedAppNL/flutter_local_notifications that referenced this pull request Feb 8, 2023
…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'
Kavantix pushed a commit to MedAppNL/flutter_local_notifications that referenced this pull request Feb 8, 2023
…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'
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Change AudioAttributes to the notifications in android
2 participants