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

startStream does not return promise #23

Closed andrewheusser closed 8 years ago

andrewheusser commented 8 years ago

The documentation says this should return a promise but is just writes a value to the board. Should we make this a promise or change the comments? If it's possible that the command won't work occasionally, we should probably return a promise.

    /**
     * Purpose: Sends a start streaming command to the board.
     * @returns {Promise} indicating if the signal was able to be sent.
     * Note: You must have successfully connected to an OpenBCI board using the connect
     *           method. Just because the signal was able to be sent to the board, does not
     *           mean the board will start streaming.
     * Author: AJ Keller (@pushtheworldllc)
     */
    OpenBCIBoard.prototype.streamStart = function() {
        this.streaming = true;
        this._reset();
        return this.write(k.OBCIStreamStart);
    };

    /**
     * Purpose: Sends a stop streaming command to the board.
     * @returns {Promise} indicating if the signal was able to be sent.
     * Note: You must have successfully connected to an OpenBCI board using the connect
     *           method. Just because the signal was able to be sent to the board, does not
     *           mean the board stopped streaming.
     * Author: AJ Keller (@pushtheworldllc)
     */
    OpenBCIBoard.prototype.streamStop = function() {
        this.streaming = false;
        return this.write(k.OBCIStreamStop);
    };
andrewheusser commented 8 years ago

stopStream doesn't return one either

andrewjaykeller commented 8 years ago

.write() returns a promise, so i figured that if i return this.write() that would in turn return a promise once .write() resolves. Is that not true?

andrewheusser commented 8 years ago

oooh, thats a good question, i'm not sure but I would guess it does. maybe shoecraft can help us on this one

andrewjaykeller commented 8 years ago

@andyh616 did you investigate this further?

tashoecraft commented 8 years ago

Looks fine to me. If this.write() is a promise, then returning it will create a Promise object, to resolve when it is done. Should be able to then chain off streamStart and stop

andrewjaykeller commented 8 years ago

Ok thank you @tashoecraft, you the real MVP. I'm closing this issue!