Strange required delay

Projects Programming
1

I am very puzzled.

I am using the ESP8266 as a server and it is plugged into a 16x2 LCD via I2C.

Everything works in the following code:

#include "src/LiquidCrystal_I2C/LiquidCrystal_I2C.h"
#include <ESP8266WiFi.h>
#include <ESP8266WebServer.h>

ESP8266WebServer server(80);
const char* ssid = "The Pink Rabbit";
const char* password = "walkingwithspiders";

const int width = 16;
const int height = 2;
const int bufferLength = 256;
LiquidCrystal_I2C lcd(0x27, width, height);

void setup() {
  WiFi.begin(ssid, password);
  while (WiFi.status() != WL_CONNECTED) {
    delay(500);
  }
  server.on("/", handleRequest);
  server.begin();
  lcd.begin(16,2);
  lcd.init();
  lcd.backlight();
}

void displayMessage(String message) {
    char longShort[height][bufferLength];
    splitAndFormat(message, longShort);
    lcd.setCursor(0, 0);
    for(int j = 0; j < width && longShort[1][j] != '\0'; j++) {
        lcd.write(longShort[1][j]);
    }
    for (int i = 0; longShort[0][i] != '\0'; i++) {
      lcd.setCursor(0, 1);
      int j;
      for(j = 0; j <  width && longShort[0][i + j] != '\0'; j++) {
          lcd.write(longShort[0][i + j]);
      }
      if(j < width) {
          lcd.write(' ');
      }
      delay(400);
    }
    lcd.clear();
}

void flash() {
    for(int i = 0; i < 5; i++) {
        lcd.backlight();
        delay(100);
        lcd.noBacklight();
        delay(100);
    }
    lcd.backlight();
}

void waitingAnimation() {
  for(int i = 0; i < width; i++) {
      lcd.setCursor(i, 0);
      lcd.print((char)126);
      delay(100);
      lcd.clear();
  }
  for(int i = width - 1; i >= 0; i--) {
      lcd.setCursor(i, 1);
      lcd.print((char)127);
      delay(100);
      lcd.clear();
  }
}

void splitAndFormat(String message, char longShort[height][bufferLength]) {
    split(message, longShort);
    format(longShort);
}

void split(String message, char longShort[height][bufferLength]) {
    char buffer[bufferLength];
    message.toCharArray(buffer, bufferLength);
    boolean first = true;
    int start = 0;
    for(int i = 0; i < bufferLength; i++) {
        if(buffer[i] == '|') {
            longShort[0][i] = '\0';
            first = false;
            start = i + 1;
            continue;
        }
        if(buffer[i] == '\0') {
            longShort[1][i] = '\0';
            break;
        }
        longShort[first ? 0 : 1][first ? i : i - start] = buffer[i];
    }
}

void format(char longShort[height][bufferLength]) {
    char buffer[bufferLength];
    for(int i = 0; i < bufferLength; i++) {
        buffer[i] = longShort[0][i];
        if(longShort[0][i] == '\0') {
            break;
        }
    }
    for(int i = 0; i < bufferLength; i++) {
        longShort[0][i] = i < width ? ' ' : buffer[i - width];
    }

    int count = 0;
    for(int i = 0; i < bufferLength; i++) {
        buffer[i] = longShort[1][i];
        if(longShort[1][i] == '\0') {
            count = i;
            break;
        }
    }
    int pad = (width - count) / 2;
    for(int i = 0; i < width; i++) {
 >>>>>>>>>>>>>>>>>>       delay(100);
        longShort[1][i] = i < pad ? ' ' : buffer[i - pad];
    }
}

void handleRequest() {
  String message = "";
  if(server.hasArg("plain")) {
      message = server.arg("plain");
      if(message == "") {
          server.send(400, "text/plain", "Invalid request.");
          return;
      }
  }
  server.send(200, "text/plain", "Displaying message to \"" + message + "\"\n");
  displayMessage(message);
}

void loop() {
  server.handleClient();
}

My question is, the delay(100) where the arrows are is required; if I take it out, the LCD no longer displays the message and the response hangs and times out.

I figured this out because I had Serial.println(something); there and I narrowed it down to that line being required; I replaced it with a delay to confirm that that was what was making it work; why the heck does it need to have a delay there?

Thanks for the help.

2

I'm not familiar with the esp. Usually removing or adding a line somewhere causing problems indicates a memory issue like writing outside the boundaries of an array.

Could not look at your code in detail as I'm using a cell phone at the moment.

3

That's good to know. I will check that

4 
first ? i : i - start

this could have negative value

5

100 millisecs is a very long delay(), especially when repeated "width" times. There must be something else wrong.

Are you trying to update the LCD too frequently?

Have you tried putting the delay() in other places in the program to find out what is the critical part? It is certainly NOT the functioning of the FOR loop where you have it.

...R

6

arduino_new:

first ? i : i - start

this could have negative value

i will always be larger than start, unless I am missing something

7

Thanks for the help! I found the bug.

I moved the delay around like you said and found that indeed somewhere in the format method is the problem. I commented stuff out etc. and then added a bunch of print statements like so:

  void format(char longShort[height][bufferLength]) {
+ Serial.println("Beginning");
      char buffer[bufferLength];
+ Serial.println("after buffer");
      for(int i = 0; i < bufferLength; i++) {
+ Serial.println(String("Copying to buffer: " + String(i) + "=" + longShort[0][i]));
          buffer[i] = longShort[0][i];
          if(longShort[0][i] == '\0') {
+ Serial.println("Broke");
              break;
          }
      }
+ Serial.println("Adding spaces");
      for(int i = 0; i < bufferLength; i++) {
+ Serial.println("Adding space or character: " + String(i) + "=" + (i < width ? ' ' : buffer[i - width]));
          longShort[0][i] = i < width ? ' ' : buffer[i - width];
+         if(longShort[0][i] == '\0') {
+ Serial.println("Broke");
+             break;
+         }
      }

+ Serial.println("Counting and copying");
      int count = 0;
      for(int i = 0; i < bufferLength; i++) {
+ Serial.println(String("Copying to buffer: " + String(i) + "=" + longShort[0][i]));
          buffer[i] = longShort[1][i];
          if(longShort[1][i] == '\0') {
+ Serial.println("Broke");
              count = i;
              break;
          }
      }
      int pad = (width - count) / 2;
+ Serial.println("Calculated pad: " + pad);
      for(int i = 0; i < width; i++) {
~ Serial.println(String("Copying to buffer: " + String(i) + "=" + (i < pad ? ' ' : buffer[i - pad])));
          longShort[1][i] = i < pad ? ' ' : buffer[i - pad];
      }
  }

(This code includes my fix and my git changes)

So the problem was this loop:

+ Serial.println("Adding spaces");
      for(int i = 0; i < bufferLength; i++) {
+ Serial.println("Adding space or character: " + String(i) + "=" + (i < width ? ' ' : buffer[i - width]));
          longShort[0][i] = i < width ? ' ' : buffer[i - width];
+         if(longShort[0][i] == '\0') {
+ Serial.println("Broke");
+             break;
+         }
      }

I wasn't checking for '\0' like the rest of them, so it would copy a bunch of garbage. It would do a reset for whatever reason at that point til I added the fix.

Thanks all!

8

Well, when I remove the prints it still gives me a problem... but that was at least a bug that could be fixed, and I am at least on the right track now.

9

I realized there's no real point to me handling the string as a char array; so I just used the built-in Arduino
String operations. I know they aren't great cause of the immutability of strings and the operations are probably slower, but for my simple IoT project it works easy and is less code.

10

Ooooh. That's not going to get a lot of support. We're pretty much allergic to String around here.

Go back to character arrays, and never test for "\0", the value in the last position of a string (small s) is 0x00. Your test will simple be,

while(s*)*
As long as s is not 0x00 the code will execute. Remember that zero in a character string is 0x30 and space is 0x20.

11

so I just used the built-in Arduino String operations

That is guaranteed to produce hard to debug, slow to develop program crashes, resulting from memory fragmentation, at least on an standard AVR-based Arduino.

In a word, DON'T.

13

Bounds checking during C-string manipulation can and should be done by using the "n" variants of the functions, like strncpy(), strncat(), strncmp(), etc.

I've certainly lost hours of work by stubbornly failing to follow my own advice!

14

kamenomagic:
i will always be larger than start, unless I am missing something

How can you be sure?

Here you have

start = i + 1;

start will be larger then i by 1 isn't it?

15

My take on this is that the delay is needed because using print statements or Strings is just a delay by another name.

A lot of LCD code works by using delays instead of looking at the display’s ready line. Sometimes these delays are just loops of nop statement. This leaves it vulnerable to “faster processors “. My guess is that in this case the library is running too fast for the LCD and it is being sent data before it is ready.

That #include looks a very non Arduino like thing, normally there is no need to put any path in those. Make sure what files are really being included and check to see if it is not feeding data too quickly. Otherwise hack the libiary to include a check on the data ready line of the LCD before feeding it data.

16

OP mentioned ESP in the opening post.

17

@kamenomagic, may I comment on your programming?
The most important thing in programming is leaning back and thinking how the global structure of the code should be.
You code seems to be code that you started to write and tried to make work along the way and then added things to it. It became something that is almost impossible to maintain.

I even suggest to start all over.

The display is used with text which has a fixed format.
You try very hard to use the String object, but the result is ugly code, and conversions to normal text and vice versa.
When not using the String object, you can format the text with a single sprintf().
If you don't like sprintf(), you could do the formatting of the text in a single function and write in comments how the text should be that is send to the display.

@arduino_new is right. You should not write code that is dangerous. I don't care if that will work or not. The code itself is a dangerous combination. I mean this combination:

const int bufferLength = 256;
int start = 0;
for(int i = 0; i < bufferLength; i++)
start = i + 1;
longShort[first ? 0 : 1][first ? i : i - start] = buffer[i];

That code should ring an alarm bell.

Don't try to be smart. Try to write code that shows what is going on with a single glance at it.
It is okay to use many if-statements. Personally, I like code that is open and spacious.

There is one more thing... why would a 5V I2C display connected via the I2C bus to a 3.3V processor work at all ?