search

don / cordova-plugin-ble-central

Bluetooth Low Energy (BLE) Central plugin for Apache Cordova (aka PhoneGap)
Apache License 2.0
942 stars 603 forks source link

Request permission for fine and background location on Android #771

Closed tiagoblackcode closed 3 years ago

tiagoblackcode commented 3 years ago

Since Android API Level 29 (Android 10) Bluetooth discovery now needs permissions for fine location as per update notes.

This change conditionally checks and requests for both fine and background location (apps with fine location are granted background location on Android 10) when running on API level 29 onwards.

I've made a pre-release tagged as 1.2.5-android-10 to test the change. I've only tested on Ionic Capacitor and seems to work correctly on Android 8 and 10.

Fixes #754 and some issues related with the scan not finding any devices on Android.

Thanks to @RoopeHakulinen for also investigating the issue.

ghost commented 3 years ago

I can't say I understand android permissions very well yet, but what about this bit about the companion device api?

Because discoverable devices might reveal information about the user's location, the device discovery process requires location access. If your app is being used on a device that runs Android 8.0 (API level 26) or higher, use the Companion Device Manager API. This API performs device discovery on your app's behalf, so your app doesn't need to request location permissions.

Is that not viable here?

tiagoblackcode commented 3 years ago

I think using the Companion Device Manager API as you suggested might be suitable for most use cases but, according to the docs and guide, it doesn't allow for continuous scan, which might prevent some apps from functioning properly:

Companion device pairing also isn't intended for continuous scanning. It is a means for implementing custom protocols over Bluetooth, BLE, and Wi-Fi that are required for the companion device to function.

ghost commented 3 years ago

it's too bad that FINE_LOCATION is the only way to support all cases :(

tiagoblackcode commented 3 years ago

Yes, definitely.

But it left me thinking there could be an additional method for a non-continuous, discrete scan using the Companion Device Manager API in the library. Something like performScan() that would trigger the callback only once. Or adding an option to the startScanWithOptions() method.

EDIT: rename startScan() into startScanWithOptions() to avoid confusion.

rolloaxel commented 3 years ago

Nice idea @tiagoblackcode, can you develop the feature ?

I can help you if you want...

tiagoblackcode commented 3 years ago

@rolloaxel let's wait for @don feedback. We also need to settle for the option vs method if this is going to be implemented.

QuentinFarizon commented 3 years ago

@tiagoblackcode I am using your branch 1.2.5-android-10, and it works better. But I encounter the following issue on a Samsung A7 running Android 10 :

Thanks anyway you and @don for your work !

Appendix :

QuentinFarizon commented 3 years ago

OK here are some thoughts @don and @tiagoblackcode :

I have pushed a version slightly different from the one of @tiagoblackcode implementing this : https://github.com/QuentinFarizon/cordova-plugin-ble-central/commit/5daa4b3075f13933868a0cd0afbc90b012797b99

Ideally, I think it should be configurable wether ACCESS_BACKGROUND_LOCATION should be required or not, because some application may never need to do background scanning.

tiagoblackcode commented 3 years ago

Hello @QuentinFarizon,

so BLE only requires ACCESS_FINE_LOCATION in Android 10 unless you want to do a background scan right? I can update my PR with that change. However, we need to support the use-case where the user (of the library) wants to perform a background scan (which is common when interacting with Bluetooth beacons). To do that, I suggest we add an option to the startScanWithOptions's options argument to request background permissions as well:

const options = {enableBackgroundScan: true};
ble.startScanWithOptions([], options, success, failure);

Regarding:

But I encounter the following issue on a Samsung A7 running Android 10 :

  • launch scan
  • permission granting UI is displayed, click "only when in use"
  • scanning returns an error
  • launch scan again
  • scan succeeds this time, strangely
  • permission granting UI is launched by your code change, because ACCESS_BACKGROUND_LOCATION is not granted The interaction between this phone/android10 and bluetooth are so strange, not sure if something can be done on your side ?

I'll try to reproduce your issue since I've also been having reports of a similar problem.

QuentinFarizon commented 3 years ago

@tiagoblackcode

so BLE only requires ACCESS_FINE_LOCATION in Android 10 unless you want to do a background scan right? I ... think so, but this should be further tested

I'm now using https://github.com/QuentinFarizon/cordova-plugin-ble-central/commit/5daa4b3075f13933868a0cd0afbc90b012797b99 in production and it works fine. As you can see, it still requestPermissions initially for ACCESS_BACKGROUND_LOCATION, but not again if it is nos granted as long as ACCESS_FINE_LOCATION is granted (which was the issue I had with @tiagoblackcode 's version)

Also, I have a check+request method to await before running the scan, using the https://github.com/dpa99c/cordova-diagnostic-plugin plugin. I have found a particular sequence of checks using this plugin, that correctly manages all cases of android10/ble interactions around location, and also works fine with previous versions of Android :

  private async checkPermissions() {
    const bluetoothAvailable = await this.diagnostic.isBluetoothAvailable();
    if (!bluetoothAvailable) {
      return Promise.reject('bluetooth_not_granted');
    }

    // Retrieve status of previous authorizations
    const locationAuthorizationStatus = await this.diagnostic.getLocationAuthorizationStatus();
    switch (locationAuthorizationStatus) {
      case this.diagnostic.permissionStatus.GRANTED:
      case this.diagnostic.permissionStatus.GRANTED_WHEN_IN_USE:
        // already granted previously
        return Promise.resolve();
      case this.diagnostic.permissionStatus.NOT_REQUESTED:
        // continue below to request authorization
        break;
      case this.diagnostic.permissionStatus.DENIED_ALWAYS:
      default:
        // User previously clicked 'always denied', maybe prompt him to go to its settings (eventually using Diagnostics plugin method for that)
        return Promise.reject('location_not_granted');
    }

    // Synchronously request location, it prompts a dialog to the user, and we await his response.
    const status = await this.diagnostic.requestLocationAuthorization(this.diagnostic.locationAuthorizationMode.ALWAYS);
    switch (status) {
      case this.diagnostic.permissionStatus.GRANTED:
      case this.diagnostic.permissionStatus.GRANTED_WHEN_IN_USE:
       // user granted permission
        return Promise.resolve();
      case this.diagnostic.permissionStatus.NOT_REQUESTED:
      case this.diagnostic.permissionStatus.DENIED_ALWAYS:
      default:
        // user denied permission, prompt him that he should accept, and why, and  to go to its settings (eventually using Diagnostics plugin method for that)
        return Promise.reject('location_not_granted');
    }
  }

Once user clicked always_deny, the app should call requestLocationAuthorization again, it wont work at all. The app should give indication to the user at what to do.

When a function request a scan, I call :

this.checkPermissions().then(() => {
    // use cordova-plugin-ble-central scanning methods

This sequence of checks should probably be integrated in this library, don't you think @don ?

don commented 3 years ago

Thanks for your patience. I'm finally getting a chance to look at this after a long delay. I've been distracted by other work.

Generally I don't like asking for more permissions than necessary. Allowing ACCESS_FINE_LOCATION is unfortunate seems to be required. I'm torn on ACCESS_BACKGROUND_LOCATION. Not everyone needs this but background scanning is very useful. Maybe that doesn't matter as much now since Android users have more control to enable and disable permissions.

I need to run in a test app but @tiagoblackcode patch looks like it will fix a lot of issues. I like @QuentinFarizon suggestion about integrating more checks like checkPermissions.

don commented 3 years ago

These changes were published to NPM as v1.3.0

QuentinFarizon commented 3 years ago

@don beware, by merging this PR as-is, I think you will hit this bug : https://github.com/don/cordova-plugin-ble-central/pull/771#issuecomment-667193474

don commented 3 years ago

@QuentinFarizon I think I solved that problem with this commit https://github.com/don/cordova-plugin-ble-central/commit/5183e5389394f9812700dc78c4f31d2a6e7e110a

QuentinFarizon commented 3 years ago

@don Nice ! I'll check it on my project.

By the way, I had to modify my "checkPermissions" method due to changes in iOS 14.0.1 and then again for iOS 14.1. Here it is now, yes it's long but every bit is necessary ! :

export enum BlePermissionsErrorType {
  BLE_NOT_AVAILABLE = 'BLE_NOT_AVAILABLE',
  LOCATION_NOT_ENABLED = 'LOCATION_NOT_ENABLED',
  LOCATION_NOT_GRANTED = 'LOCATION_NOT_GRANTED'
}

@Injectable({
  providedIn: 'root'
})
export class BlePermissionsService {

  private bluetoothState = 'unknown';

  constructor(
    private ble: BLE,
    private diagnostic: Diagnostic,
    private platform: Platform
  ) {
  }

public async checkBleAndLocationPermissions() {
    console.log('checkPermissions enter');
    const bluetoothAvailable = await this.diagnostic.isBluetoothAvailable();
    console.log('checkPermissions bluetoothAvailable ' + bluetoothAvailable);
    if (!bluetoothAvailable) {
      return Promise.reject(BlePermissionsErrorType.BLE_NOT_AVAILABLE);
    }
    const locationEnabled = await this.diagnostic.isLocationEnabled();
    if (!locationEnabled) {
      return Promise.reject(BlePermissionsErrorType.LOCATION_NOT_ENABLED);
    }
    if (this.platform.is('ios')) {
      console.log('checkPermissions startBleStateNotification');
      this.startBleStateNotification();
    }
    try {
      const locationAuthorizationStatus = await this.diagnostic.getLocationAuthorizationStatus();
      console.log('checkPermissions locationAuthorizationStatus ' + locationAuthorizationStatus);
      switch (locationAuthorizationStatus) {
        case this.diagnostic.permissionStatus.GRANTED:
        case this.diagnostic.permissionStatus.GRANTED_WHEN_IN_USE:
          if (this.platform.is('ios')) {
            await this.waitForBluetoothStateOn();
          }
          return Promise.resolve();
        case this.diagnostic.permissionStatus.NOT_REQUESTED:
          break;
        case this.diagnostic.permissionStatus.DENIED_ALWAYS:
        default:
          return Promise.reject(BlePermissionsErrorType.LOCATION_NOT_GRANTED);
      }
      const status = await this.diagnostic.requestLocationAuthorization(this.diagnostic.locationAuthorizationMode.ALWAYS);
      console.log('checkPermissions statusAfterAuthorizationRequest ' + status);
      switch (status) {
        case this.diagnostic.permissionStatus.GRANTED:
        case this.diagnostic.permissionStatus.GRANTED_WHEN_IN_USE:
          if (this.platform.is('ios')) {
            console.log('checkPermissions waitForBluetoothStateOn');
            await this.waitForBluetoothStateOn();
          }
          console.log('checkPermissions location granted');
          return Promise.resolve();
        case this.diagnostic.permissionStatus.NOT_REQUESTED:
        case this.diagnostic.permissionStatus.DENIED_ALWAYS:
        default:
          return Promise.reject(BlePermissionsErrorType.LOCATION_NOT_GRANTED);
      }
    } finally {
      if (this.platform.is('ios')) {
        await this.stopBleStateNotification();
      }
    }
  }

  private startBleStateNotification() {
    this.ble.startStateNotifications()
      .subscribe(state => {
        console.log('checkPermissions ble state changed ' + state);
        this.bluetoothState = state;
      });
  }

  private async waitForBluetoothStateOn() {
    let nbRetries = 30;
    console.log('checkPermissions bluetoothState init ' + this.bluetoothState);
    while(!(this.bluetoothState === 'on') && nbRetries) {
      console.log('checkPermissions bluetoothState ' + nbRetries + ' ' + this.bluetoothState);
      await new Promise(r => setTimeout(r, 100));
      nbRetries--;
    }
  }

  private async stopBleStateNotification() {
    await this.ble.stopStateNotifications();
  }
gduprez commented 3 years ago

Hi @don I have upgraded to v1.3.1 on my app and I'm still not receiving any events when building with sdk 29. Any idea that could help me ? i'm not able to debug java code..

We tried on OnePlus7 Oxygen Os 10, Huawei P20 Pro another Redmi Note 8 Pro. Thanks

strafton commented 3 years ago

My cordova app is currently being rejected from Google Play due to the request for Background Location in the cordova-plugin-ble-central 1.3.1 "BLE" . This is a huge issue for my team as we need bluetooth support and we are have a production issue we are fixing. Any ideas on how to disable this ASAP so it will be approved???

HankLloydRight commented 3 years ago

My cordova app is currently being rejected from Google Play due to the request for Background Location in the cordova-plugin-ble-central 1.3.1 "BLE" . This is a huge issue for my team as we need bluetooth support and we are have a production issue we are fixing. Any ideas on how to disable this ASAP so it will be approved???

See my reply here: https://github.com/don/cordova-plugin-ble-central/issues/821#issuecomment-778545615

strafton commented 3 years ago

Thank you. I will give this a try!! Sincerely, Scott

Sent from my iPhone

On Feb 12, 2021, at 9:13 PM, Hank Lloyd Right notifications@github.com wrote:

 My cordova app is currently being rejected from Google Play due to the request for Background Location in the cordova-plugin-ble-central 1.3.1 "BLE" . This is a huge issue for my team as we need bluetooth support and we are have a production issue we are fixing. Any ideas on how to disable this ASAP so it will be approved???

See my reply here: #821 (comment)

— You are receiving this because you commented. Reply to this email directly, view it on GitHub, or unsubscribe.

LocutusOfPenguin commented 3 years ago

Im abit worried to downgrade (ignoring severals commits from @don) to make it work.

isn't it also possible to just delete uses-permission android:name="android.permission.ACCESS_BACKGROUND_LOCATION" line in manifest?

HankLloydRight commented 3 years ago

Im abit worried to downgrade (ignoring severals commits from @don) to make it work.

There are only two versions between 1.2.5 and 1.3.1.
And the 1.3.0 is what created this problem to begin with. So you're only really missing the 1.3.1 changes (below). I'm using the modified 1.2.5 on android-cordova in two published apps without any problems.

= 1.3.1 = Android updated to BluetoothLeScanner removing deprecated LeScanCallback #796 Android updated to work with android-cordova@8 and android-cordova@9 #819 iOS has new plugin variable IOS_INIT_ON_LOAD to delay plugin initialization. Defaults to false. #739 #769 = 1.3.0 = Add new location permissions Android 10 (API29) #771