jonblack / arduino-menusystem

Arduino library for implementing a menu system
MIT License
194 stars 85 forks source link

NumericMenuItem and SelectFnPtr warning question #54

Closed tpak closed 7 years ago

tpak commented 7 years ago

First, thanks for this library, it is hopefully going to save me a pile of headaches. Now, if I can ever get my (very, very, very) rusty C/C++ skills back this would probably be obvious to me.

I started with the serial nav example and have hacked it to work with my 4 line LCD and 3 buttons up, down, select. So far so good; however, I have run into a compile warning that I'd like you to look at. I backported my changes to the original serial nav example for discussion sake. It is pasted at the end of this. My changes are all preceded by a line with //**

What I am trying to accomplish is, without writing a custom class, save the numeric value from a NumericMenuItem into a global variable upon exit/select from a menu item. It works but I am not sure I am doing it correctly. (nag about documentation and examples goes here) I think this is a perfect use for the SelectFnPtr is it not?

I declare and provide a function "sav_int" to be called when the NumericMenuItem mm_mi5 is exited and add it to the mm_mi5 declaration. I get the compiler warning:

Users/chris/Library/Mobile Documents/com~apple~CloudDocs/projects/Arduino/serial_nav/serial_nav.ino:42:93: warning: invalid conversion from 'void (*)(NumericMenuItem*)' to 'MenuItem::SelectFnPtr {aka void (*)(MenuItem*)}' [-fpermissive]
 NumericMenuItem mm_mi5("Level 1 - Int Item 5 (Item)", save_int, 50, -100, 100, 1, format_int);
                                                                                             ^
In file included from /Users/chris/Library/Mobile Documents/com~apple~CloudDocs/projects/Arduino/serial_nav/serial_nav.ino:10:0:
/Users/chris/Library/Mobile Documents/com~apple~CloudDocs/projects/Arduino/libraries/arduino-menusystem-master/MenuSystem.h:258:5: note: initializing argument 2 of 'NumericMenuItem::NumericMenuItem(const char*, MenuItem::SelectFnPtr, float, float, float, float, NumericMenuItem::FormatValueFnPtr)'
     NumericMenuItem(const char* name, SelectFnPtr select_fn,
     ^

Can you tell if I am doing this correctly and if not what should I change? Like I said, I am fairly rusty with my C/C++.

Also, at line 62 of my code where I cast the value from the menu item, is that the correct way to do that here? altitude = (int)(*p_menu_item).get_value(); Thanks. Chris

(I also attached my version of serial_nav.ino as zip - you should be able to just drop it in place and give it a try)

serial_nav.ino.zip

/*
 * //serial_nav.ino - Example code using the menu system library
 *
 * This example shows the menu system being controlled over the serial port.
 *
 * Copyright (c) 2015, 2016 arduino-menusystem
 * Licensed under the MIT license (see LICENSE)
 */

#include <MenuSystem.h>
#include "CustomNumericMenuItem.h"
#include "MyRenderer.h"

// forward declarations
const String format_float(const float value);
const String format_int(const float value);
const String format_color(const float value);

//**
void save_int(NumericMenuItem* p_menu_item);
int altitude = 0;

void on_item1_selected(MenuItem* p_menu_item);
void on_item2_selected(MenuItem* p_menu_item);
void on_item3_selected(MenuItem* p_menu_item);
void on_back_item_selected(MenuItem* p_menu_item);

// Menu variables

MyRenderer my_renderer;
MenuSystem ms(my_renderer);

MenuItem mm_mi1("Level 1 - Item 1 (Item)", &on_item1_selected);
MenuItem mm_mi2("Level 1 - Item 2 (Item)", &on_item2_selected);
Menu mu1("Level 1 - Item 3 (Menu)");
BackMenuItem mu1_mi0("Level 2 - Back (Item)", &on_back_item_selected, &ms);
MenuItem mu1_mi1("Level 2 - Item 1 (Item)", &on_item3_selected);
NumericMenuItem mu1_mi2("Level 2 - Txt Item 2 (Item)", nullptr, 0, 0, 2, 1, format_color);

CustomNumericMenuItem mu1_mi3(12, "Level 2 - Cust Item 3 (Item)", 80, 65, 121, 3, format_int);
NumericMenuItem mm_mi4("Level 1 - Float Item 4 (Item)", nullptr, 0.5, 0.0, 1.0, 0.1, format_float);
NumericMenuItem mm_mi5("Level 1 - Int Item 5 (Item)", save_int, 50, -100, 100, 1, format_int);

// Menu callback function

// writes the (int) value of a float into a char buffer.
const String format_int(const float value)
{
    return String((int) value);
}

// writes the value of a float into a char buffer.
const String format_float(const float value)
{
    return String(value);
}

//**
void save_int(NumericMenuItem* p_menu_item)
{
    Serial.println("inside save_int  ");
    altitude = (int)(*p_menu_item).get_value();
    //Serial.println((*p_menu_item).get_value_string());
    Serial.println(altitude);

}

// writes the value of a float into a char buffer as predefined colors.
const String format_color(const float value)
{
    String buffer;

    switch((int) value)
    {
        case 0:
            buffer += "Red";
            break;
        case 1:
            buffer += "Green";
            break;
        case 2:
            buffer += "Blue";
            break;
        default:
            buffer += "undef";
    }

    return buffer;
}

// In this example all menu items use the same callback.

void on_item1_selected(MenuItem* p_menu_item)
{
    Serial.println("Item1 Selected");
}

void on_item2_selected(MenuItem* p_menu_item)
{
    Serial.println("Item2 Selected");
}

void on_item3_selected(MenuItem* p_menu_item)
{
    Serial.println("Item3 Selected");
}

void on_back_item_selected(MenuItem* p_menu_item)
{
    Serial.println("Back item Selected");
}

void display_help() {
    Serial.println("***************");
    Serial.println("w: go to previus item (up)");
    Serial.println("s: go to next item (down)");
    Serial.println("a: go back (right)");
    Serial.println("d: select \"selected\" item");
    Serial.println("?: print this help");
    Serial.println("h: print this help");
    Serial.println("***************");
}

void serial_handler()
{
    char inChar;
    if ((inChar = Serial.read()) > 0)
    {
        switch (inChar)
        {
            case 'w': // Previus item
                ms.prev();
                ms.display();
                Serial.println("");
                break;
            case 's': // Next item
                ms.next();
                ms.display();
                Serial.println("");
                break;
            case 'a': // Back presed
                ms.back();
                ms.display();
                Serial.println("");
                break;
            case 'd': // Select presed
                ms.select();
                ms.display();
                Serial.println("");
                break;
            case '?':
            case 'h': // Display help
                ms.display();
                Serial.println("");
                break;
            default:
                break;
        }
    }
}

// Standard arduino functions

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

    ms.get_root_menu().add_item(&mm_mi1);
    ms.get_root_menu().add_item(&mm_mi2);
    ms.get_root_menu().add_menu(&mu1);
    mu1.add_item(&mu1_mi0);
    mu1.add_item(&mu1_mi1);
    mu1.add_item(&mu1_mi2);
    mu1.add_item(&mu1_mi3);
    ms.get_root_menu().add_item(&mm_mi4);
    ms.get_root_menu().add_item(&mm_mi5);

    display_help();
    ms.display();
    Serial.println("");
}

void loop()
{
    serial_handler();
}
tpak commented 7 years ago

Hey, just giving this a bump to see if anyone (especially Jon) can lend a hand. It's summer and maybe it got lost in the silence of vacations. Thanks.

jonblack commented 7 years ago

Sorry for the late reply. There are two things I noticed (just looking at the code without running it):

  1. mu_mi5 passes save_int instead of its address &save_int;
  2. save_int has the parameter NumericMenuItem* p_menu_item instead of MenuItem* p_menu_item. When save_int takes a MenuItem* you can cast it to a NumericMenuItem* in that function.

The cast on line 62 looks fine to me.

Let me know if this helps.

tpak commented 7 years ago

Thanks for getting back to me.

Hmm. Changing line 42 to this: NumericMenuItem mm_mi5("Level 1 - Int Item 5 (Item)", &save_int, 5, 0, 5, 1, format_int); and I still get the exact same thing.

If I change save_int to be declared like this: void save_int(MenuItem* p_menu_item) it complains b/c mm_mi5 is declared as NumericMenuItem which is what I need it to be - its the same error/warning but slightly different working

Also, If I passed save_int a MenuItem, I guess I could cast it but I don't understand why do that rather than have save_int accept a NumericMenuItem in the first place, its whole reason is to accept a numeric ...

Feel like I am missing something obvious here. Like I said, very rusty C++ skills.

jonblack commented 7 years ago

I'll run the code on an Arduino this week and take a look for you.

jonblack commented 7 years ago

I got this working using my second point above:

  1. save_int should take a MenuItem*
  2. Cast the MenuItem* to a NumericMenuItem*

So the function will be:

void save_int(MenuItem* p_menu_item)
{
    NumericMenuItem* p_nmi = (NumericMenuItem*) p_menu_item;

    Serial.println("inside save_int  ");
    altitude = (int) p_nmi->get_value();
    Serial.println(altitude);
}

Don't forget to change the forward declaration.

I'll close this issue. Re-open if it doesn't solve your problem.

tpak commented 7 years ago

Thanks! Happened to be in my office when this came through, that worked. Like I said, pretty rusty C skills these days. I appreciate your effort especially since this isn't a bug and is just my current proficiency level ...