search

thegecko / webusb

Node.js implementation of the WebUSB Specification
https://thegecko.github.io/webusb/
MIT License
183 stars 27 forks source link

Release interfaces on Device.close() #39

Closed dsanders11 closed 5 years ago

dsanders11 commented 5 years ago

This PR might not be perfect, kinda rushed it as the issue cropped up while I was swamped with other stuff.

Per the spec, close should release any claimed interfaces. Rather than check if it's claimed, I decided to try to release all interfaces and let releaseInterface simply ignore the ones which aren't claimed. Checking first is also perfectly reasonable.

Swallows any errors during release, as the spec doesn't mention throwing any errors here, and it wouldn't make sense to stop the close. Could definitely have better error handling.

Best way to reproduce the current bug is to claim an interface but don't release it, close the device, re-open the device, then try to close the interface (which will still be marked as claimed). Should result in a libusb not found error.

thegecko commented 5 years ago

@dsanders11 Could you look at the review comments for this?

dsanders11 commented 5 years ago

@thegecko, I saw them, I just haven't had a chance to get to them, sorry about that. Pretty swamped the next couple weeks but I'll try to get around to it.

thegecko commented 5 years ago

ack.

thegecko commented 5 years ago

@dsanders11 Any possibility you could take a look at the review comments, thanks!

dsanders11 commented 5 years ago

@thegecko, finally got around to this, apologies for the long delay. Rebased on master, made the requested changes, and deleted an extra newline that snuck into the original. Should be good to go, it's only a couple lines now as the diff is cleaner.

thegecko commented 5 years ago

Great, thanks!