openbci-archive / OpenBCI_NodeJS

Node.js SDK for the all OpenBCI Biosensor Boards
https://www.npmjs.com/package/openbci
137 stars 52 forks source link

Timeouts used to ensure commands are written rather than promise resolution #91

Closed baffo32 closed 7 years ago

baffo32 commented 7 years ago

Throughout the code, timeouts are used to wait for commands to be written. This could make maintainability more difficult if the command delay ever changes, and is nonintuitive for users of the library.

Ideally write functions should resolve a promise only when their write has completed, and this promise resolution should be used to know that the writing has completed.

A simpler approach could be to adjust .disconnect to wait until all writes have completed, but this would not solve the problem in as many cases.

Note: This is handled by the OpenBCIBoard.prototype.write function and its sister ._writeAndDrain function, which is also called elsewhere to perform internal writes. These second uses may or may not be bugs; they won't have the 10 ms delay. write() fills and empties .writeOutArray which is just a plain javascript Array but is unnecessarily treated as if it is a fixed-size buffer. It would be reasonable to place callbacks or complex objects in this array, to resolve results to the user. Alternatively, it could be abolished and replaced with a linked list of write-resolve functions.

andrewjaykeller commented 7 years ago

Agreed! Version 2 of the firmware has no delays for 99% of all cases.

Ideally write functions should resolve a promise only when their write has completed, and this promise resolution should be used to know that the writing has completed. Any idea how to link the command with resolution of it's promise call?

baffo32 commented 7 years ago

Instead of lengthening an array of commands, one could have an array of objects which each entry contain the command, the resolve function, and the rejection function. Alternatively one could have a new local function call made for each command, and use local variables. Then these functions are stored rather than the command.