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

USB Serial on 32U4 stops incrementing Serial.available() if serial buffer has ≥2 bytes in it and more are received #516

Open
AnasMalas opened this issue Dec 25, 2022 · 4 comments

Comments

@AnasMalas
Copy link

After some investigation, I was able to narrow down my code to the minimum that reproduces the bug, then I added some code that helps me locate where it is. The problem appears to be that if more than one byte is sent, Serial.available() does not increment.

I did testing on a 32U4 where I could reproduce this bug, but I could not get this bug to show up on Uno with 9600, 115200, or 2,000,000 baud, so I think it only affects USB serial.

To reproduce the bug, make sure that Serial.available returns more than 1, then send data. It will remain at the previous value despite the data being in the buffer. If no bytes or one byte are in buffer, Serial.avaible works correctly upon sending data.

Here's my example code, which shows the changes in Serial.avaiable() and prints from the serial buffer even if Serial.available() is stuck:

void setup() {
    Serial.begin(0); //No baudrate for USB serial
}

void loop() {
    static uint8_t prevInBuffer;
    static uint32_t whatsTheIssue;

    if (Serial.available() != prevInBuffer){ //Print a change in buffer, so we dont spam the monitor
        Serial.print(F("Buffer changed from "));
        Serial.print(prevInBuffer);
        Serial.print(F(" bytes to "));
        Serial.print(Serial.available());
        Serial.println(F(" bytes."));
        prevInBuffer = Serial.available();
    }
    //If a couple of characters are sent, Serial.available never updates, this condition is never 
    //satisfied unless all characters are sent together, or one character is sent while buffer is 
    //empty followed by the rest together.
    else if (Serial.available() >= 4){ 
        while (Serial.available()){
            Serial.print(char(Serial.read()));
        }
        Serial.println();
    }

    if (millis() - whatsTheIssue > 20000){ //is the issue with the buffer or Serial.available()?
        whatsTheIssue = millis();
        Serial.print(F("10 buffer characters: "));

        uint8_t i = 0;
        for (i=0;i<10;i++){
            Serial.print(char(Serial.read()));
        }
        Serial.println();
    }
}
@matthijskooijman
Copy link
Collaborator

This issue triggered my curiosity, so I decided to quickly investigate this (but I won't have time for further followups, I expect).

Turns out that, AFAICS, the behavior you are seeing is essentially unfixable with the current approach where data is only stored in dedicated USB buffer banks (DPRAM). It could be fixed by copying data from the USB buffers into regular RAM, but that would introduce an extra copy step and of course raise RAM usage, so I think that's not really an option. If you do really need to know the number of bytes pending in advance, you can always introduce such a buffer yourself and copy bytes into it while read() returns valid data.

I think this makes this issue essentially unfixable. Maybe it would be good to document this behavior somewhere, if not already done. @per1234, maybe you have ideas about this?

Basically, what currently happens:

  • When a USB packet is received, its data (up to 64 bytes) is read into one bank of DPRAM, where the microcontroller can read it.
  • While the microcontroller processes the data, if a second USB packet is received, its data is read into the second bank of DPRAM. After that, the device starts returning NAKs, preventing a third packet from being sent.
  • Once all data is read from the first bank, it is released and the second bank becomes the current one (with more data ready to read if a second packet was received in the meanwhile).
  • When you call available(), this returns the number of bytes left to read in the current bank, omitting any data in the second bank (or queued on the USB host).

Note that I could not find a way to ask the hardware how much data is pending in the second bank. But also note that this would not really help: if the data is split over more than 2 packets, there could be more data pending in the USB host that you'd never know about.

Here's the path from available() to the hardware register that has the remaining bytes in the hardware fifo:

int Serial_::available(void)
{
if (peek_buffer >= 0) {
return 1 + USB_Available(CDC_RX);
}
return USB_Available(CDC_RX);
}

u8 USB_Available(u8 ep)
{
LockEP lock(ep);
return FifoByteCount();
}

static inline u8 FifoByteCount()
{
return UEBCLX;
}

What is also interesting is that the byte count is read from the UEBCLX register only, while there are three more bits in the UEBCHX register, meaning the available count could also be wrong if sending > 255 bytes. Given there are only 2 64-byte banks that can hold data, that should be ok. At first glance it seems a waste to use only 64-byte banks (since there is 832 bytes of dedicated USB RAM available), but it turns out that most endpoints are limited to 64-bytes, only endpoint 1 supports up to 256 bytes (weirdly enough the EPSIZE register allows configuring 512-byte endpoints?). I guess the RX endpoint could be configured to be 256 bytes, leaving TX at 64 (each has 2 banks, so 2x256+2x64=640, leaving 192 bytes for the control endpoint and other USB functions), but maybe there is some other limitation, or this was just not implemented to keep things simple.

The BYCT field in the UEBCLX/UEBCHX registers is a bit weird. The datasheet section about the register suggest it just keeps a total count of available bytes (across all banks), but the "OUT endpoint management" section suggests that you need to read this many bytes before releasing the bank, implying that it contains the number of bytes in the current bank only.

Then here's the path taken by read(), which also shows clearing the current bank when it is fully read:

int Serial_::read(void)
{
if (peek_buffer >= 0) {
int c = peek_buffer;
peek_buffer = -1;
return c;
}
return USB_Recv(CDC_RX);
}

int USB_Recv(u8 ep, void* d, int len)
{
if (!_usbConfiguration || len < 0)
return -1;
LockEP lock(ep);
u8 n = FifoByteCount();
len = min(n,len);
n = len;
u8* dst = (u8*)d;
while (n--)
*dst++ = Recv8();
if (len && !FifoByteCount()) // release empty buffer
ReleaseRX();
return len;
}
// Recv 1 byte if ready
int USB_Recv(u8 ep)
{
u8 c;
if (USB_Recv(ep,&c,1) != 1)
return -1;
return c;
}

static inline void ReleaseRX()
{
UEINTX = 0x6B; // FIFOCON=0 NAKINI=1 RWAL=1 NAKOUTI=0 RXSTPI=1 RXOUTI=0 STALLEDI=1 TXINI=1
}

And this shows the hardware is configured for 2 banks of 64 bytes each:

UECFG1X = EP_DOUBLE_64;

#ifndef USB_EP_SIZE
#define USB_EP_SIZE 64
#endif

@AnasMalas
Copy link
Author

AnasMalas commented Dec 25, 2022

@matthijskooijman
Very interesting dive! Ive read it multiple times, and it isn't immediately apparent to me why you can send one character, then send a bunch more and available() does return the correct value when you send the second packet (but not subsequently)

Coincidentally, I was looking at the buffer size for FTDI USB to RS232 chips, and found that FTDI also recommend 64 byte packets (62 of which are data). FTDI urges application developers not to send charcater by character, as I was doing with my 32U4 when I found this bug. I dont know if it's related, but it may be a USB CDC thing.

This may be more of an undocumented limitation, but perhaps there is a way to get it to function like other boards

@matthijskooijman
Copy link
Collaborator

Very interesting dive! Ive read it multiple times, and it isn't immediately apparent to me why you can send one character, then send a bunch more and available() does return the correct value when you send the second packet (but not subsequently)

Hm, I had missed that observation and also cannot immediately explain this. I thought maybe the peek buffer is involved (which could store one byte in RAM and keep the rest (second packet) in the USB fifo), but it seems that is only ever filled if you call peek(), which your sketch does not.

Or maybe the fifo size returned by the BYCT register does return the size of both banks together? If so, then sending a bunch of characters at once, wait a bit and send some more character should also show the correct value for available(), but your comments imply that it only happens when the first write is 1 byte?

@AnasMalas
Copy link
Author

Yes @matthijskooijman, if the buffer already has 0 or 1 bytes in it, you can send more bytes together and they will enter the buffer and be shown by available(). After that, you can send more bytes and they will go into the buffer (as confirmed by reading then out), but available() won't update

@AnasMalas AnasMalas changed the title USB Serial on 32U4 stops updating Serial.available() despite more characters getting received USB Serial on 32U4 stops incrementing Serial.available() if serial buffer has ≥2 bytes in it and more are received Dec 31, 2022
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