Readfile on HID sometimes gives wrong values

Question

I have a connection for a WiiMote

handle = CreateFile(didetail->DevicePath, GENERIC_READ|GENERIC_WRITE, FILE_SHARE_READ, NULL, OPEN_EXISTING, FILE_FLAG_OVERLAPPED, NULL);
if(handle != INVALID_HANDLE_VALUE) {
    opened = true;
    readReportEvent = CreateEvent(NULL, TRUE, FALSE, NULL);
    memset(&readOverlapped, 0, sizeof(readOverlapped));
    readOverlapped.hEvent     = readReportEvent;
    readOverlapped.Offset     = 0;
    readOverlapped.OffsetHigh = 0;

    writeReportEvent = CreateEvent(NULL, TRUE, FALSE, NULL);
    memset(&writeOverlapped, 0, sizeof(writeOverlapped));
    writeOverlapped.hEvent    = readReportEvent;
    writeOverlapped.Offset    = 0;
    writeOverlapped.OffsetHigh = 0;
}

I have a thread which always read this handle for new messages:

while(opened && readThreadNextStatus){
    memset (readBuff, 0, 22);
    BYTE* ptrbuff = new BYTE[22];
    int readfile = ReadFile(handle, readBuff, reportLength, NULL, &readOverlapped);
    if(readfile == 0 && GetLastError() == ERROR_IO_PENDING){
        DWORD waitError;
        do 
        {
            waitError = WaitForSingleObject(readReportEvent, timeout);
        } while (waitError == WAIT_TIMEOUT && opened && readThreadNextStatus);

        if(opened && readThreadNextStatus){
            DWORD read = 0;
            if(waitError == WAIT_OBJECT_0){
                GetOverlappedResult(handle, &readOverlapped, &read, TRUE);
            }
            ResetEvent(readReportEvent);
            memcpy(ptrbuff, readBuff, 22);

        cout << "Read:  ";
        coutHex(ptrbuff);
        }
    }
}

My write function:

if(opened){
    if(!WriteFile(handle, buff, reportLength, NULL, &writeOverlapped)){
        if(GetLastError() != ERROR_IO_PENDING){
            close();
        }
    }
    WaitForSingleObject(writeReportEvent, timeout);
    DWORD write = 0;
    GetOverlappedResult(handle, &writeOverlapped, &write, TRUE);
    ResetEvent(writeReportEvent);
}
cout << "Write: ";
coutHex(buff);

Console output:

Connection established
Write: 15- 0- 0- 0- 0- 0- 0- 0- 0- 0- 0- 0- 0- 0- 0- 0- 0- 0- 0- 0- 0- 0-
Read:   0- 0- 0- 0- 0- 0- 0- 0- 0- 0- 0- 0- 0- 0- 0- 0- 0- 0- 0- 0- 0- 0-
Read:   0- 0- 0- 0- 0- 0- 0- 0- 0- 0- 0- 0- 0- 0- 0- 0- 0- 0- 0- 0- 0- 0-
Write: 15- 0- 0- 0- 0- 0- 0- 0- 0- 0- 0- 0- 0- 0- 0- 0- 0- 0- 0- 0- 0- 0-
Read:   0- 0- 0- 0- 0- 0- 0- 0- 0- 0- 0- 0- 0- 0- 0- 0- 0- 0- 0- 0- 0- 0-
Write: 15- 0- 0- 0- 0- 0- 0- 0- 0- 0- 0- 0- 0- 0- 0- 0- 0- 0- 0- 0- 0- 0-
Read:   0- 0- 0- 0- 0- 0- 0- 0- 0- 0- 0- 0- 0- 0- 0- 0- 0- 0- 0- 0- 0- 0-
Read:  20- 0- 0-10- 0- 0-49-ff-ff-ff-ff-ff-ff-ff-ff-ff-ff-ff-ff-ff-ff-ff-
Write: 15- 0- 0- 0- 0- 0- 0- 0- 0- 0- 0- 0- 0- 0- 0- 0- 0- 0- 0- 0- 0- 0-
Read:   0- 0- 0- 0- 0- 0- 0- 0- 0- 0- 0- 0- 0- 0- 0- 0- 0- 0- 0- 0- 0- 0-
Write: 15- 0- 0- 0- 0- 0- 0- 0- 0- 0- 0- 0- 0- 0- 0- 0- 0- 0- 0- 0
Read:   0- 0-
0- 0-
- 0- 0- 0- 0- 0- 0- 0- 0- 0- 0- 0- 0- 0- 0- 0- 0- 0- 0- 0- 0-

coutHex always prints the received data in hex format. Sometimes I get the right data, but sometimes the array is loaded only with 00 00 00 00 00 00 00 00 00 ... 00 00

I experienced that, when I make a write, I always get back a report, which contains just 00's and this appears before my write function make the write on the console output.

I was desperate, so I tried out this:

do 
            {
                waitError = WaitForSingleObject(readReportEvent, timeout);
                Sleep(500);
            } while (waitError == WAIT_TIMEOUT && opened && readThreadNextStatus);

I don't know why, but now it works(not fine, because it has 500ms delay).

What do you think? Maybe ReadFile and WriteFile don't work concurrently?

What causes this? Did I miss something?

score 2 · Accepted Answer · edited Jun 20 '20 at 09:12

2

There are some additional problems:

Create manual reset event (CreateEvent(NULL, TRUE, FALSE, NULL)) instead auto-reset event (CreateEvent(NULL, FALSE, FALSE, NULL)).
Check if ReadFile returned FALSE and the value of GetLastError is ERROR_IO_PENDING, and in this case wait for the event (WaitForSingleObject).
If WaitForSingleObject returns WAIT_OBJECT_0 then you call GetOverlappedResult.

Pass lpNumberOfBytesRead is not necessary if you use GetOverlappedResult because this function returns this value too.

edited Jun 20 '20 at 09:12

Community

1
1

answered Nov 13 '10 at 21:49

Luis G. Costantini R.

1,028
7
6

I have edited my original question with your ideas, please check the code. This isn't solved the problem. – Roland Soós Nov 13 '10 at 22:13
1

Put the ResetEvent(ReportEvent) after GetOverlappedResult, because when this function return the event is set (you are using TRUE as the bWait Parameter). You do not need to put the ResetEvent(ReportEvent) before GetOverlappedResul because this function resets the event. – Luis G. Costantini R. Nov 13 '10 at 23:24
Thanks, I've changed it, but same problem exist :S – Roland Soós Nov 13 '10 at 23:30
1

As Hans Passant suggest check if GetOverlappedResult return is 0 and what is the value of the GetLastError, what is the value returned in the parameter read of the GetOverlappedResult function ?. – Luis G. Costantini R. Nov 13 '10 at 23:43
with my new code it never return 0. The return value is always 1. The read parameter is always 16 – Roland Soós Nov 13 '10 at 23:58
1

Set the value of the read parameter to 0 before call GetOverlappedResult. Are you using the same Overlapped structure to read an write to the file ?. – Luis G. Costantini R. Nov 14 '10 at 00:38
Yes, the parameter always was 0 when I called GetOverlappedResult. I'm using the same overlapped stucture, do you think that this cause the error? Usually this error shows when I send a write and I got back the answer for my write. – Roland Soós Nov 14 '10 at 09:26
Thank you. When I used different overlapped for read and write, everything was fine. :) – Roland Soós Nov 14 '10 at 13:30
You are Welcome. One additional recommendation. Change the check loop: do { waitError = WaitForSingleObject(readReportEvent, timeout); } while (waitError == WAIT_TIMEOUT && opened && readThreadNextStatus); That is inneficient, and use an additional Event that you could signal when you change que value of the opened or readThreadNextStatus variables; of course you have to change the function WaitForSingleObject to MsgWaitForMultipleObjects. – Luis G. Costantini R. Nov 15 '10 at 19:51

score 1 · Answer 2 · answered Nov 13 '10 at 22:35

1

You are not supposed to repeat ReadFile after timeout, but to WaitForSingleObject again. You still have pending read. Poor man's loop (you probably should refine it, so that user can abort it):

DWORD waitError;
do
{
    waitError = WaitForSingleObject(ReportEvent, timeout);
}
while (waitError == WAIT_TIMEOUT);

answered Nov 13 '10 at 22:35

Dialecticus

16,400
7
43
103

Thanks for your answer, I added it to my code in the question. For the first run I thought it solved the problem, but after some testing I experienced again the problem. I have added a console output about the packets. – Roland Soós Nov 13 '10 at 22:54
if (waitError == WAIT_OBJECT_0) part should span all four lines until end of block. If opened && readThreadNextStatus is false from innermost loop then you don't want to print data you don't have. – Dialecticus Nov 13 '10 at 23:23

Readfile on HID sometimes gives wrong values

2 Answers2