-
Notifications
You must be signed in to change notification settings - Fork 12
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
feat: use async node-hid #80
Conversation
…nel. This is because previously we used a hack where we used .devicePath for these. This property (as far as I can tell) doesn't exist anymore, so we should not support it. As a backwards compatibility, a blind implementation for supporting { devicePath: string } was added.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I pushed some changes. @Julusian could you give them a review as well please?
this.device.write(data).catch((err) => { | ||
this.emit('error', err) | ||
}) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Just a question: Is it ok to write like this? ie we don't have to worry about concurrency or add some promise queue here?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Hmm actually yeah, this does need to be put in a queue.
close
might need to be put in the same queue too.
This updates the library to node-hid v3.0.0. This switches to using the async version of node-hid, which avoids the limitation of only being able to use 4 devices and increases usb performance by moving the blocking usb ui off of the nodejs main thread.