JamesBarwell / rpi-gpio.js

Control Raspberry Pi GPIO pins with node.js
MIT License
657 stars 116 forks source link

Use promise instead of callback #34

Closed achorein closed 8 years ago

achorein commented 8 years ago

Use promise https://github.com/kriskowal/q to be able to stack them. Very usefull in order to wait for all setup to be finished.

Instead of (ES6) :

gpio.setup(40, gpio.DIR_OUT, err => {
   if (err) { throw err; }
   gpio.setup(38, gpio.DIR_OUT, err => {
     if (err) { throw err; }
     gpio.setup(37, gpio.DIR_OUT, err => {
        if (err) { throw err; }
        // do something
     }
   }
});

use :

gpio.setup(40, gpio.DIR_OUT).then(() => {
   // do something
   return gpio.setup(38, gpio.DIR_OUT);
}).then(() => {
   return gpio.setup(37, gpio.DIR_OUT);
}).then(() => {
   // do something
}).catch(err => {
   throw err;
}

Exemple of implementation in rpi-gpio.js :

fs.writeFile(PATH + '/gpio' + pin + '/value', value, cb);

become

var deferred = q.defer();
fs.writeFile(PATH + '/gpio' + pin + '/value', value, (err) => {
   if (err) { 
      deferred.reject(err); 
   } else { 
      deferred.resolve();
   }
});
return deferred.promise;
JamesBarwell commented 8 years ago

Hi, thanks for the suggestion, but I'm not sure I want to take the module down this route, for the following reasons:

It's worth saying that this module originally came about because I was unhappy with the interfaces of other GPIO modules available, and I specifically wanted one with an err-back interface, which at the time I considered to be the most simple interface achievable while being honest about the asynchronous nature of what was happening.

One option you might like to consider would be to wrap this module in a promise style interface and distribute that as an alternative.

I'll leave this issue open for a bit in case people want to discuss it. Cheers.

canda commented 7 years ago

Some other libraries I saw opted to leave the callback as an alternative and implement Promises anyways. So you would have the 2 options to handle the asynchronous task, the callback and the returned promise. Promises will help error handling also. Promises are now natively supported in ES6, so I think it's the way to go. Observables are the new shiny thing out there, but I don't think they apply to this case; they are more for handling a stream of multiple async events. If you like the idea @JamesBarwell, I could write a PR for you to check out

canda commented 7 years ago

I ended up here because I had the same issue as #41, and thought that it would be nice to have promises for error hadling. It turns out that you can have one single error handler, which is nice. And it's flexible, so you could also have separated error handlers for each event, which is also nice. The bum is that by default, unhandled errors in promises fail silently :( Take a look at http://www.2ality.com/2016/04/unhandled-rejections.html http://jamesknelson.com/are-es6-promises-swallowing-your-errors/

But I think it's not in the scope of a library to handle application level errors, so we don't have much to do there. Errors will fail silently if unhandled. The only other option I see is to throw the error without letting the user handle it (except with a try catch block) What do you think @JamesBarwell ?

julienvincent commented 7 years ago

The bum is that by default, unhandled errors in promises fail silently :(

This isn't quite true. At the very least it isn't true anymore. Generally a promise will throw an UnhandledPromiseRejection error which will contain the stack trace that lead to the original error.

JamesBarwell commented 7 years ago

@canda I've been considering your suggestion of an additional Promises interface alongside the errback interface and think this could work well.

Very happy to receive a PR for this (with some tests please!!).

Yes, there will be some users confused about how to use the Promises just as there are today with the async err-back API, but I don't think that's something to be solved in the code. So I'd say let's just keep it a very standard implementation and just make sure the docs point people in the right direction.

dorgan commented 6 years ago

Is there documentation somewhere on how to use the promises interface? I need to setup an array of promises based on gpio.reads as I am using this for a sprinkler with pump, and do not want to turn on the pump unless at least one zone is open.

JamesBarwell commented 6 years ago

There's some brief documentation already in the readme and I will look to add some examples over time, but really this interface just wraps up the existing error-first interface.

It sounds like you might want to investigate using Promise.all to gather the results of multiple read queries. When that Promise.all resolves, you should be able to access each individual read data to check whether any of the zones are open. If you're still stuck then feel free to share some code and maybe I or others can advise.