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

fix(android): Allow setting grace period before stopForeground #2164

Merged
merged 3 commits into from
Oct 18, 2023

Conversation

dcvz
Copy link
Contributor

@dcvz dcvz commented Oct 17, 2023

This PR adds a configurable grace period - a delay before we attempt to set our service to not be in foreground mode, allowing users a chance to do player actions that might refrain us from deciding to go out of foreground state. Defaults to five seconds.

Context:
At the moment on Android as soon as we transition back to idle/stop/error we immediately set our service as not being in the foreground state (setting yourself as not in foreground is good practice). In the use of RNTP where everyone's needs are different, a user might still want to enqueue some kind of playback action once they realize the queue is over, an error has happened, etc. For example, once a user's queue ends, they may want to create a whole new queue of related music.

Immediately setting ourselves as not in foreground state may give the OS a chance to turn off network chips, or kill the service which may result in errors if you try to do any network loading (i.e. playing a remote track) after that as happens on #2159.

@dcvz dcvz requested a review from jspizziri as a code owner October 17, 2023 12:21
Copy link
Collaborator

@jspizziri jspizziri left a comment

Choose a reason for hiding this comment

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

LGTM

@dcvz dcvz merged commit b13a922 into main Oct 18, 2023
4 checks passed
@dcvz dcvz deleted the fix/load-after-end branch October 18, 2023 07:06
@elliotdickison
Copy link
Contributor

This will be particularly helpful in the workaround for #1845

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.

3 participants