Hey guys. I'm trying to set up a thread that communicates with a PC using USBHID, but I'm finding that readNB() is not working the way I would expect... I was expecting to be able to replace if (hid.read(&report)) with if (hid.readNB(&report)) and have it work straight away, but readNB() seems to be dropping some reports... To test it out, I wrote a small program based on USBHID_HelloWord that echos reports, and used SimpleHIDWrite to send some test reports with the following results:

#include "mbed.h"
#include "USBHID.h"

USBHID hid(8, 8);
HID_REPORT report;
DigitalOut led1(LED1);

int main(void)
{
    while (1) {
        //Try to read a message
        if(hid.readNB(&report)) {
            //Echo the message
            hid.send(&report);

            //Toggle the LED
            led1 = !led1;
        }
    }
}

As you can see, the first and last reports echoed fine, but the middle two didn't... Changing readNB() back to read() solves the problem and the code works again. What I find even more confusing is that the LED toggles even though the echo isn't returned... Am I missing a step here? Thanks!

Just did a couple more experiments, and it appears that readNB() is randomly returning reports without reading any data. The reports therefore have a length of 0 as seen in this picture:

Seeing as how the code for read() and readNB() is almost the same, I'm guessing that the problem lies with either readEP_NB() or readStart(). Unfortunately, glancing through the code for both functions doesn't reveal any obvious problems. As such, I'm at a stand still until I can figure out why readNB() isn't working...

I assume that this is the whole point of readNB (read non blocking)...

If there are data, then fine, if not, the function will not block until it receives data

Hmm did not fully read the post...

Can you try to remove the echo to the PC?

I'm aware that readNB() doesn't block if there's no data, but in this case it's supposed to return false and therefore the if statement will be avoided. My problem is that readNB() occationally returns true, but doesn't actually copy the available data into the report. So the if statement runs like it's supposed to, but there's no data to act on because readNB() didn't finish the job. The echoes are intentional, as they demonstrate the bug.

I just tried the same code on an official LPC1768 board, and both read() and readNB() work perfectly. Therefore, I believe the problem lies in USBHAL_LPC11U.cpp. Since there seems to be some confusion about the behavior I'm expecting, I've modified my test program to illustrate it:

#include "mbed.h"
#include "USBHID.h"

USBHID hid(8, 8);
HID_REPORT report;
DigitalOut led1(LED1);
DigitalOut led4(LED4);

int main(void)
{
    while (1) {
        //Try to read a message
        if(hid.readNB(&report)) {
            //Print the message
            printf("Got report with length = %i, data = ", report.length);
            for(int i = 0; i < report.length; i++) {
                printf("%d ", report.data[i]);
            }
            printf("\r\n");

            //Echo the message
            hid.send(&report);

            //Toggle the LED
            led1 = !led1;
        }

        //Toggle the loop LED
        led4 = !led4;
    }
}

In this version, LED4 will toggle every time the while loop loops. As such, when using read(), LED4 should only toggle once per report since read() is a blocking call, and won't return until a new report is received. By changing read() to readNB() however, it should toggle continuously since readNB() is a non-blocking call that's returning false when there's no report available, thereby avoiding the if statement but allowing the rest of the loop to run. Now imagine a USB thread that has to respond to RTOS signals/queues/mailboxes in addition to checking for reports, and you can see why readNB() is so important. My problem is that readNB() is broken on the LPC11U24. It still returns true every time I send it a report, but it will randomly fail to copy said report and instead return zero bytes. This is made evident in the Tera Term screen capture; the lines that say "Got report with length = 0, data =" should not be happening, every line should be saying "Got report with length = 8, data = XX XX XX XX XX XX XX XX".

As a side note, I also tried this program on a KL25Z, but it appears to be even more broken. Both read() and readNB() cause SimpleHIDWrite to regularly respond with "WRITE ERROR: The system cannot find the file specified" and "WRITE ERROR: A device attached to the system is not functioning", and after receiving the first report, they both start spitting out a 64B report containing junk every 6 seconds or so regardless of input.

I would like to see where the cause of the bug is.

I would begin by reducing the things that you are doing when you receive a report. Because if it is too long, you can receive several reports and you can probably miss some.

What happens if you remove the hid.send() ?

Same issue. I'm sending the reports manually one by one, so there's no way several are coming in and some are being missed.

I've opened a new issue over at GitHub regarding this, so hopefully it will be resolved soon. I've tried digging through the HAL code, but I can't seem to find the problem; it's probably a race condition or something. In the mean time, since I can't get my USB thread to work without it, I guess it's back to the drawing board...

I would be interested to see if there is the same issue if you add a sleep (where you toggle led4 for instance) instead of continuously polling.

Strangely that appears to work, although it doesn't solve the problem since the loop is once again blocking.

#include "mbed.h"
#include "USBHID.h"

USBHID hid(8, 8);
HID_REPORT report;
DigitalOut led1(LED1);

int main(void)
{
    while (1) {
        //Try to read a message
        if(hid.readNB(&report)) {
            //Print the message
            printf("Got report with length = %i, data = ", report.length);
            for(int i = 0; i < report.length; i++) {
                printf("%d ", report.data[i]);
            }
            printf("\r\n");

            //Echo the message
            hid.send(&report);

            //Toggle the LED
            led1 = !led1;
        }

        //Go to sleep
        sleep();
    }
}

Sorry I meant sleep(0.5) for instance. I think that "sleep" puts the core in sleep mode.

Hmm... Adding a wait(0.5) also seems to fix it... Am I correct in assuming this is definitely a race condition somewhere in the USBDevice library?

Definitely yes... You can remove the sleep() now.

I have no idea where it is and I have nothing to test... So I will try to give you some ideas...

Can you try this in USBHID.cpp:

bool USBHID::readNB(HID_REPORT *report)
{
    uint32_t bytesRead = 0;
    bool result;
    result = USBDevice::readEP_NB(EPINT_OUT, report->data, &bytesRead, MAX_HID_REPORT_SIZE);
    // if readEP_NB did not succeed, does not issue a readStart
    if (!result)
        return false;
    report->length = bytesRead;
    if(!readStart(EPINT_OUT, MAX_HID_REPORT_SIZE))
        return false;
    return result;
}

bool USBHID::readNB(HID_REPORT *report)
{
    uint32_t bytesRead = 0;
    bool result;
    result = USBDevice::readEP_NB(EPINT_OUT, report->data, &bytesRead, MAX_HID_REPORT_SIZE);
    // if readEP_NB did not succeed, does not issue a readStart
    if (!result)
        return false;
    report->length = bytesRead;
    if(!readStart(EPINT_OUT, MAX_HID_REPORT_SIZE))
        return false;
    return result;
}

Success! Looks like that fixed it! Should I create a pull request on GitHub with the patch?

Yep, create a pull request and close the issue :)

Thanks for your help! :D