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

Optional background tracking taking 5 random times, instead of 5 regular times per day #127

Merged

Conversation

DigitalSeneca
Copy link
Contributor

@DigitalSeneca DigitalSeneca commented May 6, 2024

fixes #9

  • Schedule a daily task at 00:00 that generated 5 random times for the day to collect the locations and send to the api

@DigitalSeneca DigitalSeneca changed the title early return Optional background tracking taking 5 random times, instead of 5 regular times per day May 6, 2024
@DigitalSeneca DigitalSeneca marked this pull request as ready for review May 9, 2024 12:41
@DigitalSeneca DigitalSeneca requested a review from epou May 9, 2024 12:43
@epou
Copy link
Member

epou commented May 9, 2024

Important for iOS:

registerPeriodicTask is only supported on iOS 13+, it might run for only 30 seconds due to iOS restrictions, but doesn't start immediately, rather iOS will schedule it as per user's App usage pattern.
⚠️ On iOS 13+, adding a BGTaskSchedulerPermittedIdentifiers key to the Info.plist for new BGTaskScheduler API disables the performFetchWithCompletionHandler and setMinimumBackgroundFetchInterval methods, which means you cannot use both old Background Fetch and new registerPeriodicTask at the same time, you have to choose one based on your minimum iOS target version. For details see Apple Docs

@DigitalSeneca DigitalSeneca force-pushed the background-tracking-random-times branch from 0f4b127 to 080fd34 Compare May 9, 2024 14:53
@epou
Copy link
Member

epou commented May 9, 2024

The idea of using a unique tag is that when the user disables the background tracking in the settings page, pending tasks could be canceled by using: Workmanager().cancelByTag("tag");

@DigitalSeneca
Copy link
Contributor Author

The idea of using a unique tag is that when the user disables the background tracking in the settings page, pending tasks could be canceled by using: Workmanager().cancelByTag("tag");

Another option is to just await Workmanager().cancelAll();

@epou
Copy link
Member

epou commented May 10, 2024

In the scenario where various tasks are concurrently running in the background, employing Workmanager().cancelAll(); might not be the most efficient approach. Instead, it's advisable to selectively cancel only those tasks associated with backgroundTracking. That's why I came up with the solution of employing a consistent tag for all tracking-related tasks. This tagging system allows for grouping these tasks together, facilitating their cancellation using a unified tag, regardless of individual task names.

@DigitalSeneca
Copy link
Contributor Author

Important for iOS:

registerPeriodicTask is only supported on iOS 13+, it might run for only 30 seconds due to iOS restrictions, but doesn't start immediately, rather iOS will schedule it as per user's App usage pattern.
⚠️ On iOS 13+, adding a BGTaskSchedulerPermittedIdentifiers key to the Info.plist for new BGTaskScheduler API disables the performFetchWithCompletionHandler and setMinimumBackgroundFetchInterval methods, which means you cannot use both old Background Fetch and new registerPeriodicTask at the same time, you have to choose one based on your minimum iOS target version. For details see Apple Docs

I've added the setup for iOS but I can't test it without a device
I've also removed some transistorsoft settings that were still there from the previous location system

In the scenario where various tasks are concurrently running in the background, employing Workmanager().cancelAll(); might not be the most efficient approach. Instead, it's advisable to selectively cancel only those tasks associated with backgroundTracking. That's why I came up with the solution of employing a consistent tag for all tracking-related tasks. This tagging system allows for grouping these tasks together, facilitating their cancellation using a unified tag, regardless of individual task names.

Okey, so the daily task is still executed, but if there's no permission enabled, we just skip it and don't schedule the 5 tasks for the day?

@epou
Copy link
Member

epou commented May 10, 2024

Okey, so the daily task is still executed, but if there's no permission enabled, we just skip it and don't schedule the 5 tasks for the day?

I was considering halting all background tracking tasks if the user disables that option in the settings page. However, if you believe it's more efficient to keep the scheduler running and simply refrain from scheduling any tasks when the option is disabled, feel free to proceed with that approach.

@DigitalSeneca
Copy link
Contributor Author

Okey, so the daily task is still executed, but if there's no permission enabled, we just skip it and don't schedule the 5 tasks for the day?

I was considering halting all background tracking tasks if the user disables that option in the settings page. However, if you believe it's more efficient to keep the scheduler running and simply refrain from scheduling any tasks when the option is disabled, feel free to proceed with that approach.

We can halt all tracking tasks, including the one with tag 'scheduleDailyTasks', but then I don't see the performance improvement of cancelling by tag instead of cancelAll();, when we're going to cancel all tasks anyways

@epou
Copy link
Member

epou commented May 14, 2024

Okay! We should only need extra care if in the future we add any other kind of task that is not related to background tracking, and only cancel these.

@DigitalSeneca DigitalSeneca requested a review from epou May 15, 2024 12:31
@DigitalSeneca DigitalSeneca requested a review from epou May 15, 2024 15:43
@DigitalSeneca DigitalSeneca merged commit c36a2d5 into Mosquito-Alert:v34 May 17, 2024
1 check passed
@DigitalSeneca DigitalSeneca deleted the background-tracking-random-times branch May 17, 2024 14:49
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