PGM_P in a struct

I can't seem to get the right combination of progmem macros and pointer deref to get this bit of code to churn out anything other than gibberish. Any help would be greatly appreciated.

As you can see, I've got a list of error codes, and a struct that links an error code with a description.

typedef enum {
  ERR_OK,
  ERR_NOT_NMEA,
  ERR_NOT_RESPONSE,
  ERR_BAD_CHECKSUM,
  ERR_UNKNOWN_MESSAGE,
  ERR_WRONG_MESSAGE,
  ERR_INVALID
} NMEA_ERR;

typedef struct {
  NMEA_ERR err;
  PGM_P msg;
} NMEA_ERR_description;

char * nmea_err_desc(NMEA_ERR);

Then, I've got the descriptions as strings in program space and an array of the error -> message structs.

PGM_P _err_ok = "Sentence OK.";
PGM_P _err_not_nmea = "Received string is not in NMEA format.";
PGM_P _err_not_response = "Received string appears to be an NMEA query.";
PGM_P _err_bad_checksum = "Checksum does not match. Message garbled.";
PGM_P _err_unknown_message = "Good sentence. Don't understand it.";
PGM_P _err_wrong_message = "Good sentence. Not what was requested.";
PGM_P _err_invalid = "Some other problem. Probably couldn't allocate memory.";

static const NMEA_ERR_description _nmea_err_desc[] =
{
  { ERR_OK, _err_ok },
  { ERR_NOT_NMEA, _err_not_nmea },
  { ERR_NOT_RESPONSE, _err_not_response },
  { ERR_BAD_CHECKSUM, _err_bad_checksum },
  { ERR_UNKNOWN_MESSAGE, _err_unknown_message },
  { ERR_WRONG_MESSAGE, _err_wrong_message },
  { ERR_INVALID, _err_invalid },
  { (NMEA_ERR)NULL, NULL }
};

Finally, there is a function that takes an error code and returns the description.

char * nmea_err_desc(NMEA_ERR e)
{
  char r[70];

  strcpy_P(r, (PGM_P)pgm_read_word(&(_nmea_err_desc[e].msg)));
  
  return r;
}

Calls to nmea_err_desc don't return the strings they're supposed to, just random bits of memory.

The non-program space version works just fine:

const char * _err_ok = "Sentence OK.";
const char * _err_not_nmea = "Received string is not in NMEA format.";
const char * _err_not_response = "Received string appears to be an NMEA query.";
const char * _err_bad_checksum = "Checksum does not match. Message garbled.";
const char * _err_unknown_message = "Good sentence. Don't understand it.";
const char * _err_wrong_message = "Good sentence. Not what was requested.";
const char * _err_invalid = "Some other problem. Probably couldn't allocate memory.";

static const NMEA_ERR_description _nmea_err_desc[] =
{
  { ERR_OK, _err_ok },
  { ERR_NOT_NMEA, _err_not_nmea },
  { ERR_NOT_RESPONSE, _err_not_response },
  { ERR_BAD_CHECKSUM, _err_bad_checksum },
  { ERR_UNKNOWN_MESSAGE, _err_unknown_message },
  { ERR_WRONG_MESSAGE, _err_wrong_message },
  { ERR_INVALID, _err_invalid },
  { (NMEA_ERR)NULL, NULL }
};

char * nmea_err_desc(NMEA_ERR e)
{
  char r[70];

  strcpy(r, _nmea_err_desc[e].msg);
  
  return r;
}

I'd really like to get these strings out of RAM and into flash where they belong.

Also, as an aside, can someone tell me where the compiler output goes for Arduino on OS-X? It's not /tmp/build.* for me. I think seeing the generated assembly and address map would be helpful here.

To put strings into progmem space, you can change your declaration as follows:

//PGM_P _err_ok = "Sentence OK.";
const char _err_ok[] PROGMEM = "Sentence OK."

There are also issues with this code you need to consider:

char * nmea_err_desc(NMEA_ERR e)
{
  char r[70];

  strcpy_P(r, (PGM_P)pgm_read_word(&(_nmea_err_desc[e].msg)));
  
  return r;
}

It is not good practice to return a pointer to a local function variable in C/C++. The char array "r" is allocated from stack memory and is only valid within the scope of the function. When your function exits, the content of "r" may be modfied and you should no longer use a reference to this memory.

You're right, returning a pointer to a function scoped variable is not the best idea. I've fixed that, but in this case it doesn't appear to be the problem.

PGM_P is a pgmspace macro that expands to const prog_char *, and prog_char is typedef'd as const char PROGMEM, so isn't PGM_P exactly the same as const char * PROGMEM?

Anyway, I changed the declaration of those strings as you suggested and I still get the same garbage on screen.

I'm just pointing out the errors I see - there may be more ... :'(

Looking at this fragment:

strcpy_P(r, (PGM_P)pgm_read_word(&(_nmea_err_desc[e].msg)));

The ".msg" element of your structure is a pointer to a string in program space. The pointer itself however is in RAM. If you take the address of this pointer (using "&") you will get a pointer to RAM. You need to get rid of the "&".

You may also want to consider the following (not necessarily to make it work, but to save additional ram):

  • Your string reference table can be implemented as a simple array where the index in the array is your "error number". That is you may not need the structure.
  • It is also possible to put the string reference array in progmem to save additional ram.
  • You could also avoid copying the string to RAM and rather use functions like print_P that can access and print strings directly from progmem one character at a time.

Here's an example taking above into account (warning this is a code fragment only):

// error codes
#define ERROR_ONE 0
#define ERROR_TWO 1
#define ERROR_THREE 2

// error strings
const char x_01[] PROGMEM = "Error 1";
const char x_02[] PROGMEM = "Error 2";
const char x_03[] PROGMEM = "Error 3";

// error string lookup table
const char *ErrTbl[] PROGMEM = {x_01,x_02,x_03};

// a macro returning a pointer to a string in progmem
#define X_PTR(_IDX_) ((const char *)pgm_read_word(&ErrTbl[_IDX_]))

// print a character string from program memory
void print_P(const char *str)
{
  char ch;
  while (true) {
    ch = pgm_read_byte(str);
    if (!ch) break;
    Serial.write(ch);
    str++;
  }
}

void test()
{
  // print an error string 
  print_P(X_PTR(ERROR_ONE));
}

Your suggestion to put the struct in progmem did the trick. I swear I tried that before, but it was late so who knows?

Finally, the code below is what I ended up with.

The Serial class really ought to have print_P and println_P functions built in. If I get really low on RAM (I’ve got 1006 bytes available right now), I’ll probably implement the print_P function you wrote below.

Thanks for your help. [smiley=beer.gif]

typedef struct {
  NMEA_ERR PROGMEM err;
  const char * PROGMEM msg;
} NMEA_ERR_description;

const char _err_ok[] PROGMEM = "Sentence OK.";
const char _err_not_nmea[] PROGMEM = "Received string is not in NMEA format.";
const char _err_not_response[] PROGMEM = "Received string appears to be an NMEA query.";
const char _err_bad_checksum[] PROGMEM = "Checksum does not match. Message garbled.";
const char _err_unknown_message[] PROGMEM = "Good sentence. Don't understand it.";
const char _err_wrong_message[] PROGMEM = "Good sentence. Not what was requested.";
const char _err_invalid[] PROGMEM = "Some other problem. Probably couldn't allocate memory.";

NMEA_ERR_description _nmea_err_desc[] PROGMEM =
{
  { ERR_OK, _err_ok },
  { ERR_NOT_NMEA, _err_not_nmea },
  { ERR_NOT_RESPONSE, _err_not_response },
  { ERR_BAD_CHECKSUM, _err_bad_checksum },
  { ERR_UNKNOWN_MESSAGE, _err_unknown_message },
  { ERR_WRONG_MESSAGE, _err_wrong_message },
  { ERR_INVALID, _err_invalid },
  { (NMEA_ERR)NULL, NULL }
};

char * nmea_err_desc(NMEA_ERR e)
{
  char * r = (char *)malloc(60 * sizeof(char));
  
  if(r != NULL)
    strcpy_P(r, (PGM_P)pgm_read_word(&(_nmea_err_desc[e].msg)));

  return r;
}

The Serial class really ought to have print_P and println_P functions built in.

So, submit it as a suggestion, in the Uno Punto Zero forum.

Thanks for the tip, I'll do that.