Improve Android Service lifecycle/notification management. #849
Conversation
audio_service/android/src/main/java/com/ryanheise/audioservice/AudioService.java
Outdated
Show resolved
Hide resolved
| @@ -592,6 +596,7 @@ private void deactivateMediaSession() { | |||
| } | |||
| // Force cancellation of the notification | |||
| getNotificationManager().cancel(NOTIFICATION_ID); | |||
| notificationCreated = false; | |||
There was a problem hiding this comment.
Because of this change notification won't update when go ready -> idle -> ready, not sure why.
There was a problem hiding this comment.
Ah, yes I was wondering whether this would break anything, although the logic seemed right. That said, when I planned out this change, I originally wanted to get rid of notificationCreated completely so that it would be possible to display the notification even before entering the foreground state. Maybe with !idle being used to determine whether updateNotification should show the notification rather than notificationCreated.
| private boolean isActuallyPlaying() { | ||
| return playing && (processingState == AudioProcessingState.loading | ||
| || processingState == AudioProcessingState.buffering | ||
| || processingState == AudioProcessingState.ready); | ||
| } |
There was a problem hiding this comment.
This fixes playing=true and idle thing. It's also looks ok to exit foreground state when it's completed or error.
There was a problem hiding this comment.
But other problem is if you try to stop the idle service - nothing happens, I'll try to think about this...
There was a problem hiding this comment.
The above is only used when entering foreground. When exiting foreground, it uses a broader condition (simply !playing).
|
There's no problem with discussing the specifics of a pull request here, since we now have two concrete pull request to look at and comment on. It's easier that way to review and comment using the GitHub tools.. |
|
I have no problem with discussing the details of this PR here. Just wanted you to take a look at what I proposed in the issue, because this correlates with what you made here, and perhaps is a better solution. |
ryanheise
force-pushed
the
feature/service_lifecycle
branch
from
5a6dd55 to
6e49376
Compare
audio_service/android/src/main/java/com/ryanheise/audioservice/AudioServiceConfig.java
Outdated
Show resolved
Hide resolved
audio_service/android/src/main/java/com/ryanheise/audioservice/AudioServicePlugin.java
Outdated
Show resolved
Hide resolved
Try to be more reasonable in some edge case state transitions.