Skip to content

Use multi-Node.js-version HID.node, and process.nextTick for callbacks #17

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Open
wants to merge 3 commits into
base: master
Choose a base branch
from

Conversation

SkyLined
Copy link

Here's my current version of the master branch of BlinkStick, with the changes I proposed in the forum:

  1. Use multiple versions of HID.node to support Node.js versions 0.8.0 and up (except 0.11.0-0.11.15 because these are incompatible with Nan and thus HID.node cannot currently be built for them).
  2. Use process.nextTick to call all callbacks. This prevents a Node.js crash because of stack exhaustion if a loop exists where the callback of a BlinkStick method calls another BlinkStick method, etc.

Tested with multiple Node.js versions on windows 8.1

SkyLined added 3 commits March 23, 2015 10:33
* use process.nextTick to call callbacks, which prevents Node./js
crashing on recursive callbacks (these would cause stack exhaustion in
the original code.
* On Windows, add pre-compiled HID.node files for multiple versions of
Node.js. It should now work with Node.js version v0.8.0 and higher,
except for v0.11.0-v0.11.15.
* Adjust code to attempt to load each pre-compiled HID.node file until
one works, or report an error if none of them do.
* Add "-multi-hid" to identify this particular build
* Variable "isWin" is used later on in the file, so cannot be removed as
it was in a previous commit.
@pci
Copy link

pci commented May 12, 2015

Whilst trying to move some code from fedora to windows I came across the windows 8 issue, I can confirm this PR works on windows 8.1 with node 0.12.2 (64bit) 👍

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants