Fix 10 second delay in WiFiClient read methods when no data is available to read

[mgcookson]

Problem Summary

Prior to v1.3.0 of core, invoking either WiFiClient::read() or WiFiClient::read(uint8_t *buf, size_t size) when there was no data available to read would result in those methods returning immediately with expected return codes (-1 for the former and 0 for the latter).

As of v1.3.0, both 'read' methods now unexpectedly delay for 10 seconds in the 'no data available' scenario before returning the expected return codes. This change in behavior was introduced via PR #349.

Proposed Fix

This PR contains a proposed fix that allows the state machine used to read (WiFi) modem responses to immediately transition to the next state when the indicated data length in the response is zero, instead of getting stuck in the 'read data' state.

Research Notes

Example affected code excerpts with results

Code invoking WiFiClient::read()

byte buf[512];
Serial.print("starting read at millis=");Serial.println(millis());
int r = _client.read();
Serial.print("finished read at millis=");Serial.print(millis());
Serial.print(" with result=");Serial.println(r); 

Results when run under core 1.2.2

starting read at millis=10334
finished read at millis=10342 with result=-1

Results when run under core 1.4.1

starting read at millis=10614
finished read at millis=20619 with result=-1

Code invoking WiFiClient::read(uint8_t *buf, size_t size)

byte buf[512];
Serial.print("starting read at millis=");Serial.println(millis());
int r = _client.read(buf, 1);
Serial.print("finished read at millis=");Serial.print(millis());
Serial.print(" with result=");Serial.println(r); 

Results when run under core 1.2.2

starting read at millis=10192
finished read at millis=10200 with result=0

Results when run under core 1.4.1

starting read at millis=10189
finished read at millis=20194 with result=0

Modem.cpp debug output before and after proposed fix

Enabling modem debug output using modem.debug(Serial, 3) yields the following results:

Results when run under core 1.4.1 prior to this fix

REQUEST: AT+CLIENTRECEIVE=0,1023

RAW: + State 0
C State 1
L State 1
I State 1
E State 1
N State 1
T State 1
R State 1
E State 1
C State 1
E State 1
I State 1
V State 1
E State 1
: State 1
<SP> State 2
0 State 2
| State 2
O State 3
K State 3
<CR> State 3
<LF> State 3
Final State 3 res 3
<-RAW END
   ANSWER: OK

   Result: TIMEOUT

Results when run under core 1.4.1 after this fix

REQUEST: AT+CLIENTRECEIVE=0,1023

RAW: + State 0
C State 1
L State 1
I State 1
E State 1
N State 1
T State 1
R State 1
E State 1
C State 1
E State 1
I State 1
V State 1
E State 1
: State 1
<SP> State 2
0 State 2
| State 2
O State 5
K State 8
<CR> State 8
<LF> State 8
Final State 9 res 0
<-RAW END
   ANSWER:
   Result: OK

[CLAassistant]

All committers have signed the CLA.

[andreagilardoni]

I would remove the state transition at line 297 and keep the state change next to each other. This will improve the readability.

Suggested change

	if (sized_read_size == 0) {
	state = at_parse_state_t::Res;
	}
	if (sized_read_size != 0) {
	state = at_parse_state_t::Sized;
	} else {
	state = at_parse_state_t::Res;
	}

[mgcookson]

Thanks for the good suggestion - I have added another commit to address.

[andreagilardoni]

@mgcookson Thanks for your contribution, it is really appreciated!