(untested) Signal overlapped from caller in LUsb0_ControlTransfer #33
Conversation
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This is an attempt to deal with libusb 1.0.24 or later which always uses asynchronous control transfers. It will pass an OVERLAPPED structure and use this afterwards to check that the transfer has finished. It will still be forced into a synchronous transfer here, but by signalling the finished transfer the caller will not be reporting timeouts for actual finished transfers. Signed-off-by: Tormod Volden <[email protected]>
|
@TravisRo Hi Travis, please take a look when you got the time. Thanks. |
|
One thing that might be missing is passing the number of bytes read/written back to the caller through the overlapped. I remember seeing in someone's log files that completed transfers always were listed with 0 length, it was probably a similar issue. In libusb windows_force_sync_completion() does but it also calls PostQueuedCompletionStatus() so maybe my one-liner SetEvent() above is not enough. |
|
It seems that we were able to fix this in libusb instead without too much trouble, so I will close this "hot fix" merge request which probably is questionable anyway. I will keep #34 open, which I think makes more sense, at least as an RFC. |
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
This is an attempt to deal with libusb 1.0.24 or later which always uses
asynchronous control transfers. libusb will pass an OVERLAPPED structure and
use this afterwards to check that the transfer has finished.
The transfer will still be forced synchronous here, but by
signalling the finished transfer the caller will not be reporting
timeouts for actual finished transfers.
Additional merge request comments:
I am not sure if this is correct, if it compiles, or if it solves any problem. There is however some timeouts showing up in libusb, for example in libusb/libusb#94 (comment) which I could possibly think is caused by this.
The libusbK.sys backend deals properly with both sync and async requests. The libusb0.sys backend would up to now ignore the overlapped for an async request. I think the rework in libusb 1.0.24 made everything async. The suggested change makes it "fake async". Probably it could be made to work again with libusb changes instead, but I think offering a "fake async" here is good anyway.
Alternatively the overlapped from the caller can be passed down to the DeviceIOControl instead of the extra overlapped currently created in _usb_io_sync(). I will suggest this in another merge request. It still doesn't make it true async. I think the repacking done in usb_control_msg() requires allocating a new buffer for setup + outgoing data, and it is not obvious for me how to free that buffer at the right point in time later, if going asynchronous.