search

xamarin / XamarinComponents

Plugins for Xamarin
MIT License
1.99k stars 692 forks source link

[XPlat/ExposureNotification] Suggestion to use a foreground service job for Android #984

Open tmurakami opened 4 years ago

tmurakami commented 4 years ago

Hi,

Google recently published the Exposure Notifications implementation guide. https://developers.google.com/android/exposure-notifications/implementation-guide

According to the guide,

Use WorkManager

To handle the periodic background fetch of these files, we highly recommend using Jetpack WorkManager. Specifically, use a foreground service job that displays a visible notification to the user to reduce the chance of falling into the rare bucket.

That guide is for the ExposureWindow mode. However, I think we should follow the above recommendation for v1 mode as well because COCOA (the EN app in Japan that relies on the Xamarin EN package) has the issue of delayed downloading diagnosis keys, which may be due to the download job (BackgroundFetchWorker) falling into the rare bucket.

Do you have any plans to update the ExposureNotification package to use a foreground service job?

mattleibow commented 3 years ago

This does seem like a good thing to do. Thanks for bringing it up.

EDIT

Looking at the code, I think we are using the default configuration, so the normal rescheduling should be in place. However, using a foreground service still might be a good option.

tmurakami commented 3 years ago

Thank you for your reply.

BTW, are you planning to change the Work constraints in ExposureNotification.android.cs? https://github.com/xamarin/XamarinComponents/blob/2da9d10ecc65572e59e478dc1532f40a4bc80af4/XPlat/ExposureNotification/source/Xamarin.ExposureNotification/ExposureNotification.android.cs#L126-L131

I think SetRequiresDeviceIdle(true) is a bit strict. In the exposure-notifications-android app, that constraint has already been removed. https://github.com/google/exposure-notifications-android/commit/1b215ca7fc99cac292b56a87751421bd1065df75#diff-66b86b8a10a7fa4c739be56d73b34ea8d4c27a587e3186a710bbcb83fb2f375eL121-L138

tmurakami commented 3 years ago

I think SetRequiresDeviceIdle(true) is a bit strict. In the exposure-notifications-android app, that constraint has already been removed.

The reason why exposure-notifications-android doesn't use setRequiresDeviceIdle() seems to be because the app's minSdkVersion is 21.