Help me untangle this bit of code!

Hey, everyone!

I’m making a basic menu system for my game. It’ll store each menu item’s type, (button or checkboxes, and a few other cosmetic things) X and Y coordinates, and a pointer. Here’s my code so far.

– Edit –
more context for that “a pointer” bit:
Based on the type of menu item, it’ll change what type of pointer it is.
If it’s a button/invertedButton, it’ll be a function pointer in ROM.
If it’s a checkbox, it’ll be a boolean variable pointer in RAM.
– End edit –

I have three questions:

  1. How do I fix all these errors and warnings?
  2. Can I have a pointer that points to either RAM or ROM based on the menu item’s type …or am I sorely mistaken?
  3. I really can’t wrap my head around pointers. I get that they’re positions in memory and all that stuff, but I don’t get all the weird *s and &s and ->s that show up in modern C++. Can someone give me some pointers on this?

Can I suggest you put the whole code up into GitHub (or whatever) so we can see ’ all these errors and warnings’?

I see what you did there. http://www.cplusplus.com/doc/tutorial/pointers/

Are you looking for a C or C++ (OOP) way to do it?

If you wanna go the C++ way, you can create a MenuItem class, then create subclasses for various types of buttons, like BooleanMenuItem, FunctionMenuItem etc. They can all have a virtual method clicked that will do different things in different subclasses. @Pharap will probably give you more ideas here.

Also you may want to take a look at unions.

Anyway, if your menu is really simple (just a bunch of items without submenus) I wouldn’t complicate it too much, I’d simply create an array of strings, then check which one was clicked in a switch. Sometimes simples is better.

1 Like

They’re actually all inherited from C and haven’t changed in decades.

The only thing C++ did in terms of pointers was to add references (another use of &).

Pun intended? :P

* can either be used to create a pointer type or to dereference a pointer.
For example T * x; means that x is a pointer to an object of type T,
and then *x would ‘dereference’ x to get the value of the object being pointed to.
(Technically it gets a reference to the object pointed to, but I don’t want to complicate matters if you don’t understand references.)

& takes the address of a variable and returns a pointer.
E.g. T * x = &y; would mean that x is a pointer to an object of type T and the object it points to is y. &y alone means 'the address of y'.

-> dereferences the pointer on the left hand side and then acts as a . on the object the pointer was pointing to.
In other words x->y is translated to (*x).y.
In fact (*x).y will work perfectly. -> is just a shorthand.

All of these explanations also apply to C as far as I’m aware because they were inherited from C.

On the Arduboy (and any code using Arduino or avr-libc),
reading from progmem can’t be done through a normal dereference.
You have to use pgm_read_byte or one of the variants.
For reading whole objects, you’re best off using memcpy_P.
For reading pointers use pgm_read_ptr and static_cast the result (e.g. static_cast<const char *>(pgm_read_ptr(stringTable))).

On the Arduboy a pointer can point to RAM, ROM (progmem) or even EEPROM, there’s no way of knowing which just by looking at the pointer, which is why you have to be careful about how you use them and why the F macro exists (to tell print/println that “this is a pointer to a string in progmem, not a string in RAM”).

So yes, it’s possible, but you have to be careful about what you’re doing.
If you try to read a progmem pointer as a RAM pointer or a RAM pointer as a progmem pointer then you’ll end up reading the wrong thing and the result could be disasterous.

(Disasterous as in your program breaks and does weird things, not disasterous as in your Arduboy gets destroyed. It should be almost impossible to brick your Arduboy by writing buggy code.)

You haven’t given us enough information to solve all your problems, but I’ve tried to solve what I can…

I started trying to fix your problems, but soon discovered that there isn’t actually enough code here to figure out how to fix your errors.

Over half the errors are "x was not declared in this scope",
which means that it can’t compile because it needs some of the things you’ve left out:

  • MENU_OTHER
  • menuItem_FuncPointer
  • boolPointer1
  • boolPointer2
  • TO_
  • ButtonState
  • ab
  • BLACK
  • WHITE

What little I can tell you about what’s wrong with it (just from reading the error list, so this may not be all the problems):

Your first problem is that you’re using ... outside a comment (here and here to be precise).
... does actually have a meaning in C++, but only in very specific contexts, hence this is upsetting the compiler.

Secondly, MENU_ITEMS_LOOKUP is supposed to be an array of const MenuItemInfo *, but you’re trying to initialise the pointers with {MENU_ITEMS_MAIN, 4}.
In other words, you’re trying to initialise pointers to objects with the objects themselves.
Presumably MENU_ITEMS_LOOKUP should instead be an array of const MenuItemInfo.

Your third problem is with (*(menuItems[index].actionPointer))().
actionPointer is a void *, not a function pointer, so you can’t try to call it.
In fact, the only things you can do with a void * is reassign it or cast it to another type.
Most of the time resorting to void * is a bad idea because it removes important type information and is often a sign of a bad design.

Reading the comment on actionPointer (Can be either a ROM function pointer or a RAM variable pointer)…
@drummyfish’s suggestion of using a union is possibly a better option in this case,
but I can’t really offer up a solution without knowing more.
Specifically I’d need to know:

  • What types of things actionPointer would be pointing to
  • Whether the type of object actionPointer points to depends soleley on the value of MenuItem::type or whether there’s more to it

I’m very concerned about what TO_ is defined as.
I worry in general when I find macros cropping up in code because they generally aren’t needed and should be avoided like the plague for anything other than conditional compilation.

While I think of it, don’t use ‘macro case’ (all caps and underscores) for anything other than macros, it’s considered bad practice in C++.

By the way, if you want I can show you a way to make it possible to write:

const MenuItemInfo MENU_ITEMS_LOOKUP[]
{
	MENU_ITEMS_MAIN,
	MENU_ITEMS_MODE,
	MENU_ITEMS_MODE_A,
	MENU_ITEMS_MODE_B,
	MENU_ITEMS_OPTIONS
};

And have the length automatically inferred by the compiler.
It requires using a (fairly simple) template function though,
so I’d need to explain how it works.


I usually avoid cplusplus.com because it’s been known to get important things wrong before (mainly in the documentation of the standard library, but also misusing terminology), but that particular part of the tutorial looks fine for an intro at least.

It does some stuff I don't like though:
  • Declaring local variables before they’re used
  • Not putting spaces around binary operators
  • Redundant brackets on return values
    • E.g. return (g); should simply be return g;
  • Using function pointers ‘the long way’
    • E.g. (*functocall)(x, y) instead of just functocall(x, y)
  • Bad variable names
    • E.g. functocall instead of functionToCall or just function
    • psize instead of just size
  • using namespace std; - don’t do this!

I’d like to also offer up this tutorial though.

Specifically these sections:


Actually that might not be the best way.
If this were desktop programming then sure, that’s how UI stuff is often done,
but virtrual functions chew up a fair bit of memory on Arduboy.

As I said earlier in my comment, unions could be an option,
but whether they’re the best option depends on how they’re used and what they’re used for.

And they absolutely must have some way of knowing which member of the union is the active member (e.g. through enum-based tagging), otherwise you’re looking at undefined behaviour.

Two possible problems with that.

Firstly switches become horrible to maintain after a few items.
A progmem array oriented approach makes the code more data driven and allows the menu definitions to be spread across separate files instead of squashed into one big monolith.

Secondly, they can often end up using more memory than a hand-crafted progmem manipulation approach.
The compiler will sometimes turn the code into a jump table,
but I’ve found that it doesn’t do that nearly as much as it should do.

switches are only a reasonable option if there are only a few menus and the logic in them is quite simple.

2 Likes