The USB keyboard subsystem seems to malfunction if we do not push_input a KeyboardReport every time poll() comes back ready

[mutantbob]

I have been continuing my experiments with the Teensy 4 USB keyboard subsystem.

One thing I have observed is that if I do not perform an hid.push_input(x) every time device.poll(&mut[hid]) comes back ready, then the keyboard seems to "lock up". Sometimes it gets stuck on a single key, sometimes on no key, and it never goes back to working.

Here is an example test routine I have been using:

fn keyboard_mission4<P: Pin>(
    pins: &mut MyPins<P>,
    gpt1: &mut GPT,
    hid: &mut HIDClass<BusAdapter>,
    device: &mut UsbDevice<BusAdapter>,
) -> ! {
    let mut pushed_back = None;

    let mut bacon = Ia::default();

    let mut counter = 0;

    loop {
        if device.poll(&mut [hid]) {
            let mode = RotaryMode::get(pins);

            let rollover = match mode {
                RotaryMode::Position1 => 0,
                RotaryMode::Position2 => 1,
                RotaryMode::Position3 => 10,
                RotaryMode::Position4 => 100,
                RotaryMode::Unknown => 0,
            };

            counter += 1;

            if counter > rollover {
                let keyboard_report = if !pins.switch_pin.is_set() {
                    match pushed_back.take() {
                        Some(x) => x,
                        None => bacon.one_usb_pass(&mut pins.led, &pins.switch_pin, gpt1, hid),
                    }
                } else {
                    simple_kr1(0, 0)
                };

                match hid.push_input(&keyboard_report) {
                    Ok(_) => {}
                    Err(_) => pushed_back = Some(keyboard_report),
                }
                counter = 0;
            }
        }

        support::time_elapse(gpt1, || {
            pins.led.toggle();
        });
    }
}

If the rollover is anything other than 0, the keyboard gets stuck and fails to cycle through the message embedded in Ia. (the LED still blinks, so I am optimistic the loop is still running).

I was hoping someone who understands the USB subsystem better might be able to explain what is going wrong. If what I am observing is a problem with the USB driver/library, that means this API is a bit fragile and might benefit from a wrapper layer to make things easier for first-time users.

[mciantyre]

If you move the LED toggling inside the if device.poll(&mut [hid]) branch, like

if device.poll(&mut [hid]) {
    //
    // Everything else,
    // with a NON-ZERO rollover.
    //

    support::time_elapse(gpt1, || {
        pins.led.toggle();
    });
} // end poll

does the example still blink? I'm assuming a non-zero rollover. I think that if it blinks, it only blinks once.

I think the troublesome code demonstrates a chicken-or-egg problem:

• In order for poll to return true, we must enter the PollResult data branch of the poll() call.
• The most important endpoint for this USB HID class at this point is (I think?) the interrupt IN endpoint. Once that endpoint signals ep_in_complete, poll() returns true.
• The interrupt IN endpoint signals ep_in_complete only after the input report supplied to hid.push_input has been flushed to the host. Basically, we need to call hid.push_input at least once. This effect is observed by device.poll() later on.
• We can't call hid.push_input, because we're not entering the poll() branch, and the inner branch when rollover != 0.

When rollover == 0, the example just so happens to enter poll, enter counter > rollover, and call hid.push_input. After a couple more loops, poll returns true again, counter > rollover again, call hid.push_input again...

Could you separate polling from the generation of input reports?

[mutantbob]

The separation of polling from the generation of input reports is definitely possible. I used to have them separated before, but was annoyed by how push_input() was returning WouldBlock most of the time.

Your description of the behavior of poll() does not match my interpretation of the docstring in usb-device/src/device.rs.

"Returns true if one of the classes may have data available for reading or be ready for writing, false otherwise. "

Is the device ready for writing? Is there a function other than poll that will tell me when push_input will not block?

[mciantyre]

"Returns true if one of the classes may have data available for reading or be ready for writing, false otherwise. "

Great observation. I agree with your interpretation of device's poll() method.

But as the imxrt-usbd bus implementer, I have a different requirement. I'm only expected to signal "an IN transfer completed" once. See the PollResult::Data's ep_in_complete field documentation:

An IN packet has finished transmitting. This event should only be reported once for each completed transfer.

To meet this requirement, I clear the "IN transfer complete" bits on every bus poll() (see here).

Unless the device is expected to latch the ep_in_complete bits and keep signaling true, these documented behaviors don't seem compatible. What do you think?

This might be worth a larger discussion with the upstream project. I'm thinking that either

• device's poll() documentation is ~wrong~ could be misinterpreted. If the device doesn't latch the bits, it should read similarly to the ep_in_complete documentation. "Returns true if one of the classes [...] ~or be ready for writing~ or has completed a scheduled write." (I suppose the existing docs don't explicitly say "it always returns true while its ready for writing"; in this interpretation, the existing docs are not wrong.) Or,
• the ep_in_completedocs are wrong. As the bus implementer, I should latch those bits until the device writes new data to the endpoint. "This event should ~only be reported once for each completed transfer.~ be reported while the bus can accept data on the endpoint."

Or something else is at play, down in the imxrt-usbd driver.

[mutantbob]

I do not know how much code uses the UsbDevice API, but my very shortsighted guess is that it should be improved to better match what using code needs from it.

My extremely limited use case is "can I call push_input without getting a WouldBlock ?" and the API does not have an exact function for that. Other apps are certainly asking other questions, but I have not made an attempt to find any other such source code.

I definitely feel that at least the upstream documentation needs to be refined.

[mutantbob]

I separated the poll() from the push_input() and it worked fine (although if I turned the rollover up to 1_000_000, there was enough delay to let the computer start auto-repeating the keystrokes), so your instinct was correct.

[mciantyre]

Thanks for sharing your troubleshooting. If everything is working, I'll close this issue. Reach out if you hit any other issues in this driver.