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

NullPointerException when reading peripheral value on Android #47

Closed
ekuleshov opened this issue Jan 28, 2024 · 9 comments · Fixed by #50
Closed

NullPointerException when reading peripheral value on Android #47

ekuleshov opened this issue Jan 28, 2024 · 9 comments · Fixed by #50
Assignees
Labels
bug Something isn't working

Comments

@ekuleshov
Copy link
Contributor

ekuleshov commented Jan 28, 2024

I have a simple Device Name descriptor declared in a service registered with PeripheralManager and running it in Android app.

When calling char read from nRF connect on another device, I'm getting the following exception.
Tested on Amazon KFRAWI tablet with Android 11 (API 30) and on Samsung S23 with Android 14 (API 34).
The same code works on iOS and MacOS.

java.lang.NullPointerException: null cannot be cast to non-null type dev.yanshouwang.bluetooth_low_energy_android.MyGattDescriptorArgs
  at dev.yanshouwang.bluetooth_low_energy_android.MyPeripheralManager.onDescriptorReadRequest(MyPeripheralManager.kt:371)
  at dev.yanshouwang.bluetooth_low_energy_android.MyBluetoothGattServerCallback.onDescriptorReadRequest$lambda$7(MyBluetoothGattServerCallback.kt:91)
  at dev.yanshouwang.bluetooth_low_energy_android.MyBluetoothGattServerCallback.$r8$lambda$9GtPtGoebEvyBgdNRTL29uj-pLw(Unknown Source:0)
  at dev.yanshouwang.bluetooth_low_energy_android.MyBluetoothGattServerCallback$$ExternalSyntheticLambda5.run(Unknown Source:10)
...

I also see the same issue even with the example app. Just run it on Android device, connect to it with nRF connect and tap to read some readable chars.

@yanshouwang
Copy link
Owner

I have no idea why nRF call readDescriptor when readCharacteristic.

But the error maybe occured on Android if you read the CLIENT_CHARACTERISTIC_CONFIG_UUID descriptor because I didn't cache it. The iOS/macOS handled read/write descriptor requests by system so it works.

I will cache this descriptor in the next version.

@yanshouwang yanshouwang self-assigned this Jan 29, 2024
@yanshouwang yanshouwang added the bug Something isn't working label Jan 29, 2024
@ekuleshov
Copy link
Contributor Author

Thank you. Perhaps you can also add some safe guard to the Android code it won't crash the whole app and could check mDescriptorsArgs[hashCode], so it won't be unconditionally cast to MyGattDescriptorArgs:

fun onDescriptorReadRequest(
device: BluetoothDevice, requestId: Int, offset: Int, descriptor: BluetoothGattDescriptor
) {
val status = BluetoothGatt.GATT_SUCCESS
val hashCode = descriptor.hashCode()
val descriptorArgs = mDescriptorsArgs[hashCode] as MyGattDescriptorArgs
val value = descriptorArgs.valueArgs
val sent = mServer.sendResponse(device, requestId, status, offset, value)
if (!sent) {
Log.e(TAG, "onDescriptorReadRequest: send response failed.")
}
}

@ekuleshov
Copy link
Contributor Author

BTW, curios, why are you using BluetoothGattDescriptor.hashCode() there instead, for example, of BluetoothGattDescriptor.getUuid() ?

@yanshouwang
Copy link
Owner

BTW, curios, why are you using BluetoothGattDescriptor.hashCode() there instead, for example, of BluetoothGattDescriptor.getUuid() ?

The UUID can be duplicated, it is allowed to define characteristics or descriptors with the same UUID, so we can't locate by UUID, the best way to do this is use device address and attribute handle to locate an GATT attribute, but the attribute handle only available on Winows, so I use hashCode instead, it's more likely unique then the UUID.

I wonder if I should to add null check codes, if I did that, it will hard to find out what errors occured, I think the better way is to keep the logic to be correct to avoid mistakes caused by wrong codes like this.

@ekuleshov
Copy link
Contributor Author

I wonder if I should to add null check codes, if I did that, it will hard to find out what errors occured, I think the better way is to keep the logic to be correct to avoid mistakes caused by wrong codes like this.

Crashing the whole app is really really bad... Perhaps you could add a global try/catch in the plugin channel handler and communicate this exception to Flutter?

@yanshouwang
Copy link
Owner

yanshouwang commented Jan 29, 2024

I wonder if I should to add null check codes, if I did that, it will hard to find out what errors occured, I think the better way is to keep the logic to be correct to avoid mistakes caused by wrong codes like this.

Crashing the whole app is really really bad... Perhaps you could add a global try/catch in the plugin channel handler and communicate this exception to Flutter?

It's app developer's duty to catch unhandle exceptions(both flutter and native side), we can't ensure a plugin will never throw errors, plugin should use try catch as little as possible.

But indeed I should make this plugin more and more stable by fix errors.

@ekuleshov
Copy link
Contributor Author

Unfortunately there is nothing can be done in the Flutter app when plugin's Android channel implementation is throwing an uncaught exception like this. It has to be handled in the plugin.

@yanshouwang
Copy link
Owner

Unfortunately there is nothing can be done in the Flutter app when plugin's Android channel implementation is throwing an uncaught exception like this. It has to be handled in the plugin.

Not exactly, you can catch unhandled exceptions on native side:
https://medium.com/@boyrazgiray/how-to-catch-handle-exceptions-globally-in-android-d3447064df14
https://stackoverflow.com/questions/29672305/how-to-handle-all-the-exceptions-in-ios-app

But even you catch those unhandled errors, how to deal these errors correctly is not easy because the business logic already blocked.

@ekuleshov
Copy link
Contributor Author

Normally you don't need to modify the main Android activity. It is generated by Flutter and it is better if it stays that way.

The Flutter plugins are handling all the errors and they should return appropriate exception to the consuming app. E.g. something like a system error, which from the app point of view is not much different than Bluetooth being turned off.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working
Projects
None yet
Development

Successfully merging a pull request may close this issue.

2 participants