why does Esp32 client.read() often return -1 while there is still data to be read?

[tonychunxizhu]

Hardware:

Board: ESP32 Dev Module(ESP32-WROOM-32)
Core Installation/update date:
IDE name: Arduino IDE 1.8.13
Flash Frequency: 80Mhz
PSRAM enabled: Disabled
Upload Speed: 921600
Computer OS: Windows 10

Description:

I am trying to test WiFi data transfer between cell phone and Esp32, the routine is:
1, filename
2, file size
3, file data
when ESP32 reads files via WiFi, even there is still data in, client.read() often return -1, I have to add other conditions to check reading finished or not.

My questions is why there are so many failed read, any ideas are highly appreciated.

Sketch: (leave the backquotes for [code formatting](https://help.github.com/articles/creating-and-highlighting-code-

#include <SPI.h>
#include <WiFi.h>
#include <WiFiClient.h>
#include <WiFiAP.h>
#define

// Set these to your desired credentials.
const char* ssid = "TestAP";
const char* password = "12345678";

WiFiServer server(6666);

int i,j;
char toClient[128]={0};

byte cmd;
unsigned long pretime=0;
unsigned long curtime=0;

void setup()
{
i=0;
Serial.begin(115200);
Serial.println("begin...");

// You can remove the password parameter if you want the AP to be open.
WiFi.softAP(ssid, password);
IPAddress myIP = WiFi.softAPIP();
Serial.print("AP IP address: ");
Serial.println(myIP);
server.begin();

Serial.println("Server started");

}

// the loop function runs over and over again until power down or reset
void loop()
{

WiFiClient client = server.available();   // listen for incoming clients

if(client)                                // if you get a client,
{
    
    Serial.println("New Client.");           // print a message out the serial port
    Serial.println(client.remoteIP().toString());


    while(client.connected())               // loop while the client's connected
    {
        while(client.available()>0)                // if there's bytes to read from the client,
        {
            char c = client.read();             // read a byte, then
            if(UPLOADFILE ==c){

              pretime=millis();
              uint8_t filename[32]={0};
              uint8_t bFilesize[8];
              long filesize;
              int segment=0;
              int remainder=0;
              uint8_t data[512];
              int len=0;
              int totallen=0;
              
              Serial.println("\download file\n");
              delay(50);
              len=client.read(filename,32);                  
              delay(50);
              len=client.read(bFilesize,8);
              filesize=BytesToLong(bFilesize);
              segment=(int)filesize/512;
              delay(50);

              i=0;  //succeed times
              j=0; //fail times
             ////////////////////////////////////////////////////////////////////
             //problem occures here, to many "-1" return value
            // test result Serial output  is in this comment section
           // download file
           //file name test24.94m.mp3,size 24941639, total read 24941639, segment 48714, succeed 49725 times, failed 278348 times
           //time splashed 32981 ms, speed 105115 Bps
           //Client Disconnected.
            // if there were no read problem, I should read 48,715 times and finish.
           //But it read 328,073 times, including 278,348 falied times, wasted too much time
              while(((len=client.read(data,512))!=-1) || (totallen<filesize))
              {
                if(len>-1) {
                  totallen+=len; 
                  i++;
                }
                else{
                  j++;
                }
                               
              }
             ///loop read end , too many times read fail//////////////////////////////////////////////////////////////////
              
              sprintf(toClient, "\nfile name %s,size %d, total read %d, segment %d, succeed %d times, failed %d times\n",filename,filesize,totallen,segment,i,j);
              Serial.write(toClient);
                
              curtime=millis();

              sprintf(toClient, "time splashed %d ms, speed %d Bps\n", curtime-pretime, filesize*1000/(curtime-pretime));
              Serial.write(toClient);
              
              client.write(RETSUCCESS);                  
            }
            else
            {
              Serial.write("Unknow command\n");
            }
        }
    }
           
            
    // close the connection:
    client.stop();
    Serial.println("Client Disconnected.");
}

}

[tonychunxizhu]

the Serial output is

download file
file name test24.94m.mp3,size 24941639, total read 24941639, segment 48714, succeed 49725 times, failed 278348 times
time splashed 32981 ms, speed 105115 Bps
Client Disconnected.

if there were no reading problems, it should read only 48,715 times and finish.
But it read 328,073 times, including 278,348 failed times, which wasted too much time.

Thanks a lot.

[MHotchin]

I just ran into this. The problem is in "arduino-esp32/libraries/WiFi/src/WiFiClient.cpp".

If the client has run out of data that has been received, it will return an error, instead of returning 0 (indicating no data available right now).

Starting line 106:

    int read(uint8_t * dst, size_t len){
        if(!dst || !len || (_pos == _fill && !fillBuffer())){
            return -1;

Change to:

    int read(uint8_t * dst, size_t len){
        if(!dst || !len || (_pos == _fill && !fillBuffer())){
            return _failed ? -1: 0;

I'll report this as a specific issue, but I believe this is the root of your problem.

[MHotchin]

Oh, and as a workaround - try changing from read() to readBytes().

[tonychunxizhu]

Hi, @MHotchin thank you for your help.

I changed WiFiClient.cpp line 108 from "return -1;" to “return _failed ? -1: 0;”

then the program stuck in the while loop. When I changed read() to readBytes(), it also stuck in the while loop, no matter whether I modified WiFiClient.cpp or kept it original version.
Any other ideas?

Thanks again.

              filesize=BytesToLong(bFilesize);
              i=0;  //succeed times
              j=0;  //fail times
                               
              while(((len=client.read(data,512))!=-1) || (totallen<filesize))
              {
              //stuck here
               ...
              }
              //never came out the while loop above
             sprintf(toClient, "\nfile name %s,size %d, total read %d, succeed %d times, failed %d times\n",filename,filesize,totallen,i,j);
             ...

I just ran into this. The problem is in "arduino-esp32/libraries/WiFi/src/WiFiClient.cpp".

If the client has run out of data that has been received, it will return an error, instead of returning 0 (indicating no data available right now).

Starting line 106:

    int read(uint8_t * dst, size_t len){
        if(!dst || !len || (_pos == _fill && !fillBuffer())){
            return -1;

Change to:

    int read(uint8_t * dst, size_t len){
        if(!dst || !len || (_pos == _fill && !fillBuffer())){
            return _failed ? -1: 0;

I'll report this as a specific issue, but I believe this is the root of your problem.

[MHotchin]

With this fix, if you do get -1 (or any negative value) as a return val, that's an actual error - you're done. So, I think you need to && the conditions, not ||.

Also, if you get zero bytes, you may need to delay() to allow other ESP32 code to run, otherwise the loop will hog the processor. Not sure if that's relevant to your problem here, though.

while(((len=client.read(data,512)) >=0 ) && (totallen<filesize))
{
   if (len == 0) {
      delay(1);
      continue;
   }
   ...
}

[lbernstone]

@MHotchin If this is a solid fix, can you submit it as a PR?

[tonychunxizhu]

@MHotchin Thanks you.
You are right.
The problem is fixed.
However, delay(1) could significantly reduce the zero-reading times, but the total time of reading almost keeps same.
I need to figure out why the client sending speed is slow. But it's not Esp32's problem.

Thanks again.

With this fix, if you do get -1 (or any negative value) as a return val, that's an actual error - you're done. So, I think you need to && the conditions, not ||.

Also, if you get zero bytes, you may need to delay() to allow other ESP32 code to run, otherwise the loop will hog the processor. Not sure if that's relevant to your problem here, though.

while(((len=client.read(data,512)) >=0 ) && (totallen<filesize))
{
   if (len == 0) {
      delay(1);
      continue;
   }
   ...
}

[MHotchin]

@MHotchin If this is a solid fix, can you submit it as a PR?

Submitted as PR #4448

[stale]

[STALE_SET] This issue has been automatically marked as stale because it has not had recent activity. It will be closed in 14 days if no further activity occurs. Thank you for your contributions.

[stale]

[STALE_DEL] This stale issue has been automatically closed. Thank you for your contributions.