espressif / arduino-esp32

Arduino core for the ESP32
GNU Lesser General Public License v2.1
13.68k stars 7.42k forks source link

WifiClient flush() should send all data instead of clearing input (as defined in Arduino specification) #9379

Open imwhocodes opened 8 months ago

imwhocodes commented 8 months ago

Related area

WiFi / Ethernet

Hardware specification

All ESP32 versions

Is your feature request related to a problem?

Arduino describe Stream::flush() as "sending all outgoing outstanding data" https://www.arduino.cc/reference/en/language/functions/communication/stream/streamflush/

Stream is the base interface class for all IO implementations (WiFi, HarwareSerial, USBCDC, SPI, Wire)

So also in Serial arduino interface behaviour is maintained: https://www.arduino.cc/reference/en/language/functions/communication/serial/flush/

HardwareSerial here in Esp32 is conformant too:

https://github.com/espressif/arduino-esp32/blob/0a26a8c4e3f89d4c436078f3b360c7e5e09dbc35/cores/esp32/HardwareSerial.cpp#L472-L475

https://github.com/espressif/arduino-esp32/blob/0a26a8c4e3f89d4c436078f3b360c7e5e09dbc35/cores/esp32/esp32-hal-uart.c#L753-L766

But then WiFiClient::flush() is still maintaining old arduino behaviour of clearing RX input:

https://github.com/espressif/arduino-esp32/blob/0a26a8c4e3f89d4c436078f3b360c7e5e09dbc35/libraries/WiFi/src/WiFiClient.cpp#L162-L167

This is not compliant with the Stream interface

Describe the solution you'd like

WiFiClient::flush() to actually send al outstanding data to the client to the other end without clearing the input buffer

Describe alternatives you've considered

Overloading WiFiClient class with a custom flush function

Additional context

Yes is true that original WiFiClient::flush() from arduino define a different behaviour https://www.arduino.cc/reference/en/libraries/wifi/client.flush/ But it is also true that that documentation is both obsolete and it don't even adhere to the Stream::flush() interface from which it inherit https://www.arduino.cc/reference/en/language/functions/communication/stream/streamflush/

I have checked existing list of Feature requests and the Contribution Guide

JAndrassy commented 8 months ago

it is Print:flush

imwhocodes commented 8 months ago

it is Print:flush

It is not mentioned anymore in "original" Arduino: https://www.arduino.cc/reference/en/language/functions/communication/print/

JAndrassy commented 8 months ago

It is not mentioned anymore in "original" Arduino:

still not mentioned. but it is in Print and it should be there as it is an output related function

imwhocodes commented 8 months ago

It is not mentioned anymore in "original" Arduino:

still not mentioned. but it is in Print and it should be there as it is an output related function

It is mentioned in Stream documentation

Also given both documentation, implementation in other Arduino-esp32 (derived from Stream) and common sense (from the meaning in cpp std to python and any existing library in between) it should send all data

One example where this is broken is if you have a class or function that take * Stream as input and relay on it polymorphic implementation, you can't know what is going to happen