don / cordova-plugin-ble-central

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

Scan should emit complete-callback #586

Open Domvel opened 6 years ago

Domvel commented 6 years ago

The scan method should have a complete callback. This saves a separate timeout implementation. Current workaround: Add e.g. setTimeout(done, time); on a invoke of scan .

In ionic the complete event is not fired for the rxjs observable. I expected:

this.ble.scan([], 3).subscribe(device => {
  console.log('Device discovered: ', device);
}, error => {
  console.log('Cannot scan.');
}, () => {
  console.log('Scan completed. After 3 seconds. But this event will never fired.');
});

One use case: Disable and re-enable a scan button. Or just know when the scan is completed. Without extra code of timers.

Dirty workaround: Add a timeout to the observable. It fires when no other events are emitted within this time. But you will get an error event, not complete. You could also wrap it in a complete new observable and directly pass the events incl. a complete event by setTimeout().

example for a (dirty) workaround:

public scan(): Observable<any> {
  const timeoutSeconds = 1;

  return new Observable(sub => {
    this.ble.scan([], timeoutSeconds).subscribe(
      device => sub.next(device),
      error => sub.error(error));

    setTimeout(() => {
      sub.complete();
    }, timeoutSeconds * 1000);
  });
}

Idk how to inject a complete event to an existing observable. merge, concat, zip does not work. In my opinion, the original scan method of this plugin should have a complete callback. What you think?

btw. I'm wondering how is the way from this native cordova plugin until ionic wrapper. And how does it wrap? This wrapper has to know the original callbacks to implement the obervable. I have not idea / not found it. How is the workflow? Where is the repository of it? And who should implement this? Currently on Ionic I access the requestMTU method directly by the window object like in cordova, not the typed file. Because it's not up to date. But a typefile is not a wrapper. What wrapps it in the Observable?

don commented 6 years ago

I don't really want to change the API by adding another callback or break the contract by send a complete message through the existing callback, since that would break existing code.

There are 3 options: 1) Like you mentioned, run a timer that matches the scan time. This is very straightforward and what I normally do. https://github.com/don/ionic-ble-examples/blob/master/scan/src/pages/home/home.ts#L35 2) Use startScan and stopScan. I do this when I need more control. 3) Wrap the scan function. Your example had issues, but I'm sure this could be made to work.

I'm not sure about your Ionic vs Cordova questions. Other people have written the Ionic wrappers they work well but I haven't looked Into the implementation.

Domvel commented 6 years ago

np. this is just a feature request / suggestion. For my case, I wrapped the method. Btw. I wrapped the whole library in my custom BLE service to simplify the usage. For example the device id is managed automatically. So my read / write methods do not need the device id anymore. :) And for the scan, I implemented the completed callback. 👍 All fine. But it's up to you to decide to close this issue.