JiriBilek / WiFiSpi

SPI library for Arduino AVR and STM32F1 to connect to ESP8266
GNU Lesser General Public License v3.0
62 stars 13 forks source link

How to manage maximum socket connections? #20

Closed fredlcore closed 3 years ago

fredlcore commented 3 years ago

EDIT: The problem described here results from reaching the maximum limit of sockets (apparently 4). The question thus is how to manage or determine the current number of socket connections, see third post below.

The main page of my project on the Arduino Due side connects to my GitHub repository and downloads a file containing the current version and then informs the user if an update is available. This works great also with WiFiSpi. However, the result of that query is only seen on the website the very first time I call the main page. Afterwards, the code still goes through, but no more text is sent to the browser.

To send text to the browser, I don't use client.write() or .print() directly but fill a buffer that is then sent to the browser once it's full or at the end of the website. With the EthernetClient library, I use client.flush() for making sure that at the end of each call all characters are transmitted. I see that this function is not yet implemented in WiFiSpi, but I'm not really sure that this is the root cause of the problem because it does work the first time after each restart. I have also removed client.stop() and also added a delay to make sure that there is enough time for the Arduino to send away all reimaining data in the buffer, but no success.

Do you have any idea where the problem could lie?

Here's the code in question if that is of any relevance:

  if(enable_version_check){
    printlnToWebClient(PSTR("<BR><BR>Checking for newer version...<BR>"));
    flushToWebClient();
    httpclient.connect("bsb-lan.de", 80);
    httpclient.println("GET /bsb-version.h");
    httpclient.println();

    unsigned long timeout = millis();
    while (millis() - timeout < 3000 && !httpclient.available()) {
    }

    int major = -1;
    int minor = -1;
    int patch = -1;
    char version_number[8] = { 0 };
    while (httpclient.available()) {
      char c = httpclient.read();
      if (c == '\"') {
        int index = 0;
        do {
          c = httpclient.read();
          version_number[index] = c;
          index++;
        } while (c != '\"');
        version_number[index-1] = '\0';
        if (major < 0) {
          major = atoi(version_number);
        } else if (minor < 0) {
          minor = atoi(version_number);
        } else if (patch < 0) {
          patch = atoi(version_number);
        }
      }
    }
    httpclient.stop();

    if ((major > atoi(MAJOR)) || (major == atoi(MAJOR) && minor > atoi(MINOR)) || (major == atoi(MAJOR) && minor == atoi(MINOR) && patch > atoi(PATCH))) {
      printToWebClient(PSTR("New version available: "));
      printFmtToWebClient(PSTR("<A HREF=\"https://github.com/fredlcore/bsb_lan/archive/master.zip\">%d.%d.%d</A><BR>\r\n"), major, minor, patch);
    } else {
      printToWebClient(PSTR("Version up to date."));
    }
  }

Currently, the condition "Version up to date" is true and I can output the result to the serial port, but it is not printed to the web browser.

This is what printToWebClient() does:

int printToWebClient(const char *format){
  int len = strlen(strcpy(bigBuff + bigBuffPos, format));
  bigBuffPos += len;
  if(bigBuffPos > OUTBUF_USEFUL_LEN){
    flushToWebClient();
  }
  return len;
}

void flushToWebClient(){
  if(bigBuffPos > 0){
    client.write(bigBuff, bigBuffPos);
    bigBuffPos = 0;
    return;
  }
  client.flush();
}
fredlcore commented 3 years ago

Just to add: I tried to write directly to the client in addittion to the buffer: The first time after reboot, the message is printed twice (once from the buffer, once from client.println()), the second time not even once...

fredlcore commented 3 years ago

Ok, I think I found the culprit: Immediately after httpclient.connect("bsb-lan.de", 80); it is no longer possible to write to the client instance. I assume that all four sockets are eaten up the second time the main page is called (which I can't really explain to myself, because I use stop() at the end of each loop). Is there a way to find out how many sockets are currently occupied and a way to shut some of them down manually?

JiriBilek commented 3 years ago

Free sockets: The function WiFiSpiClass::getSocket() will return the first free socket number or SOCK_NOT_AVAIL.

Flushing the data: The data are immediately transmitted to the ESP after WiFiClient.print() and similar. Then I think they are given to the network layer and sent. So I think there is no flush() required.

If the problems still persist, please make minimal sketch showing the problems so I can run it here on my hardware. I am testing on STM32F103 instead of Arduino but I hope it does not make a difference.

fredlcore commented 3 years ago

Ok, will try to do so. Just one observation which might be a shortcut: I output WiFiSpiClass::getSocket() each time at the beginning of my loop() function. Since I set up two servers in the setup() code, getSocket() outputs "2" continuously until I call the remote http website. Immediately afterwards, getSocket() returns "0". All remaining functions work as before, just the remote http call is not executed and it seems the function from within .connect() is called is exited. Otherwise I cannot explain to me why the rest of the function is not executed, but the program continues otherwise normal. But I will make more tests and hope to come up with a isolated test case.

fredlcore commented 3 years ago

I think I may have found the reason for this problem: When creating a server through WiFiSpiServer.begin(), this server is allocated the currently next free socket. At the beginning of my sketch, this is socket 0. However, if a client is created from this server through .available(), and this client is stopped (either explicitly through .stop() or implicitly by leaving its scope), the socket (here: socket 0) will be freed by writing _state[0] with NA_STATE. So the next client instance will be given socket 0 by getSocket(), although socket 0 is still "owned" by the server instance. And since the server instance does not know about this, it still thinks that it should use socket 0, because the socket index is only determined once in the .begin() function.

That is why in my case both the client derived from WiFiSpiServer.available() as well as the explicitly created client through WiFiSpiClient (by extension through ._connect()) will use socket 0 to write their data. Since httpclient.stop() will close socket 0, the rest of the web page that the client derived from .available() wants to write to the browser, does not go through anymore.

Of the cuff, I have no idea how to fix this, because WiFiClient.stop() would somehow have to know if this socket belongs to a client derived from a WiFiSpiServer where the socket will remain blocked - or if it comes from an explicit WiFiSpiClient instance where the socket should be freed after it's been stopped.

Or maybe it's just too late and I get it all wrong ;)...

JiriBilek commented 3 years ago

Hold on, I have to refresh my memory. However, I found an old code I used for server testing. Maybe it is useful for you. I used to run it on STM32F103. The PB12 is used for resetting the ESP. You can omit it completely.

#include <WiFiSpi.h>

char ssid[] = "*****"; //  your network SSID (name)
char pass[] = "*****";    // your network password

int status = WL_IDLE_STATUS;
// if you don't want to use DNS (and reduce your sketch size)
// use the numeric IP instead of the name for the server:
//IPAddress server(74,125,232,128);  // numeric IP for Google (no DNS)
char server[] = "www.google.com";    // name address for Google (using DNS)

// Initialize the server
// with the IP address and port of the server
// that you want to connect to (port 80 is default for HTTP):
WiFiSpiServer TCPserver(80);

void setup() {
  pinMode(PB12, OUTPUT);
  digitalWrite(PB12, HIGH);

  //Initialize serial and wait for port to open:
  Serial.begin(115200);
  while (!Serial.available()) {
    ; // wait for serial port to connect. Needed for native USB port only
  }

  Serial.println("Reset ESP8266");

  digitalWrite(PB12, LOW);
  delay(100);
  digitalWrite(PB12, HIGH);
  delay(500);

  // Initialize the WiFiSpi library
  WiFiSpi.init(SS, 100000);

  // check for the presence of the shield:
  if (WiFiSpi.status() == WL_NO_SHIELD) {
    Serial.println("WiFi shield not present");
    WiFiSpi.status();
    // don't continue:
    while (true);
  }

/*  String fv = WiFiSpi.firmwareVersion();
  if (fv != "0.1.4") {
    Serial.println(fv);
    Serial.println("Please upgrade the firmware");
  }*/

  // attempt to connect to Wifi network:
  while (status != WL_CONNECTED) {
    Serial.print("Attempting to connect to SSID: ");
    Serial.println(ssid);
    // Connect to WPA/WPA2 network.
    status = WiFiSpi.begin(ssid, pass);
    Serial.println(status);
    Serial.print("Done with begin");

    // wait 1 second for connection:
    delay(1000);
  }
  Serial.println("Connected to wifi");
  printWifiStatus();

  Serial.println("\nStarting connection to server...");

  TCPserver.begin();
  if ( TCPserver.status() )
  {
    Serial.println("\nStarted server");
  }
}

void loop() {
  static uint16_t counter = 0;

  // if there are incoming bytes available
  // from the server, read them and print them:
  WiFiSpiClient client = TCPserver.available();
  if ( client )
  {
    Serial.println("Found client");

    if ( client.connected() )
    {
      while (! client.available() ) {}

/*      while ( client.available() ) {
        char c = client.read();
        Serial.write(c);
      }*/
      char input[400];
      uint16_t inputLen;
      while ((inputLen = client.available()) > 0) {
          if (inputLen > sizeof(input) - 1)
              inputLen = sizeof(input) - 1;
          client.read(reinterpret_cast<uint8_t*>(input), inputLen);
          input[inputLen] = 0;
          Serial.print(input);
      }
    }

    const uint16_t responseCode = 200;
    char buffer[1024];
    char header[100];
    char body[100];
    snprintf(body, sizeof body, PSTR("Test %d\r\n"), ++counter); 
    snprintf(header, sizeof header, PSTR("HTTP/1.1 %d OK\r\nContent-Type: text/plain\r\nConnection: close\r\nContent-Length: %i\r\n\r\n"), responseCode, (int)strlen(body)); 
    strcpy(buffer, header);
    strcat(buffer, body);

    client.print(buffer);

    client.stop();
  }
}
fredlcore commented 3 years ago

That code should be running fine (and is running fine with me). What should become a problem is if you

  1. add a second client in the form of client2 = WiFiSpiClient(); at the global section
  2. Make a client2.connect("www.google.com");inside the if ( client.connected()) condition
  3. Stop client2 before client.print(buffer);

The first run should be fine because TCPserver.available() will be assigned socket 0 and socket 0 will be marked as used. client2.connect() will then be given socket 1 and client2.stop() will free socket 1. All fine until here. However, at the end of the script, client.stop(); will free socket 0. It will use it again in the next iteration of loop() and everything written to client will be displayed into the browser until client2.connect() is called. Because available() won't mark socket 0 as used, getSocket() will return 0 and thus use the same socket as TCPserver has been assigned at the time of creation in the global section.

It boils down to the problem that _state is only modified at initialization for WiFiSpiServer but for WiFiSpiClients every time connect() or stop() is called.

fredlcore commented 3 years ago

One idea I have would be to add 128 to the value stored in _state for each index position that is occupied by a server socket. That way, WiFiSpiServer.stop() would still free the socket by setting it to NA_STATE, but WiFiSpiClient.stop() could check for a value smaller than 128 and only then free the socket by setting _state to NA_STATE.

I tried it here and it works fine, but I don't want to make a pull request because I don't know if this has other implications I can't oversee right now. Could you try these changes in your setting and decide if you want to keep it?

In WiFiSpiClient.cpp change stop() to this:

void WiFiSpiClient::stop() {
  if (_sock == SOCK_NOT_AVAIL)
    return;

  ServerSpiDrv::stopClient(_sock);

  int count = 0;
  // wait maximum 5 secs for the connection to close
  while (status() != CLOSED && ++count < 500)
    delay(10);

  if (WiFiSpiClass::_state[_sock] < 128) {           // <--- this line is new
    WiFiSpiClass::_state[_sock] = NA_STATE;
    _sock = SOCK_NOT_AVAIL;
  }                                                  // <--- this line is new
}

And in WiFiSpiServer.cpp change begin() to this:

void WiFiSpiServer::begin()
{
    uint8_t sock = WiFiSpiClass::getSocket();

    if (sock != SOCK_NOT_AVAIL)
    {
        _sock = sock;
        ServerSpiDrv::startServer(_port, _sock);
        WiFiSpiClass::_server_port[_sock] = _port;
        WiFiSpiClass::_state[_sock] = _sock + 128;    // <--- this line is changed
    }
}
JiriBilek commented 3 years ago

@fredlcore I modified my server example and nested a client reading your file. Runs fine. Please try it.

Edit: This is by no means an ideal example, only a test.

/*
  Web server with nested client

This sketch creates a TCP server on port 80.
 On every request on the TCP server reads a page on testing website (http://bsb-lan.de),
 prints the data read and outputs 'Test' and a increasing number of the test.
 Uses WiFi ESP8266 module.

 This example is written for a network using WPA encryption.

 Circuit:
   1. On ESP8266 must be running (flashed) WiFiSPIESP application.

   2. Connect the Arduino to the following pins on the esp8266:

            ESP8266         |
    GPIO    NodeMCU   Name  |   STM32F103
   ===================================
     15       D8       SS   |   PA4
     13       D7      MOSI  |   PA7
     12       D6      MISO  |   PA6
     14       D5      SCK   |   PA5
     RESET                  |   PB12

    Note: If the ESP is booting at the moment when the SPI Master (i.e. Arduino) has the Select line HIGH (deselected)
    the ESP8266 WILL FAIL to boot!

 */

#include <WiFiSpi.h>

char ssid[] = "*****"; //  your network SSID (name)
char pass[] = "******";    // your network password

int status = WL_IDLE_STATUS;

// Initialize the Ethernet client library
// with the IP address and port of the server
// that you want to connect to (port 80 is default for HTTP):
WiFiSpiServer TCPserver(80);

void setup() {
  pinMode(PB12, OUTPUT);
  digitalWrite(PB12, HIGH);

  //Initialize serial and wait for port to open:
  Serial.begin(115200);
  while (!Serial.available()) {
    ; // wait for serial port to connect. Needed for native USB port only
  }

  Serial.println("Reset ESP8266");

  // Reset the ESP  
  digitalWrite(PB12, LOW);
  delay(100);
  digitalWrite(PB12, HIGH);
  delay(500);

  // Initialize the library
  WiFiSpi.init(SS, 100000);

  // check for the presence of the shield:
  if (WiFiSpi.status() == WL_NO_SHIELD) {
    Serial.println("WiFi shield not present");
    WiFiSpi.status();
    // don't continue:
    while (true);
  }

  String fv = WiFiSpi.firmwareVersion();
  if (fv != "0.2.3") {
    Serial.println(fv);
    Serial.println("Please upgrade the firmware");
  }

  // attempt to connect to Wifi network:
  while (status != WL_CONNECTED) {
    Serial.print("Attempting to connect to SSID: ");
    Serial.println(ssid);
    // Connect to WPA/WPA2 network. Change this line if using open or WEP network:
    status = WiFiSpi.begin(ssid, pass);
    Serial.println(status);
    Serial.print("Done with begin");

    // wait 1 second for connection:
    delay(1000);
  }
  Serial.println("Connected to wifi");
  printWifiStatus();

  Serial.println("\nStarting connection to server...");

  TCPserver.begin();
  if ( TCPserver.status() )
  {
    Serial.println("\nStarted server");
  }
}

void loop() {
  static uint16_t counter = 0;

  // if there are incoming bytes available
  // from the server, read them and print them:
  WiFiSpiClient client = TCPserver.available();
  if (client)
  {
    Serial.println("Created client");

    if ( client.connected() )
    {
      while (! client.available() ) {}

/*      while ( client.available() ) {
        char c = client.read();
        Serial.write(c);
      }*/
      char input[400];
      uint16_t inputLen;
      while ((inputLen = client.available()) > 0) {
          if (inputLen > sizeof(input) - 1)
              inputLen = sizeof(input) - 1;
          client.read(reinterpret_cast<uint8_t*>(input), inputLen);
          input[inputLen] = 0;
          Serial.print(input);
      }
    }

    // Read the data from web
    WiFiSpiClient inpCli;
    if (inpCli.connect("bsb-lan.de", 80)) {
        Serial.println("connected to server");
        // Make a HTTP request:
        inpCli.println("GET /bsb-version.h HTTP/1.1");
        inpCli.println("Host: blb-lan.de");
        inpCli.println("Connection: close");
        inpCli.println();

        delay(2000);

        while (inpCli.available()) {
            char c = inpCli.read();
            Serial.write(c);
        }
        inpCli.stop();
    }

    // Output data to the server connection
    const uint16_t responseCode = 200;
    char buffer[1024];
    char header[100];
    char body[100];
    snprintf(body, sizeof body, PSTR("Test %d\r\n"), ++counter); 
    snprintf(header, sizeof header, PSTR("HTTP/1.1 %d OK\r\nContent-Type: text/plain\r\nConnection: close\r\nContent-Length: %i\r\n\r\n"), responseCode, (int)strlen(body)); 
    strcpy(buffer, header);
    strcat(buffer, body);

    client.print(buffer);

    client.stop();
  }
}

void printWifiStatus() {
  // print the SSID of the network you're attached to:
  Serial.print("SSID: ");
  Serial.println(WiFiSpi.SSID());

  // print your WiFi shield's IP address:
  IPAddress ip = WiFiSpi.localIP();
  Serial.print("IP Address: ");
  Serial.println(ip);

  // print the received signal strength:
  long rssi = WiFiSpi.RSSI();
  Serial.print("signal strength (RSSI):");
  Serial.print(rssi);
  Serial.println(" dBm");
}
JiriBilek commented 3 years ago

One idea I have would be to add 128 to the value stored in _state for each index position that is occupied by a server socket. That way, WiFiSpiServer.stop() would still free the socket by setting it to NA_STATE, but WiFiSpiClient.stop() could check for a value smaller than 128 and only then free the socket by setting _state to NA_STATE.

Slowly, I am getting the idea what's wrong. The WiFiSpiClass::_state[] for the server is cleared upon client stopping and reused sometimes later. But the server is still running and the library should use another "socket". Then the bug is in releasing the statevariable and the fix should be: not deleting the state when WiFiSpiClass::_server_port[_sock] != 0. I need to confirm this idea and to test it. My example worked because the lifetime of the client that came from TCP server overlaps the one from the nested client.

fredlcore commented 3 years ago

Exactly, here's the code that I modified slightly and it reproduces my error:

/*
  Web server with nested client

This sketch creates a TCP server on port 80.
 On every request on the TCP server reads a page on testing website (http://bsb-lan.de),
 prints the data read and outputs 'Test' and a increasing number of the test.
 Uses WiFi ESP8266 module.

 This example is written for a network using WPA encryption.

 Circuit:
   1. On ESP8266 must be running (flashed) WiFiSPIESP application.

   2. Connect the Arduino to the following pins on the esp8266:

            ESP8266         |
    GPIO    NodeMCU   Name  |   STM32F103
   ===================================
     15       D8       SS   |   PA4
     13       D7      MOSI  |   PA7
     12       D6      MISO  |   PA6
     14       D5      SCK   |   PA5
     RESET                  |   PB12

    Note: If the ESP is booting at the moment when the SPI Master (i.e. Arduino) has the Select line HIGH (deselected)
    the ESP8266 WILL FAIL to boot!

 */

#include "src/WiFiSpi/src/WiFiSpi.h"

char ssid[] = "Network"; //  your network SSID (name)
char pass[] = "Password";    // your network password

int status = WL_IDLE_STATUS;

// Initialize the Ethernet client library
// with the IP address and port of the server
// that you want to connect to (port 80 is default for HTTP):
WiFiSpiServer TCPserver(80);

void setup() {

  //Initialize serial and wait for port to open:
  Serial.begin(115200);
  Serial.println("Reset ESP8266");

  // Reset the ESP  

  // Initialize the library
  WiFiSpi.init(13);

  // check for the presence of the shield:
  if (WiFiSpi.status() == WL_NO_SHIELD) {
    Serial.println("WiFi shield not present");
    WiFiSpi.status();
    // don't continue:
    while (true);
  }

  String fv = WiFiSpi.firmwareVersion();
  if (fv != "0.2.3") {
    Serial.println(fv);
    Serial.println("Please upgrade the firmware");
  }

  // attempt to connect to Wifi network:
  while (status != WL_CONNECTED) {
    Serial.print("Attempting to connect to SSID: ");
    Serial.println(ssid);
    // Connect to WPA/WPA2 network. Change this line if using open or WEP network:
    status = WiFiSpi.begin(ssid, pass);
    Serial.println(status);
    Serial.println(WiFiSpi.localIP());
    Serial.print("Done with begin");

    // wait 1 second for connection:
    delay(1000);
  }
  Serial.println("Connected to wifi");
  printWifiStatus();

  Serial.println("\nStarting connection to server...");

  TCPserver.begin();
  if ( TCPserver.status() )
  {
    Serial.println("\nStarted server");
  }
}

void loop() {
  static uint16_t counter = 0;

  // if there are incoming bytes available
  // from the server, read them and print them:
  WiFiSpiClient client = TCPserver.available();
  if (client)
  {
    Serial.println("Created client");

    if ( client.connected() )
    {
      while (! client.available() ) {}

/*      while ( client.available() ) {
        char c = client.read();
        Serial.write(c);
      }*/
      char input[400];
      uint16_t inputLen;
      while ((inputLen = client.available()) > 0) {
          if (inputLen > sizeof(input) - 1)
              inputLen = sizeof(input) - 1;
          client.read(reinterpret_cast<uint8_t*>(input), inputLen);
          input[inputLen] = 0;
          Serial.print(input);
      }
    }

    // Output data to the server connection
    const uint16_t responseCode = 200;
    char buffer[1024];
    char header[100];
    char body[100];
    snprintf(body, sizeof body, PSTR("Test %d\r\n"), ++counter); 
    snprintf(header, sizeof header, PSTR("HTTP/1.1 %d OK\r\nContent-Type: text/plain\r\n\r\n"), responseCode); 
    strcpy(buffer, header);
    strcat(buffer, body);

    client.println(buffer);
    client.println("Now calling external website...");

    // Read the data from web
    WiFiSpiClient inpCli;
    if (inpCli.connect("bsb-lan.de", 80)) {
        Serial.println("connected to server");
        // Make a HTTP request:
        inpCli.println("GET /bsb-version.h HTTP/1.1");
        inpCli.println("Host: blb-lan.de");
        inpCli.println("Connection: close");
        inpCli.println();

        delay(2000);

        while (inpCli.available()) {
            char c = inpCli.read();
            client.write(c);
        }
        inpCli.stop();
    }

    client.println("Footer of website");

    client.stop();
  }
}

void printWifiStatus() {
  // print the SSID of the network you're attached to:
  Serial.print("SSID: ");
  Serial.println(WiFiSpi.SSID());

  // print your WiFi shield's IP address:
  IPAddress ip = WiFiSpi.localIP();
  Serial.print("IP Address: ");
  Serial.println(ip);

  // print the received signal strength:
  long rssi = WiFiSpi.RSSI();
  Serial.print("signal strength (RSSI):");
  Serial.print(rssi);
  Serial.println(" dBm");
}
fredlcore commented 3 years ago

My solution is similar to yours, I just treat Bit 8 as a "server socket flag". But using "server_port" is of course also an option. Thanks for taking the time to look into this!

JiriBilek commented 3 years ago

So the solution might be:

void WiFiSpiClient::stop() {
  if (_sock == SOCK_NOT_AVAIL)
    return;

  ServerSpiDrv::stopClient(_sock);

  int count = 0;
  // wait maximum 5 secs for the connection to close
  while (status() != CLOSED && ++count < 500)
    delay(10);

  if (WiFiSpiClass::_server_port[_sock] == 0)  // <---------------- THE FIX
    WiFiSpiClass::_state[_sock] = NA_STATE;
  _sock = SOCK_NOT_AVAIL;
}

Before committing, I have to test it but feel free to try it :)

fredlcore commented 3 years ago

Great, works here! So, _sock = SOCK_NOT_AVAIL should still be set? Just for my understanding what happens there...

JiriBilek commented 3 years ago

No, the socket must be blocked until the TCP server closes it, otherwise another client or server would use it. Oh, my bad, didn't read carefully the question. Yes, the _sock is a variable local to WiFiClient.

fredlcore commented 3 years ago

Ok, undestood, thanks!