search

capacitor-community / bluetooth-le

Capacitor plugin for Bluetooth Low Energy
MIT License
281 stars 85 forks source link

feat(android): add requestEnable method #591

Closed pwespi closed 12 months ago

pwespi commented 12 months ago

closes #590

gregoiregentil commented 12 months ago

Thank you for taking of #590. But don't you think that RequestEnable API should be integrated into Enable API? Because it takes care of the same feature but with different Android version. Am I clear in my comment? Or do you think that the user should check the Android version in his typescript logic?

peitschie commented 11 months ago

Just in case it's of any interest, here's an example of how this intent-based approach can be wired up so the success/fail result can still be returned via the promise-based API: https://github.com/don/cordova-plugin-ble-central/blob/073c80dbc269341a4822663bde85d090e98d32c4/src/android/BLECentralPlugin.java#L1332-L1350 (obviously, some adaption required!)

(Just driving by while checking if I needed this change too!)

gregoiregentil commented 11 months ago

My understanding (my wish) is to keep only the Enable call in my app. And in the Android code of the plugin, it should detect that we are in version 33+ of Android, so then it should use onActivityResult as explained in the previous comment, which will trigger the OS to show the new popup for the user to enable Bluetooth. Am I misunderstanding?

pwespi commented 11 months ago

Thank you all for your feedback.

I understand that you would like to have a single method to call in your app. My goal was that requestEnable would become that method. It should handle the result properly, as mentioned by @peitschie (thank you for the link!). This will be fixed with: https://github.com/capacitor-community/bluetooth-le/pull/595.

In general I'd like to avoid that a method has a different implementation depending on the SDK version (which is not always possible). And the implementation with ACTION_REQUEST_ENABLE actually works on all supported Android versions. Therefore my plan was to have the new method requestEnable which supports all version and deprecate enable. I should probably have used the implementation with ACTION_REQUEST_ENABLE from the beginning, like the cordova-plugin-ble-central plugin does, and never have used the bluetoothAdapter.enable(). The Android documentation says:

The [enable()](https://developer.android.com/reference/android/bluetooth/BluetoothAdapter#enable()) method is provided only for applications that include a user interface for changing system settings, such as a "power manager" app.

So it was meant for "power manager" apps, and not for apps that just use Bluetooth. That's another reason why I would like to deprecate it.

@gregoiregentil Once https://github.com/capacitor-community/bluetooth-le/pull/595 is merged, would you be able to switch to requestEnable completely, or is there a problem with that?