Uno resets when using Serial.print() - memory problem?

Projects Programming
1

Hello,

I'm writing a handling sketch for a DFRobot LCD KLeypad Shield, to be run on an Uno.

The sketch has quite a lot of text options to show on the LCD and I started getting compiler warnings along the lines of "memory low - stability issues may occur".

To try to fix this, I've moved all the text options (which were in a 15x4x2 char* array) into program memory using PROGMEM, like this:

PROGMEM const char str_00[] = "Sensors      ";
PROGMEM const char str_01[] = "Actuators    ";
PROGMEM const char str_02[] = "Control      ";

PROGMEM const char str_10[] = "Position     ";
PROGMEM const char str_11[] = "Light        ";
PROGMEM const char str_12[] = "Temperature  ";
PROGMEM const char str_13[] = "Range        ";

PROGMEM const char str_20[] = "[Solenoid]   ";

...etc...

PROGMEM const char str_D1[] = "[Count]      ";
PROGMEM const char str_D2[] = "[Degrees]    ";
PROGMEM const char str_D3[] = "[RPM]        ";

PROGMEM const char str_E0[] = "[Open-Loop]  ";
PROGMEM const char str_E1[] = "[On/Off]     ";
PROGMEM const char str_E2[] = "[PID]        ";

...plus some strings that aren't unique:

PROGMEM const char str_BL[] = "             ";

PROGMEM const char str_RT[] = "  >";
PROGMEM const char str_LT[] = "<  ";
PROGMEM const char str_RL[] = "< >";
PROGMEM const char str_NA[] = "   ";

...and then constructed an array to hold them all:

PROGMEM const char* const options[15][4][2] = {{{str_00,str_RT},{str_01,str_RT},{str_02,str_RT},{str_BL,str_NA}},
                                               {{str_10,str_RL},{str_11,str_RL},{str_12,str_RL},{str_13,str_RL}},
                                               {{str_20,str_LT},{str_21,str_LT},{str_22,str_LT},{str_BL,str_NA}},
                                               {{str_30,str_RL},{str_31,str_RL},{str_BL,str_NA},{str_BL,str_NA}},
                                               {{str_40,str_LT},{str_41,str_LT},{str_BL,str_NA},{str_BL,str_NA}},
                                               {{str_50,str_LT},{str_51,str_LT},{str_52,str_LT},{str_53,str_LT}},
                                               {{str_60,str_RL},{str_61,str_RL},{str_BL,str_NA},{str_BL,str_NA}},
                                               {{str_70,str_RL},{str_71,str_RL},{str_72,str_RL},{str_BL,str_NA}},
                                               {{str_80,str_LT},{str_81,str_LT},{str_82,str_LT},{str_BL,str_NA}},
                                               {{str_90,str_LT},{str_91,str_LT},{str_BL,str_NA},{str_BL,str_NA}},
                                               {{str_A0,str_LT},{str_A1,str_LT},{str_BL,str_NA},{str_BL,str_NA}},
                                               {{str_B0,str_LT},{str_B1,str_LT},{str_B2,str_LT},{str_BL,str_NA}},
                                               {{str_C0,str_LT},{str_C1,str_LT},{str_C2,str_LT},{str_C3,str_LT}},
                                               {{str_D0,str_LT},{str_D1,str_LT},{str_D2,str_LT},{str_D3,str_LT}},
                                               {{str_E0,str_LT},{str_E1,str_LT},{str_E2,str_LT},{str_BL,str_NA}}};

...which is all from the language reference here: PROGMEM - Arduino Reference

Reading stuff out of the array is now a bit of a faff, but it works like this:

        lcd.setCursor(0,0);
        strcpy_P(cLCDBuffer, (char*)pgm_read_word(&(options[iCurMenu][iCurOption][0])));
        //lcd.print(options[iCurMenu][iCurOption][0]);
        lcd.print(cLCDBuffer);

...where iCurMenu and iCurOption are indexes updated from the keypad.

Having navigated the menu, the user can then press a SELECT button, which does the following:

     case btnSELECT:                                   //SELECT button pressed
       if(!keyPressed) {
         switch(iMode){
           case modeMENU:
             //Use the pgm macro to get the vurrent option text  grom program memory
             strcpy_P(cLCDBuffer, (char*)pgm_read_word(&(options[iCurMenu][iCurOption][0])));
             //if(options[iCurMenu][iCurOption][0][0] == '[') {
             //check the first character of the current option string
             if(cLCDBuffer[0] == '['){
               breadcrumbs[iBCIndex][0] = iCurMenu;    //Add the current menu to the breadcrumb array...
               breadcrumbs[iBCIndex][1] = iCurOption;  //...and the option selected
               iBCIndex++;                             //Increment the breadcrumb array index
               
               Serial.print("SELECT:");
               
               for(int i=0;i<iBCIndex;i++) {
                 Serial.print(breadcrumbs[i][0]);
                 Serial.print(",");
                 Serial.print(breadcrumbs[i][1]);
                 Serial.print(":");
                 strcpy_P(cLCDBuffer, (char*)pgm_read_word(&options[breadcrumbs[i][0]] [breadcrumbs[i][1]] [0]));
                 Serial.print(cLCDBuffer);
               }

...the menu and option selections are tracked in a breadcrumbs[10][2] array (10x [menu you were in][option you selected]), so here, just to check it's all worked OK, I'm looping through that array, picking out the text from the options[15][4][2] array and echoing to the serial monitor.

Now, here's the problem - this worked fine before I put the options array (and all my other arrays) in program memory. Whilst the menu system itself works (the LCD displays as expected and the keypad responds as expected), this reporting to the serial monitor resets the Uno.

It gets as far echoing:

SE⸮

...where the weird character is a square shape on the LCD and either jumps back to the start of the menu routine or completely resets the Uno.

What I think might be happening is whatever buffer the serial.print function uses (which I read somewhere is in dynamic RAM) is somehow colliding with other stuff in RAM that shouldn't be there, but is, because I've used PROGMEM to move it about...or something. To be honest, I haven't got a clue why it's doing this, but it wasn't before I started using PROGMEM...

I've attached my entire code, but it's quite long to go over line by line here - I can try to explain what I was hoping it would do if anybody has the time to have a look at it for me...

Sorry this is a "my code's broken, can somebody please fix it for me" request, but I've been bashing away at this for days and just going round in circles :frowning:

Thanks for any help at all!

:slight_smile:

LCD_Keypad_Shield_Menu_02.ino (28.9 KB)

2

The first thing that I would check is that you are not exceeding any array bounds and writing on memory outside of the array

3

Thanks for that - it's a good point and I wouldn't be at all surprised if I'm writing outside of the bounds of an array somewhere, but why would this only become apparent during a Serial.print()?

Even if I comment out everything except the Serial.print("SELECT:"); in my code, it still crashes, despite not attempting to access any array data...

<pulling_hair_out_smiley_yet_to_be_defined>

4

Now, here's the problem - this worked fine before I put the options array (and all my other arrays) in program memory.

If it used to work before you moved the data, and now doesn't, doesn't it seem likes that you are accessing the data incorrectly?

How much memory did you save moving the 120 pointers into PROGMEM? Was that savings worth the effort required to now access the data?

Your complete code is missing, but it seems clear that there is far too much of it, when you don't KNOW that you can access the data properly.

5

Hi Paul,

thanks very much for your thoughts on this problem.

Before moving all my text into PROGMEM, I was using 75% of dynamic memory; afterwards, I'm using about 25%, so the saving is significant.

I'll admit, moving all my constants into PROGMEM is probably overkill and I think having my array of function pointers in PROGMEM stops it from working altogether (so I moved it back out again), but with only 2048 bytes of dynamic RAM to play with, every byte counts!

I'm not sure it's a case of accessing the data incorrectly - I haven't changed the access method (i.e. Serial.print()) between the non-PROGMEM version and the PROGMEM version. It's a bit of a moot point through, really, as without moving the options array into PROGMEM, there's not enough dynamic RAM for the program to run at all...

Are you logged into the forum? My code should appear at the bottom of my first post, next to a paperclip icon - the link is LCD_Keypad_Shield_Menu_02.ino . I'm not sure I follow you when you say I don't know I can access the data properly - the LCD.print routines work fine, it's just the serial.print that fails.

Thanks for your time on this - it's very much appreciated :slight_smile:

6

Before moving all my text into PROGMEM, I was using 75% of dynamic memory; afterwards, I'm using about 25%, so the saving is significant.

Yes, moving the strings was important. But, you added pointers to the strings, which point to the location in PROGMEM, and then you moved the pointers into PROGMEM. THAT saved very little SRAM, for all the extra work you need to do, fetching first the pointer and then fetching the pointed to data.

I'm not sure it's a case of accessing the data incorrectly - I haven't changed the access method (i.e. Serial.print()) between the non-PROGMEM version and the PROGMEM version.

Serial.print() knows how to print data from SRAM. It does not know how to print data from PROGMEM. So, first you must access the data for it to print. But, that data is in PROGMEM, and so is the pointer to it. So, you need to get the pointer from PROGMEM, and then you need to get the data that it points to from PROGMEM. So, it IS a matter of accessing the data incorrectly.

7

Hi Paul,

thanks for replying so quickly - I'm sure I'll get to the bottom of this in no time with your help!

You're quite right about serial.print() printing only from SRAM, which is exactly what this line addresses:

strcpy_P(cLCDBuffer, (char*)pgm_read_word(&(options[iCurMenu][iCurOption][0])));

...it reads the text string out of the options array and places it in a buffer (cLCDBuffer) in SRAM - I then either LCD.print or Serial.print the buffer.

There were two reasons I moved all the strings in the options menu into PROGMEM individually and then put the pointers in the options array; firstly, I'd tried just moving the array of strings into PROGMEM and kept getting the error variable options must be constant to be moved into PROGMEM or something roughly like that - I can't remember the exact wording. This was despite the options array being declared constant, so I assumed there was some sort of non-obvious error with the way I was doing it. Secondly, the PROGMEM language reference page I then followed does it this way, so I assumed this was the way you had to do it - it certainly works for the LCD.print routine.

Thanks for your continued help with this :slight_smile:

8

It gets as far echoing:

    SE⸮

I agree with others: it looks like you exceeded array bounds and stepped on the RAM being printed. The two good characters you can see are safely written into the UART and eventually arrive at the PC. They would not be affected by RAM writes.

I would add to the other suggestions that you call Serial.flush() after each debug print section. This will wait for the debug prints to finish before the sketch continues:

             breadcrumbs[iBCIndex][0] = iCurMenu;    //Add the current menu to the breadcrumb array...
               breadcrumbs[iBCIndex][1] = iCurOption;  //...and the option selected
               iBCIndex++;                             //Increment the breadcrumb array index
               
               Serial.print("SELECT:");
               Serial.flush();
               
               for(int i=0;i<iBCIndex;i++) {
                 Serial.print(breadcrumbs[i][0]);
                 Serial.flush();
                 Serial.print(",");
                 Serial.print(breadcrumbs[i][1]);
                 Serial.flush();
                 Serial.print(":");
                 strcpy_P(cLCDBuffer, (char*)pgm_read_word(&options[breadcrumbs[i][0]] [breadcrumbs[i][1]] [0]));
                 Serial.print(cLCDBuffer);
                 Serial.flush();
               }

You should add Serial.print + flush to display array indices before you access the arrays.

9

Thanks for that - I'll definitely check all my array bounds and assignments again to make sure I'm not making a mess of that somewhere.

I feel a bit thick for having never come across Serial.flush() before, but I'll give that a go, too and see what difference it makes - unfortunately, I haven't got an Arduino handy at the moment to upload my code to and try it...

Thanks very much for your help with this :slight_smile:

10

Ah Ha! That was it - adding serial.flush fixed the serial.print problem - then I could see an errant bit of maths was setting an array index to -1 and that was what was causing the Uno to reset.

Thanks very much indeed -dev and all for your help; I don't know how long it would have taken me to figure out flushing the buffer was the solution :slight_smile:

:slight_smile:

11

Thanks very much indeed -dev and all for your help; I don't know how long it would have taken me to figure out flushing the buffer was the solution

It wasn't the solution. Fixing the array index problem was. Flushing the buffer simply made debugging more effective.

Of course, defensive coding would not have allowed the problem to occur in the first place.

EVERY place where I write to an array, I make sure that the index is in range. You should get into that habit, too, especially as you have now been burned by not doing that.

12

Absolutely - you may consider me suitably chastened.

Many thanks for your helpful input.

:slight_smile: