Serial.print(char) crashes Mega - Board Toasted?

Projects Programming
22

You can append complete code as an attachment. Less risk that copy/paste by somebody that wants to look at it overrides something or that you missed a few lines.

This line in fanOnEvery() does not do anything useful

data + 15 * dirc;

Further, the second argument of itoa() is a pointer to an existing character array. Yours is just a pointer pointing to somewhere. Did not check all the code but this is definitely wrong in handleSchedule() and one of the possible causes of your application crashing.

There might be a lot more wrong.

23

Just at a quick glance, you're not using itoa() correctly.

  ....

  int fanFor = 15; //readEEprom("fanFor");
  int fanEvery = 120; //readEEprom("fanEvery");

  ...

  itoa(fanEvery, fanEvery, 10);
  strcpy(html8, fanEvery);

  ...

itoa is defined as

char * itoa ( int value, char * str, int base );

By passing fanEvery twice you're saying to itoa: "convert the value fanEvery into a string, then copy that string into memory location starting at the location pointed to by fanEvery"
That will overwrite whatever is in memory located at fanEvery (and a variable number of subsequent bytes). I'm not surprised you're getting crashes.

You're then trying to strcpy fanEvery (an integer) into a cstring. ie You're passing an integer instead of a pointer to a character (a cstring) to strcpy.

maybe better this way...

  ....

  int fanFor = 15; //readEEprom("fanFor");
  int fanEvery = 120; //readEEprom("fanEvery");
  char buffr[10];

  ....

  itoa(fanFor, buffr, 10);
  strcpy(html6, buffr);

  ...
  itoa(fanEvery, buffr, 10);
  strcpy(html8, buffr);
  ...

I've set buffr arbitrarily at 10. Size buffr to be at least one character larger than the largest number of digits your think your integer variables will contain.

Another issue is the following:

  ...
  char* tempSetting;

  ...
 
  itoa(72, tempSetting, 10);
  ...
  strcpy(html2, tempSetting);
  ...

while the call to itoa is correct there, 1) there is no need for it and 2) your going to run into problems with it anyway.

  1. There is no need because you're converting a constant that you already know. Just create the string "72"
  2. You're going to run into problems in that while you are using a pointer to a char, that pointer points nowhere when you created it, and you have not actually allocated space for the string you're wanting to create with itoa(). when you call itoa() it happily creates the string "72" then copies that string to memory starting at the location pointed to by tempsetting, which is ...where? undefined!

Back to point 1, you don't actually appear to be doing anything with that string other than to copy it into html2. You've already defined html2 and allocated enough space for it, just use html2 in your call to itoa()

ie: itoa(72, html2,10);

I'm assuming here that the 72 will actually be a variable value later on down the track.
If that's not the case, just re-define html2 at the start to be

char html2[100] = "72";

(although, using 100 bytes to store '72' seems a bit wasteful :wink:

24

Adding to my previous reply (had a bit mpre time to check)

You have scheduleTemp, scheduleHH and scheduleMM declared as global variables. You however have them as pointers in the mentioned function; those are different variables.

I suggest that you crank up the warning levels in file->preferences to all so the compiler does warn you about this type of possible mistakes. You can ignore the warnings related to the EEPROM library.

Some warnings that the compiler gives

C:\Users\sterretje\AppData\Local\Temp\arduino_modified_sketch_38323\sketch_nov05a.ino:442:14: warning: ISO C++ forbids comparison between pointer and integer [-fpermissive]

   if (AA == -1) strcpy(AA, '9');

              ^

===

C:\Users\sterretje\AppData\Local\Temp\arduino_modified_sketch_490644\sketch_nov05a.ino:442:31: warning: invalid conversion from 'char' to 'const char*' [-fpermissive]

   if (AA == -1) strcpy(AA, '9');

                               ^

===

C:\Users\sterretje\AppData\Local\Temp\arduino_modified_sketch_490644\sketch_nov05a.ino:646:12: warning: passing 'const String' as 'this' argument of 'String& String::operator=(const char*)' discards qualifiers [-fpermissive]

   tempM[0] = "Hysteresis";

            ^

===

C:\Users\sterretje\AppData\Local\Temp\arduino_modified_sketch_38323\sketch_nov05a.ino:328:20: warning: iteration 30 invokes undefined behavior [-Waggressive-loop-optimizations]

     if (tempArray[i] > 0) {

                    ^

C:\Users\sterretje\AppData\Local\Temp\arduino_modified_sketch_38323\sketch_nov05a.ino:327:3: note: containing loop

   for (int i = 0; i < 31; i++) { //Add up temps

   ^

tempArray only has 30 elements; counting 0..29.

So this check is wrong

  if (tempReadI > 30) tempReadI = 0;

and as a result the below line will more than likely create problems (it definitely does touch memory that does not belong to the array) and can be the cause of a crash.

  tempArray[tempReadI] = tempRead;
25

Oh, and get rid of String (capital S). Once you have everything fixed, you still run the risk of memory fragmentation and as a result running out of memory at run time.

26

Getting rid of Strings is my #3 priority now. It's still a big one. I fixed the array to [31] so it fills properly.

27

I'm about to give up. This darn thing on an Uno will cause it to print horizontal spaces after the sixth iteration of the for loop just after the "cu" of "currentBlock". On the Mega, it reboots.

What do you think is going on?

Oh and scheduleArray returns zero or appears to, but that's my function. It reads/writes an array stored in EEPROM. The row is specified by currentBlock, a char. Column by a char* of a string like "scheduleHH".

#include <EEPROM.h>

char currentBlock;

void setup() {
  Serial.begin(9600);
  char html0[550] = { 0 }, html1[100] = { 0 }, html2[100] = { 0 }, html3[100] = { 0 }, html4[100] = { 0 };
  char scheduleHHA[4] = { 0 };
  char scheduleMMA[4] = { 0 };
  char scheduleTempA[4] = { 0 };
  int scheduleHHi, scheduleMMi, scheduleTempi;
 char buff[10] = { 0 };
 
  currentBlock = 'A';
  strcpy(html0, "<!DOCTYPE html><html><head><title>Thermostat Schedule</title></head><body><hr>");
  strcpy(html1, "<table><tr><th> Time </th > <th> Temp. </th> <tr><td><a href=AHHUp> + </a>");


  strcat(html0, html1);
  for (int i = 0; i < 8; i++) {
    currentBlock++;
    Serial.print("currentBlock:");
    Serial.println(currentBlock);
    
    scheduleHHi = scheduleArray(currentBlock, 'r', "scheduleHH", 0);
    scheduleMMi = scheduleArray(currentBlock, 'r', "scheduleMM", 0);
    scheduleTempi = scheduleArray(currentBlock, 'r', "scheduleTemp", 0);

itoa(scheduleHHi,buff,10);
strcpy(scheduleHHA,buff);
itoa(scheduleMMi,buff,10);
strcpy(scheduleMMA,buff);
itoa(scheduleTempi,buff,10);
strcpy(scheduleTempA,buff);


    Serial.print("ScheduleTemp:"); Serial.println(scheduleTempA);

    strcpy(html2, "<a href=");
    strcat(html2, currentBlock);
    strcat(html2, "HHDown> - </a>");
    strcat(html2, scheduleHHA);
    strcat(html2, "<a href=");
    strcat(html2, currentBlock);
    strcat(html2, "HHUp> + </a>");
    
    strcat(html2, ": <a href=");
    strcat(html2, currentBlock);
    strcat(html2, "MMDown> - </a>");
    strcat(html2, scheduleMMA);
    strcat(html2, "MMUp> + </a>");
    strcat(html2, " </td> <td>");


    strcat(html3, ": <a href=");
    strcat(html3, currentBlock);
    strcat(html3, "TempDown> - </a>");
    strcpy(html3, scheduleTempA);
    strcat(html3, "TempUp> + </a>");
    strcat(html3, " </td></tr><tr><td>");

    strcat(html0, html2);
    strcat(html0, html3);
  }
  strcpy(html4, " </table> </body> </html> ");

  strcat(html0, html4);
  //Serial.println(html0);

}

void loop() {
  // put your main code here, to run repeatedly:
Serial.print("Ping!");
delay(1000);
}


int scheduleArray(char Block, char rw, char* item, int data) {
  int block;
  int out;
  int arrTimeHH[8], arrTimeMM[8], arrTemp[8];

  out = 0;

  if (Block == 'A') block = 0;
  if (Block == 'B') block = 1;
  if (Block == 'C') block = 2;
  if (Block == 'D') block = 3;
  if (Block == 'E') block = 4;
  if (Block == 'F') block = 5;
  if (Block == 'G') block = 6;
  if (Block == 'G') block = 7;
  //Serial.print('['); Serial.print(item); Serial.print(']');
  if (rw == 'r') { //Read data
    EEPROM.get(210, arrTimeHH);
    EEPROM.get(230, arrTimeMM);
    EEPROM.get(250, arrTemp);
    if (strcmp(item, "timeHH") == 0) {
      out = arrTimeHH[block];
    }
    if (strcmp(item, "timeMM") == 0) {
      out = arrTimeMM[block];
    }
    if (strcmp(item, "temperature") == 0) { //Do NOT use "temp" - for some reason doesn't work.
      out = arrTemp[block];
    }
  }
  if (rw == 'w') { //Write data
    if (strcmp(item, "timeHH") == 0) {
      arrTimeHH[block] = data;
      EEPROM.put(210, arrTimeHH);
    }
    if (strcmp(item, "timeMM") == 0) {
      arrTimeMM[block] = data;
      EEPROM.put(230, arrTimeMM);
    }
    if (strcmp(item, "temperature") == 0) {
      arrTemp[block] = data;
      EEPROM.put(250, arrTemp);
    }
  }
  return out;
}
28

It's WORKING!

This is all I wanted, a basic WWW interface for changing schedule and temperature on a programable thermastat:

Time Temp.

    • 0 + : - 0 + 60: +
  • 2 + : - 3 + 62: +
  • 4 + : - 6 + 64: +
  • 6 + : - 9 + 66: +
29

So for others to learn, what did you need to fix?

30

I had to allocate enough space for all the strings to fit.