Broke my code using PROGMEM

Hi all,

I have been working on a menu system and got it working nicely, but decided it needed to store the strings in PROGMEM. I have managed to set up PROGMEM ok, and can Serial.print the buffer, but I simply cannot get the string into the display menu function? It just displays random characters. I know that you have to handle the strings differently but i'm stumped!

Any help would be very much appreciated, as I have now spent 2 days reading the various pages covering PROGMEM and can't work out where to start.

#include <Adafruit_GFX.h>
#include <Adafruit_SSD1306.h>

#define maxItemSize 16

const int itemsPerScreen = 9;
const int fontSize = 8;


static int pinA = 2; // Our first hardware interrupt pin is digital pin 2
static int pinB = 3; // Our second hardware interrupt pin is digital pin 3
static int enSW = 9;

volatile byte aFlag = 0; // let's us know when we're expecting a rising edge on pinA to signal that the encoder has arrived at a detent
volatile byte bFlag = 0; // let's us know when we're expecting a rising edge on pinB to signal that the encoder has arrived at a detent (opposite direction to when aFlag is set)
volatile uint16_t encoderPos = 0; //this variable stores our current value of encoder position. Change to int or uin16_t instead of byte if you want to record a larger range than 0-255
volatile uint16_t
oldEncPos = 0; //stores the last encoder position value so we can compare to the current reading and see if it has changed (so we know when to print to the serial monitor)
volatile byte reading = 0; //somewhere to store the direct values we read from our interrupt pins before checking to see if we have moved a whole detent

#define maxItemSize 11
#define OLED_RESET 4
Adafruit_SSD1306 display(OLED_RESET);
 
PROGMEM const char Messaging[] ="Messaging";
PROGMEM const char UVScanner[] ="UV Scanner";
PROGMEM const char SecuritySystem[] ="Security";
PROGMEM const char BACK[] ="Main Menu";

int cnt = 0; 
int itemSelected, subMenuSelected;
int itemsToDisplay=0;

const char* const menu_table[] PROGMEM = {Messaging, UVScanner, SecuritySystem, BACK};


char menu[50];    // make sure this is large enough for the largest string it must hold



void setup() {

  display.begin();
  pinMode(enSW, INPUT_PULLUP); 
  pinMode(pinA, INPUT_PULLUP); 
  pinMode(pinB, INPUT_PULLUP);
  attachInterrupt(0,PinA,RISING);
  attachInterrupt(1,PinB,RISING); 
  display.clearDisplay();  
  display.setTextSize(fontSize/8);
  display.setTextColor(WHITE);
  display.println("Initialising...");
  display.display();
  Serial.begin(9600);
  while(!Serial);//wait for serial to connect
  
  for (int i = 0; i < 4; i++)  //Build menu table
  {
    strcpy_P(menu, (char*)pgm_read_word(&(menu_table[i]))); // Necessary casts and dereferencing, just copy.
    Serial.println(menu);
   delay( 250 );
  }
  

  
}

void loop() {
  
  

  display.setTextSize(1);
  display.clearDisplay();
  display.setCursor(25,0);
  display.println(F("Menu System"));
  display.setCursor(20,25);
  display.println(F("Press control to  enter the menu system"));
  display.display();
  display.setTextSize(fontSize/8);

  
 // Enter the settings menu if select Switch is pressed
 if(digitalRead(enSW)==0){
    while(digitalRead(enSW)==0);//wait till switch is released.
    
    itemSelected = displayMenu(*menu,sizeof(menu)/maxItemSize ); 
    switch(itemSelected){
        
         case 0: 
             // subMenuSelected = displayMenu(subMenu0, sizeof(subMenu0)/maxItemSize);
                break;
                
         case 1:
             //   subMenuSelected = displayMenu(subMenu1, sizeof(subMenu1)/maxItemSize);
                break;
                
         case 2:
             //   subMenuSelected = displayMenu(subMenu2, sizeof(subMenu1)/maxItemSize);
                break;
  
         default: break;       
              
     }
      
   }   
    
 }

 




// This function accepts the a 2D character array and its length and display it on the screen.
// The function montiers the encoder position and moves the menu up and down.
// It returns the selected menu option to the calling functions
     
int displayMenu(char *menuInput[], int menuLength){
    Serial.println(menuLength);
    int curPos,startPos, endPos;
 do{ 
      
            startPos = encoderPos%menuLength;    
            display.clearDisplay();
      
            endPos = itemsPerScreen;
            
            if(menuLength < itemsPerScreen)
            {
                 endPos = menuLength -startPos;  
            }
      
            if((menuLength-startPos)<itemsPerScreen)
            {
                endPos = menuLength -startPos;
            }

      
            for(cnt = 0; cnt<=(endPos-1); cnt++){
                if(cnt == 0)
                {
                  display.setCursor(0,0);
                  display.print(F("Scroll to select"));
                  display.drawFastHLine(0,10,200,WHITE);
                  display.setCursor(0,16);// sets the selection pointer
                  display.print(F("->"));
                }
                
                display.setCursor(16,16 + cnt*fontSize);// sets the height of the submenu text
                display.println(*menuInput[cnt+startPos]);
                
            }
            
            display.display();
            cnt = 0;

          if(oldEncPos != encoderPos) {
            oldEncPos = encoderPos;   
          }  
 }while(digitalRead(enSW));
 while(digitalRead(enSW)==0); //wait till switch is reseleased 
 return startPos;
}

/*******************************Utility Functions *******************************/
 


void PinA(){
  cli(); //stop interrupts happening before we read pin values
  reading = PIND & 0xC; // read all eight pin values then strip away all but pinA and pinB's values
  if(reading == B00001100 && aFlag) { //check that we have both pins at detent (HIGH) and that we are expecting detent on this pin's rising edge
    encoderPos --; //decrement the encoder's position count
    bFlag = 0; //reset flags for the next turn
    aFlag = 0; //reset flags for the next turn
  }
  else if (reading == B00000100) bFlag = 1; //signal that we're expecting pinB to signal the transition to detent from free rotation
  sei(); //restart interrupts
}


void PinB(){
  cli(); //stop interrupts happening before we read pin values
  reading = PIND & 0xC; //read all eight pin values then strip away all but pinA and pinB's values
  if (reading == B00001100 && bFlag) { //check that we have both pins at detent (HIGH) and that we are expecting detent on this pin's rising edge
    encoderPos ++; //increment the encoder's position count
    bFlag = 0; //reset flags for the next turn
    aFlag = 0; //reset flags for the next turn
  }
  else if (reading == B00001000) aFlag = 1; //signal that we're expecting pinA to signal the transition to detent from free rotation
  sei(); //restart interrupts
}
const char* const menu_table[] PROGMEM = {Messaging, UVScanner, SecuritySystem, BACK};

By putting the pointers in PROGMEM, you've saved 8 bytes of SRAM and doubled the level of effort required to fetch the data. A fair trade? I doubt it.

Ah, I have cut the rest of the arrays out of the code until I get the first level of menu working. There is a fair amount of strings in the next levels of menus. I thought it was best to get the first layer working then add in the rest.

So this code is no way near the final amount of PROGMEM usage. The dynamic memory according to the compiler drops from 96% down to 70% ish.

Cheers

The pointers are not that big so putting them in RAM isn't that bad and makes it a lot easier to read it back. It's the strings you want in PROGMEM.

And uhm

#include <avr/pgmspace.h>

:wink:

And instead of copying the string to RAM and then print it you can print it from RAM. Just cast the pointer to it in PROGMEM as __FlashStringHelper*

Woops, <avr/pgmspace.h> didn't survive the cut and paste from the longer code into the post.

Would it be possible to get an example of how i'd use __FlashStringHelper*

I have no idea how to use that to get the strings into the menu.

Plus, are you saying that I have things in PROGMEM that don't need to be there? I followed an online example and thought i was on the right track.

Cheers

Yes. Put the strings in PROGMEM. The pointers which point to the strings don't have to be in PROGMEM. Usually it's better to keep those in SRAM.

Larp-fx:
Would it be possible to get an example of how i'd use __FlashStringHelper*

I have a number of these sprinkled around.

if (activeScreen == menuMode) { // mode display
    lcd.print( (__FlashStringHelper*) &indexModeTextP[textAndParms[activeScreen].parmVal]);
  }         //                       /  ^   ^   ^  address of  the print data  ^   ^   ^ 
//                 address operator /

// In this example the LCD print position has been previously set.

I have been trying various things over the last 24 hours and have made some tweaks to the code.

It still doesn't work, but it does do more. It now does the following:

When you fire it up it prints char menu to the serial monitor, and that displays the four strings nicely.

You then press the rotary encoder button and it should display those 4 menu options on the oled. It doesn't?

All I get is four lines each showing "Main menu"

I can scroll through them, but I cannot for the life of me work out why it won't display the other options?

I have also tried removing the "PROGMEM" from menu_table, which I thought would then NOT store the pointers in PROGMEM, but that gives me an error? I know someone suggested only storing the strings in progmem and not the pointers. Tips on how to sort that would be great?

Also, as a side note, could someone tell me why i get the same 4 options printed to the serial port regardless of what number i put in the [] box after the char menu? If i leave the box blank i get an error, but I was under the impression that this had to at least equal the number of characters in the longest sting I have stored?

New code:

#include <Adafruit_GFX.h>
#include <Adafruit_SSD1306.h>
#include <avr/pgmspace.h>

#define maxItemSize 11

const int itemsPerScreen = 9;
const int fontSize = 8;


static int pinA = 2; // Our first hardware interrupt pin is digital pin 2
static int pinB = 3; // Our second hardware interrupt pin is digital pin 3
static int enSW = 9;

volatile byte aFlag = 0; // let's us know when we're expecting a rising edge on pinA to signal that the encoder has arrived at a detent
volatile byte bFlag = 0; // let's us know when we're expecting a rising edge on pinB to signal that the encoder has arrived at a detent (opposite direction to when aFlag is set)
volatile uint16_t encoderPos = 0; //this variable stores our current value of encoder position. Change to int or uin16_t instead of byte if you want to record a larger range than 0-255
volatile uint16_t
oldEncPos = 0; //stores the last encoder position value so we can compare to the current reading and see if it has changed (so we know when to print to the serial monitor)
volatile byte reading = 0; //somewhere to store the direct values we read from our interrupt pins before checking to see if we have moved a whole detent

#define OLED_RESET 4
Adafruit_SSD1306 display(OLED_RESET);
 
PROGMEM const char Messaging[] ="Messaging";
PROGMEM const char UVScanner[] ="UV Scanner";
PROGMEM const char SecuritySystem[] ="Security";
PROGMEM const char BACK[] ="Main Menu";

int cnt = 0; 
int itemSelected, subMenuSelected;
int itemsToDisplay=0;

const char* const menu_table[] PROGMEM = {Messaging, UVScanner, SecuritySystem, BACK};


char menu[50];    // make sure this is large enough for the largest string it must hold



void setup() {

  display.begin();
  pinMode(enSW, INPUT_PULLUP); 
  pinMode(pinA, INPUT_PULLUP); 
  pinMode(pinB, INPUT_PULLUP);
  attachInterrupt(0,PinA,RISING);
  attachInterrupt(1,PinB,RISING); 
  display.clearDisplay();  
  display.setTextSize(fontSize/8);
  display.setTextColor(WHITE);
  display.println("Initialising...");
  display.display();
  Serial.begin(9600);
  while(!Serial);//wait for serial to connect
  
  for (int i = 0; i < 4; i++)  //Build menu table
  {
    strcpy_P(menu, (char*)pgm_read_word(&(menu_table[i]))); // Necessary casts and dereferencing, just copy.
    Serial.println(menu);
   delay( 250 );
  }
  

  
}

void loop() {
  
  

  display.setTextSize(1);
  display.clearDisplay();
  display.setCursor(25,0);
  display.println(F("Menu System"));
  display.setCursor(20,25);
  display.println(F("Press control to  enter the menu system"));
  display.display();
  display.setTextSize(fontSize/8);

  
 // Enter the settings menu if select Switch is pressed
 if(digitalRead(enSW)==0){
    while(digitalRead(enSW)==0);//wait till switch is released.
    
    itemSelected = displayMenu(menu ,sizeof(menu)/maxItemSize ); 
  
    switch(itemSelected){
        
         case 0: 
             // subMenuSelected = displayMenu(subMenu0, sizeof(subMenu0)/maxItemSize);
                break;
                
         case 1:
             //   subMenuSelected = displayMenu(subMenu1, sizeof(subMenu1)/maxItemSize);
                break;
                
         case 2:
             //   subMenuSelected = displayMenu(subMenu2, sizeof(subMenu1)/maxItemSize);
                break;
  
         default: break;       
              
     }
      
   }   
    
 }

 




// This function accepts the a 2D character array and its length and display it on the screen.
// The function montiers the encoder position and moves the menu up and down.
// It returns the selected menu option to the calling functions
     
int displayMenu(char *menuInput, int menuLength){
    Serial.println("debug at selection");
    int curPos,startPos, endPos;
 do{ 
      
            startPos = encoderPos%menuLength;    
            display.clearDisplay();
      
            endPos = itemsPerScreen;
            
            if(menuLength < itemsPerScreen)
            {
                 endPos = menuLength -startPos;  
            }
      
            if((menuLength-startPos)<itemsPerScreen)
            {
                endPos = menuLength -startPos;
            }

      
            for(cnt = 0; cnt<=(endPos-1); cnt++){
                if(cnt == 0)
                {
                  display.setCursor(0,0);
                  display.print(F("Scroll to select"));
                  display.drawFastHLine(0,10,200,WHITE);
                  display.setCursor(0,16);// sets the selection pointer
                  display.print(F("->"));
                }
                
                display.setCursor(16,16 + cnt* fontSize );// sets the height of the submenu text
                display.print(menuInput);//[cnt+startPos] <<WTF does this do?
                
                
            }
            
            display.display();
            cnt = 0;

          if(oldEncPos != encoderPos) {
            oldEncPos = encoderPos;   
          }  
 }while(digitalRead(enSW));
 while(digitalRead(enSW)==0); //wait till switch is reseleased 
 return startPos;
}

/*******************************Utility Functions *******************************/
 


void PinA(){
  cli(); //stop interrupts happening before we read pin values
  reading = PIND & 0xC; // read all eight pin values then strip away all but pinA and pinB's values
  if(reading == B00001100 && aFlag) { //check that we have both pins at detent (HIGH) and that we are expecting detent on this pin's rising edge
    encoderPos --; //decrement the encoder's position count
    bFlag = 0; //reset flags for the next turn
    aFlag = 0; //reset flags for the next turn
  }
  else if (reading == B00000100) bFlag = 1; //signal that we're expecting pinB to signal the transition to detent from free rotation
  sei(); //restart interrupts
}


void PinB(){
  cli(); //stop interrupts happening before we read pin values
  reading = PIND & 0xC; //read all eight pin values then strip away all but pinA and pinB's values
  if (reading == B00001100 && bFlag) { //check that we have both pins at detent (HIGH) and that we are expecting detent on this pin's rising edge
    encoderPos ++; //increment the encoder's position count
    bFlag = 0; //reset flags for the next turn
    aFlag = 0; //reset flags for the next turn
  }
  else if (reading == B00001000) aFlag = 1; //signal that we're expecting pinA to signal the transition to detent from free rotation
  sei(); //restart interrupts
}

If you leave the array in memory, the first bit just becomes

for(byte i = 0; i < 4, i++){
    Serail.printlln((__FlashStringHelper*)&menu_table[i])
  }

And for the menu on the screen, why use a do while? Now you keep on drawing the same while the button is pressed.

And about the problem, in displayMenu() you use menuInput which is just the global menu. menu is only filled as a test in setup() and is last filled (from the start!) with "Main Menu". That's all there is in menu, not "Messaging UV Scanner Security Main Menu". That also would not make sense. Then you have the text in PROGMEM and the first thing you do is load them all in RAM. Consuming as much RAM (even more) as not using PROGMEM at all.

Try to print them the same way as in setup() or easier, as I showed you.

Hi, Thanks for the reply. Just so that I get it clear in my head before I go messing around in the code too much more i would do......

Change this:

const char* const menu_table[] PROGMEM = {Messaging, UVScanner, SecuritySystem, BACK};

to this:

const char* const menu_table[]  = {Messaging, UVScanner, SecuritySystem, BACK};

Then I would change this bit of code:

for (int i = 0; i < 4; i++)  //Build menu table
  {
    strcpy_P(menu, (char*)pgm_read_word(&(menu_table[i]))); // Necessary casts and dereferencing, just copy.
    Serial.println(menu);
   delay( 250 );
  }

to this:

for(byte i = 0; i < 4; i++){
    Serial.println((__FlashStringHelper*)&menu_table[i]);
  }

Is that correct?

Yes, that way it should still make the debug Serial print on startup.

Damn, that just prints random characters to the serial monitor on start up.

I know it must be something simple, but I am blind for looking now. Been staring at it for days now.

Cheers

Larp-fx:

for(byte i = 0; i < 4; i++){

Serial.println((__FlashStringHelper*)&menu_table[i]);
 }




Is that correct?

No. Drop the &.

   Serial.println((__FlashStringHelper*)menu_table[i]);

Thankyou so very very much.

New it was going to be one of those simple things that you just can't see.

That part of the code works beautifully now.

Time to start struggling with the rest of it now.

Cheers

@oqibidipo, you're right!

If you add this function

inline __FlashStringHelper* PtoF(const  char * const p){
  return (__FlashStringHelper*) p;
}

it becomes even simpler:

Serial.println(PtoF(menu_table[i]));

Not that it really because simpler, but I think an easy function name is easier to remember/type.

And the Adafruit GFX library just extends the Print class as well so it should be able to handle the __FlashStringHelper as well (and thus PtoF()).

[/quote]

septillion:
And for the menu on the screen, why use a do while? Now you keep on drawing the same while the button is pressed.

And about the problem, in displayMenu() you use menuInput which is just the global menu. menu is only filled as a test in setup() and is last filled (from the start!) with "Main Menu". That's all there is in menu, not "Messaging UV Scanner Security Main Menu". That also would not make sense. Then you have the text in PROGMEM and the first thing you do is load them all in RAM. Consuming as much RAM (even more) as not using PROGMEM at all.

Try to print them the same way as in setup() or easier, as I showed you.

Could you explain this bit a little for me, I don't understand totally? Thanks

First or second part?

Both please, if you would be so kind.

Now that I can print the arrays using flashstringhelper, I assume that there's a better way to construct the menu tree.

thank you

  1. One the menu is drawn, don't redraw it unless you change something. So draw and wait.

Better option: draw menu and wait in a while() doing nothing on user input
Best option: don't make blocking code. Let the loop() loop. And when you want to display the menu, draw it and return to the loop() and remember the menu is open. If there is user input, act on it (redraw (parts), go to other menu etc) but return to the loop() again.

  1. You know try to use the global variable menu to print the menu on the display. But there is just one place you put anything in menu and that's in setup().
 for (int i = 0; i < 4; i++)  //Build menu table
  {
    strcpy_P(menu, (char*)pgm_read_word(&(menu_table[i]))); // Necessary casts and dereferencing, just copy.
    Serial.println(menu);
   delay( 250 );
  }

And each loop of that for-loop will start at the beginning of menu. So after the first loop it holds "Messaging", after the second "UV Scanner", after the third "Security" and after the last "Main Menu". So when you enter the loop() all menu contains is "Main Menu". Simply, don't use the menu varaible in your program, it's not needed and be declaring it the memory saving of PROGMEM is undone because it will consume 50 bytes...

Larp-fx:
Now that I can print the arrays using flashstringhelper, I assume that there's a better way to construct the menu tree.

Yes, the same way you printed to Serial :wink:

display.println((__FlashStringHelper*)menu_table[0]); //etc
//or
display.println(PtoF(menu_table[0]));

septillion:

display.println((__FlashStringHelper*)menu_table[0]); //etc

//or
display.println(PtoF(menu_table[0]));

I assume that i'll need to call the four items for the first menu one at a time when I draw the menu? or is there a better way?