beagleboard / bonescript

Scripting tools for the BeagleBoard and BeagleBone
http://beagleboard.org
MIT License
32 stars 9 forks source link

Revisit handling of synchronous calls #19

Open jadonk opened 6 years ago

jadonk commented 6 years ago

From @jadonk on February 12, 2014 17:58

Having both synchronous and asynchronous implementations of all the functions has become a pain to maintain. I've used node-fibers to 'my.wait_for' completion of all asynchronous events in order to provide a synchronous return value, but this means that the synchronous calls must be run within a fiber such that they can block (wait for the callback, but allow the other functions to run). To facilitate this, I'm considering bringing back the setup/run functions. The concern is this will cause some existing applications to break. Please file feedback on this issue if you have strong opinions.

Example of what will break:

var b = require('bonescript');

b.pinMode('P9_14', b.ANALOG_OUTPUT);
var value = b.analogRead('P9_36');
b.analogWrite('P9_14', value);

This simplest way to make this work synchronously with the new implementation is:

var b = require('bonescript');

function setup() {
    b.pinMode('P9_14', b.ANALOG_OUTPUT);
    var value = b.analogRead('P9_36');
    b.analogWrite('P9_14', value);
 }

The most desired way to approach this problem is to rewrite your code as asynchronous:

var b = require('bonescript');

b.pinMode('P9_14', b.ANALOG_OUTPUT, undefined, undefined, undefined, onPinMode);

function onPinMode() {
    b.analogRead('P9_36', onAnalogRead);
}

function onAnalogRead(x) {
    b.analogWrite('P9_14', x.value);
}

This has some slightly non-obvious performance advantages if you aren't familiar with node.js's asynchronous architecture. It also has the advantage of working over the RPC mechanism that allows you to run BoneScript within a web page and will be the primary method I will teach BoneScript from here on out.

To simplify writing this in the future I'll handle callbacks in other function argument positions in functions like b.pinMode(pin, mode, callback), rather than requiring all of the arguments. The last argument will always be a callback.

If this just seems unmanageable to you, please let me know!

Copied from original issue: jadonk/bonescript#75

jadonk commented 6 years ago

From @shepting on March 15, 2014 2:48

I'm all for removing synchronous logic!

jadonk commented 6 years ago

I got some really negative feedback. I'm now starting a dual-implementation approach so that I can have a very clean asynchronous implementation. I'll have a parallel implementation that is fully synchronous in the same repo and sharing global variables. Calling from within a fiber will result in usage of the asynchronous versions.

jadonk commented 6 years ago

From @adityapatadia on September 15, 2014 8:9

Having synchronous calls goes against design philosophy of nodejs which aims to do all I/O asynchronously. We have written https://github.com/theoctal/octalbonescript to be fully async except for digitalWrite where both implementation are there.

jadonk commented 6 years ago

I don't know. Maybe there is some way to discourage people from using synchronous logic. My first thought was to have them only do it from within setup(). If there was some reasonable way to add a synchronization point, perhaps with fibers, just at the end of the function in the outer wrapper, that'd be good. ...and then if we required people do something explicit to specify it.

I'm thinking something like:

var b = require('bonescript');
b.defaultWait(0);
var x = b.digitalRead('P1_10', 'wait'); // synchronous
b.digitalWrite('P1_11', b.HIGH);  // asynchronous without a callback
b.defaultWait(1);
b.digitalWrite('P1_11', b.LOW);   // synchronous

...and then maybe warnings could be spit out if you don't call b.defaultWait() and don't explicitly say either 'wait' or provide a callback?

I'm not sure I figured out a good way to do that with fibers.

jadonk commented 6 years ago

Going back and reading my note, I guess that fibers will never be a solution outside of setup() and run(). Perhaps if we are outside of setup() or run(), then if b.defaultWait() is set or if 'wait' is provided as an argument for the callback value, then we can print out a nice error with a URL that will teach folks how to call setup() and run() or learn how to do asynchronous programming?

It'll likely break a lot of people, but hopefully they can get over it. Guess it is better to make the code more maintainable. Man, now I remember why I never got this done.

vaishnavachath commented 6 years ago

I will try to implement the said solution of printing out an error with info to adapt to the change if 'wait' is provided as an argument or defaultWait() is set(when outside setup() and run()), along with addressing the issue.