search

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

Bugfix: channelSet always fails due to bad resolve() #121

Closed khwilson closed 7 years ago

khwilson commented 7 years ago

The resolve function of a Promise takes exactly one argument. All further arguments are ignored. Thus, the newChannelSettingsObject that was being returned by the channelSet Promise was ignored and the corresponding value in openBCIBoard was always undefined.

Thus, any calls to channel set would actually break the openBCISample code as the channelSettingsArray contained an undefined.

This fixes the bug.

andrewjaykeller commented 7 years ago

Hey @khwilson please fix up the tests for these. please see contribute in the read me. I changed the base branch to development. Great find. I'm a little shocked there is not an auto test for this already.

khwilson commented 7 years ago

Ah, the contribute was in the README. Was looking around for it and didn't see it. Sorry about that!

Also, before fixing the tests, is this an acceptable solution or would you prefer something else? I ask before fixing the tests because there's going to be a MASSIVE find/replace because all the tests only check if a single argument is returned....

khwilson commented 7 years ago

Also, the contribute doesn't mention the OpenBCI:Development branch as being the base branch. Would you like me to submit at PR for that while I'm here?

andrewjaykeller commented 7 years ago

is this an acceptable solution

Yes if this is true then let's get this in the 1.4.1 release!

You should have to change it looks like 27 tests from openBCIConstants.js.

Also, the contribute doesn't mention the OpenBCI:Development branch as being the base branch. Would you like me to submit at PR for that while I'm here?

Yes please if it's not already in the development branch and being planned in. Also not sure if there is anything in the contribute about adding your change in the changelog.md when you're done!

Thanks!

khwilson commented 7 years ago

Okiedokes, fixed up the tests. Worked on my box; hopefully Travis will be equally happy. Thanks for all the hard work. Also, see #122 for the README typos I spotted.

Best,

Kevin

andrewjaykeller commented 7 years ago

Kevin looks like great, waiting on the three auto checks to pass now :)

andrewjaykeller commented 7 years ago

Thanks @khwilson!!!