arduino-libraries / Ethernet

Ethernet Library for Arduino
http://arduino.cc/
259 stars 264 forks source link

EthernetClient flush() is pre-1.0 input discard #27

Open agdl opened 8 years ago

agdl commented 8 years ago

From @PaulStoffregen on September 15, 2013 15:13

EthernetClient.cpp in 1.5.x has old pre-1.0 flush() function that discards input. Arduino 1.0 changed flush() to complete transmission.

https://github.com/arduino/Arduino/blob/ide-1.5.x/libraries/Ethernet/src/EthernetClient.cpp#L122

void EthernetClient::flush() {
  while (available())
    read();
}

Copied from original issue: arduino/Arduino#1576

agdl commented 8 years ago

From @cmaglie on September 25, 2013 12:33

Fixed the wrong behaviour of dropping buffered chars, but I'm not sure that the flush waits for real the end of transmission. I should write a test for it.

PaulStoffregen commented 6 years ago

Fix for this here: https://github.com/PaulStoffregen/Ethernet/commit/17abf2b4df37000d784c811dcf20073dec65269a

(close this issue when this code become version 2.0)

PaulStoffregen commented 6 years ago

While this is fixed, I'm leaving it open since there are still several places in DHCP and DNS referencing the old function. They're labeled with FIXME comments.

nyfrk commented 5 years ago

@PaulStoffregen Can you make it so that flush() is respecting the timeout? When flush() is invoked when the client lost the connection it is blocking pretty long until the status eventually changes.

Maybe we could change it to something like this:

void EthernetClient::flush()
{
    unsigned long start = 0;
    auto prevFreesize = W5100.SSIZE;

    while (sockindex < MAX_SOCK_NUM) {
        uint8_t stat = Ethernet.socketStatus(sockindex);
        if (stat != SnSR::ESTABLISHED && stat != SnSR::CLOSE_WAIT) return;
        if (Ethernet.socketSendAvailable(sockindex) >= W5100.SSIZE) return;

        auto freesize = Ethernet.socketSendAvailable(sockindex);
        if (freesize >= W5100.SSIZE) return;
        if (freesize != prevFreesize) {
            prevFreesize = freesize;
            start = millis();
        } else {
            if (millis()-start >= _timeout) {
                Ethernet.socketClose(sockindex);
                sockindex = MAX_SOCK_NUM;
                //stop(); // calling stop would start another timeout period
                return;
            }
        }
        delay(1);
    }
}

This will reset the timeout period whenever a byte has been sent. If no byte has been sent for more than _timeout ms the socket is closed.

PaulStoffregen commented 5 years ago

I am currently working on a hardware project. Not planning to do anything with Ethernet until late-2019, or maybe even 2020 or later. Maybe someone else will take this up sooner. Eventually I'll return to Ethernet for many pending requests, but I've learned the hard way not to do something "quick & easy" on any library when I truly do not have the time & attention for proper testing.