dariuszseweryn / RxAndroidBle

An Android Bluetooth Low Energy (BLE) Library with RxJava3 interface
http://polidea.github.io/RxAndroidBle/
Apache License 2.0
3.43k stars 583 forks source link

The library claims client state is RxBleClient.State.READY even when location service is off (Android 10 location changes) #742

Closed sandy-8925 closed 3 years ago

sandy-8925 commented 3 years ago

Describe the bug This is for applications targeting API 22. When the location service is off, RxBleClient.observeStateChanges() returns RxBleClient.State.READY even though Android location services is off. This is due to the check in LocationServicesStatusApi23.isLocationProviderEnabledRequired() that assumes that location services is not required for apps targeting API 22 - on Android 10 (API 29), this is not the case. Location services is required regardless of which Android version the app targets.

To Reproduce Steps to reproduce the behavior:

  1. Create app targeting API 22
  2. Have app observe RxBleClient.observeStateChanges()
  3. After installing the app, grant location permission (either normal or 'only while app is in use')
  4. Ensure Bluetooth is on and Location is off in Android settings
  5. Launch the app
  6. RxBleClient.State.READY is emitted by RxBleClient.observeStateChanges() which should not be the case

Specific application where the above situation occurs - https://github.com/sandy-8925/BLExplorer/blob/modernization_v2/android/src/main/java/org/ligi/blexplorer/BluetoothController.kt#L56

Expected behavior RxBleClient.State.LOCATION_SERVICES_NOT_ENABLED should be emitted by RxBleClient.observeStateChanges() in this case

Smartphone (please complete the following information):

Logs from the application when bug occurs (this will greatly help in quick understanding the problem) To turn on logs use:

RxBleClient.updateLogOptions(new LogOptions.Builder()
        .setLogLevel(LogConstants.DEBUG)
        .setMacAddressLogSetting(LogConstants.MAC_ADDRESS_FULL)
        .setUuidsLogSetting(LogConstants.UUIDS_FULL)
        .setShouldLogAttributeValues(true)
        .build()
);
// please paste the logs here
dariuszseweryn commented 3 years ago

Is the app in your case able to scan? Or Is there some kind of an error emitted? I see no logs.

dariuszseweryn commented 3 years ago

@sandy-8925 Ping?

sandy-8925 commented 3 years ago

Is the app in your case able to scan? Or Is there some kind of an error emitted? I see no logs.

It's not able to scan i.e no devices show up. Only after location services is turned on, and the app is restarted, is it able to scan and pick up devices.

I don't think any error was emitted, but I'll try reproducing it and see if there's anything relevant in the logs (like stacktrace or system error/warning).

dariuszseweryn commented 3 years ago

I have just pushed a branch fix/location_check_api_28 which uses a new function in LocationManager to check if location is on. Could you try that branch/commit and report if it fixes the problem? I do not have access to such a device.

You can use JitPack for testing purposes.

dariuszseweryn commented 3 years ago

@sandy-8925 Ping?

sandy-8925 commented 3 years ago

I tested, and unfortunately, the problem still remains. The problem is in this function - LocationServicesStatusApi23.isLocationProviderEnabledRequired() which checks return !isAndroidWear && targetSdk >= Build.VERSION_CODES.M

Since the target SDK of the app is API 22, this function returns false, thus resulting in the library thinking that location service is not required at all. The function you changed CheckerLocationProvider.isLocationProviderEnabled() never gets called because of this.

dariuszseweryn commented 3 years ago

Correct me if I am wrong but you should not be able to upload such an application to Play Store now? Anyway – could you get me information which Android OS stops respecting target SDK < 22 and checks for location permission anyway?

sandy-8925 commented 3 years ago

That limitation applies to the Play Store only, it doesn't apply to sideloaded Android apps. They library isn't limited to only being used in Play Store apps.

This app I used it in, is an open source app that I forked. It's distributed through FDroid.

The problem occurs on Android 10 and above, which seems to enforce location permission requirement for Bluetooth scanning, regardless of target level.

dariuszseweryn commented 3 years ago

Do you know if the same does apply to android wear?

sandy-8925 commented 3 years ago

I am not aware of the behaviour on WearOS , but it should be the same.

dariuszseweryn commented 3 years ago

Check the latest commit on that branch

sandy-8925 commented 3 years ago

I tested with that latest change, and can confirm that it works correctly now.

BTW, it seems it's missing a dependency on RxRelay, the app keeps crashing because of the missing dependency. I was able to fix it by adding implementation 'com.jakewharton.rxrelay2:rxrelay:2.1.1' to the list of Gradle dependencies.

dariuszseweryn commented 3 years ago

Jitpack does not seem to generate a proper POM file and other dependencies are not correctly fetched. This should go away with a proper release

sandy-8925 commented 3 years ago

Ok, it looks like problem solved then. Thanks!

dariuszseweryn commented 3 years ago

Fix available in 1.12.1