PROGMEM issues, again

Using Arduino Programming Questions
#1

In a quest to back away from a 60% ram usage message I finally investigated PROGMEM. After a lot of stumbling and assistance I managed to get some strings into PROGMEM and bring them out for display on my 16x2 LCD. The template to create them is copied from Nick Gammon’s forum.

My next target is a list of error messages. The codes defining each type of error are fixed in the VL6180X TOF sensor. As can be seen below, these codes are not sequential. To relate error code to message text I created a 2D array (of structures) using the received error code as the argument for a search loop. It partly works - the loop will retrieve, and I can print to the LCD, the correct error code. The lcd.print() however, shows nothing. My latest attempt, shown below, is lifted from the AVR user manual.

Creation of the data and a short bit to test the function call

//  LASER ERROR MESSAGE TEXT
//
typedef struct  {         // create structure
  int8_t errorCode;        // 6180 error code
  char* errorText[22];    // error description
} laserErr_t;

const char LasErrSt0[] PROGMEM = "System Error";
const char LasErrSt1[] PROGMEM = "ECE Failure";
const char LasErrSt2[] PROGMEM = "Convergence fail";
const char LasErrSt3[] PROGMEM = "Range Ignore";
const char LasErrSt4[] PROGMEM = "SNR Error";
const char LasErrSt5[] PROGMEM = "Raw Underflow";
const char LasErrSt6[] PROGMEM = "Raw Overflow";
const char LasErrSt7[] PROGMEM = "Range Underflow";
const char LasErrSt8[] PROGMEM = "Range Overflow";

const laserErr_t laserMsg[2][9] PROGMEM = {
  1, LasErrSt0,
  6, LasErrSt1,
  7, LasErrSt2,
  8, LasErrSt3,
  11, LasErrSt4,
  12, LasErrSt5,
  13, LasErrSt6,
  14, LasErrSt7,
  15, LasErrSt8,
  };
byte noOfErrMsgs = (sizeof (laserMsg) / sizeof (laserErr_t)) / 2;
/*
    END VARIABLE DECLARATIONS  --  SETUP ROUTINE
*/
void setup() {
  Serial.begin(9600); // for debug only
  lcd.begin(16, 2);// open comms. with 1602 display unit
  lcd.createChar(0, ramAtTop);
  lcd.createChar(1, ramAtLoad);
  lcd.createChar(2, ramAtBottom);
  lcd.createChar(3, rotateCCW);
  lcd.createChar(4, rotateCW);
  // diagnostic_display();
  status = 13;
  displayLaserError(status);
  while (1);

Function to print error messages

// variable i will hold index to code and text

// print buffer is declared as: char lcdPrintBufr[22];

void displayLaserError(int errNum) {
  for ( int i = 0; i < noOfErrMsgs; i++) { // nine error messges currently
    if (errNum == pgm_read_byte(&laserMsg[0][i])) {
      memcpy_P(&lcdPrintBufr, (&laserMsg[1][i]), sizeof lcdPrintBufr);
      //(&(mydata[i][j]));
      break;
    }
    lcd.clear();
    lcd.print("Laser Err no. ");
    lcd.print(errNum);
    lcd.setCursor(0, 01);
   //  memcpy_P(&lcdPrintBufr, &laserMsg[1][1], sizeof lcdPrintBufr);
     lcd.print(lcdPrintBufr);
  }
}

No doubt it’s something blindingly simple but I’m out of ideas. Throw me a bone?

#2

I didn't study whole code, just after brief look: you have &lcdPrintBufr in memcpy but the pointer to the lcdPrintBufr[22] array is just lcdPrintBufr or &lcdPrintBufr[0].
Another problem is using the memcpy instead of strcpy. The progmem strings are defined with variable length, it means the string ends with the NULL character and it is not defined behind. You have to align buffered string separately.

#3

I'm pretty sure you don't want 22 character pointers in your structure. One is sufficient.

typedef struct  {         // create structure
  int8_t errorCode;        // 6180 error code
  char* errorText;    // error description
} laserErr_t;

And why have two structures per message?

const laserErr_t laserMsg[2][9] PROGMEM = {
  1, LasErrSt0,
  6, LasErrSt1,
  7, LasErrSt2,
  8, LasErrSt3,
  11, LasErrSt4,
  12, LasErrSt5,
  13, LasErrSt6,
  14, LasErrSt7,
  15, LasErrSt8,
  };

Shouldn't this be more like:

const laserErr_t laserMsg[9] PROGMEM = {
  {1, LasErrSt0},
  {6, LasErrSt1},
  {7, LasErrSt2},
  {8, LasErrSt3},
  {11, LasErrSt4},
  {12, LasErrSt5},
  {13, LasErrSt6},
  {14, LasErrSt7},
  {15, LasErrSt8}
  };
#4

Budvar10:
I didn't study whole code, just after brief look: you have &lcdPrintBufr in memcpy but the pointer to the lcdPrintBufr[22] array is just lcdPrintBufr or &lcdPrintBufr[0].
Another problem is using the memcpy instead of strcpy. The progmem strings are defined with variable length, it means the string ends with the NULL character and it is not defined behind. You have to align buffered string separately.

This is part of the already working code:

typedef struct {
  char stationtext[22];
} stationlist;
const stationlist turretPosnTextP[] PROGMEM = {"Crimp",
                                               "Bullet Seat",
                                               "Powder Drop",
                                               "Prime/Deprime",
                                               "Raise to Unload ",
                                               "Lower to Unload " //final space erases turret char
                                              };
int8_t turretPosnNoParms_s = sizeof(turretPosnTextP) / 2;
//
// and this correctly accesses and displays the above strings
//
memcpy_P (&lcdPrintBufr, &turretPosnTextP[textAndParms[screenNow].parmVal], sizeof lcdPrintBufr);
lcd.print(lcdPrintBufr);

I don't understand how this is different from the non-working stuff.

Also, this line:

 strcpy_P(&lcdPrintBufr, (&laserMsg[1][i]), sizeof lcdPrintBufr);

Generates the error code:

laser_error:5: error: cannot convert 'char ()[22]' to 'char' for argument '1' to 'char* strcpy_P(char*, const char*)'

strcpy_P(&lcdPrintBufr, (&laserMsg[1]), sizeof lcdPrintBufr);

  • ^*
#5

You can .print() direct from PROGMEM. Just cast the PROGMEM address as (__FlashStringHelper*)

void displayLaserError(int errNum) {
  char * message;  // PROGMEM address of message
  
  lcd.clear();
  lcd.print("Laser Err no. ");
  lcd.print(errNum);

  for ( int i = 0;  i < noOfErrMsgs;  i++) { // nine error messges currently
    if (errNum == pgm_read_byte(&laserMsg[i].errorCode)) {
      // Found the message that matches the error code
      message = pgm_read_word(&laserMsg[i].errorText);  // Get PROGMEM address from PROGMEM
      lcd.setCursor(0, 01);
      lcd.print( (__FlashStringHelper*) message);
      break;  // Don't bother looking for another match
    }
  }
}
#6

johnwasser:
And why have two structures per message?

Because I’m easily confused. :slight_smile: Come to think of it, that would be a good screen name.

The code is revised per your advice and will compile but still does not work.

//  LASER ERROR MESSAGE TEXT
//
typedef struct  {         // create structure
  int8_t errorCode;        // 6180 error code
  char* errorText[];    // error description
} laserErr_t;

const laserErr_t laserMsg[] PROGMEM = {
  1, "System Error",
  6, "ECE Failure",
  7, "Convergence fail",
  8, "Range Ignore",
  11, "SNR Error",
  12, "Raw Underflow",
  13, "Raw Overflow",
  14, "Range Underflow",
  15, "Range Overflow"
};
byte noOfErrMsgs = (sizeof (laserMsg) / sizeof (laserErr_t)) / 2;
/*
    END VARIABLE DECLARATIONS  --  SETUP ROUTINE
*/
void setup() {
  Serial.begin(9600); // for debug only
  lcd.begin(16, 2);// open comms. with 1602 display unit
  lcd.createChar(0, ramAtTop);
  lcd.createChar(1, ramAtLoad);
  lcd.createChar(2, ramAtBottom);
  lcd.createChar(3, rotateCCW);
  lcd.createChar(4, rotateCW);
  // diagnostic_display();
  status = 11;
  displayLaserError(status);
  while (1);

Now the search fails. With the added print statements the number 0-8 are printed on the serial monitor.
What I believe are the contents of the referenced memory locations are printed after the index.

void displayLaserError(int errNum) {
  char * message;  // PROGMEM address of message
 
  lcd.clear();
  lcd.print("Laser Err no. ");
  lcd.print(errNum);

  for ( int i = 0;  i < noOfErrMsgs;  i++) { // nine error messges currently
    Serial.print(i); 
    Serial.print(" ");
    Serial.println(pgm_read_byte(&laserMsg[i].errorCode));
    if (errNum == pgm_read_byte(&laserMsg[i].errorCode)) {
      // Found the message that matches the error code
      message = pgm_read_word(&laserMsg[i].errorText);  // Get PROGMEM address from PROGMEM
      lcd.setCursor(0, 01);
      lcd.print( (__FlashStringHelper*) message);
      //iprint = i;
      lcd.setCursor(14, 0);
      lcd.print(i);
      break;  // Don't bother looking for another match
    }
  }

Additionally, including a number in the declaration of laserMsg yields this error message - too many initializers for ‘const laserErr_t [9]’

#7 
  char* errorText[];    // error description
  • ? -
#8

[quote author=Coding Badly date=1505327854 link=msg=3411150]

  • ? - [/quote]

Brackets removed. Code now compiles and loop executes as desired. A serial monitor display from the loop with status assigned a value of ‘15’ appears as follows:

index/value from table

0 1
1 6
2 7
3 8
4 11
5 12
6 13
7 14
8 15

//  LASER ERROR MESSAGE TEXT
//
typedef struct  {         // create structure
  int8_t errorCode;        // 6180 error code
  char* errorText;    // error description
} laserErr_t;

const laserErr_t laserMsg[] PROGMEM = {
  1, "System Error",
  6, "ECE Failure",
  7, "Convergence fail",
  8, "Range Ignore",
  11, "SNR Error",
  12, "Raw Underflow",
  13, "Raw Overflow",
  14, "Range Underflow",
  15, "Range Overflow"
};
byte noOfErrMsgs = (sizeof (laserMsg) / sizeof (laserErr_t));
/*
    END VARIABLE DECLARATIONS  --  SETUP ROUTINE
*/
void setup() {
  Serial.begin(9600); // for debug only
  lcd.begin(16, 2);// open comms. with 1602 display unit
  lcd.createChar(0, ramAtTop);
  lcd.createChar(1, ramAtLoad);
  lcd.createChar(2, ramAtBottom);
  lcd.createChar(3, rotateCCW);
  lcd.createChar(4, rotateCW);
  Serial.println();
  status = 15;
  displayLaserError(status);
  while (1);

Still, the result of the function…

void displayLaserError(int errNum) {
  char * message;  // PROGMEM address of message
 
  lcd.clear();
  lcd.print("Laser Err ");
  lcd.print(errNum);

  for ( int i = 0;  i < noOfErrMsgs;  i++) { // nine error messges currently
    Serial.print(i); 
    Serial.print(" ");
    Serial.println(pgm_read_byte(&laserMsg[i].errorCode));
    if (errNum == pgm_read_byte(&laserMsg[i].errorCode)) {
      // Found the message that matches the error code
      message = pgm_read_word(&laserMsg[i].errorText);  // Get PROGMEM address from PROGMEM
      lcd.setCursor(0, 01);
      lcd.print( (__FlashStringHelper*) message);
      lcd.setCursor(14, 0);
      lcd.print(i);
      break;  // Don't bother looking for another match
    }
  }

}

is:

Laser Err 15 8
16 chrs of !<#_&c@ 8j)-S`

#9

The version below, with the changes I recommended, compiles fine for me after adding the stuff that your snippets didn’t include. I have not yet tried to run it.

// include the library code:
#include <LiquidCrystal.h>

// these constants won't change.  But you can change the size of
// your LCD using them:
const int numRows = 2;
const int numCols = 16;

// initialize the library with the numbers of the interface pins
LiquidCrystal lcd(12, 11, 5, 4, 3, 2);

const byte noOfErrMsgs = 9;

const char LasErrSt0[] PROGMEM = "System Error";
const char LasErrSt1[] PROGMEM = "ECE Failure";
const char LasErrSt2[] PROGMEM = "Convergence fail";
const char LasErrSt3[] PROGMEM = "Range Ignore";
const char LasErrSt4[] PROGMEM = "SNR Error";
const char LasErrSt5[] PROGMEM = "Raw Underflow";
const char LasErrSt6[] PROGMEM = "Raw Overflow";
const char LasErrSt7[] PROGMEM = "Range Underflow";
const char LasErrSt8[] PROGMEM = "Range Overflow";

typedef struct  {         // create structure
  int8_t errorCode;        // 6180 error code
  const char* errorText;    // error description
} laserErr_t;

const laserErr_t laserMsg[noOfErrMsgs] PROGMEM = {
  {1, LasErrSt0},
  {6, LasErrSt1},
  {7, LasErrSt2},
  {8, LasErrSt3},
  {11, LasErrSt4},
  {12, LasErrSt5},
  {13, LasErrSt6},
  {14, LasErrSt7},
  {15, LasErrSt8}
};

void displayLaserError(int errNum) {
  char * message;  // PROGMEM address of message

  lcd.clear();
  lcd.print("Laser Err no. ");
  lcd.print(errNum);

  for ( int i = 0;  i < noOfErrMsgs;  i++) { // nine error messges currently
    if (errNum == pgm_read_byte(&laserMsg[i].errorCode)) {
      // Found the message that matches the error code
      message = pgm_read_word(&laserMsg[i].errorText);  // Get PROGMEM address from PROGMEM
      lcd.setCursor(0, 01);
      lcd.print( (__FlashStringHelper*) message);
      break;  // Don't bother looking for another match
    }
  }
}

void setup() {}
void loop() {}
#10

johnwasser:
The version below, with the changes I recommended, compiles fine for me after adding the stuff that your snippets didn’t include. I have not yet tried to run it.

Thank you! The code in post #9 works as desired.

For future reference, it seems my main error was not including curly braces around the array elements. Is this correct?

const laserErr_t laserMsg[noOfErrMsgs] PROGMEM = {
  {1, LasErrSt0},
  {6, LasErrSt1},
  {7, LasErrSt2},
  {8, LasErrSt3},
  {11, LasErrSt4},
  {12, LasErrSt5},
  {13, LasErrSt6},
  {14, LasErrSt7},
  {15, LasErrSt8}
};

Also, ram usage is now down to 41%. That should leave sufficient headroom from here on out.

#11

dougp:
For future reference, it seems my main error was not including curly braces around the array elements. Is this correct?

Your THREE main errors were:

  • Not putting brackets around the initializers for each structure.
  • Declaring a two by N array to hold N structures.
  • Declaring 'errorText' as an array of 22 character pointers.
#12

Bollixed it up but good, eh?

Again, many thanks.

#13

dougp:
Bollixed it up but good, eh?

Even with decades of experience I have made similar messes when trying to figure out why a construct that made sense to me did not make sense to the compiler. We're always learning.

#14

If you don’t mind all of the messages taking up the space of the longest message you can avoid having the string constants separate from the structure initialization. (The compiler WILL generate a warning if an initializer string that is larger than the character array it is initializing.)

// include the library code:
#include <LiquidCrystal.h>

// these constants won't change.  But you can change the size of
// your LCD using them:
const int numRows = 2;
const int numCols = 16;

// initialize the library with the numbers of the interface pins
LiquidCrystal lcd(12, 11, 5, 4, 3, 2);

const byte noOfErrMsgs = 9;

typedef struct  {         // create structure
  int8_t errorCode;        // 6180 error code
  const char errorText[17];    // error description
} laserErr_t;

const laserErr_t laserMsg[noOfErrMsgs] PROGMEM = {
  {1, "System Error"},
  {6, "ECE Failure"},
  {7, "Convergence fail"},
  {8, "Range Ignore"},
  {11, "SNR Error"},
  {12, "Raw Underflow"},
  {13, "Raw Overflow"},
  {14, "Range Underflow"},
  {15, "Range Overflow"}
};

void displayLaserError(int errNum) {
  lcd.clear();
  lcd.print("Laser Err no. ");
  lcd.print(errNum);

  for ( int i = 0;  i < noOfErrMsgs;  i++) { // nine error messges currently
    if (errNum == pgm_read_byte(&laserMsg[i].errorCode)) {
      // Found the message that matches the error code
      lcd.setCursor(0, 01);
      lcd.print( (__FlashStringHelper*) &laserMsg[i].errorText);
      break;  // Don't bother looking for another match
    }
  }
}

void setup() {}
void loop() {}
#15

That's what I was trying to achieve in my initial attempts. A few extra bytes of PROGMEM don't bother me at this point. There's 32K and I've only used a third of that. I just like the look of things, with the error numbers aligned with their respective messages as literals as opposed to text contained within yet another variable.

Saved for later digestion.

#16

johnwasser:
If you don’t mind all of the messages taking up the space of the longest message you can avoid having the string constants separate from the structure initialization.

Something like this should solve even that problem…

// include the library code:
#include <LiquidCrystal.h>

// these constants won't change.  But you can change the size of
// your LCD using them:
const int numRows = 2;
const int numCols = 16;

// initialize the library with the numbers of the interface pins
LiquidCrystal lcd(12, 11, 5, 4, 3, 2);

const byte noOfErrMsgs = 9;

typedef struct  {         // create structure
  int8_t errorCode;        // 6180 error code
  uint8_t errorTextOffset;    // error description
} laserErr_t;

#define ET01 "System Error\0"
#define ET02 "ECE Failure\0"
#define ET03 "Convergence fail\0"
#define ET04 "Range Ignore\0"
#define ET05 "SNR Error\0"
#define ET06 "Raw Underflow\0"
#define ET07 "Raw Overflow\0"
#define ET08 "Range Underflow\0"
#define ET09 "Range Overflow\0"

const laserErr_t laserMsg[noOfErrMsgs] PROGMEM = {
  {1, 0},
  {6, sizeof(ET01)},
  {7, sizeof(ET01 ET02)},
  {8, sizeof(ET01 ET02 ET03)},
  {11, sizeof(ET01 ET02 ET03 ET04)},
  {12, sizeof(ET01 ET02 ET03 ET04 ET05)},
  {13, sizeof(ET01 ET02 ET03 ET04 ET05 ET06)},
  {14, sizeof(ET01 ET02 ET03 ET04 ET05 ET06 ET07)},
  {15, sizeof(ET01 ET02 ET03 ET04 ET05 ET06 ET07 ET08)}
};

static char const errorText[] PROGMEM = ET01 ET02 ET03 ET04 ET05 ET06 ET07 ET08 ET09;

void displayLaserError(int errNum) {
  lcd.clear();
  lcd.print("Laser Err no. ");
  lcd.print(errNum);

  for ( int i = 0;  i < noOfErrMsgs;  i++) { // nine error messges currently
    if (errNum == pgm_read_byte(&laserMsg[i].errorCode)) {
      // Found the message that matches the error code
      lcd.setCursor(0, 01);
      lcd.print( (__FlashStringHelper*) &errorText[pgm_read_byte(&laserMsg[i].errorTextOffset)] );
      break;  // Don't bother looking for another match
    }
  }
}

void setup() {}
void loop() {}
#17

[quote author=Coding Badly date=1505756256 link=msg=3416700]
Something like this should solve even that problem…[/quote]

Thank you. I’ll investigate that once I have a few other issues resolved.

The levels of complexity of this language seem limitless from this vantage point.

#18

So, I cleared a number of other issues and am now back at progmem. This code:

void displayTurretPosn(uint8_t trtPosition) {
  lcd.setCursor(0, 1); // column 0, line 2
  lcd.print(blankLine);//n
  lcd.setCursor(0, 1); //n
  if (textAndParms[menuMode].parmVal >= 1 && textAndParms[menuMode].parmVal <= 3)
    lcd.print(textAndParms[menuStation].bottomLineText);
  else {
    char * message;
    message = pgm_read_word(&turretPosnTextP[trtPosition]);  // Get message's PROGMEM address
    lcd.setCursor(0, 01);
    lcd.print( (__FlashStringHelper*) message); // prints directly from PROGMEM

    // next two lines work ok
    //memcpy_P (&lcdPrintBufr, &turretPosnTextP[trtPosition], sizeof lcdPrintBufr);
    //lcd.print(lcdPrintBufr);
    //Serial.print(message);
  }
}

does not work. The (16x2 LCD) display goes to all reverse video blanks. If the lines from char * message to lcd.print( (__FlashString… are commented out and the two lines currently commented out are uncommented, mem_cpy_P to lcd.print… the display is as desired.

I copied the char * message stuff from another function which prints direct from progmem, hat tip @JohnWasser.

void displayLaserError(int errNum) {
  char * message;  // PROGMEM address of message
  lcd.clear();
  lcd.print("Laser Err no. ");
  lcd.print(errNum);
  if (errNum <= 5) errNum = 1; // Error no's 1-5 encompassed by code '1'
  for ( int i = 0;  i < noOfErrMsgs;  i++) { // Nine original error messges
    // One user-added code 99 'sensor not found'

    if (errNum == pgm_read_byte(&laserMsg[i].errorCode)) {
      // Found the message that matches the error code
      message = pgm_read_word(&laserMsg[i].errorText);  // Get message's PROGMEM address
      lcd.setCursor(0, 01);
      lcd.print( (__FlashStringHelper*) message); // prints directly from PROGMEM
      break;  // Don't bother looking for another match
    }
  }
  //Serial.print("unrecoverable laser error - code ");
  //Serial.print(status);

  while (1);
}

This is the source of the text I’m trying to print.

typedef struct {
  char stationtext[17];
} stationlist;
const stationlist turretPosnTextP[] PROGMEM =
{ "Crimp",
  "Bullet Seat",
  "Powder Drop",
  "Prime/Deprime",
  "Raise to Unload ",
  "Lower to Unload " //final space erases turret char
};

I’ve fussed with this thing for a couple of hours off and on and am getting nowhere. Could I get a pointer? No pun intended.

Controller = UNO.

#19 
    char * message;
    message = pgm_read_word(&turretPosnTextP[trtPosition]);  // Get message's PROGMEM address
    lcd.setCursor(0, 01);
    lcd.print( (__FlashStringHelper*) message); // prints directly from PROGMEM

You are fetching the first two bytes of the message structure as if it were a pointer. It isn’t. You are storing message in PROGMEM like in my SECOND example but using code from the FIRST example to fetch a pointer to the message. You should not mix the two. The code from the SECOND example is:

  for ( int i = 0;  i < noOfErrMsgs;  i++) { // nine error messges currently
    if (errNum == pgm_read_byte(&laserMsg[i].errorCode)) {
      // Found the message that matches the error code
      lcd.setCursor(0, 01);
      lcd.print( (__FlashStringHelper*) &laserMsg[i].errorText);
      break;  // Don't bother looking for another match
    }
  }

Since you are not searching this table for a matching error code you can simplify the code to:

      // Found the message that matches the error code
      lcd.setCursor(0, 01);
      lcd.print( (__FlashStringHelper*) &turretPosnTextP[trtPosition].stationtext);
#20

johnwasser:
Since you are not searching this table for a matching error code you can simplify the code to:

      // Found the message that matches the error code

lcd.setCursor(0, 01);
     lcd.print( (__FlashStringHelper*) &turretPosnTextP[trtPosition].stationtext);

Yes, that line works perfectly. One of my failed attempts did include placing a literal behind the dot *&turretPosnTextP[trtPosition].1 * but that didn't work either. I can only presume it was broken some other way.

I do notice the FlashString method uses twenty-six more bytes than memcpy_P.

To wrap up, the way I read it; FlashStringHelper* is a pointer to the address of &turretPosnTextP[trtPosition].stationtext).

Again thank you.