ekstrand / ESP8266wifi

ESP8266 Arduino library with built in reconnect functionality
MIT License
450 stars 235 forks source link

odd-ball delays? #50

Open kiwichrish opened 6 years ago

kiwichrish commented 6 years ago

Hi-ho, Firstly thanks for your work on this library, I was having debugging issues with a library of my own for a small project and couldn't see the wood for the trees!

But... What's with the odd-ball delays? You've called it niceDelay() and I can understand using millis() if you're releasing control back to loop() for multi-tasking, but, well... I don't get it. :-)

flodmotorgrodan commented 6 years ago

Reading without cheking .available()?

Yes an impressing library!

I was also was wondering about the delays.

The reason could be it fixed a problem with using readChar() in listenForIncomingMessage() where there is no check before reading the input stream.

I removed the delays completely and made a readCharW(ms)

char SerialESP8266wifi::readCharW(int timeout) {
    if (_serialIn->available()) {
        return readChar();
    }

    unsigned long stop = millis() + timeout;
    do {
        if (_serialIn->available()) {
            return readChar();
        }
    } while (millis() < stop);
    return 0;
} 

..and used that wherever i found readChar() being used after a successful readCommand(). readCommand() finds a keyword, but you have no clue if there are more characters in the input buffer and should not read without checking .available() first.

Patrt 2 - What about integrating a Stream?

The accumulation of data in the send command was inventive. It got me thinking of using a Stream class as input instead. Then you can have the Stream point to the msgOut buffer and when you flush it some underlying code in the stream calls the send() method when stream tends to be near full as well.

Maybe some skilled developer can redesign the WiFi class to inherit from Stream or maybe expose a Stream property.

Here my send() function test:

//Something like this?..
bool SerialESP8266wifi::send(uint8_t channel, MemoryStream& stream){
    watchdogConnect();
    writeCommand(CIPSEND);
    _serialOut -> print(channel);
    writeCommand(COMMA);
    _serialOut -> println(stream.available());
    byte prompt = readCommand(1000, PROMPT, ERROR);
    if (prompt != 2) {
        while (stream.available())
        {
            _serialOut->write(stream.read());       
        }
        byte sendStatus = readCommand(5000, SEND_OK, BUSY);
        if (sendStatus == 1) {
            //msgOut[0] = '\0';
            if(channel == SERVER_CHAN)
                flags.connectedToServer = true;
            return true;
        }
    }
    else
    {
        //ERROR Assume not connected
        flags.connectedToServer = false;
    }

#if WIFI_FOOTPRINT == 2

    if(channel == SERVER_CHAN)
        flags.connectedToServer = false;
    else
        _connections[channel-0x30].connected = false;

#endif