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.

Sign up for GitHub

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

Update PdService.java #120

Open
wants to merge 2 commits into
base: master
Choose a base branch
from
Open

Update PdService.java #120

Changes from all commits
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
b054841
Update PdService.java
rafaelgustavo123 May 15, 2022
f2c645c
Update PdCore/src/main/java/org/puredata/android/service/PdService.java
rafaelgustavo123 May 17, 2022
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
10 changes: 8 additions & 2 deletions PdCore/src/main/java/org/puredata/android/service/PdService.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -241,8 +241,14 @@ private Notification makeNotification(Intent intent, int icon, String title, Str
notificationManager.createNotificationChannel(channel);
}
}

PendingIntent pi = PendingIntent.getActivity(getApplicationContext(), 0, intent, 0);

PendingIntent pi = null;
if (Build.VERSION.SDK_INT >= Build.VERSION_CODES.M) {
pi = PendingIntent.getActivity(getApplicationContext(), 0, intent, PendingIntent.FLAG_UPDATE_CURRENT | PendingIntent.FLAG_IMMUTABLE);
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I am not sure about setting FLAG_UPDATE_CURRENT here. On the one hand, this will just replace an existing notification so it will not show multiple notifications, if startAudio() is called multiple times. On the other hand, this should have been prevented in startAudio().

Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

What's the solution here? We need to set one or the other per the original discussion on SO.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

What's the solution here? We need to set one or the other per the original discussion on SO.

I would pick FLAG_IMMUTABLE, but I am unsure about the FLAG_UPDATE_CURRENT_PART.

} else {
pi = PendingIntent.getActivity(getApplicationContext(), 0, intent, PendingIntent.FLAG_UPDATE_CURRENT);
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I am against FLAG_UPDATE_CURRENT here. This may change the behaviour on older Android versions, and the existing code with using 0 was working in many apps.

}

return new NotificationCompat.Builder(PdService.this, TAG)
.setSmallIcon(icon)
.setContentTitle(title)
Expand Down