yanshouwang / bluetooth_low_energy

A Flutter plugin for controlling the bluetooth low energy.
https://pub.dev/packages/bluetooth_low_energy
MIT License
46 stars 15 forks source link

IllegalStateException on Android #86

Open ekuleshov opened 1 month ago

ekuleshov commented 1 month ago

The following IllegalStateException on some customer devices reported in Sentry. Can't reproduce this on my own devices. Using bluetooth_low_energy 6.0.2.

It looks like the mBluetoothGattServerCallback may not be set at the time it is called:

https://github.com/yanshouwang/bluetooth_low_energy/blob/78e74f7793312394e6221de89560cd7c9e5791cf/bluetooth_low_energy_android/android/src/main/kotlin/dev/hebei/bluetooth_low_energy_android/MyPeripheralManager.kt#L27-L35

Dart exception:

File "my_api.g.dart", line 1482, in MyPeripheralManagerHostAPI.addService
  File "<asynchronous suspension>"
  File "my_peripheral_manager.dart", line 135, in MyPeripheralManager.addService
  File "<asynchronous suspension>"

Android exception:

PlatformException(IllegalStateException, java.lang.IllegalStateException, Cause: null, Stacktrace: java.lang.IllegalStateException
    at x2.z1.z(MyPeripheralManager.kt:30)
    at x2.z2$a.u(MyAPI.g.kt:30)
    at x2.z2$a.b(Unknown Source:0)
    at x2.n2.a(Unknown Source:2)
    at l3.a$b.a(BasicMessageChannel.java:18)
    at z2.c.l(DartMessenger.java:19)
    at z2.c.m(DartMessenger.java:42)
    at z2.c.i(Unknown Source:0)
    at z2.b.run(Unknown Source:12)
    at android.os.Handler.handleCallback(Handler.java:873)
    at android.os.Handler.dispatchMessage(Handler.java:99)
yanshouwang commented 1 month ago

I think this error is throwed by line 82 or line 165, is the bluetooth state poweredOn before you call the addService method?

https://github.com/yanshouwang/bluetooth_low_energy/blob/78e74f7793312394e6221de89560cd7c9e5791cf/bluetooth_low_energy_android/android/src/main/kotlin/dev/hebei/bluetooth_low_energy_android/MyPeripheralManager.kt#L82

https://github.com/yanshouwang/bluetooth_low_energy/blob/78e74f7793312394e6221de89560cd7c9e5791cf/bluetooth_low_energy_android/android/src/main/kotlin/dev/hebei/bluetooth_low_energy_android/MyPeripheralManager.kt#L160-L171

ekuleshov commented 1 month ago

@yanshouwang I think so, here is my init code below. The removeAllServices and addService are inside _initBlePeripheral()

    PeripheralManager pm = PeripheralManager();

    pm.stateChanged.listen((event) async {
      final state = event.state;
      if (Platform.isAndroid && state == BluetoothLowEnergyState.unauthorized) {
        await pm.authorize();
      }
      if (event.state == BluetoothLowEnergyState.poweredOn) {
        _initBlePeripheral(pm);
      } else {
        _disposeBlePeripheral(pm);
      }
    });

    BluetoothLowEnergyState state = pm.state;
    if (state == BluetoothLowEnergyState.poweredOn) {
      await _initBlePeripheral(pm);
    }
yanshouwang commented 1 month ago
    PeripheralManager pm = PeripheralManager();

    pm.stateChanged.listen((event) async {
      final state = event.state;
      if (Platform.isAndroid && state == BluetoothLowEnergyState.unauthorized) {
        await pm.authorize();
      }
      if (event.state == BluetoothLowEnergyState.poweredOn) {
        _initBlePeripheral(pm);
      } else {
        _disposeBlePeripheral(pm);
      }
    });

    BluetoothLowEnergyState state = pm.state;
    if (state == BluetoothLowEnergyState.poweredOn) {
      await _initBlePeripheral(pm);
    }

The code seems right, so this error was throwed by the line 165(addService returns false)?

Need more information about this or how to reproduce this issue with some steps

ekuleshov commented 1 month ago

The code seems right, so this error was throwed by the line 165(addService returns false)? Need more information about this or how to reproduce this issue with some steps

Unfortunately this is all I've got so far. The stack traces are reported by Sentry from user's devices. I will poke around locally and will monitor crash reports.

But I wonder plugin could handle it more gracefully than:

private val server get() = mServer ?: throw IllegalStateException() 
yanshouwang commented 1 month ago

Yes, I think I should add some description to each exception. Also maybe you can upload the debug symbols to Sentry to get the entire stack trace?

See this docs for details

ekuleshov commented 1 month ago

upload the debug symbols to Sentry to get the entire stack trace?

Already did that. Will have to wait for a few days when users will pickup a new app release...

ekuleshov commented 1 month ago

Here is the most recent unobfuscated stacktrace with bluetooth_low_energy: 6.0.2 running on Pixel 6 Pro with Android 12.

PlatformException: PlatformException(IllegalStateException, java.lang.IllegalStateException, Cause: null, Stacktrace: java.lang.IllegalStateException
    at dev.hebei.bluetooth_low_energy_android.MyPeripheralManager.getServer(MyPeripheralManager.java:82)
    at dev.hebei.bluetooth_low_energy_android.MyPeripheralManager.removeAllServices(MyPeripheralManager.java:185)
    at dev.hebei.bluetooth_low_energy_android.MyPeripheralManagerHostAPI$Companion.setUp$lambda$20$lambda$19(MyPeripheralManagerHostAPI.java:1351)
    at dev.hebei.bluetooth_low_energy_android.MyPeripheralManagerHostAPI$Companion.$r8$lambda$OnoTl9oJ8yjgRSaVQDt0sDSzAFs(Unknown Source:0)
    at dev.hebei.bluetooth_low_energy_android.MyPeripheralManagerHostAPI$Companion$$InternalSyntheticLambda$1$7f73d5bfd3ddafa853578382aa442d6d585ec1fa0e35f9033c4f2c22db4dbd19$9.onMessage(Unknown Source:2)
    at io.flutter.plugin.common.BasicMessageChannel$IncomingMessageHandler.onMessage(BasicMessageChannel.java:261)
    at io.flutter.embedding.engine.dart.DartMessenger.invokeHandler(DartMessenger.java:292)
    at io.flutter.embedding.engine.dart.DartMessenger.lambda$dispatchMessageToQueue$0(DartMessenger.java:319)
    at io.flutter.embedding.engine.dart.DartMessenger.$r8$lambda$2j2MERcK825A5j1fv5sZ7xB2Iuo(Unknown Source:0)
    at io.flutter.embedding.engine.dart.DartMessenger$$InternalSyntheticLambda$1$5dd8b6f7959f08bc8717eff7469e77e06ef5aed51cc0cee17f1e13794798223f$0.run(Unknown Source:12)
...
  #0      MyPeripheralManagerHostAPI.removeAllServices (package:bluetooth_low_energy_android/src/my_api.g.dart:1526)
  File "<asynchronous suspension>"
  #2      MyPeripheralManager.removeAllServices (package:bluetooth_low_energy_android/src/my_peripheral_manager.dart:151)
  File "<asynchronous suspension>"
...
yanshouwang commented 1 month ago

@yanshouwang I think so, here is my init code below. The removeAllServices and addService are inside _initBlePeripheral()

    PeripheralManager pm = PeripheralManager();

    pm.stateChanged.listen((event) async {
      final state = event.state;
      if (Platform.isAndroid && state == BluetoothLowEnergyState.unauthorized) {
        await pm.authorize();
      }
      if (event.state == BluetoothLowEnergyState.poweredOn) {
        _initBlePeripheral(pm);
      } else {
        _disposeBlePeripheral(pm);
      }
    });

    BluetoothLowEnergyState state = pm.state;
    if (state == BluetoothLowEnergyState.poweredOn) {
      await _initBlePeripheral(pm);
    }

This code seems has an issue when the state is unauthorized. After the authorize mehtod complete, the app will run into the else block which will call the _disposeBlePeripheral method.

If the user denied the permission, you will run into this error.

ekuleshov commented 1 month ago

This code seems has an issue when the state is unauthorized. After the authorize mehtod complete, the app will run into the else block which will call the _disposeBlePeripheral method.

Are you suggesting that PeripheralManager.stopAdvertising() and PeripheralManager.removeAllServices() methods require BLE permission be granted on Android?

yanshouwang commented 1 month ago

Are you suggesting that PeripheralManager.stopAdvertising() and PeripheralManager.removeAllServices() methods require BLE permission be granted on Android?

All methods require permission when being called, the if else blocks in your code should not be called when state is unauthorized. You can use switch instead of if else, or use else if instead of the second if

ekuleshov commented 1 month ago

All methods require permission when being called, the if else blocks in your code should not be called when state is unauthorized. You can use switch instead of if else, or use else if instead of the second if

It is counter-intuitive to require permission on destructing/removing methods...

E.g. when the app BLE permission is manually removed in the system settings and then BLE power goes off - user would be asked for BLE permission in order to shut things down? It would be more natural to not request permission in that case.

yanshouwang commented 1 month ago

There is an issue in your code. You call authorize when status is unauthorized, and you should return after that. But you still check the state after that. The event.state is still unauthorized.

If the state is still denied by user. The state will keep unauthorized. Then you need show a warning UI to let user jump to the system settings to grant the permission.

If the permission is granted by user. You will receive another state event with poweredOn or poweredOff. If the state is poweredOff. You should show a warning UI to let user turn on the Bluetooth.

You should only continue the business when state is poweredOn.

yanshouwang commented 1 month ago

E.g. when the app BLE permission is manually removed in the system settings and then BLE power goes off - user would be asked for BLE permission in order to shut things down? It would be more natural to not request permission in that case.

You don't need to dispose anything when BLE is turned off. Just show a warning message to user is OK.

ekuleshov commented 1 month ago

I updated my init code to this:

...
    pm.stateChanged.listen((event) async {
      if (event.state == BluetoothLowEnergyState.poweredOn) {
        _initBlePeripheral(pm);
      } else {
        _disposeBlePeripheral(pm);
      }
    });

    BluetoothLowEnergyState state = pm.state;
    if (state == BluetoothLowEnergyState.poweredOn) {
      await _initBlePeripheral(pm);
    }
    ...

  Future<void> _initBlePeripheral(PeripheralManager pm) async {
    if (Platform.isAndroid && pm.state == BluetoothLowEnergyState.unauthorized) {
      bool res = await pm.authorize();
      if (!res) {
        return;
      }
    }
   ...

But I'm still getting this stack trace (Amazon Fire 7 gen 7, Android 5.1.1)

PlatformException: PlatformException(IllegalStateException, java.lang.IllegalStateException, Cause: null, Stacktrace: java.lang.IllegalStateException
    at i3.z1.R(MyPeripheralManager.kt:8)
    at i3.z1.g(MyPeripheralManager.kt:1)
    at i3.z2$a.w(MyAPI.g.kt:6)
    at i3.z2$a.g(MyAPI.g.kt)
    at i3.p2.a(R8$$SyntheticClass)
    at w3.a$b.a(BasicMessageChannel.java:18)
    at k3.c.l(DartMessenger.java:19)
    at k3.c.m(DartMessenger.java:42)
    at k3.c.i(DartMessenger.java)
    at k3.b.run(R8$$SyntheticClass)
    at android.os.Handler.handleCallback(Handler.java:739)
        ...
, null)
  #0      MyPeripheralManagerHostAPI.removeAllServices (package:bluetooth_low_energy_android/src/my_api.g.dart:1526)
  File "<asynchronous suspension>"
  #2      MyPeripheralManager.removeAllServices (package:bluetooth_low_energy_android/src/my_peripheral_manager.dart:151)
  File "<asynchronous suspension>"
  ...
yanshouwang commented 1 month ago

You need to request permission when the state is unauthorized. If the permission is authorized by user, the state will be poweredOn or poweredOff. Call removeAllServices when the state is poweredOn. Do not call any methods when state is poweredOff.

ekuleshov commented 1 month ago

You need to request permission when the state is unauthorized. If the permission is authorized by user, the state will be poweredOn or poweredOff. Call removeAllServices when the state is poweredOn. Do not call any methods when state is poweredOff.

It look odd... When BLE state is powered off I probably should not be asking user's permission for bluetooth?

How does the stage transition look like when the app does not have BLE permissions and BLE us powered on/off? Could you please add the complete state chart to the plugin documentation.

Also, after unauthorized state is raised and user grants permission, does framework automatically trigger the poweredOn state?

Having an unauthorized along with other power and availability states is somewhat cumbersome to manage. It would have been simpler and more intuitive to manage if unauthorized won't be a state, but some kind of separate property/stream.

yanshouwang commented 1 month ago

You need to request permission when the state is unauthorized. If the permission is authorized by user, the state will be poweredOn or poweredOff. Call removeAllServices when the state is poweredOn. Do not call any methods when state is poweredOff.

It look odd... When BLE state is powered off I probably should not be asking user's permission for bluetooth?

How does the stage transition look like when the app does not have BLE permissions and BLE us powered on/off? Could you please add the complete state chart to the plugin documentation.

Also, after unauthorized state is raised and user grants permission, does framework automatically trigger the poweredOn state?

Having an unauthorized along with other power and availability states is somewhat cumbersome to manage. It would have been simpler and more intuitive to manage if unauthorized won't be a state, but some kind of separate property/stream.

This behavior is the same as the CoreBlutooth. The state will always be unauthorized if the user doesn't agree the app to use the BLE. After authorized, the state will update to poweredOff or poweredOn automatically.

See this discussion for more details.

ekuleshov commented 1 month ago

This behavior is the same as the CoreBlutooth. The state will always be unauthorized if the user doesn't agree the app to use the BLE. After authorized, the state will update to poweredOff or poweredOn automatically.

See this discussion for more details.

Hmm. That discussion is related to iOS, yet the unauthorized state is Android only?...

I'm still unclear if the state change event will be send after user gives permission after an unauthorized event is processed and PeripheralManager.authorize() is called?

graph TD;
  unauthorized --> poweredOff
  unauthorized --> poweredOn
  poweredOff --> poweredOn
  poweredOff --> unauthorized
  poweredOn --> poweredOff
  poweredOn --> unauthorized
  unknown
  unsupported
yanshouwang commented 4 weeks ago

I use the Apple's way instead of Google's in this plugin. The state will always be unauthorized before you call the authorize or showAppSettings method on Android. The permission is requested by the system automatically on iOS. This is necessary to keep the same business cross platforms.

I think the graph should be like this. The app will be killed by system when the permission is disallowed by user from the app settings.

graph TD;
unknown --> unsupported & unauthorized & poweredOff & poweredOn
unauthorized --> poweredOff & poweredOn
poweredOff <--> poweredOn
ekuleshov commented 4 weeks ago

The app will be killed by system when the permission is disallowed by user from the app settings.

That sounds scary... In other BLE libs removing Bluetooth permission simply cut off the scan/connectivity but I don't see why the app should be killed.

Also, under what circumstances the application will get the unknown state event?

yanshouwang commented 4 weeks ago

That sounds scary... In other BLE libs removing Bluetooth permission simply cut off the scan/connectivity but I don't see why the app should be killed.

The system will restart the app when user remove any permission of the app. This is a normal behavior on Android and iOS. It's unrelated to plugins.

Also, under what circumstances the application will get the unknown state event?

The unknown state is the default value. The state changed event will never trigger this value.