New Arduino library: MenuSystem

Development Libraries
33

I'm also reluctant to add more classes. The arduino doesn't have a huge amount of memory, and generic libraries should be small so more of that memory can be used by the client application.

Yes I agree with you

The interface change in your example also isn't ideal: it would break existing code. That said, I think the 'MenuItem' might be better off if the handler function pointer was passed in the constructor.

I also agree that it would brake the actual code, but just the menu creation part, at least for me isn't big deal to repair it. Your idea of making it on the constructor look good, today night I will update my mods to allow that, so you can do like this:

MenuSystem mm;
Menu mu("Main Menu");
MeniItem mi1("Menu Item 1",&on_mi1);
mu.add(mi1);

or

MenuSystem mm;
Menu mu("Main Menu");
MeniItem mi1("Menu Item 1")
mi1.set_select_function(&on_mi1);
mu.add(mi1);

I still propose this change, because as I see, setting the callback function, should be responsibility of the object (menuItem) itself and not of the composite class. Other point that support my change:

Menu const* Menu::add_menu(Menu* pMenu)
{
    pMenu->set_parent(this);

    _menu_components[_num_menu_components] = pMenu;

    if (_num_menu_components == 0)
        _p_sel_menu_component = pMenu;

    _num_menu_components++;

    return pMenu;
}
void Menu::add_item(MenuItem* pItem, void (*on_select)(MenuItem*))
{
    // Prevent memory overrun
    if (_num_menu_components == MAX_MENU_ITEMS)
        return;

    _menu_components[_num_menu_components] = pItem;

    pItem->set_select_function(on_select);

    if (_num_menu_components == 0)
        _p_sel_menu_component = pItem;

    _num_menu_components++;
}

If you see both methods are almost the same, regarding the fact that add_item check for space (that it would a good idea to check on add_menu to) and that add_item sets the callback function in the process. So if you extract the responsibility of setting the callback funct to the MenuItem class you will be able to have generic add method like this

MenuComponent const* Menu::add_menu(MenuComponent* pComponent)
{
    // Prevent memory overrun
    if (_num_menu_components == MAX_MENU_ITEMS)
        return NULL;

    pMenu->set_parent(this);

    _menu_components[_num_menu_components] = pComponent;

    if (_num_menu_components == 0)
        _p_sel_menu_component = pComponent;

    _num_menu_components++;

    return pComponent;
}

The only thing is that set_parent should be virtual on MenuComponent ad we have 2 options:

  • Make a generic method on MenuComponent that return false or anything that do nothing and declare it on Menu (as it)
  • Add a parent private/protected (not sure which one) on the MenuComponent and implement on Menucomponent so it can work for both classes
    The first option add program memory
    The second one need more ram to accept this new var on all items and menus (doesn't found a case where knowing the parent of an item will be useful)
    Anyway you save some program memory transforming two methods into one

Regarding my SetupItem, look at https://github.com/niesteszeck/menusystem/blob/master/examples/LCDAdvancedNav/LCDAdvancedNav.ino
In this example I was making, I show how to toggle a led, how to display an analog read, and how to setup a var (string in this case) that represent a date+time (that is where I want to go). I you see it is really difficult to mix the display+the menusystem+ setting a var.
I need to save states to know if I'm on the menu or in the callback function that is trigger from the menu to it can do setting var related work.
So I decided to expand the menuSystem to allow me to add this new class. So I make something like this (I don't have the exact code here, so I just writing what i remember):

SetupItem(char* name, int value)
    : Menucomponent(name),
    _value(value) 
{}
boolean SetupItem::increment(){
    _value++;
}
boolean SetupItem::decrease(){
    _value--;
}
char * SetupItem::get_name(){
    sprintf(_display_name,"%s: %d",_name,_value);
    return _display_name:
}
    int get_value() {
    return _value;
}

So I can do something like this:

Menu mu("Main Menu");
SetupItem si1("Month", 0);
SetupItem si1("Day", 0);
SetupItem si1("Year", 0);
MenuItem mi1("Set Date", &on_set_date);
MenuItem mi2("Cancel", &on_cancel);
mu.add(mi1);

while(1){
    if (button==up)
        ms.increase();
    if(nutton==down)
        ms.decrese();
    display(); //this method ask the menusystem with get_names to display it over serial or anything
}

void on_cancel () {
    ms.back();
}
void on_set_date(){} // this func ask menusystem for vars values and set the date accordingly

The output should be like

Setup var 1: 0

If I press the up button 10 times:

Setup var 1: 10

Because the SetupMenu is added to a menu next will be jump to next var to set up (fe: from month to day).

I know that all this changes implies a lot of small changes everywhere, so let me show my code so you can see all together; the example (SerialNav) working with the modifications to show how to set a date (I want to set up .

Other point that I just realize while I was writing this, is that a lot of the changes I had to made to the lib allow anyone to grab menusystem and expand it under they needs, as I did making a new SetupItem class.

I also don't want to say that your lib is poor or anything like that, I just trying to use it and I found this way the easiest to help me do what I want. I am very grateful of your work (it opened my eyes about composite pattern that is a good way to achieve a menu) and hopefully we can expand the functionality of your lib.
*If I'm spelling something really wrong (from programming point) please bright me :wink: