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

Inconsitent meaning of WiFiUDP::flush() across cores. #8312

Open
everslick opened this issue Sep 15, 2021 · 8 comments
Open

Inconsitent meaning of WiFiUDP::flush() across cores. #8312

everslick opened this issue Sep 15, 2021 · 8 comments

Comments

@everslick
Copy link
Contributor

AFAICS ESP8266 is the only Arduino platform where WiFiUDP::flush() behaves like what one expects on a POSIX system. Meaning sending/writing any pending data. But Arduino chose to use it as a means to discard buffers. And that's how it is implemented on most (all?) cores AND stated in the documentation. https://github.com/esp8266/Arduino/blob/master/cores/esp8266/Udp.h#L79

On ESP8266 WiFiUDP::flush() just calls endPacket() which is not really useful, while a function to drain the receive buffer IS. At least IMHO.

I propose to change the flush() method according to the Arduino standard.

@d-a-v
Copy link
Collaborator

d-a-v commented Sep 15, 2021

But Arduino chose to use it as a means to discard buffers

I don't fully agree:

https://www.arduino.cc/en/Reference/ClientFlush
https://www.arduino.cc/reference/en/language/functions/communication/serial/flush/

and in Stream: https://www.arduino.cc/reference/en/language/functions/communication/stream/streamflush/

And that's how it is implemented on most (all?) cores

It was so before we fixed it (if I remember correctly)

AND stated in the documentation. https://github.com/esp8266/Arduino/blob/master/cores/esp8266/Udp.h#L79

virtual void flush() =0; // Finish reading the current packet

This comment indeed does not reflect what's under the hood and has had taken its source from
https://github.com/arduino-libraries/Ethernet/blob/7c32bbe146bbe762093e4f021c3b12d7bf8d1629/src/Ethernet.h#L202

Serial and Ethernet do not mention that buffer is cleared. Stream's flush() is similar to the two previous ones plus "clear the buffer" but it does not say which one is cleared (outbound or inbound, API should be crystal clear).

Beside, official Ethernet implementation does not seem to discard anything.

Do you have links to other implementations not behaving similarly as in this core ?

Should we update this unclear comment ?

@JAndrassy
Copy link
Contributor

virtual void flush() is in Print class in Arduino-API. Print handles output. the decision was made in Arduino 1.0 in 2011 https://www.arduino.cc/en/Main/ReleaseNotes

https://www.arduino.cc/reference/en/language/functions/communication/serial/flush/

@d-a-v
Copy link
Collaborator

d-a-v commented Sep 15, 2021

So the current behavior which is waiting for the output buffer (Print) made into output packets (implying that the output buffer becomes empty at the end of the call) is compliant, no ?

@everslick
Copy link
Contributor Author

Do you have links to other implementations not behaving similarly as in this core ?

Well, first thing that comes to mind is the "big brother" ESP32:

https://github.com/espressif/arduino-esp32/blob/master/libraries/WiFi/src/WiFiUdp.cpp#L268-L273

But maybe it's ESP32 that is not compliant.

Probably the general meaning of flush() changed at one point of time, but I'm pretty sure to have seen it used as "clearing buffers" plenty of times.

So, I agree, the code comment should be changed, and the ESP32 implementation.

@everslick
Copy link
Contributor Author

On the other hand:

https://www.arduino.cc/en/Reference/WiFiUDPFlush

The API documentation is clear as it says:

Discard any bytes that have been written to the client but not yet read.

@d-a-v
Copy link
Collaborator

d-a-v commented Sep 15, 2021

Well I missed that one. So Ethernet UDP and WiFi UDP do not behave similarly ?

@everslick
Copy link
Contributor Author

So Ethernet UDP and WiFi UDP do not behave similarly ?

right. :-(

@JAndrassy
Copy link
Contributor

JAndrassy commented Sep 15, 2021

only the doc is wrong. see the release notes I linked in my comment. there is the complete history. search "flush"

https://github.com/arduino-libraries/Ethernet/blob/master/src/EthernetUdp.cpp#L176-L179

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

3 participants