Expedite Samsung Screen Off Detections #941
Conversation
… on Samsung to quicly switch to a non-empty scan filter if the screen goes off.
|
Can I get a beta with this change and I can start testing? |
|
@atotalnoob You'd probably have to clone or fork and test yourself. One concern I have is that there is a difference between You might need to test for both screen off || screen locked. |
((KeyguardManager) context.getSystemService(Context.KEYGUARD_SERVICE)).isKeyguardLocked() |
|
Thanks for the feedback about the locked screen, @paulpv. Indeed, I have not tested whether a screen that is locked but turned on currently prevents detections with this library. If it does, then the code that decides whether to use an empty scan filter or a non-empty scan filter may need to be augmented to check for Any help testing this PR would be appreciated, @atotalnoob and @paulpv. I am on travel for the next few weeks and don't have access to a Samsung device. I will try testing on other Android devices with the Samsung check stubbed out to see that it doesn't break anything, but that won't help test it against Samsung, which is what it is designed to help. I'm also a little worried about state/timing bugs with this change, so if initial testing goes well, I think it is a good idea to get this out in the wild bundled with apps as a beta release and look for any new crash reports caused by this change before merging this PR. I'll roll that beta release after I do my initial testing, and I can put it into a couple of apps I have in the Play Store. Any help doing the same would be appreciated. |
|
Unfortunately, testing has uncovered a fundamental problem with this solution. On Android 8+ apps are not allowed to run for more than a few minutes in the background without a foreground service. If the user puts the app doing beacon scanning to the background, but keeps the screen on, then turns the screen off a bit later, the application will no longer be active to respond to the Broadcast event to change to a non-empty scan filter for the screen being off. A different solution is necessary. |
…ching to filtered scans after screen off.
|
It shouldn't be required that this library provide that solution. |
|
Simplest example of creating a service I have come up with: https://github.com/paulpv/ForegroundServiceAPI26/blob/master/app/src/main/java/com/github/paulpv/foregroundserviceapi26/MainService.kt No other logic needs to be in the service. |
|
Even with foreground service, I am seeing this issue with Samsung that it stops monitoring when the screen is off. I have not pulled down this branch however because I have yet to upgrade to AndroidX, so not sure if this will fix the problem |
|
@stepheaw correct. Background scanning requires multiple things, one being a usable filter that appears to behave differently on Samsung (undefined if others), two being a foreground service, three being a PendingIntent scan, and perhaps others. |
|
For the record, I have all three and an still unable to reliably scan in the background on many models, not just Samsung. |
|
I have all three of these pretty sure but as soon as I click the screen off on Samsung it stops working. On the Pixel its working fine, On the OnePlus I needed to pin the app |
|
@paulpv I would advise you look into the capabilities of this library. If one provides a notification to it, it is able to launch scanning as a foreground service. |
|
Thanks @atotalnoob, I was just going by @davidgyoung 's comments. If he intimates a concern that this library doesn't implement a foreground service, then I went with that. |
|
On further thought, it is possible to do this, because the empty scan filter is only used during an active scan cycle when the application is guaranteed to be active, either via a foreground service or by running in the foreground. I have tested the results and found it to work as expected on a Google Pixels 3a with the Samsung manufacturer check stubbed. I will put this into a beta test release. |
|
Beta release is available here: https://github.com/AltBeacon/android-beacon-library/releases/tag/2.16.4-beta-1 |
davidgyoung
changed the title
|
I've been testing this beta for a few hours, works like a dream on my samsung There is a slight pause when the screen turns off, but I'd say it's probably <1 second and doesn't make much of a difference, anyway. @davidgyoung Would you like some logs? |
|
This may be unrelated to this PR, but my below comment is related to In reviewing the code I am a little concerned about the usage of
Furthermore, if your app was actively scanning when it is force-closed (via user-solicited, app-update, permission-change, and confirmed this even happens when stopping the process while paused in the debugger) then the started PendingIntent will never be re-createable and thus will never be able to be stopped; ie: orphaned/leaked. When that re-started process wants to legitimately start a new scan, it will probably create a new PendingIntent instance that will also get onReceive, which can multiply the number of scan results. The only way to stop the incoming PendingIntents is to turn off Bluetooth and then back on. This issue was a big deal for the Android-Scanner-Compat-Library to fix. This problem affects any caller of the |
|
Belay some of my concern. I was speaking from memory of many different tests/experiments, but testing raw Android OS calls I am not seeing it as bad as I stated above, but these still are areas to test w/ any implementation. |
|
Just to follow up on this, I've been testing the latest changes to this branch on a Samsung S9+. I've found that the broadcast receiver that was added is able to keep my app detecting beacons in the background (with foreground service). Amazing work, thank you so much! |
|
@stepheaw, thank you for the feedback. Can you please explain what symptoms you saw before with a foreground service but without this change? Did your app really stop detecting? Anything custom on your configuration like scan periods and such. |
|
@davidgyoung for a little bit of background, I am taking this library and generating a Xamarin binding library, so not sure how that affects things from the start. My use case for beacons is that I am turning on and off a piece of beacon hardware, and I want to wake up my app even if the app is force closed, and preform some action. Right now, I do not have any custom configuration regarding scan periods and I have setEnableScheduledScanJobs set to false. Although, I think I tried every variation I could to get this to work a few weeks ago. On previous versions of this library, on a Samsung phone, my app would be monitoring beacons just fine with the app open - until I hit the power button. Once I hit the power button, the adb logs had absolutely no output with anything related to beacons at all. When I hit the power button again to wake the app up, then I start to get logs that things are happening. I did not see this behavior on my pixel devices and was related to Samsung only. |
|
Thanks. @stepheaw. I think this makes sense. However I would expect that prior to this change you would eventually see logs again (maybe after 5 mins or so) after a full scan cycle completed with the screen off. |
|
@davidgyoung Maybe I was too impatient before to realize this - so I'm not sure. I haven't extensively gone though all the capabilities of this library until now, so I'm not as knowledgeable as some of the other folks on some of the nuances. |
|
@davidgyoung what still needs to be tested in order to get this merged? |
|
I just released this beta into an app in the Google Play Store. I'd like to monitor the results over the next week or so to see if any new crashes reports pop up as a result of this change. |
|
This is now in a 2.16.4 release. I had the beta in a production app in the Play Store for a week and noticed no new crashes. |
|
Hmmm...I am not seeing any devices scanned when I turn off my screen. When I turn off my screen the following is logged (issue highlighted w/ excessive |
This change is designed to solve the problem reported in #936 where a Samsung Device will be slow do detect a beacon (it will take an extra full scan period) if the app is put to the background with the screen on, then subsequently the screen goes off.
This is caused by the fact that an empty scan filter is set up when the app is pushed to the background, then disallowed by Samsung once the screen goes off. The required non-empty screen off scan filter is not set up until the next scan cycle ends.
The solution is to start a BroadcastReceiver on Samsung devices for this case to look for screen off events before the next scan cycle starts. If a screen off is detected, switch immediately to a non-empty scan filter.
This is a work in progress as I have not yet tested it on a Samsung device.