Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Moving Menu Names and Items into PROGMEM #32

Open
jecottrell opened this issue May 16, 2016 · 7 comments
Open

Moving Menu Names and Items into PROGMEM #32

jecottrell opened this issue May 16, 2016 · 7 comments

Comments

@jecottrell
Copy link

jecottrell commented May 16, 2016

I'm running up against a dynamic memory limit with an involved menu tree. I've tried all of the PROGMEM ideas presented by Dean Camera (fourwalledcubicle) and I haven't been successful. Is there any way to achieve that?

Here is the beginning of my menu system:

MenuSystem ms;
Menu mm("HOME");
  MenuItem mm_mi1("Status");
  Menu mu2("MODES");
    Menu mu21("AUTO");
      MenuItem mu21_mi1("Select");
    Menu mu22("MANUAL");
      MenuItem mu22_mi1("Select");
      MenuItem mu22_mi2("Move Doors");
      MenuItem mu22_mi3("Move Nest Gates");
      MenuItem mu22_mi4("Switch Lights");
  Menu mu3("SETTINGS");
    MenuItem mu3_mi1("Set Time");
    MenuItem mu3_mi2("Set Date");
    Menu mu31("DOOR SCHEDULE");
      MenuItem mu31_mi1("Door Open Time");
      MenuItem mu31_mi2("Door Close Time");
    Menu mu32("NEST SCHEDULE");
      MenuItem mu32_mi1("Nest Open Time");
      MenuItem mu32_mi2("Nest Close Time");
    Menu mu33("LIGHT SCHEDULE");
      Menu mu331("LIGHT AM SCHED");
        MenuItem mu331_mi1("Lights AM On");
        MenuItem mu331_mi2("Lights AM Off");
      Menu mu332("LIGHT PM SCHED");
        MenuItem mu332_mi1("Lights PM On");
        MenuItem mu332_mi2("Lights PM Off");

Thanks for developing and sharing your code. It's been an INCREDIBLE help with my latest project.

John

@jonblack
Copy link
Owner

jonblack commented May 16, 2016

I'm glad you find the library useful.

As you've said, to store the MenuItem strings in Flash memory instead of SRAM you need to use PROGMEM. I don't see that in your example above, so I'll show you what I expect to see:

const PROGMEM char menu_text_home[] = {"HOME"};
Menu mm(menu_text_home);

I've never done this so that code above is not tested.

Realise that there's more than one way to run out of memory. You more or less have three things competing:

  • Stack
  • Heap
  • Static data

If you make lots of nested/recursive function calls, a stack frame is added each time, increasing the stack size; nothing is freed until a function exits.

Static data are things like the MenuSystem instances. They are when the program starts and not freed until it ends, so that space is in most cases not recoverable.

Lastly you have the heap. This is where dynamically allocation data goes (e.g. created with malloc). Memory here can be freed once it's no-longer required.

Since I can only count 300 bytes of strings, assuming I can count, that leaves 1748 bytes on an UNO, which is still quite a bit of SRAM. Of course, that doesn't include all the Menu objects and anything else you have. It's also possible that some other library is hogging your SRAM.

The following function (taken from adafruit) can be called at points in your code to see how much RAM is available. This might help with debugging. You can also look into avr-size, although that doesn't include anything that happens at runtime. I've tried neither of these so I have no idea if they work/will help.

int freeRam () 
{
  extern int __heap_start, *__brkval; 
  int v; 
  return (int) &v - (__brkval == 0 ? (int) &__heap_start : (int) __brkval); 
}

It'll probably (sadly) a case of looking closely at how much RAM is being used by various parts of your code until you find the bit that's the problem. It might be that all the code is fine and that you just have a lot of it, in which case, you need more RAM - a Mega2560 has 8k which is four times as much as the Uno. :)

Good luck. Get in touch if you need more help.

@jecottrell
Copy link
Author

jecottrell commented May 16, 2016

Thanks for the extensive explanation and the time you spent to write it. I have some basic understanding of what's going on under the hood, but only just enough to be dangerous.

As I had mentioned, I tried every implementation of PROGMEM that was applicable from here: http://www.github.com/abcminiuser/avr-tutorials/blob/master/Progmem/Output/Progmem.pdf?raw=true without any luck. I just tried your suggestion with the same results. A single, gibberish character is displayed at 0,0 on the LCD.

This is on a 328 part, so the 2048 limitation is correct. I'm currently at 1,489 and still have a number of menus to add. I will go through my code and clean up any problem areas that I can find, but right now the addition of more menus is eating up space fastest. I can already see a few areas that can free up some space, but I'm not sure it will be enough if I can't get the menu text into flash.

The sad thing is, I originally had planned to use the 2560, but was talked out of it.

Thanks again,

John

@jecottrell
Copy link
Author

So, I think I've found the solution but I'm not sure how to implement it. I was having a similar problem after declaring a constant array of constant character arrays with PROGMEM. Like this:

const char time_str[]  PROGMEM = "  Time: ";
const char date_str[]  PROGMEM = "  Date: ";
const char batt_str[]  PROGMEM = "  Batt: ";
const char wind_str[]  PROGMEM = "  Wind: ";
const char water_str[] PROGMEM = " Water: ";
const char light_str[] PROGMEM = "Lights: ";
const char door_str[]  PROGMEM = " Doors: ";
const char nest_str[]  PROGMEM = " Nests: ";

// define a string table array of strings in PROGMEM
const char* const status_string_table[] PROGMEM = {
  time_str,
  date_str,
  batt_str,
  wind_str,
  water_str,
  light_str,
  door_str,
  nest_str
}; 

If I tried to print by directly referencing the table of arrays, it wouldn't work, like this:

void displayStatuses()
{
  lcd.clear();
  lcd.setCursor(0,0);
  lcd.print(status_string_table[status_display_index]);
}

But, after finding and using this approach, everything worked fine:

void displayStatuses()
{
  // Buffer for printing our arrays
  char status_buffer[16];

  strcpy_P(status_buffer, (char*)pgm_read_word(&(status_string_table[status_display_index])));

  lcd.clear();
  lcd.setCursor(0,0);
  lcd.print(status_buffer);
}

So the question would be, how to do that in this function:

void displayMenu()
{
  // Buffer for printing our arrays
  char status_buffer[16];

  lcd.clear();
  lcd.setCursor(0,0);
  // Display the menu
  Menu const* cp_menu = ms.get_current_menu();


????
  strcpy_P(status_buffer, (char*)pgm_read_word(&(cp_menu->get_name())));
????

  //lcd.print("Current menu name: ");
  lcd.print(cp_menu->get_name());

  lcd.setCursor(0,1);
  lcd.print('*');

  lcd.print(cp_menu->get_selected()->get_name());
}

@jonblack
Copy link
Owner

jonblack commented May 17, 2016

That page has the following to say about strcpy_P:

The strcpy_P function copies a string from program space to a string in RAM ("buffer"). Make sure your receiving string in RAM is large enough to hold whatever you are retrieving from program space.

This means that it copied from FLASH to SRAM. Because the MenuItem components are built at the start and are always in memory, strcpy_P is going to immediately put all the nice FLASH strings into SRAM, and you have the same problem again. At least, that's how I see it. It seems PROGMEM is only useful when you throw away the strings immediately after using them.

The only way PROGMEM would help is if this library would read a string from FLASH when get_name is called. I have no idea if that's feasible nor if it's a good idea, but hopefully you get my point.

It's an interesting problem you have and I wish I could give you a solution other than "you need more RAM" and "optimize your code". In any case, I'm going to leave this issue open to remind myself to look at the library's memory usage - perhaps it can be improved.

@eadf
Copy link
Contributor

eadf commented Jun 2, 2016

One way to deal with this problem is to keep an alternative name pointer inside each MenuComponent.
One of this pointers isconst char*and the other isconst __FlashStringHelper *, only one of these can be set and the other one is always NULL.

__FlashStringHelperis the "arduino way" to deal withPROGMEMstrings, you use the macroF()to convert a string to progmem. The type is also cast into__FlashStringHelper* so that the code can 'know' if the string is in flash or ram. As you already noticed it is not useful to 'read' a progmem string with the normal char* functions.

An alternative to dual pointers is to use an union:

    typedef union {
        const char* _name;
        const __FlashStringHelper * _pname;
    } Char_ptr_t;

But then a bool has to be stored alongside the union to differentiate between the two.

@eadf
Copy link
Contributor

eadf commented Jun 3, 2016

Being capable of storing 'flash memory' strings side by side with 'dynamic memory' inside MenuComponents will also require the signature or getName() to change. It will need to return the data via a 'dynamic memory' buffer or a stack allocated String (the class).

@jonblack jonblack added this to the Future milestone Jun 15, 2016
@jonblack jonblack removed the question label Aug 5, 2017
@jonblack jonblack removed this from the Future milestone Aug 5, 2017
@ghost
Copy link

ghost commented Dec 4, 2020

Okay, I may have figured it out....

const char string_example[] PROGMEM = {"Example"};

instead of
Menu menuOne("Example");

I have
Menu menuOne(string_example);

I created this function to print to the LCD screen:

`void LCDPrint(const char *str)
{
      char c;
      if (!str)
            return;
      while ((c = pgm_read_byte(str++)))
      {
            lcd.print(c);
      }
}`

And then modified the MyRenderer class to simply be this:

class MyRenderer : public MenuComponentRenderer
{
public:
      void render(Menu const &menu) const
      {
            lcd.clear();
            lcd.setCursor(0, 0);
            LCDPrint((const char *)(menu.get_name()));
            lcd.setCursor(0, 1);
            menu.get_current_component()->render(*this);
      }

      void render_menu_item(MenuItem const &menu_item) const
      {
           LCDPrint((const char *)(menu_item.get_name()));
      }

      void render_back_menu_item(BackMenuItem const &menu_item) const
      {
            LCDPrint((const char *)(menu_item.get_name()));
      }

      void render_numeric_menu_item(NumericMenuItem const &menu_item) const
      {
           LCDPrint((const char *)(menu_item.get_name()));
      }

      void render_menu(Menu const &menu) const
      {
           LCDPrint((const char *)(menu.get_name()));
      }
};

So far it seems to print to the LCD perfectly well but I'm unsure if it's just reading the strings from PROGMEM initially and then storing them in RAM afterwards.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

3 participants