WiFiUDP not reading last byte

[henrikkunzelmann]

The WiFiUDP implementation is not reading the last byte in WiFiUDP::read().

Example (code is working fine on ESP8266 and ESP32 with one byte extra padding when sending packets to it):

int size = udp->parsePacket();
udp->read(buffer, size);

Size is for example 17 but read() reads only 16 bytes. The bug is in the cbuf class: cbuf::write() copies 16 bytes because cbuf::room() returns only 16 bytes (and not the correct length of 17 bytes).

The bug in the cbuf class should be fixed, but also a new method should be added to WiFiUDP:

int WiFiUDP::parsePacket(char* buffer, size_t size) (or some other signature).

parsePacket copies the packet content in the user provided buffer and returning the size of the packet. With this approach there is no need to create an internal cbuf instance and saving some memory overall.

[me-no-dev]

I agree that cbuf might need to be looked at, though that same cbuf is running on esp8266 also and is used by many libs, so it's strange to me to not be working.
int size = udp->parsePacket(); is used by many(all) libraries that use UDP and that is the original signature from Arduino. I'm not sure that adding another method will do any good.

[henrikkunzelmann]

I've patched the "bug" in cbuf::room() by using:

size_t cbuf::room() const
{
    if(_end >= _begin) {
        return _size - (_end - _begin);
    }
    return _begin - _end - 1;
}

room() will then return the correct free size (e.g. 17 bytes), but then available() returns 0, because _begin is the same value as _end (wrap_if_bufend).

Proposed fix for this problem is to actually patch WiFiUDP::parsePacket():

rx_buffer = new cbuf(len);

should be

rx_buffer = new cbuf(len + 1);

The usage of cbuf should be checked (is cbuf actually working correctly and only used "wrong" in WiFiUDP?).

I agree that cbuf might need to be looked at, though that same cbuf is running on esp8266 also and is used by many libs, so it's strange to me to not be working.

For me it's actually strange that the one-off bug was not catched by anyone else here.

int size = udp->parsePacket(); is used by many(all) libraries that use UDP and that is the original signature from Arduino. I'm not sure that adding another method will do any good.

I can kinda agreee that overloading the method maybe confusing to other users (e.g. using parsePacket(buf, sizeof(buf)) and then trying to use WiFiUDP::read() or so). I will provide a pull request with my approach.

[me-no-dev]

done :)