Skip to content
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

Missing HID InputReports #53

Open
AlainCo opened this issue Mar 8, 2018 · 7 comments
Open

Missing HID InputReports #53

AlainCo opened this issue Mar 8, 2018 · 7 comments

Comments

@AlainCo
Copy link

AlainCo commented Mar 8, 2018

Hi,
I've succeeded in exchanging with a HId device (a Ledger nano S).
It worked well, but now on some long sequence of inputreport (3 reports), I miss on report.

all is in purejavahidapi.windows.HidDevice.runReadOnBackground()

I've investigated and I've discovered that the missing report is obtained if,
after the ReadFile()
and the GetOverlappedResult()

I do agains a GetOverlappedResult() which obtaine a different report, the 2nd block.
but if I read again, I always obtain the same second block.

If I do a ReadFile() (with or without a ResetEvent() ), I miss the 2nd report.

I've tried to set an event following this idea
https://groups.google.com/d/msg/comp.os.ms-windows.programmer.win32/YmmR3AP4iXU/dro42wfM5ZcJ
m_InputReportOverlapped.hEvent = CreateEvent(null, true, false, null);
but nothing change

if I do many GetOverlappedResult() even resetting the m_InputReportLength[0]=0;,
each time it says "m_InputReportLength=65" and read the same 2nd report...

with or without ResetEvent(m_InputReportOverlapped.hEvent), nothing change...

I don't know Win32 API, except few page on MSDN about the cited functions...
Do you have an idea... If it works, I will post the patch...
Too bad it was nearly working. I'm not far...
Best regards

@nyholku
Copy link
Owner

nyholku commented Mar 9, 2018

Hi, looking at the code I'm wondering if the ReadFile actually sometimes (in your case) returns true indicating that the read has completed in which case perhaps the GetOverlapped call should not be made ... I don't have time right now to investigate (and I don't have hardware that would cause this problem) but you might want to try to follow that thought.

If you look at my PureJavaComm here

https://github.com/nyholku/purejavacomm/blob/de137e36484cfc4e3b5bd2b93b5e4d84aed88f0a/src/jtermios/windows/JTermiosImpl.java#L472

you see that the control flow of the code is subtly different and does not do GetOverlapped if ReadFile returns true.

@AlainCo
Copy link
Author

AlainCo commented Mar 9, 2018

I've tested, and the readfile sometime complete synchronously and fill Overlapped and byteread, but the getOverlappedResults give the same answer.

The problem happens independently of Readfile synchronous or not (il fact it only happend for a async call) when I get successively getoverlappedResult.
it get the first report...
then the second report, then always the second, but no change in behavior... byteread is alsways 65 even if I reset it to zero.

It is as if the overlapped was re-written asynchronously, like a shared memory.
In a way it have some coherent with HID, which considers it is a report, not a message...
after all, you can rewrite an old report (mouse position, key pressed) with a report which is newer...
But I would have expected this happens only after you reset event...

for application that abuse HID philosophy to exchange data it is tragic.

My first hope is to detect block changing when GetOverlappedResult or not, and if a new block is overwritten, call back against, else continue with ReadFile.
what is strange is that ResetEvent does nothing, good or bad, there or missing.
Code will be awful if I do that.

The snippet of purejavacomm will help me.
I'll go back to avoiding getOverlappedResult when in sync ReadFile and see if I can find a trick.

@AlainCo
Copy link
Author

AlainCo commented Mar 9, 2018

I could make it work, but only by

  1. CreateEvent with manualReset=false in the Overlapped
  2. ResetEvent
  3. ReadFile
  4. as you propose only if IO PENDING -> GetOverlappedResult wait=true
  5. if readbytes>0 get the data, and copy from the Overlapped input memory, call the listener
  6. keep a copy of reportId and data
  7. try again to GetOverlappedResult wait=false
  8. if success, and readbytes>0 and data/reportId is different from previous ->call the listener and retry from step 6
  9. else retry from step 2

Here is the patch, but there is tons of printf to remove:

patch-purejavahid-lostreport-20180309.txt

@AlainCo
Copy link
Author

AlainCo commented Mar 9, 2018

I was afraid the time inside the listener would make me lose some HID reports, but no...
even with sleep(1000) inside the listener, with my trick all works.

here is my trace:

ResetEvent...
ReadFile...
ReadFile -> err=997
ReadFile -> IO pending
GetOverlappedResult wait
sending OutputReport chunk: 0101050000001AE002000115058000002C8000003C800000008000000000000000
GetOverlappedResult -> byteread=0
buffer: empty
ResetEvent...
ReadFile...
ReadFile -> err=997
ReadFile -> IO pending
GetOverlappedResult wait
GetOverlappedResult -> byteread=65
buffer: len=64 buf=0101050000008D410417FD98CC1E5D95190C6528F0ECBD48DD6A816D36E040A4B0706F1FE2E65D1AB97CE820622B97E1A2F00DB84F15E54BCC543472B5E0A18000
do listener len=64 buf=0101050000008D410417FD98CC1E5D95190C6528F0ECBD48DD6A816D36E040A4B0706F1FE2E65D1AB97CE820622B97E1A2F00DB84F15E54BCC543472B5E0A180
retry GetOverlappedResult nowait
late GetOverlappedResult succeed lasterror=0 byteread=65
buffer: len=64 buf=01010500015508979FF61C62F8E428663130373039374537313732443845424546634246333130374546444539303131333234373942334D4A1AE2DB28073CA300
do listener len=64 buf=01010500015508979FF61C62F8E428663130373039374537313732443845424546634246333130374546444539303131333234373942334D4A1AE2DB28073CA3
retry GetOverlappedResult nowait
late GetOverlappedResult succeed lasterror=0 byteread=65
buffer: len=64 buf=01010500015508979FF61C62F8E428663130373039374537313732443845424546634246333130374546444539303131333234373942334D4A1AE2DB28073CA300
buffer: no change len=64 buf=01010500015508979FF61C62F8E428663130373039374537313732443845424546634246333130374546444539303131333234373942334D4A1AE2DB28073CA3
ResetEvent...
ReadFile...
ReadFile -> byteread=65
buffer: len=64 buf=0101050002E6359AA1C62F3EB8B426342D3F46F1F090F54EC6DD7074900000000000000000000000000000000000000000000000000000000000000000003CA300
do listener len=64 buf=0101050002E6359AA1C62F3EB8B426342D3F46F1F090F54EC6DD7074900000000000000000000000000000000000000000000000000000000000000000003CA3
device close()
retry GetOverlappedResult nowait
late GetOverlappedResult succeed lasterror=0 byteread=65
buffer: len=64 buf=0101050002E6359AA1C62F3EB8B426342D3F46F1F090F54EC6DD7074900000000000000000000000000000000000000000000000000000000000000000003CA300
buffer: no change len=64 buf=0101050002E6359AA1C62F3EB8B426342D3F46F1F090F54EC6DD7074900000000000000000000000000000000000000000000000000000000000000000003CA3
device closed!

I don't understand why the inputreport is updated instantly with the second report, but then that the third report wait patiently to be readFile and getOverlappedResult.

note that the first report is read after a IO pending then getOverlappedResult, not after a synchronous ReadFile. I don't understand what is happening...

Maybe we should ask to Windows programmers... I am not one of them.

@nyholku
Copy link
Owner

nyholku commented Mar 9, 2018

Hi, thanks for the patch but the raw file or pull request is much more usable. Patches don't fit well my workflow.

@AlainCo
Copy link
Author

AlainCo commented Mar 9, 2018

Here it is, I've added comments and commented traces.

It's crazy, I've just removed the print, and now the bug reappears (probably a timing problem).
8(

What corrected the bug was just a 1ms sleep in the loop, after the listener callback.

I'll look on monday.

anyway here is my situation. (

HidDevice.java.txt
Kernel32Library.java.txt

with the sleep 1ms
HidDevice.java.2.txt

I'll look if the solution is not to have a bigger buffer with many reports

@AlainCo
Copy link
Author

AlainCo commented Mar 12, 2018

I've tested using bigger buffers (8 times the 64bytes of usual blocks), but there is no change (byteread is always 64+1).

I've tested flooding the ledger, and finally the Thread.sleep(); sloud be >11ms ... I decided 20ms and it works. I've heard that depending on USB2 or 3, fast or not, minimum delay for reports is 1ms or 10ms... not sure it is related...

here is the version that

works now

HidDevice.java.txt

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

No branches or pull requests

2 participants