ATMega2560 SRAM crash. Advice for reducing SRAM fragmentation.

Hello guy, long time reader, first post.

A little background on the project. I’m writing a program for a standalone atmega2560 board. This unit needs to run 24/7 for months on end. It also has an LCD screen (NHD-0420CW-AG3) where error codes can will be displayed, engine status, totalized hours, runtime, etc etc. I’ve written the vast majority of the code already, and the IC is only 11% filled. SRAM is the issue. When a message pops up, the unit will crash. I did some research and it appears to be a stack crash.

What I’m looking for is advice, opinions, and experiences for writing stable code without memory leaks and wisely using the heap. Due to the amount of available flash space, I’m not concerned with putting as much as possible flash.

I’ve read a number of articles on this forum, and abroad, summarizing ways to do this (standard buffers of multiple size pre-declared in memory/Storing strings in EEPROM/PROGMEM any constants.) I’m not as advanced of a C programmer as I’d like, I’m using a lot of strcpy / strcat / itoa. What would be the best solution?

Is IO an issue? If not, use a 1284P, 16K SRAM, should give you some breathing room and room to grow.
Attach your code for advice on moving stuff like displayed text to flash.

Alviss:
What would be the best solution?

Post your code if you want to get more than general tips.

CrossRoads:
Is IO an issue? If not, use a 1284P, 16K SRAM, should give you some breathing room and room to grow.
Attach your code for advice on moving stuff like displayed text to flash.

Footprint size is more of the issue. There’s also some peripherals available on the 2560, that influenced the choice. Additionally, it’s the passed the point of a redesign. I greatly appreciate the input though, I’ll design with SRAM availability more in-mind in the future.

#define SCRN_BUFFER  21
char line0[SCRN_BUFFER] = {};
char line1[SCRN_BUFFER] = {};
char line2[SCRN_BUFFER] = {};
char line3[SCRN_BUFFER] = {};

/*
const char msg0 PROGMEM = "Standing by";
const char msg1 PROGMEM = "Contactor Open";   
const char msg2 PROGMEM = "Low Voltage";   
const char msg3 PROGMEM = "Cold Batteries";   
const char msg4 PROGMEM = "Oil Change due soon";   
const char msg5 PROGMEM = "Lost time. Aquiring...";   
const char msg6 PROGMEM = "RTC Malfunction";   
const char msg7 PROGMEM = "Driver Malfunction";   
const char msg8 PROGMEM = "Voltage Reg. failure";   
const char msg9 PROGMEM = "Battery TC failure";   
const char msg10 PROGMEM = "Poor Sat. fix";   
const char msg11 PROGMEM = "No GPS fix";      
const char msg12 PROGMEM = "Engine Failed Start";   
const char msg13 PROGMEM = "Engine Overspeed Fail";         
const char msg14 PROGMEM = "Engine Stall Fail";   
const char msg15 PROGMEM = "Engine Overheat Fail"; 
const char msg16 PROGMEM = "Engine Low oil pressure";   
const char msg17 PROGMEM = "No GPS or RTC. Call Tech";   
const char msg18 PROGMEM = "BMS FAIL, CALL TECH.";   
*/
//--------------------------------------------------------------------------------------------
//
void handle_screenFlip()
{  
  //processLine("", 0); 
  //processLine("", 1); 
  //processLine("", 2);
  //processLine("", 3);

  if (diagMode)
  {
    //displayTime();
    displayBatBank();
    display12v(); 
    /*
    if (!Dispflip) {
      display12v();
      displayEngineHours();
    } else {
      
      displayEngine();
    }
    */
  }
  else
  {
    //displayEngineHours();

    if (!Dispflip)
    {
      handleScreenMessage();
      //display12v();
    }
    else
    {
      if (run_engine && !fail)
        displayEngine();
      //displayTime();
      //displayBatBank();
    }
  }
  
  Dispflip = Dispflip ? false : true;
}

//--------------------------------------------------------------------------------------------
// This function displays the current time and date
void displayTime() 
{  
  char* buff;
  
  itoa(now.year(), buff, 10);
  strcat(line0, buff);
  strcat(line0, "/");
  
  /*
  char line0[SCRN_BUFFER] = {};
  
  itoa(now.year(), buff, 10);
  strcat(line0, buff);
  strcat(line0, "/");
  
  itoa(now.month(), buff, 10);
  strcat(tmp, buff);
  strcat(tmp, "/");
  
  itoa(now.day(), buff, 10);
  strcat(tmp, buff);
 
  strcat(tmp, " ");
  
  itoa(now.hour(), buff, 10);
  strcat(tmp, buff);
  strcat(tmp, ":");
  
  itoa(now.minute(), buff, 10);
  strcat(tmp, buff);
  strcat(tmp, ":");
  
  itoa(now.second(), buff, 10);
  strcat(tmp, buff); 
  */

  byte bytes[SCRN_BUFFER] = {};
  for (int i = 0; i < SCRN_BUFFER; i++)
     bytes[i] = line0[i];
     
  write_line(bytes, 0);
}

//--------------------------------------------------------------------------------------------
//
void displayEngine()
{
  char tmp[SCRN_BUFFER] = {};
  if (eng_running)
  {
    strcpy(tmp, "Charging Bank...");  
    displayRuntime();
  }
  else
    strcpy(tmp, "Starting Engine..."); 
     
  processLine(tmp, 1);            
}

//--------------------------------------------------------------------------------------------
//
void displayRPM()
{
  char tmp[SCRN_BUFFER] = {};
  char dtext[10];
  
  strcat(tmp, "RPM: ");
  itoa(engineRPM, dtext, 10);
  
  for (int i = 0; i < (8 - strlen(dtext)); i++)
     strcat(tmp, " "); 
  
  strcat(tmp, dtext);
     
  processLine(tmp, 1);            
      
}

//--------------------------------------------------------------------------------------------
//
void handleScreenMessage()
{
  char tmp[SCRN_BUFFER] = {};
 
if (stateCode == 0)
{
  if (!run_engine)  
    strcpy(tmp, "Standing By");
}
else if (stateCode == 1)
    strcpy(tmp, "Contactor Open");   
else if (stateCode == 2)
    strcpy(tmp, "Low Voltage");   
else if (stateCode == 3)
    strcpy(tmp, "Cold Batteries");   
else if (stateCode == 4)
    strcpy(tmp, "Oil Change due soon");   
    
else if (stateCode == 10)
    strcpy(tmp, "Lost time. Aquiring...");   
else if (stateCode == 11)
    strcpy(tmp, "RTC Malfunction");   
else if (stateCode == 12)
    //strcpy(tmp, "Standing By");   
    strcpy(tmp, "Driver Malfunction");   
else if (stateCode == 13)
    strcpy(tmp, "Voltage Reg. failure");   
else if (stateCode == 14)
    strcpy(tmp, "Battery TC failure");   
    
else if (stateCode == 20)
    strcpy(tmp, "Poor Sat. fix");   
else if (stateCode == 21)
    strcpy(tmp, "No GPS fix");      
    
else if (stateCode == 30)
    strcpy(tmp, "Engine Failed Start");   
else if (stateCode == 31)
    strcpy(tmp, "Engine Overspeed Fail");         
else if (stateCode == 32)
    strcpy(tmp, "Engine Stall Fail");   
else if (stateCode == 33)
    strcpy(tmp, "Engine Overheat Fail"); 
else if (stateCode == 34)
    strcpy(tmp, "Engine Low oil pressure");   
  
else if (stateCode == 39)
    strcpy(tmp, "No GPS or RTC. Call Tech");   
else if (stateCode == 40)
    strcpy(tmp, "BMS FAIL, CALL TECH.");        
else
  sprintf(tmp, "%d", stateCode);

  processLine(tmp, 1); 
}  

//--------------------------------------------------------------------------------------------
// This function displays the hours/mins the engine has been running.
// minutes are displayed in 1/10 of an hour
// 6 mins = 0.1 hours, 30 mins = 0.5 hours, etc.
int displayRuntime()
{
  char tmp[SCRN_BUFFER] = {};     
  char dtext[10];
  float fval;
  
  fval = float(runTime) / 60;
  dtostrf(fval, 1, 1, dtext);
 
  strcat(tmp, "Running:");
  
  for (int i = 0; i < (8 - strlen(dtext)); i++)
     strcat(tmp, " "); 
  
  strcat(tmp, dtext);
  strcat(tmp, " Hrs");
  
  processLine(tmp, 3); 
  return 1;
}

//--------------------------------------------------------------------------------------------
// This function displays the hours the engine has.
int displayEngineHours()
{
  char tmp[SCRN_BUFFER] = {};     
  char hrtext[10];
  char mintext[3];
  char oilChange[9];
  
  itoa(totEngineHrs, hrtext, 10);
  itoa(totEngineMins, mintext, 10);
  itoa(nextOilChange, oilChange, 10);
  
  strcat(tmp, hrtext);
  strcat(tmp, " : ");
  strcat(tmp, mintext);
  
  strcat(tmp, " : ");
  strcat(tmp, oilChange);  
  
  if (diagMode)
    processLine(tmp, 3); 
  else
    processLine(tmp, 0);     
  
  return 1;
}

//--------------------------------------------------------------------------------------------
// This function handles updating the 12v entry
void display12v() 
{ 
  char dtext[4];
  //float fval;
  char tmp[SCRN_BUFFER] = {};
  
  strcat(tmp, "Sys V: ");
  strcat(tmp, "   ");
  
  itoa(v12, dtext, 10);
  
  //fval = float(v12) / 100;
  //dtostrf(fval, 1, 2, dtext);
  
  strcat(tmp, dtext);
  strcat(tmp, "v"); 

  processLine(tmp, 1); 
}

//--------------------------------------------------------------------------------------------
// This function handles updating the 24v entry 
void displayBatBank()
{
  char dtext[4];
  //float fval;
  char tmp[SCRN_BUFFER] = {};
  
  strcat(tmp, "Bank: ");
  strcat(tmp, "   ");
  
  itoa(v24, dtext, 10);
  //fval = float(v24) / 100;
  //dtostrf(fval, 1, 2, dtext);
  
  strcat(tmp, dtext);
  strcat(tmp, "v"); 

  processLine(tmp, 1); 
}

//--------------------------------------------------------------------------------------------
// This function is a helper and wrapper to write text easily
void processLine(char* t, int r) 
{    
  char pad[SCRN_BUFFER] = {};
  snprintf(pad, sizeof(pad), "%-20s", t);
  
  byte bytes[SCRN_BUFFER] = {};
  for (int i = 0; i < SCRN_BUFFER; i++)
     bytes[i] = pad[i];
     
  write_line(bytes, r);
}

As you can see here, the code is very pock-marked with comment-outs.

I really feel as if I’m going about much of the code very amateurishly. I’m not very familiar with string manipulation.

This piece of code will probably crash the system (sooner or later)

void displayTime()
{  
  char* buff;
  
  itoa(now.year(), buff, 10);
  strcat(line0, buff);
  strcat(line0, "/");

You are writing to an uninitialized pointer. :cry:

And for saving RAM use F() macro or other means of moving the string constants to PROGMEM.

http://www.atmel.com/webdoc/AVRLibcReferenceManual/pgmspace.html

Whandall:
You are writing to an uninitialized pointer. :cry:

line0? When I was actually using this function, line0 was not commented out. Again, sorry for the pock-mark of comments and such.
I've attempted to use F() as much as possible, especially for the stateCode messages. But during compilation I get,

display.ino:159:30: error: cannot convert 'const __FlashStringHelper*' to 'const char*' for argument '2' to 'char* strcpy(char*, const char*)'
cannot convert 'const __FlashStringHelper*' to 'const char*' for argument '2' to 'char* strcpy(char*, const char*)'

Errors. Which shows me my absence of experience manipulating strings. What is suggested I do in this case?

No, not line0, buff. itoa places the converted string ... somewhere.

 char* buff;
 
  itoa(now.year(), buff, 10);

There are a lot of str..._P functions (working from PROGMEM) described here

http://www.atmel.com/webdoc/AVRLibcReferenceManual/group__avr__pgmspace.html

BTW, why do you use buff at all?

void displayTime()
{ 
  itoa(now.year(), line0, 10);
  strcat_P(line0, PSTR("/"));

It was close. This should be functional equivalent:

void displayTime()
{
  itoa(now.year(), line0+strlen(line0), 10);
  strcat_P(line0, PSTR("/"));

Oooh! I see, I see! (says the blind man). I'll implement these changes and report back.

Whandall:
BTW, why do you use buff at all?

I seem to have gravely misunderstood the usage of this function.

Thank you kindly, Whandall.

Works like a hot damn! Thank you, Whandall.