Fix success return type

[MaximBelov]

No description provided.

[peitschie]

Hi @MaximBelov ,

Are you able to provide any context for this change? I understand that you've added a bool return to each some success callbacks, but it's not at all apparent to me what use-case you've encountered that's driven this change.

Can you please provide an example of the issue you're addressing by this modification?

[MaximBelov]

Hi @peitschie
My changes are required for the correct type:

I use wrapper for convenience and I will show you an example of TS code

https://github.com/danielsogl/awesome-cordova-plugins/blob/master/src/%40awesome-cordova-plugins/plugins/ble/index.ts#L201

    const result = await this.bluetoothLowEnergy.isConnected(device.id).catch((error) => { console.error(error); return false});
    // with my changes
    const connectionStatus = result ? 'connected' : 'disconnected';
    // current implementation
    const connectionStatus = result === 'OK' ? 'connected' : 'disconnected';

   const isEnabled = await this.bluetoothLowEnergy.isEnabled();
    if (isEnabled) {
      this.bluetoothLowEnergy.startScan([]).subscribe({
        next: (config: BluetoothBLEConnectionConfig) => {},
        error: async (_error) => {},
      });
    }

Some time ago, I had a similar issue with urbanairship plugin
You can find more information here

urbanairship/urbanairship-cordova#427 (review)

[peitschie]

Right... I can appreciate the confusion a bit there!

The default implementation of isConnected uses either the callbacks (success/error) with no result, or if using the promise version (withPromises.isConnected) will resolve or reject the promise.

If you're wanting to turn this into a bool return, you can handle this in your wrapping code by adding a then like this:

const result = await this.bluetoothLowEnergy.isConnected(device.id).then(() => true).catch((error) => { console.error(error); return false});

I don't have anything against the approach you've implemented, and certainly appreciate you taking the effort to raise this merge request, but it's not a change I want to make as it affects the interface and type definitions for these methods.

I'll update the isConnected examples to be more explicit about how to best achieve the scenario you've outlined, as I agree it's a common use-case.

I repeat again though, thanks for your contribution efforts here! I'll close this out and raise the documentation improvements in preference.

[peitschie]

Writing the documentation gave me a hint as to how to better support the use-case you've got here, without introducing any backwards compatibility changes.

I've introduced an overload of withPromises.isConnected that takes a false parameter as the second value to indicate you want a boolean return, rather than a promise rejection.

With this tweak, you should now be able to write the above as:

// Promises with boolean return
const isConnected = await ble.withPromises.isConnected(device_id, false);

Let me know if you're happy with this approach, and I'll cut it into a full release!