0

I am running the following code on my ESP8266 (AI-Thinker ESP8266MOD).

I send an HTTP GET request and a pin is set to high for a 1/2 second.

However, after some time (sometimes 1hr, 2hrs, 12hrs, totally random), it stops responding to HTTP requests. I think it might be because of heap fragmentation due to the String objects I'm using.

Questions:

How can I replace the request (in loop()) and s and text (in GenerateResponse()) objects with char arrays? After I replace the String objects with char arrays, how can I use the indexOf() method and + operator on the char arrays?

Will this approach help with heap fragmentation?

Is it possible to find out if heap fragmentation is the problem?

#include <ESP8266WiFi.h>

const char* ssid = "myWifi";
const char* wifiPassword = "y76ggS";
const char* passwordToOpenDoor = "/81"; //password should begin with a slash

const int doorPin = 5;

WiFiServer server(301); //Pick any port number you like
WiFiClient client;

void setup() {
  Serial.begin(115200);
  delay(10);
  Serial.println(WiFi.localIP());

  pinMode(doorPin, OUTPUT);
  digitalWrite(doorPin, 0);

  Serial.println();
  Serial.print("Connecting to ");
  Serial.println(ssid);

  WiFi.begin(ssid, wifiPassword);

  while (WiFi.status() != WL_CONNECTED) {
    delay(500);
    Serial.print(".");
  }
  Serial.println("");
  Serial.println("WiFi connected");

  server.begin();
  Serial.println("Server started. Diagnostics info:");
  Serial.println(WiFi.localIP());
}

void loop() {
  client = server.available();
  if (!client) {    
    return;
  }

  while(!client.available()){
    delay(1);  
  }

  String request = client.readStringUntil('\r'); 
  client.flush();

  Serial.println(request);  
  if (request.indexOf(passwordToOpenDoor) != -1) { //Is password correct?    
    GenerateResponse("Password is correct");
    OpenDoor();    
  }
  //Got a GET request and it wasn't the favicon.ico request, must have been a bad password:
  else if (request.indexOf("favicon.ico") == -1) {  
    GenerateResponse("Password is incorrect.");
  }  
}

void OpenDoor() {
  digitalWrite(doorPin, 1);
  delay(500);
  digitalWrite(doorPin, 0);
}

void GenerateResponse(String text) {
  Serial.println(text);
  String s = "HTTP/1.1 200 OK\r\n";
  s += "Content-Type: text/html\r\n\r\n";
  s += "<!DOCTYPE HTML>\r\n<html>\r\n";
  s += "<br><h1><b>" + text + "</b></h1>";
  s += "</html>\n";
  client.flush();
  client.print(s);
  delay(1);
}
Improve this question
4
  • Further to all the answers provided so far, when using char array, String.indexOf() can be replaced by strstr(str1, str2). It return position number when str2 match part of the str1.
    – hcheung
    Commented May 5, 2020 at 23:59
  • For learning, read `<cstring> that have all the function for c string and array manipulation that you need when handling char array.
    – hcheung
    Commented May 6, 2020 at 0:20
  • @hcheung on this line "else if (strstr(request, "favicon.ico") == -1) {" I get "ISO C++ forbids comparison between pointer and integer [-fpermissive]"
    – David Klempfner
    Commented May 6, 2020 at 11:27
  • I had to remove the comparison with 1 and -1 in both if statements to get it to compile.
    – David Klempfner
    Commented May 6, 2020 at 11:30

3 Answers 3

Reset to default
2

You already received good answer about some general ideas for replacing String objects with C strings. Here I will try to add some more tricks more specifically targeted to your actual situation.

I can see two places in your program where the use of String shines by its convenience, and replacing it with C strings will require some effort. The first is the use of Stream::readStringUntil(char) for getting the first request line. Here I see no better option (see edit below) than reading the stream one character at a time, and putting those characters into an array:

const size_t input_buffer_length = 256;

// Read a line of text up to the first '\r'.
// Returns a NUL-terminated string, without the final '\r',
// into a statically allocated buffer.
// Warning: this never times out.
const char *readline(Stream &input)
{
  static char buffer[input_buffer_length];
  size_t pos = 0;  // writing position within the buffer
  int c;           // current character
  while ((c = input.read()) != '\r') {
    if (c >= 0 && pos < sizeof buffer - 1) {
      buffer[pos++] = c;
    }
  }
  buffer[pos] = '\0';  // terminate the string
  return buffer;
}

You would use it like this:

const char *request = readline(client);

For more robustness, you may want to add a timeout. Note also that this implementation is no reentrant, as it uses a static buffer. This is not an issue for your use case.

Edit: As noted by hcheung in a comment, you do not need to implement this readline() function, as you can use Stream::readBytesUntil(), which handles the timeout and is provided by the Arduino core:

char buffer[input_buffer_length];
client.readBytesUntil('\r', buffer, input_buffer_length);

The other place is the usage of String::operator+=(const char *) for generating the response. Here I would advice you against using strcat() or strncat(): these will require extra memory to store the concatenated string which you probably do not need. Instead, you can simply print() the pieces one by one:

void GenerateResponse(const char *text) {
  Serial.println(text);
  client.println("HTTP/1.1 200 OK");
  client.println("Content-Type: text/html");
  client.println();
  client.println("<!DOCTYPE HTML>");
  client.println("<html>");
  client.print("<br><h1><b>");
  client.print(text);
  client.println("</b></h1></html>");
  client.flush();
  delay(1);
}

Alternatively, you can make use of the implicit concatenation of string literals in order to reduce the number of calls to client.print():

void GenerateResponse(const char *text) {
  Serial.println(text);
  client.print(
      "HTTP/1.1 200 OK\r\n"
      "Content-Type: text/html\r\n"
      "\r\n"
      "<!DOCTYPE HTML>\r\n"
      "<html>\r\n"
      "<br><h1><b>"
  );
  client.print(text);
  client.print("</b></h1></html>\r\n");
  client.flush();
  delay(1);
}
Improve this answer
2
  • 1
    Instead of creating a customised readline() function, it probably cleaner to use client.readBytesUntil('\r', buffer, length) since both readStringUntil() and readBytesUntil() are all inherited from Stream class.
    – hcheung
    Commented May 6, 2020 at 0:33
  • 1
    Thanks for your answer. It's been running now for over 12 hrs :) Looks like the String objects were the issue. I'll let you know if it's still working after a couple of days.
    – David Klempfner
    Commented May 6, 2020 at 23:30
2

So here is code to get you started:

myMessageArray [256] = {'\0'}; // Define global array large enough and zero terminate

the + method is replaced by

strcpy (myMessageArray, "Text to Add"); 
// Initializes the myMessageArray starts at index 0
strcat (myMessageArray, "More text to Add"); 
// Appends to the myMessageArray starts at current index

converting numeric values to chars:

uint_16_t myNumberValue  = 31253;
char numBuffer [16] = {'\0'}; // Helper buffer for conversions

itoa (myNumberValue,numBuffer,10);  // converts an integer to a base 10 (decimal) char
itoa (myNumberValue,numBuffer,2);  // converts an integer to a base 2 (binary) char
itoa (myNuberValue,numBuffer,16);  // converts an integer to a base 16 (hex) char

itoa initilizes a char array so we need the helper array:

strcat (myMessageArray, numBuffer);

to convert float we use

dtostrf(floatVariable, StringLengthIncDecimalPoint, numVarsAfterDecimal, numBuffer);

to convert chars back to int use

int16_t myIntVar = atoi(numBuffer);

to convert chars back to floats use

float myFloatVar = atof(numBuffer, decimalsToShow); // using just atof(numBuffer)
gives you standard x.XX only 2 decimals

A working IndexOf for char arrays:

/*******************************************************************************/
/**
    \brief Finds the index of the given value in the array starting at the given index
    \author codebreaker007
    \param [in] targetArray the array to search through for the char
    \param [in] valueToFind the value to find
    \param [in] startIndex the index to start searching at
    \return the index of the value within the array

    \details This method returns INDEX_NOT_FOUND^(-1) for a null input array.
        A negative startIndex is treated as zero. A startIndex larger than the array
        length will return INDEX_NOT_FOUND(-1)
*/
/********************************************************************************/
int8_t indexOf(char* targetArray, char valueToFind, uint16_t startIndex = 0) {
  if (targetArray == NULL) {
    return INDEX_NOT_FOUND;
  }
  if (startIndex <= 0) {
    startIndex = 0;
  }
  for (uint16_t i = startIndex; i < strlen(targetArray); i++) {
    if (valueToFind == targetArray[i]) {
      return i;
    }
  }
  return INDEX_NOT_FOUND;
}

To compare char arrays you should look into the strcmp function and if you want to checkthe first n char tomatchyou use:

if (strncmp(myMessageArray , "POST", 4 ) == 0) { 
// compares the first 4 chars if it matches = 0, see details in the arduino doku

Hope this comprehensive intro to char arrays helps you for your current and future projects. How to find out - convert to char arrays and if its runnibg stable that was it. Before that conversion you can never rule out String class as culprit.

Improve this answer
7
  • Nice thorough answer. (Voted). I tend to use sprintf() and printf() in C, which is too heavy-weight for most Arduino boards. I am also not sure about the WiFiClient and WiFiServer classes and whether they have methods that return char arrays rather than strings.
    – Duncan C
    Commented May 5, 2020 at 14:33
  • 1
    This way is perfect to have buffer overflows. I propose to use snprintf function.
    – SBF
    Commented May 5, 2020 at 15:53
  • 1
    All the standard classes on ESP8366/32 use char (after learning the String heap problem in com heavy szenarios) As snprintf and similar all use temporary char arrays - look into the sources - this delays the problem of heap fragmentation, but is not solving it @SBF where do you see buffer overflows I propose global char arrayscompiled to flash - I have esps running for 1.5 years now with heavy use of char arrays not a single reset/crash - same program with String-class crashes after 30+ mins
    – Codebreaker007
    Commented May 5, 2020 at 17:26
  • 2
    Re “A working IndexOf for char arrays”: or just use the good old strchr().
    – Edgar Bonet
    Commented May 5, 2020 at 19:26
  • @Codebreaker007: I totally agree to avoid Sting class and use char buffer, however, if your buffer is too small when you strcat you have a buffer overflow. snprintf() has an argument to limit the maximum number of bytes to be used in the buffer. The generated string has a length of at most n-1, leaving space for the additional terminating null character.
    – SBF
    Commented May 5, 2020 at 19:31
1

You asked: "After I replace the String objects with char arrays, how can I use the indexOf() method and + operator on the char arrays?"

Short answer: You can't. If you want to use C strings (aka arrays of char), you'll have to use C string functions.

The String methods operate on String objects. The common wisdom is to avoid the String class entirely since it relies on heap memory, and Arduinos are far, far too memory-starved to be able to use a class like String which creates temporary objects on the heap.

Improve this answer

Your Answer

By clicking “Post Your Answer”, you agree to our terms of service and acknowledge you have read our privacy policy.

Not the answer you're looking for? Browse other questions tagged or ask your own question.