Possible stack overflow.

Hello all. I’ve been quite befuddled lately at my thermostat which I started several years ago. I had a very similar sketch running quite well up until about two years ago when I moved back home from California. About six months ago, I purchased a house and reinstalled my thermostat and ever since then, after a while, all activity just stops. Only resetting the arduino makes it work. I’m using a mega 2560 and at compile it shows about 12% sram usage. I suspect, despite this, I’m getting a stack overflow. My latest code, shown below seems to freeze after about an hour. I’ve tried making what ever variables and objects I can find global in an attempt to allocate memory. I’m using many different pieces of hardware each with their own libraries that may use all kinds of local variables but I suspect the ethernet code is the culprit.

Recently, Ive added an Uno to the garage and got them to talk to each other. The only reason I’m posting both is because theyre relatively similar. They both use the Adafruit LCD shield and ethernet shield. The ethernet code is almost identical and they both freeze after about the same amount of time. The uno shows about 37% of sram usage.

My thermostat code is very long but documented as thoroughly as I can. The second sketch is still in the prototype phase but hopefully someone can take a look and point me in the same direction. Thanks very much in advance.

My thermostat code is too long to post so here is a “rubbishbin” link:
Thermostat v8.2
I have also attached it. Not sure which is better.

My second code is as follows:

#include <Wire.h>
#include <Adafruit_RGBLCDShield.h>
#include <SPI.h>         
#include <Ethernet.h>
#include <ICMPPing.h>
#include <OneWire.h>
#include "Adafruit_MCP9808.h"

//#define debug //comment out to skip all serial code.

#define RED 0x1
#define YELLOW 0x3
#define GREEN 0x2
#define TEAL 0x6
#define BLUE 0x4
#define VIOLET 0x5
#define WHITE 0x7

#define ONE_WIRE_BUS    2
#define ONE_WIRE_PWR    3
#define ONE_WIRE_GND    4
#define garageGnd       5
#define mainDoorPin     6
#define sideDoorPin     7
#define sideDoorLockPin 8

#define REQ_BUF_SZ      60 //size of buffer used to capture HTTP requests.

Adafruit_RGBLCDShield lcd = Adafruit_RGBLCDShield();

OneWire   oneWire(ONE_WIRE_BUS);
byte      addr[8];
byte      mac []   =   {0xDE, 0xAD, 0xBE, 0xEF, 0xFE, 0xED}; //mac address for ethernet shield
byte      ip  []   =   {192, 168, 1, 178};                   //ip address for ethernet shield

EthernetServer server (80);

char HTTP_req[REQ_BUF_SZ] = {0}; //buffered HTTP request stored as null terminated string.
char req_index            =  0;  //index into HTTP_req buffer.
EthernetClient client;

int8_t tempGarage;
unsigned long timeNow, timeTemp;
uint8_t mainDoor, sideDoor, sideDoorLocked = 0;

void setup() {
  pinMode(garageGnd,       OUTPUT);
  pinMode(mainDoorPin,     INPUT_PULLUP);  
  pinMode(sideDoorPin,     INPUT_PULLUP);
  pinMode(sideDoorLockPin, INPUT_PULLUP);
  pinMode     (ONE_WIRE_PWR, OUTPUT);
  pinMode     (ONE_WIRE_GND, OUTPUT);
  digitalWrite(ONE_WIRE_PWR, HIGH);
  digitalWrite(ONE_WIRE_GND, LOW); 
  digitalWrite(garageGnd,    LOW);
  Ethernet.begin(mac, ip);
  #ifdef debug
    Serial.begin  (19200);
  lcd.begin     (16, 2);
  lcd.print     (F("Garage Test!"));
  delay         (1000);
} //end setup().

void loop() {
  timeNow = millis();
  if (digitalRead(mainDoorPin) == HIGH) {
    mainDoor = 1;
  } //end if.
  else {
    mainDoor = 0;
  } //end else.
  if (digitalRead(sideDoorPin) == HIGH) {
    sideDoor = 1 ;
  } //end if.
  else {
    sideDoor = 0;
  } //end else. 
  if (digitalRead(sideDoorLockPin) == HIGH) {
    sideDoorLocked = 0;
  } //end if.
  else {
    sideDoorLocked = 1;
  } //end else.
  client = server.available();
  if (client) {
    boolean currentLineIsBlank = true;
    while (client.connected()) {
      if (client.available()) {
        char d = client.read();
        if (req_index < (REQ_BUF_SZ - 1)) {
          HTTP_req[req_index] = d;
          req_index ++;
        } //end if.
        if (d == '\n' && currentLineIsBlank) {
          if (StrContains(HTTP_req, "garageStatus")) { //request from thermostat.
          client.println("HTTP/1.1 200 OK");
          client.println("Access-Control-Allow-Origin: *");
          if (StrContains(HTTP_req, "getTemp")){
            client.println("Content-Type: text/xml\r\nConnection: keep-alive\r\n");
            client.print  ("<?xml version = \"1.0\" ?><inputs><tG>");
            client.print  (tempGarage);  
            client.print  ("</tG><mD>");
            client.print  (mainDoor);
            client.print  ("</mD><sD>");
            client.print  (sideDoor);
            client.print  ("</sD><sDL>");
            client.print  (sideDoorLocked);
            client.print  ("</sDL></inputs>");
          } //end if.
          req_index = 0;
          StrClear(HTTP_req, REQ_BUF_SZ); //clear the buffer.
        } //end if.
        if (d == '\n') {
          currentLineIsBlank = true;
        } //end if.
        else if (d != '\r') {
          currentLineIsBlank = false;
        } //end else if.
      } //end if client.available().
    } //end while.
  } //end if client.    
  if (timeNow - timeTemp > 1000) { // update temps once per second.
    tempGarage  = round(cToF(getOneWireTemp(addr)));
    timeTemp = timeNow;
  } //end if.
  lcd.setCursor(0, 0);
  lcd.print    (F("Garage Temp: "));
  lcd.print    (tempGarage);
  lcd.print    (F("F       "));
  lcd.setCursor(0, 1);
  if (mainDoor == 0 && sideDoor == 0 && sideDoorLocked == 1) {
    lcd.print  (F("Garage Secure   "));
  } //end if.
  else if (mainDoor == 0 && sideDoor == 0 && sideDoorLocked == 0){
    lcd.print  (F("Door Unlocked   "));
  }  //end else if.
  else if (mainDoor == 0 && sideDoor == 1){
    lcd.print  (F("Side Door Open  "));
  } //end else if.
  else if (mainDoor == 1 && sideDoor == 0) {
    lcd.print  (F("Main Door Open  "));
  } //end else if.
  else if (mainDoor == 1 && sideDoor == 1) {
    lcd.print  (F("Both Doors Open!"));
  } //end else if.
 } //end loop().

float cToF(float c) { // convert float C to int F.
  return ((1.8 * c) + 32.0);
} //end cToF().

float getOneWireTemp(byte *str) { // get OneWire temp.
  byte data[2]; //array to store data retrieved from sensor.
  oneWire.write(0x44, 1); //start conversion.
  oneWire.write(0xBE); //read scratchpad.
  for (byte i = 0; i < 2; i ++) {
    data[i] = oneWire.read(); //collect data.
  } //end for.
  int16_t raw = (data[1] << 8) | data[0]; //convert raw data to C.
  return (float)raw / 16.0;
} //end getOneWireTemp().

void StrClear(char *str, char length) {
  for (uint8_t i = 0; i < length; i ++) {
    str[i] = 0;
  } //end for.
} //end StrClear().

//searches for the string sfind in the string str
//returns 1 if string found
//returns 0 if string not found
char StrContains(char *str, char *sfind) {
  char found = 0;
  char index = 0;
  char len;

  len = strlen(str);
  if (strlen(sfind) > len) {
    return 0;
  } //end if.
  while (index < len) {
    if (str[index] == sfind[found]) {
      found ++;
      if (strlen(sfind) == found) {
        return 1;
      } //end if.
    } //end if.
    else {
      found = 0;
    } //end else.
    index ++;
  } //end while.
  return 0;
} //end StrContains().

thermostat_mega_newest5.ino (31.4 KB)

It's actually pretty rare to overflow the stack. You have to have a recursive function that runs away. (I haven't looked too closely at your code to exclude this possibility.)

More common is memory fragmentation in the heap. Using the String objects in a particularly stupid way will cause the heap to grow over time and collide with the stack. You don't appear to be using String either.

Either of these cases will be uncovered by a freeRam() function. Make it print the free ram once per minute or something and watch it for a few minutes.

Here's a representative line from the pastebin code...

 if (clockChecks[2] - clockChecks[1] > heaterDelay) { //check to see if heater has been off for at least 2 min.

OK, so you made an array called clockChecks because you know it's bad to have numbers in variable names. But you only ever use constants in the array! How the heck do you remember that clockChecks[1] is the heater time? You could do either clockChecks[Heater_Time] or just heaterTime. This has got to be very confusing to debug just a few days after it's been written. The fact that you had to write such a long comment for a simple expression means that it's already an issue.

The loop() function is also a little on the big side. Try to move all of that button-switch stuff into separate functions. Preferably into separate files. That will make it easier to debug.

    while (client.connected()) {

Are you sure it's not getting stuck here?

Thanks!!! Ill look into some other code for ethernet client. I agree that my clockChecks array is a little obscure. I have trouble remembering which one is which from time to time.

I have been unable to resolve this. My latest code seems to freeze after about 24 hours. Any help is very appreciated!

My latest thermostat code.

It's just one of those days. We're doing our best to get things back up and running.
If this is taking too long you can write us on the Forum, or check the Error Log.

That's absolutely normal on my system (Win8 64bit, FireFox 56.0.2). I have never been able to access it, always got that message.