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

feat: support for background mode iOS 13+ #28

Merged

Conversation

duyhungtnn
Copy link
Collaborator

@duyhungtnn duyhungtnn commented Aug 16, 2023

Changes

  • Enable background mode for the example app
  • Add func for register background task
  • Fix foreground task could not register the first time

@duyhungtnn duyhungtnn force-pushed the fix/background-task-did-not-work branch from cbcce95 to b79fe04 Compare August 16, 2023 09:34
@duyhungtnn duyhungtnn force-pushed the fix/background-task-did-not-work branch from b79fe04 to ee7f6f8 Compare August 24, 2023 03:24
@duyhungtnn duyhungtnn changed the title feat: enable background mode for the example app feat: background mode for iOS 13+ Aug 24, 2023
@duyhungtnn duyhungtnn force-pushed the fix/background-task-did-not-work branch from 72fd7c4 to b30c23d Compare August 24, 2023 14:50
@duyhungtnn duyhungtnn marked this pull request as ready for review August 24, 2023 14:52
@duyhungtnn duyhungtnn marked this pull request as draft August 24, 2023 14:56
@duyhungtnn
Copy link
Collaborator Author

Tested on a real device to verify it works
Screenshot 2023-08-24 at 21 19 05

Screenshot 2023-08-24 at 21 20 37

@duyhungtnn duyhungtnn force-pushed the fix/background-task-did-not-work branch from 186382e to f2d7b70 Compare August 25, 2023 14:50
@duyhungtnn duyhungtnn marked this pull request as ready for review August 25, 2023 15:45
Bucketeer/Sources/Internal/Scheduler/TaskScheduler.swift Outdated Show resolved Hide resolved
}

@objc func onForeground() {
// noted: from iOS 16, the app will receive both UIApplication.didFinishLaunchingNotification
// & UIScene.willConnectNotification when the app launching
guard isOnForeground == false else { return }
Copy link
Member

Choose a reason for hiding this comment

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

The isOnForeground is not thread-safe.
Also, I think we should always reschedule the foreground tasks when they change instead of ignoring them when they are already in the foreground for this case.

How about registering the notifications according to the OS version?
So we can avoid calling twice from iOS 16.
Also, I didn't find the notification willConnectNotification described in the comment.

Let me know your thoughts.

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

I updated this logic a little, it seems we were using incorrect notifications for listening.
Currently no more duplicate events for on_foreground after app launched

@duyhungtnn duyhungtnn force-pushed the fix/background-task-did-not-work branch 6 times, most recently from 4602db2 to 71ab47b Compare August 28, 2023 15:50
BKTBackgroundTask.enable()
return self
}
}
Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Supporting SwiftUI from iOS 14

ContentView()
}
.enableBKTBackgroundTask()
}
Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Here is how to register background task for SwiftUI app

@@ -83,7 +89,7 @@ final class TaskScheduler {
onForeground()
}

@objc private func onUISceneWillEnterForeground() {
@objc private func onUISceneDidActive() {
component.config.logger?.debug(message: "[TaskScheduler]: onUISceneWillEnterForeground")
Copy link
Member

Choose a reason for hiding this comment

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

nit: onUISceneWillEnterForeground -> onUISceneDidActive

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

fixed

@cre8ivejp
Copy link
Member

@duyhungtnn, please fix the conflict rebasing from the main branch.

@duyhungtnn duyhungtnn force-pushed the fix/background-task-did-not-work branch from f01dabc to aa04ad5 Compare August 31, 2023 07:17
@duyhungtnn duyhungtnn force-pushed the fix/background-task-did-not-work branch from aa04ad5 to 9a3934d Compare August 31, 2023 07:18
@duyhungtnn
Copy link
Collaborator Author

@cre8ivejp I fixed the conflict and rebase the code

@duyhungtnn duyhungtnn force-pushed the fix/background-task-did-not-work branch from b9d5422 to 68b365c Compare September 4, 2023 03:47
@cre8ivejp cre8ivejp changed the title feat: background mode for iOS 13+ feat: support for background mode iOS 13+ Sep 5, 2023
@cre8ivejp cre8ivejp merged commit 0161cdf into bucketeer-io:main Sep 5, 2023
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.

2 participants