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

The library should require location services to be enabled to scan BLE devices only when absolutely needed #106

Closed
mfatiga opened this issue Dec 14, 2016 · 11 comments
Assignees
Milestone

Comments

@mfatiga
Copy link

mfatiga commented Dec 14, 2016

Summary

Since Android 6.0 it is required to hold ACCESS_COARSE_LOCATION or ACCESS_FINE_LOCATION permissions to scan BLE devices but there is no requirement to enable Location Services.

The docs don't mention a requirement for Location Services to be turned on, only the requirement for ACCESS_COARSE_LOCATION or ACCESS_FINE_LOCATION permissions.

I have created a couple of BLE scanners without using any libraries, and have also been using Android Beacon Library (and have also contributed to it) for lots of projects in my company, and have never required Location Services to be enabled for scanning to work.

Actual result

The library throws an exception when calling scanBleDevices if the Location Services are not enabled.

Expected result

The library should only require ACCESS_COARSE_LOCATION or ACCESS_FINE_LOCATION permissions. The Location Services don't have to be turned on for the scanning to work properly.

@uKL uKL self-assigned this Dec 14, 2016
@uKL
Copy link
Collaborator

uKL commented Dec 14, 2016

Hey,

thanks for the report. Did you check https://code.google.com/p/android/issues/detail?id=189090?

If that's true what you say, could you provide a pull request with this one changed, still with scanning working as expected? Thanks.

@mfatiga
Copy link
Author

mfatiga commented Dec 14, 2016

I have read through the issue in the link you provided and it seems this doesn't affect all Android devices or Android versions. I think the library should not explicitly require the Location Services to be on for all Android versions and devices, because the majority of the devices will scan normally without Location Services being turned on. I have tested BLE scanning on dozens of devices and have never encountered one that requires Location Services to be on to enable scanning.
I currently don't have the time to do a pull request but it's a matter of removing one else if expression in the RxBleClientImpl.scanBleDevices method:

else if (checkIfLocationAccessIsEnabledIfRequired()) {
    return Observable.error(new BleScanException(BleScanException.LOCATION_SERVICES_DISABLED));
}

@dariuszseweryn
Copy link
Owner

Hello @mfatiga
First of all I want to thank you for your report.
Unfortunately some devices require location to be on and some do not. Unless it is possible to determine from the code if a specific device needs to have location turned on it will be a lot of burden to maintain a blacklist of devices that need it. Plus we will have a whole new set of issues about RxAndroidBle scan is not working. For now we plan to stick with the requirement for the location as we also do have different things on our plates. However - if you will have more time - you're welcome to dig into the topic and propose a solution.
Kind Regards

@dariuszseweryn dariuszseweryn changed the title The library should NOT require location services to be enabled to scan BLE devices The library should require location services to be enabled to scan BLE devices only when absolutely needed Dec 15, 2016
@dariuszseweryn
Copy link
Owner

@mfatiga Could you share how have you tested the need of having Location Services being turned on? According to the topic that @uKL has mentioned - if you have tested with an app that was build for target SDK <=22 then it is possible that none of the devices needed the Services to be turned on.

@mfatiga
Copy link
Author

mfatiga commented Dec 21, 2016

@dariuszseweryn I have been building all of the apps with target SDK 23 or greater, and none of the apps I work on currently require location so the location services are turned off on all of my test devices.

Daily test devices (the apps are tested on those devices on a daily basis):
LG G4 - Android 6.0
Samsung Galaxy S5 neo - Android 6.0.1
Samsung Galaxy S6 - Android 6.0.1
Samsung Galaxy Tab A (7.0'') - Android 5.1.1
Samsung Galaxy Tab 10.1'' - Android 6.0
Huawei CAM-L21 - Android 6.0

Me and my coworkers have also tested the apps on our personal devices and on many other devices from app users.

@dariuszseweryn
Copy link
Owner

I am building an app with SDK 25 and tested it with Location Services turned OFF:

  • Huawei ALE-L21, Android 6.0 - OK
  • Samsung SM-G900F, Android 6.0.1 - OK
  • Motorola Nexus 6, Android 7.0 - Not OK
  • LGE Nexus 5X, Android 7.0 - Not OK
    So far it seems that the Nexus line needs to have Location Services turned ON. Though it may not be the only case. Unless it is somehow possible to determine wether that is needed during the runtime - I will not make the restriction more loose. I only plan to push a check wether the app was built for SDK <=22 - then in compatibility mode - the system doesn't require Location Services to be ON even on Nexus devices.
    If you will have a moment - feel free to search wether runtime check for Location Services requirement is possible.

dariuszseweryn added a commit that referenced this issue Dec 29, 2016
…atibility mode (targetSdk < 23)

Summary: Related: #106

Reviewers: michal.zielinski, pawel.urban

Reviewed By: pawel.urban

Differential Revision: https://phabricator.polidea.com/D2042
@dariuszseweryn
Copy link
Owner

@mfatiga As it is not possible to currently distinguish at runtime which devices need Location Services to being turned on - I am closing. I could only add a check for the devices SDK and build SDK of the library - which I did.

If the situation will change - feel free to reopen.

Best Regards

@mfatiga
Copy link
Author

mfatiga commented Feb 3, 2017

Thank you for all the effort!

@iLeafSolutionsPvtLtd
Copy link

Hi, I just came across the same issue. Very few devices require location to be turned on for BLE callbacks and most of them are runs on Android Oreo.

@piotr-pawlowski
Copy link

I think you should not require location services to be enabled. On Samsung devices with Android 6+ BLE scanning works properly without location. In my app I inform users that location services should be enabled if there is no scan results, but this info disappears when first scan result occurs. I do not want to force users to enable location services.

@dariuszseweryn
Copy link
Owner

I have added an option to ScanSettings to disable Location Services check. You can try it out by using 1.9.0-SNAPSHOT version (see snapshots section of readme)

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

5 participants