$cordovaBLE.startNotification shouldn't use a Promise

[don]

$cordovaBLE.startNotification shouldn't use a Promise.

ble.startNotification calls the success callback whenever the value of a Bluetooth characteristic changes. This callback may be called many times so a promise won't work here.

I'm not sure what the right solution is for ng-cordova. Maybe the user could just handle ble.startNotification in their service?

I have a non-ionic example that uses the bleno test service to demonstrate notifcations. You can run this test peripheral on OS X or Linux. Clone the repo and node test.js

create a new cordova project

$ cordova create notify com.example.notify Notify

replace notify/www/js/index.js with https://gist.github.com/don/d9fef2fb45fbd67a372d

$ cd notify
$ cordova plugin add cordova-plugin-ble-central
$ cordova platform add android
$ cordova run --device

[gortok]

This is also covered in issue #750; and it's an issue we also experience in the (different) BluetoothLE plugin wrapper we use at @Jewelbots: https://github.com/Jewelbots/ng-cordova-bluetoothle.

Some plugins handle this through an eventing system, using $rootScope.emit instead of returning a value from a promise resolution:

https://github.com/gortok/ng-cordova/blob/phonegap-push-plugin-addition/src/plugins/push_v5.js#L21

But we also seemingly fixed this issue in [https://github.com/tubaman/ng-cordova/commit/e28650183a5e1b206c9cefee75c2454644636f6a](this commit) for scan; it appears this should also be used for the startNotification function?

[mhartington]

@gortok So should this be fixed with #836 merged in?

[gortok]

@mhartington #750 and #836 targeted the same bit of code; the code in question here is a different bit (startNotification vs. scan); so the code in #750 would need to be added for the startNotification function for this to be resolved.

[mhartington]

Is that already in the works?

[gortok]

@mhartington No. I'm currently integrating the wrapper for the push plugin; Expected that to be finished soon, and if no one jumps on it before then I can take a look at this.

[don]

q.notify might work, but that infers the promise will be resolved at some point, which is incorrect for a bluetooth notification. The bluetooth notification lives until the user disconnects (or less common, until it's manually stopped.)

[gortok]

@don That's a great point. It's also something I've struggled with, promises are the new hotness but they don't catch these sorts of situations. Observerables are obviously preferred; and are generally done using $rootScope.emit() / $rootScope.on() in AngularJS 1.x. This is probably the route we should go for the BLE wrapper; much like this: https://github.com/gortok/ng-cordova/blob/phonegap-push-plugin-addition/src/plugins/push_v5.js#L21

[don]

@gortok I wouldn't want to emit the same event for all notification. Having one callback per characteristic notification makes sense for BLE services I use. I need to write an BLE ionic app to figure some of this out.

[gortok]

@don That's a really good point. Perhaps we could dynamically register events to listen for, instead of the hardcoding that's done in the push plugin?

[jonalling]

@don if it helps, this is the development app I am writing to explore BLE in Ionic.

https://github.com/jonalling/cordovaDev

Scan, Connect, Read all work, Notify only returns the one value which we've been discussing.

[jonalling]

As a workaround I was able to get startNotification to work in a controller using $scope.$apply in the callback. I'm skipping ngCordova for this function for now.

$ionicPlatform.ready(function(){
      var deviceId = $stateParams.deviceId;
      var serviceUUID = data.service;
      var characteristicUUID = data.characteristic;
      ble.startNotification(deviceId, serviceUUID, characteristicUUID, function (result) {
        $scope.$apply(function() {
          var dataResult = new Uint8Array(result);
          $scope.dataValue.unshift(dataResult[0]);
        });
      }, function (error) {
        // error
      });
}); // end $ionicPlatform.ready

[don]

@jonalling thanks for the update. I think skipping ngCordova for notifications is the best solution for now.

[tomasbedrich]

Shouldn't this be closed by #1079 ?