Listening Server Looses Data

[FrancMunoz]

I've been working hard in MKRGSM these days and found another issue, this time related to listening TCP sockets. Using as a base your server/client examples, I've created a complex system that sends and transmits data through TCP/IP.

All works fine the first time on listening socket, but the second or third time listening server looses chars (communication between MKR and Modem), that follows an infinite loop caused by the while(ready()==0); statements. I've changed these statements following the same strategy used in Moodem::waitForResponse function, but this din't solved the issue totally: the infinite loop doesn't occur but received data is incomplete so I my parser fails.

Finally, After spending a lot of time thinking that was somenthing not optimized, and optimizing code. I opted to downgrade baudrate in Modem.cpp to 9600. Then all magically worked. So in Modem.cpp changed last line:

ModemClass MODEM(SerialGSM, 9600, GSM_RESETN, GSM_DTR);

Please consider downgrade baudrate to lower speed, 115200 it's pretty and the examples work fine but then just adding few lines... all turns dark.

I would like to contribute this project, is it possible?

Hope it helps! And thank you for your great job!

[akash73]

Hi @FrancMunoz, yes we are happy if you want to contribute.
just open a PR and we will test it.
can you please share a sketch to test?

thanks for you time

[sandeepmistry]

Hi @FrancMunoz,

Thank you for purchasing a board, and letting us know about this issue.

Could you please share a sketch (preferably minimal) that reproduces the issues and with specific steps if it's not clear in the sketch.

I suspect the SAMD21's software UART buffer is overflowing, maybe because the RTS pin is not getting set early enough. The threshold can be adjust here: https://github.com/arduino/ArduinoCore-samd/blob/master/cores/arduino/Uart.cpp#L25

[FrancMunoz]

Hi @akash73 and @sandeepmistry ,

Will be a pleaseure to contribute I will do a PR with my proposals.

I'm preparing the sketch, I will prepare the example to make a PR as a base of the issue.

I think you are on the right way @sandeepmistry , the buffer is overflowing, that explains why downgrading baudrate works. The lost data ever was 2 or three bytes if it helps.

Let me prepare the sketches.

Thank you!!!

[sandeepmistry]

Ok, so bumping #define RTS_RX_THRESHOLD 10 to 16 in Uart.cpp might be a way to fix it then.

In any case, example sketch to reproduce would be awesome :)

[FrancMunoz]

Hi @sandeepmistry!

I've created a PR with a server example, just add a delay(50); in loop and it will fail at second or third try. I've tested #define RTS_RX_THRESHOLD 16 in Uart.cpp but didn't fixed. Downgrading baud rate stills working.

Tomorrow will do a PR for a buffered client example that is faster than reading char by char.

Thank you!

[sandeepmistry]

Hi @FrancMunoz,

Thanks for creating pull request #12, could you please clarify exactly where the delay(50); should be placed to reproduce the issue. I tried right after loop() { and things seem fine.

There was also one modification I needed to get the sketch working here:

        if(client.connected()) {         
            delay(500);  // <-- added so we wait for incoming bytes
            while(client.available()>0) {

For my setup, the socket did not have receive data right away. I'd also appreciate some clarification on what the lost data is, is it request data to the server or response data to the client? Maybe you can share a example of what is missing too.

Note: I'm currently testing with the master version of the SAMD core: https://github.com/arduino/ArduinoCore-samd - will also try to roll back to the v1.6.17 SAMD core release to see if that has any issues.

[FrancMunoz]

Hi @sandeepmistry!

I'm very busy these days... I apologize, will try to reply as fast as i can. I've read all your replies for both bugs, it seems we don't play with same conditions so will be difficult to debug ... I will try with latest samd core (thank you for the advice didn't noticed about). Right now I've latest MKRGSM and the one i've modified to get the #12 works without any issue (10 days without interruption). I've to mention that in my modified version I made more changes this changes are basically a loop that remains waiting for data what a byte is received AND WAIT 20ms to check if more data is available (i did it trying not to loose any byte i thought that weren't relevant when I notified the bug).

void ModemClass::poll()
{
	bool reading=false;
	bool avail=false;

  while ((avail=_uart->available()) || reading) {
    if(!avail) {
    	delay(20);
    	continue;
    }
    char c = _uart->read();
    //reading=true;
    if (_debug) {
      Serial.write(c);
    }

    _buffer += c;

    switch (_atCommandState) {
      case AT_COMMAND_IDLE:
      default: {

        if (_buffer.startsWith("AT") && _buffer.endsWith("\r\n")) {
          reading=false;
          _atCommandState = AT_RECEIVING_RESPONSE;
            //Serial.println("Vacia Buffer 2");
            //Serial.println(_buffer);
          _buffer = "";
        }  else if (_buffer.endsWith("\r\n")) {
        	reading=false;
          _buffer.trim();

          if (_buffer.length()) {
          	callUrcHandlers(_buffer);
          }
            //Serial.println("Vacia Buffer 3");
            //Serial.println(_buffer);
          _buffer = "";
        }

        break;
      }

      case AT_RECEIVING_RESPONSE: {
        if (c == '\n') {
          int responseResultIndex = _buffer.lastIndexOf("OK\r\n");
          if (responseResultIndex != -1) {
            _ready = 1;
          } else {
            responseResultIndex = _buffer.lastIndexOf("ERROR\r\n");
            if (responseResultIndex != -1) {
              _ready = 2;
            } else {
              responseResultIndex = _buffer.lastIndexOf("NO CARRIER\r\n");
              if (responseResultIndex != -1) {
                _ready = 3;
              }
            }
          }

          if (_ready != 0) {
			reading=false;
            if (_lowPowerMode) {
              digitalWrite(_dtrPin, HIGH);
            }

            if (_responseDataStorage != NULL) {
              _buffer.remove(responseResultIndex);
              _buffer.trim();

              *_responseDataStorage = _buffer;

              _responseDataStorage = NULL;
            }

            _atCommandState = AT_COMMAND_IDLE;
            //Serial.println("Vacia Buffer 4");
            //Serial.println(_buffer);
            _buffer = "";
            return;
          }
        }
        break;
      }
    }
  }
}

So, about your delay, i don't need it at all. I can't understand why you put a delay there, the GSMClient::available() called from GSMServer::available function will wait for that (or this is what i understood... maybe i'm wrong), I use the 20ms from bug #10, so here could be the reason of why you have to wait, could you please try to put 50ms delay from bug #10? :

...
  MODEM.sendf("AT+USORD=%d,0", _socket, 0);
  **if (MODEM.waitForResponse(10000, &response) == 1) {**
    if (response.startsWith("+USORD: ")) {
...

About the delay... using #12 just put a delay before checkServer call in main loop:

void loop() {
    int cMillis=millis();
    
    if(gsmStarted) {  
        **delay(50);**
        checkServer();

        // Blink Led
        if(cMillis-timerLed>=250) {
            blinkState=!blinkState;
            digitalWrite(PIN_BLINK, blinkState);
        }
    }
}

The missed chars are ever one byte or two from SARA so when an incomming connection arrives first bytes from serial are missing (that's why downgraded baudrate)... then the +UUSOLI command received from SARA is incomplete something like +UUSO:0, so handleURC doesn't parse the command... and data is lost.

I hope i've explained it more accurately.

Thank you for your effort!

[sandeepmistry]

Ok, let's take a step back.

Could you please provide a sketch to reproduce the issue with the original MKRGSM (without changes) and the SAMD 1.6.17 or master core. No rush, whenever you can :)

So, about your delay, i don't need it at all. I can't understand why you put a delay there, the GSMClient::available() called from GSMServer::available function will wait for that (or this is what i understood... maybe i'm wrong),

I was hitting this condition, and then Chrome was complaining about an invalid HTTP response:

 +            } else {
 +                // Send plain response, so we can use non HTTP like fsockopen from PHP.
 +                client.println(response);
 +            }
 +            delay(1000); // Let data reach its destination.
 +            Serial.println("Response Send.");
 +        }
 +        Serial.println("Close remote client.");
 +        client.stop();

[alvarolb]

I am also experiencing some issues while reading from a socket. In my case, there are two bytes available in the buffer (as reported by the available command), and doing partial readings uses to fail, i.e., reading one byte after another instead of the whole available data. In this case, the second read uses to fail. However, adding a small delay between readings like delay(1), seems to improve the problem. Any hint?

[sandeepmistry]

Hi @alvarolb,

Could you please share a minimal sketch to reproduce the issue, along with any debug logs (add MODEM.debug() to the sketch. Also, what version of the library are you using?

If it's not related to the server, please open a new issue to track this.

[FrancMunoz]

Hi! Just would like to inform that I've made some tests and upgraded speed to 57600bps and it's working without issues can't use 115200 but will make more testings.

[Rocketct]

Hi @FrancMunoz i have test your example #12, it works with the speed 921600, second i have tried to impose a time out in checkServer() function instead of a delay in main as follow:

void checkServer() {
    GSMClient client;
    bool reply = false;
    String data = "";
    int received = 0; 
    int ready;
    String response = "";
    uint8_t buffer[256];
    int avail, read;
    bool isHTTP = false;
    int s;
    
    
    client=server.available();
    if(client) {
        Serial.println("Request Received");
               
        if(client.connected()) {   
          **int timeout=millis() + 4000;**
          
          
          **while(client.available()==0 && millis()<timeout);**    
            while(client.available()>0) {
                 avail=client.read(&buffer[0], 255); 
        
                while(avail>0 && received<1024) {
                    received+=avail;
                    buffer[avail]=0; // Set last char to NULL to terminate String and concat.
                    data.concat(String((char *)buffer));
                    avail=client.read(&buffer[0], 255);
                }
            }

            if(data.length()>0) {
                Serial.print("Received: ");
                Serial.print(received);
                Serial.println(" bytes.");

                if(data.startsWith("GET /")) {
                    isHTTP=true;
                    s=data.indexOf(' ', 5);
                    if(s>-1) {
                        data=data.substring(5, s);
                    }                    
                } 
                
                // Parse message
                if(data=="hello") {
                    digitalWrite(PIN_STATE, 1);
                  
   response="Hello!;
                } else if(data=="bye") {
                    digitalWrite(PIN_STATE, 0);
                    response="Good Bye!";
                } else {
                    response="Can't understand "+data;
                }
            }

            if(isHTTP) {
                // Send HTTP Header
                client.println("HTTP/1.1 200 OK");
                client.println("Content-Type: text/html");
                client.println("Connection: close");
                client.println();

                // Send HTTP Data
                client.println("<html><body>");
                client.println("<h3>MKRGSM 1400 Server v0.1</h3>\n");
                if(response.length()>0) {
                    client.print("<p>");
                    client.print(response);
                    client.println("</p>");
                }
                client.println("</body></html>");
            } else {
                // Send plain response, so we can use non HTTP like fsockopen from PHP.
                client.println(response);
            }
            delay(1000); // Let data reach its destination.
            Serial.println("Response Send.");
        }
        Serial.println("Close remote client.");
        client.stop();
    }
}

and it seem work fine because wait that all character are available, could you try this modifications?

[FrancMunoz]

Hi @Rocketct !

Sure! I will try your suggestion and will tell. In my case any hangs occurred anymore at 57600, and never needed the time out but it's a nice update since it will manage in case of communication failure.

Thank you will inform you soon!

[FrancMunoz]

Hi! I can confirm that the issue is gone I've tested at 921600 with no changes in my sketches and all is working with any issue. So I suggest this issue to be closed.

Thank you!

[Rocketct]

ok @FrancMunoz i have to ask only one think witch version you use of the library?

[FrancMunoz]

Hi @Rocketct ! I'm using SAMD 1.6.18 and my modified MRKGSM from 1.1.2 that you can find in #27. Also I've tested with 1.1.2 and worked so I consider this issue is resolved.

With that, I mean that this issue: chars are lost, is no longer occurring... but now, we have another issue as I've commented in #27, SARA does not respond to all AT commands and as much time is running much fails are happening, so with my modified library arduino doesn't hangs but after a day sending data each minute it stops to send anything because SARA does not respond to (some or all) AT commands (it gets collapsed? internal thresholds?) ... As this is the same behavior as #27 and no chars are lost I think this could be closed and continue there.

I guess #27 issue is in SAMD core not in MKRGSM... will investigate more.

Thank you!

[sandeepmistry]

@FrancMunoz thanks for the update. Based on you comment from #11 (comment) I will close this.

[jpmeijers]

I'm using TinyGSM with a Ublox Sara G450 on a custom board with the SAMD21.

Communication via UART to the modem at 115200 works perfect in most cases, but I have a very similar issue with longer TCP connections. Downloading a 100kB file using the TCP AT commands, reading the data in 649 byte blocks using AT+USORD=0,649. Sometimes it happens that the SAMD21 loses some bytes (both times I counted it lost 54 bytes in the middle of the response).

A strange effect that I saw was that it happened more often when the watchdog timer is enabled. javos65/WDTZero#4

My theory is that the Arduino Serial library's buffer gets filled quicker than what we process it, overflowing and dropping data. Does this make sense? I will try and increase the Serial buffer first, and then maybe decrease the size of the chunks I read from the modem.