Handle receiving a ZLP in USB_Available

[sandeepmistry]

To address https://github.com/arduino/Arduino/issues/6669.

If a ZLP is received the FifoCount will be zero, but RXOUTI will still be triggered.

@NicoHood could you please try this change out to ensure it's ok HID API wise. Thanks.

[ArduinoBot]

✅ Build completed.

Please test this code using one of the following:

⬇️ http://downloads.arduino.cc/javaide/pull_requests/arduino-PR-6886-BUILD-734-linux32.tar.xz
⬇️ http://downloads.arduino.cc/javaide/pull_requests/arduino-PR-6886-BUILD-734-linux64.tar.xz
⬇️ http://downloads.arduino.cc/javaide/pull_requests/arduino-PR-6886-BUILD-734-windows.zip
⬇️ http://downloads.arduino.cc/javaide/pull_requests/arduino-PR-6886-BUILD-734-macosx.zip
⬇️ http://downloads.arduino.cc/javaide/pull_requests/arduino-PR-6886-BUILD-734-linuxarm.tar.xz

ℹ️ The linuxarm build is still experimental and may not be always available.

[NicoHood]

@sandeepmistry I currently do not have the hardware nor time to test this, I am sorry. Thanks for mentioning me though. If the official arduino hid libraries work, mine should work too.

[kervinck]

I found this issue because I just debugged a very similar "256-byte transfer problem on the Arduino Micro" in my project. See kervinck/gigatron-rom#36

I was about to file a problem report when I found this issue report here. However, I just tested arduino-PR-6886-BUILD-734-macosx.zip and it doesn't resolve my case. I have opened https://github.com/arduino/Arduino/issues/7838 although I do have a workaround that seems to work in my project.

The reason I'm commenting here is that you may want to take a quick look at it. My conclusion and workaround are using a different observation, namely that the datasheet sequence is not followed by USBCore.cpp, specifically this:

RXOUTI shall always be cleared before clearing FIFOCON

The way USB_Recv is written I don't think ReleaseRX will ever be called twice in a row. And that is exactly what gets me out of the hangup situation.

[sandeepmistry]

@kervinck would you be able to open a new pull request for your suggested changes?

[kervinck]

That would take me one or two weeks of study, cleanup and testing. Sorry.

[CLAassistant]

Thank you for your submission! We really appreciate it. Like many open source projects, we ask that you sign our Contributor License Agreement before we can accept your contribution.
_{You have signed the CLA already but the status is still pending? Let us recheck it.}

[obra]

I've just tested this patch against a longstanding issue I've had where, only on macOS, sending 384 or more bytes to the AVR USB Serial device from the host causes USB Serial to lock up.

...and it appears to solve the issue.

Without this patch, sending the following string (or any string of equal or greater length) from the Classic Arduino IDE's serial console will cause the USB Serial connection to stop reading input, but only on macOS.

0123456789abcdef0123456789abcdef0123456789abcdef0123456789abcdef0123456789abcdef0123456789abcdef0123456789abcdef0123456789abcdef0123456789abcdef0123456789abcdef0123456789abcdef0123456789abcdef0123456789abcdef0123456789abcdef0123456789abcdef0123456789abcdef0123456789abcdef0123456789abcdef0123456789abcdef0123456789abcdef0123456789abcdef0123456789abcdef0123456789abcdef01234567890abcde

The trivial repro sketch for this failure is

void setup() {Serial.begin(9600);}
void loop()  {if (Serial.available()) {Serial.write(Serial.read());}}

@cmaglie Apologies for tagging you directly, but who's the right code owner to see about getting this PR either merged or rejected?

[tlyu]

Confirmed the observation by @kervinck that this fix is incomplete. If the application calls USB_Recv before USB_Available when a ZLP has been read, it will stop reading new input until it calls USB_Available. (The USB hardware might accept and ACK an additional packet, due to double buffering, but the application can't read it.)

This additional patch appears to fix it in our use case (diff is against the keyboardio fork of the core):

--- a/avr/cores/keyboardio/USBCore.cpp
+++ b/avr/cores/keyboardio/USBCore.cpp
@@ -245,6 +245,10 @@ int USB_Recv(u8 ep, void* d, int len)

        LockEP lock(ep);
        u8 n = FifoByteCount();
+       if (!n && HasOUT()) {
+               ReleaseRX(); // handle ZLP
+               n = FifoByteCount();
+       }
        len = min(n,len);
        n = len;
        u8* dst = (u8*)d;

I also agree that the datasheet says to always clear RXOUTI before FIFOCON, but this doesn't seem to cause problems in practice. It also doesn't seem to be the problem here. The AVR core doesn't use interrupts for non-control endpoints, so it's less important to clear RXOUTI as soon as possible. (FIFOCON seems to be what controls whether the USB hardware will send NAK to the host.)

[tlyu]

This probably needs to be moved to https://github.com/arduino/ArduinoCore-avr