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

odd-ball delays? #50

Open
kiwichrish opened this issue Aug 16, 2017 · 1 comment
Open

odd-ball delays? #50

kiwichrish opened this issue Aug 16, 2017 · 1 comment

Comments

@kiwichrish
Copy link

kiwichrish commented Aug 16, 2017

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
Copy link

flodmotorgrodan commented Nov 13, 2017

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

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

2 participants