Progmem function pointers issue in class

How to acces program memory properly?

((VoidFnct)pgm_read_word(&menuItems[1].action))(); // This crashes, does not work.

I am stuck with this :confused:

menuItems.h

#ifndef menuItems_h
#define menuItems_h

#include 
#include "menu/menu.h"

typedef void (*VoidFnct)();

typedef struct item
{
    int id;
    void (menu::*action)(void);
} ITEM;

static const int menuItemsTotal = 53;

static constexpr ITEM menuItems[] PROGMEM = {
    {1, &menu::hello},
    {2, &menu::hello},
    {3, &menu::hello},
};

#endif

menu.h

#ifndef menu_h
#define menu_h

#include 

#include "openGLCD.h"

class menu
{
public:
    void hello();
private:
};
#endif

menu.cpp

#include 
#include "menu/menu.h"
#include "menu/menuItems.h"

...

void menu::test()
{
    ...
    ((VoidFnct)pgm_read_word(&menuItems[1].action))(); // This crashes, does not work.
    ...
}

void menu::hello()
{
    Serial.println("hello");
}

...

main.cpp

#include 

#include "openGLCD.h"

#include "menu/menu.h"
#include "menu/menuItems.h"

menu MENU;

void setup()
{
    Serial.begin(115200);

    BUTTON.begin();
}

void loop()
{
    BUTTON.run();

    if (BUTTON.getStatus() == BUTTON_HOLD)
    {
        //MENU.hello(); // This is working untill i have to execute function like below

        ((VoidFnct)pgm_read_word(&menuItems[1].action))(); // This crashes, does not work.
    }
}

No one?

May I’m misreading your code, but functions are always already in PROGMEM.

The code you posted can not compile and can not work.

There is a difference between a pointer to a function and a pointer to a member function.

Whandall: There is a difference between a pointer to a function and a pointer to a member function.

Indeed: https://isocpp.org/wiki/faq/pointers-to-members

Well the code compiles without warnings/errors (PlatformIO). But when I test it on my mega2560 i got for some reason int value changes. I have isolated that error to the line where I call: ((VoidFnct)pgm_read_word(&menuItems[1].action))();

The code you've posted CANNOT possibly compile without error. Do NOT post code you have not first actually compiled and run. Post the ACTUAL code you are having a problem with.

Regards, Ray L.

The message has the following error or errors that must be corrected before continuing: The message exceeds the maximum allowed length (9000 characters).

I just thought I post the actual Problem where I have isolated and not all the garbage. Sorry for my beginner mistake…

menu.cpp (28 KB)

menu.h (3.83 KB)

menuItems.h (2.82 KB)

main.cpp (3.51 KB)

That's still not enough to be able to compile. At the very least, you're missing all these include files:

#include 
#include 
#include 
#include 
#include 
#include 
#include 
#include 
#include 
#include 

Even if you had posted all those, not many people are going to be interested in wading through all that junk just to get your code to compile. Please create an MRE that recreates your problem using the smallest possible code. Also, if you haven't already, read the article about pointers to member functions that I linked in Reply #4.

Ok did a minimal ino sketch in a zip file. Compiles without error but crashes when call pointer to member…

gfvalvo:
Indeed: Standard C++

I have read.

The easy way will be to do it like this:

void menuHello()
{
    MENU.hello();
    return;
}

But would like to have it this way:
https://isocpp.org/wiki/faq/pointers-to-members#array-memfnptrs

Can you tell me how to do this with PROGMEM and pgm_read_word?

PointerToMemberTest_1.zip (7.64 KB)

This version of the sketch will compile. I expect the actual execution will still fail because the member function is being called without an object instance.
Sorry about the double-spacing. I think the file contains CRLF line endings.

typedef void (*pVoidFnct)();
class menu
{
  public:
    void hello();
    void test();
  private:
};


typedef struct item
{
  int id;
  void (menu::*action)(void);
} ITEM;


static const int menuItemsTotal = 53;


static constexpr ITEM menuItems[] PROGMEM =
{
  {1, &menu::hello},
  {2, &menu::hello},
  {3, &menu::hello},
};


void menu::test()
{
  ((pVoidFnct)pgm_read_word(&menuItems[1].action))(); // This crashes, does not work.
}


void menu::hello()
{
  Serial.println("hello");
}


menu MENU;


void setup()
{
  Serial.begin(115200);
}


void loop()
{
  MENU.hello(); // This is working untill i have to execute function like below


  ((pVoidFnct)pgm_read_word(&menuItems[1].action))(); // This crashes, does not work.
}

Not related to your question, but why are you declaring id, idLevel1, and idLevel2 as int then reading them with pgm_read_byte? That will appear to work because int is stored least-significant-byte first and all your numbers will fit in a byte, but it does waste space. Also, the actual text for each menu item is not being stored in PROGMEM, only the pointer to it.

Don't have time to look at the function pointer this morning.

david_2018: Not related to your question, but why are you declaring id, idLevel1, and idLevel2 as int then reading them with pgm_read_byte? That will appear to work because int is stored least-significant-byte first and all your numbers will fit in a byte, but it does waste space. Also, the actual text for each menu item is not being stored in PROGMEM, only the pointer to it.

Don't have time to look at the function pointer this morning.

I know will change it to byte later. Well at time i save 25-30% of RAM on a mega2560

((MENU).*(menuItems[52].action))();

Doas the trick without PROGMEM. What about with?

I modified @johnwasser's code using the techniques described in https://isocpp.org/wiki/faq/pointers-to-members#array-memfnptrs and made it "work".

The compiler was very ornery about casting a uint16_t into a pointer to a member function. So, I resorted to memcpy().

#include "Arduino.h"

class Menu;
typedef void (Menu::*pVoidFnct)();
#define CALL_MEMBER_FN(objectPtr, functPtr) ((objectPtr)->*(functPtr))()

class Menu {
  public:
    void hello();
    void goodBye();
    void thankYou();
  private:
};

void Menu::hello() {
  Serial.println("Hello");
}

void Menu::goodBye() {
  Serial.println("Good Bye");
}

void Menu::thankYou() {
  Serial.println("Thank You");
}

struct ITEM {
  uint16_t id;
  void (Menu::*action)(void);
};

constexpr ITEM menuItems[] PROGMEM = {
  { 1, &Menu::hello },
  { 2, &Menu::goodBye },
  { 3, &Menu::thankYou },
};

constexpr uint8_t numMenuItems = sizeof(menuItems) / sizeof(menuItems[0]);

Menu menu;

void setup() {
  pVoidFnct pointer = nullptr;
  uint16_t addr;

  Serial.begin(115200);
  delay(1000);

  for (uint8_t i = 0; i < numMenuItems; i++) {
    addr = pgm_read_word(&menuItems[i].action);
    memcpy(&pointer, &addr, 2);
    CALL_MEMBER_FN(&menu, pointer);
  }
}

void loop() {
}

Serial Port Output:

Hello
Good Bye
Thank You

Here's a variation using memcpy_P. It also works:

#include "Arduino.h"

class Menu;
typedef void (Menu::*pVoidFnct)();
#define CALL_MEMBER_FN(objectPtr, functPtr) ((objectPtr)->*(functPtr))()

class Menu {
public:
  void hello();
  void goodBye();
  void thankYou();
  private:
};

void Menu::hello() {
  Serial.println("Hello");
}

void Menu::goodBye() {
  Serial.println("Good Bye");
}

void Menu::thankYou() {
  Serial.println("Thank You");
}

struct ITEM {
  uint16_t id;
  void (Menu::*action)(void);
};

constexpr ITEM menuItems[] PROGMEM = {
    { 1, &Menu::hello },
    { 2, &Menu::goodBye },
    { 3, &Menu::thankYou },
};

constexpr uint8_t numMenuItems = sizeof(menuItems) / sizeof(menuItems[0]);

Menu menu;

void setup() {
  pVoidFnct pointer = nullptr;

  Serial.begin(115200);
  delay(1000);

  for (uint8_t i = 0; i < numMenuItems; i++) {
    memcpy_P(&pointer, &menuItems[i].action, sizeof(pVoidFnct));
    CALL_MEMBER_FN(&menu, pointer);
  }
}

void loop() {
}

Also, it would probably be useful to have the entire menu ITEM available at once:

#include "Arduino.h"

class Menu;
typedef void (Menu::*pVoidFnct)();
#define CALL_MEMBER_FN(objectPtr, functPtr) ((objectPtr)->*(functPtr))()

class Menu {
  public:
    void hello();
    void goodBye();
    void thankYou();
  private:
};

void Menu::hello() {
  Serial.println("Hello");
}

void Menu::goodBye() {
  Serial.println("Good Bye");
}

void Menu::thankYou() {
  Serial.println("Thank You");
}

struct ITEM {
  uint16_t id;
  void (Menu::*action)(void);
};

constexpr ITEM menuItems[] PROGMEM = {
  { 1, &Menu::hello },
  { 2, &Menu::goodBye },
  { 3, &Menu::thankYou },
};

constexpr uint8_t numMenuItems = sizeof(menuItems) / sizeof(menuItems[0]);

Menu menu;

void setup() {
  ITEM tempItem;

  Serial.begin(115200);
  delay(1000);

  for (uint8_t i = 0; i < numMenuItems; i++) {
    memcpy_P(&tempItem, menuItems + i, sizeof(ITEM));
    Serial.print("ID: ");
    Serial.print(tempItem.id);
    Serial.print(" - ");
    CALL_MEMBER_FN(&menu, tempItem.action);
  }
}

void loop() {
}

Output:

ID: 1 - Hello
ID: 2 - Good Bye
ID: 3 - Thank You

How exactly is this not working? It looks to me like the code you attached is calling the function properly, I can get as far as the menu::showMenuLevel1() function, but without a rotary encoder I’m seeing it loop there and not return. Inserting a return statement near the top of that function backs out to the main loop, so I don’t really see a problem with the progmem.

Why are you calling loop() from within the menu? Will this lead to a recursive loop?

void menu::exit()
{
#ifdef DEBUG_MENU
  Serial.println(F("[MENU] Exit"));
#endif

  for (int i = 0; i < encoderHistory; i++)
  {
    encoderPosInLevel_1[i] = 0;
    encoderPosInLevel_2[i] = 0;
    encoderPosInLevel_3[i] = 0;
    encoderPosInLevel_4[i] = 0;
  }

  minEncoderPos = 0;
  maxEncoderPos = 0;

  for (int i = 0; i <= 1; i++)
  {
    firstLineInLevel_1[i] = 0;
    cursorAtLineInLevel_1[i] = 0;
  }

  currentLevel = 0;
  oldLevel = 0;

  loop(); // <<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<< call to loop()
  return;
}

Hmm I am Not sure, if you are having only one trouble. I am seeing right now at least two seperate possible Problem cases:

First, you said you are using mega 2560. This Board has 256kb progmem while others have only 64. That way, using progmem read, you can only access in the first 64kb. It is possible, that you will get out of bounds depending on data size you have. So I can encourage you to use progmem_far functions.

Second, I see you use non-int variables inside of here:

pgm_read_word(&menuItems[1].action)

you have to get the memory position of menuItems[1].action first and THEN you can read from memory.

Here is a working example, check out especially RadarPositionsAPI and RadarPositions to see how I implemented the Memory Usage on MEGA. It helped several members with similar problems. Have a great one: https://github.com/Stefan-Kosker/totally-legal-blitzermelder/tree/master/MotorbikeEnhancher3000/FullProject/main