WiFiClient does not handle partial writes

[lasselukkari]

Description

I believe the WiFiClient write function does not handle cases where the lwip_send_r does not write all the data: https://github.com/arduino/nina-fw/blob/master/arduino/libraries/WiFi/src/WiFiClient.cpp#L90-L96

The return value of the lwip_send_r function defines how many bytes were actually written but the code in this repository expects it to send all or return an error. For reference this is how the ESP32 Arduino core implements the retry logic: https://github.com/espressif/arduino-esp32/blob/master/libraries/WiFi/src/WiFiClient.cpp#L367-L408. The send function there is just a wrapper for the lwip_send_r.

Most likely this is the reason why these issues report the WiFiNINA WiFiClient writes to fail. https://github.com/arduino-libraries/WiFiNINA/issues/129 https://github.com/lasselukkari/aWOT/issues/89

Environment

ARDUINO MKR WIFI 1010 macOS Catalina 10.15.7 Arduino IDE 1.8.13 WiFiNina 1.8.0 nina-fw 1.4.1

Reproduction

To reproduce the problem here is a sketch that has been modified from the WiFiNINA WiFiWebServer example. It writes a 100000 byte payload to the client in 1kB chunks. Reload the page a few times with a browser and it will fail to send the complete payload randomly.

#include <SPI.h>
#include <WiFiNINA.h>

#define WIFI_SSID ""
#define WIFI_PASSWORD ""

WiFiServer server(80);

void setup() {
  Serial.begin(9600);

  WiFi.begin(WIFI_SSID, WIFI_PASSWORD);
  while (WiFi.status() != WL_CONNECTED) {
    delay(500);
    Serial.print(".");
  }
  Serial.println(WiFi.localIP());

  server.begin();
}

void loop() {
  // listen for incoming clients
  WiFiClient client = server.available();
  if (client) {
    Serial.println("new client");
    // an http request ends with a blank line
    boolean currentLineIsBlank = true;
    while (client.connected()) {
      if (client.available()) {
        char c = client.read();
        // if you've gotten to the end of the line (received a newline
        // character) and the line is blank, the http request has ended,
        // so you can send a reply
        if (c == '\n' && currentLineIsBlank) {
          // send a standard http response header
          client.println("HTTP/1.1 200 OK");
          client.println("Content-Type: text/plain");
          client.println("Content-Length: 100000");
          client.println("Connection: close");  // the connection will be closed after completion of the response
          client.println();
          for (int i = 0; i < 100; i++) {
            client.print(
              "****************************************************************************************************"
              "****************************************************************************************************"
              "****************************************************************************************************"
              "****************************************************************************************************"
              "****************************************************************************************************"
              "****************************************************************************************************"
              "****************************************************************************************************"
              "****************************************************************************************************"
              "****************************************************************************************************"
              "****************************************************************************************************"
            );
          }

          break;
        }
        if (c == '\n') {
          // you're starting a new line
          currentLineIsBlank = true;
        } else if (c != '\r') {
          // you've gotten a character on the current line
          currentLineIsBlank = false;
        }
      }
    }

    if (client.getWriteError()) {
      Serial.println("Client write error detected");
    }

    // give the web browser time to receive the data
    delay(1);
    client.stop();
  }
}

[ocrdu]

Problem reproduced on Arduino Nano 33 IoT, Windows, Arduino IDE 1.8.9, WiFiNina 1.8.0, nina-fw 1.4.1.

[giulcioffi]

Hi @lasselukkari and @ocrdu! Could you please let me know if the two PRs referenced above this message solve this issue?

[lasselukkari]

Hi. I can try it out but it would be easier if you could provide us a precompiled binary for the nina-fm.

[giulcioffi]

Here you find the binary: nina-fw.zip

[lasselukkari]

After flashing the board with the provided firmware (pain in the ass because it randomly fails about 90% of the time) my MKR WIFI 1010 is unable to connect to the wifi router. I reflashed the NINA_W102-v1.4.1.bin and wifi connection works again. Went back to the nina-fw.binand no wifi. So for some reason the wifi does not connect with updated firmware.

[giulcioffi]

On my side the fw seems to work fine. I am using a MKR WiFi 1010 with the latest release of ArduinoCore-samd. Could you please try the connection uploading a simple WiFiWebClient? If this works, please replace the setup of your sketch (the one you posted above) with the following:

int status = WL_IDLE_STATUS;

void setup() {

  Serial.begin(9600);
  while(!Serial); 

  while (status != WL_CONNECTED) {
    Serial.print("Attempting to connect to SSID: ");
    Serial.println(WIFI_SSID);
    status = WiFi.begin(WIFI_SSID, WIFI_PASSWORD);

    delay(1000);
  }
  Serial.println("Connected to wifi. Local IP: ");
  Serial.println(WiFi.localIP());

  server.begin();
}

[lasselukkari]

I get "Communication with WiFi module failed!" with your firmware. With the 1.4.1 it works. Are you sure you gave me the right file?

[lasselukkari]

Do you use the espotool to flash the esp32. If so what are your parameters for it? Your bin file is 0.2 mb smaller than the stock firmware. Is that expected? Quickly looking at the binary content of both firmware versions it seems that they are really different.

[giulcioffi]

I'm sorry, before I sent to you the binary of just the nina application...but you need the full one, my fault. You find it here: NINA_W102.bin.zip.

And yes, I do upload it with the esptool. My procedure is the following:

• upload SerialNINAPassthrough
• run python /path/to/esptool.py --chip esp32 --port /dev/ttyACM1 --baud 921600 --before default_reset --after hard_reset write_flash -z --flash_mode dio --flash_freq 40m 0x0000 /path/to/NINA_W102.bin

Please let me know if this works fine and sorry again.

[lasselukkari]

The firmware works now and the bug seems to be fixed too. It also fixed the original problem described by the user of my library here: https://github.com/lasselukkari/aWOT/issues/89.

Just out of curiosity why doesn't this firmware use the Arduino core for the ESP32?

[giulcioffi]

Thank you for testing! About you question, I don't have an answer but maybe @facchinm can help here :)

[facchinm]

@lasselukkari we needed to be able to update the idf independently and when we started the arduino core was lagging behind :slightly_smiling_face: I think we could switch one day or another; PRs are always well accepted :wink:

[lasselukkari]

@facchinm I completely understand that. The ESP32 arduino core was really buggy for a long time. Now things seem to have settled down.

If the plan is to move to use the official Arduino ESP32 core these changes to fix this problem are maybe going to the wrong direction. Now the actual retry logic is implemented to WiFiNina codebase while ESP32 Arduino core would do it too. The switch would be easier one day if the WiFiNina would be kept as simple as possible and all the real logic would be in the firmware. I don't see this as big problem but it is always harder to keep two projects in sync if the logic is split between them.