abandonware/noble

Modify bindings protocol to be able to pass errors to callback functions.

Opened this issue · 1 comments

Currently some binding methods can never pass an error to the callback. Example:

const discoverServices = function (uuids, callback) {
  if (callback) {
    this.once('servicesDiscover', services => {
      callback(null, services);  //hardcoded null for error. 
    });
  }

  this._noble.discoverServices(this.id, uuids); // calls the binding. 
};

I propose that we define a protocol for indicating an error.
Some ideas:

  1. Keep the current events structure, and send back the string "ERROR" in the data. This is simple, but doesn't allow for an error message...but most bindings console log the error anyway.
  2. Add sister events to existing events (servicesDiscoverError), but this would require managing adding a once for 2 events, and clearing the once for the event that didnt happen. Messy.
  3. Audit events list to see if any expect a string as the response data. I dont think any do. We could consider a string to be an error message.

The benefit to doing this would be that instead of having to wait for a timeout on a given operation, we can fail instantly and proceed.

Also I think we need to add deprecation messages on some of the poorly named events, and migrate them to new names. For example:
servicesDiscover -> Response, contains array of serviceIds. rename discoveredServiceIds? etc?
servicesDiscovered - > Also a response, but contains an array of service objects. rename discoveredServiceObjects? etc?

I can make the changes, looking for feedback on the protocol used for indicating an error, and event name changes?

@pixel-shock
@atrovato
Any thoughts?