neu-rah / ArduinoMenu

Arduino generic menu/interactivity system
GNU Lesser General Public License v2.1
929 stars 189 forks source link

Arduinomenu doesn't work with Mega + SD Card + ST7920 display #391

Open hicksan opened 1 year ago

hicksan commented 1 year ago

This is purely due to the data structure used by ArduinoMenu and the memory addressing is using PROGMEM for AVRs and RAM for non-AVRs and AVRs with dynamic menus – but hasn’t implemented that feature in the u8g2 attachment.

It works fine with serial output, just not on the screen (displays garbage). This only applies to dynamic menus (eg file list from Sd card), not static (hardcoded).

This works fine with non-AVR processors - I have switched to an ESP8266 instead - but would prefer to run it on a MEGA to gain the use of the extra I/O pins.

hicksan commented 1 year ago

Here is an example of initializing an option without macros for an non-AVR (PROGMEM) device. //define "Op 1" void op1Func(); promptShadow op1Info("Op 1",(callback)op1Func,enterEvent); prompt op1(op1Info); And here the same for an AVR (PROGMEM) device. //define "Op 1" void op1Func(); const char op1Text[] PROGMEM="Op 1"; const promptShadowRaw op1InfoRaw PROGMEM={(callback)op1Func,_noStyle,op1Text,enterEvent,noStyle}; const promptShadow& op1Info=(promptShadow)&op1InfoRaw; prompt op1(op1Info); OP macro automates all of that, but it must be used inside a MENU macro. You will notice how the data structure differs. This is triggered by the MENU_USERAM being defined.

There are two types of menu structures: static and dynamic. The static ones are defined and fixed entirely within the code. The dynamic ones are defined in the code but content is fixed at run time based on reading other data (like reading the directory list from an SD card). These do not have a fixed memory allocation. If MENU_USERAM is NOT used then these will be written, like the fixed ones, in the program memory space. With fixed menus that would not be a problem but with dynamic menus it results in the program being corrupted with all the behaviour that we saw.

The dynamic menus are a new feature. ArduinoMenu has not implemented that feature in the u8g2 library interface, so it is trying to print only from PROGMEM. If we are using MENU_USERAM this will result in it trying to read the menu data from the wrong place.

Here is a quote from the Utils page:

idx_t print_P(Print& s,const char at,idx_t len) print a PROGMEM string, a string that resides on flash memory. On AVR devices you can not print the char because the system will try to read from ram instead. There are some more data residing in flash, be careful when handling pointers to internal data. print_P(Serial,item.getText());

So I think the problem lies with this line (138) in u8g2Out.h:

                                            idx_t printRaw(const char* at,idx_t len) override {
                                                            trace(Serial<<"u8g2Out::printRaw"<<endl);
                                                            trace(Serial<<"["<<at<<"]");
                                                            return print((__FlashStringHelper*)at);
                                              // const char* p=at;
                                              // uint8_t ch;
                                              // for(int n=0;(ch=memByte(at++))&&(len==0||n<len);n++) {
                                                            //            trace(Serial<<"["<<ch<<"]");
                                              //   write(ch);
                                              // }
                                              // return at-p-1;
                                            }

So I think it is that “const char * at” that is causing the problem.

I believe the u8g2Out.h lib is based on the (earlier) Adafruit_gfx library. The data structure (line 19 of u8g2Out.h) reads:

            class u8g2Out:public gfxOut {
                            public:

There is an AdafruitGfxOut.h library, as part of ArduinoMenu. This in turn references Adafruit_GFX.h which is a ‘core’ Arduino library. I think gfxout is defined there.

Adafruit-GFX-Library/Adafruit_GFX.h at master • adafruit/Adafruit-GFX-Library (github.com)

I am now experimenting with the MENU_DEBUG function (definition in menuDefs.h, line #9) but can’t seem to get it to work.

The output is supposed to go to MENU_DEBUG_OUT if defined (how? Filename?) otherwise defaults to Serial – and serial is already in use as menu output so how to access all that trace data?

hicksan commented 1 year ago

Any progress on this issue, please, Rui?

neu-rah commented 1 year ago

Hi!

debug will mix with normal output, its a mess, the MENU_DEBUG_OUT is just a gateway that allows cleaning all debug info from the build when needed.

In principle all things that work with Adafruit_GFX should work with the menu, any compile errors?

hicksan commented 1 year ago

Debug is not the issue here, Rui. I was simply trying to get debug to help figure out exactly where the error is. The problem, stated at the top of this thread, is that your data structures in ArduinoMenu use PROGMEM for AVR processors and RAM for non-AVRs but the u8g2 attachment does not.

It compiles fine and runs perfectly through serial but displays garbage to the screen of the ST7920 display when used with a Mega though not with an ESP8266. The error is in the dynamic menu functions, so normal menus are fine but file lists from an SD card, for example, trigger garbage.

neu-rah commented 1 year ago

there is a define to force use ram, MENU_USERAM, and you got it... I'm focusing on new menu version, where i'm not forcing the FLASH thing among many other things :\ thanks for your interest, will you also be interesting on testing the new stuff?

I got almost all item types working on a PC console (mcu Serial is very similar)

still have to do scrolls, graphics with measuring and clipping, drawing modes and triggers, events, etc...

hicksan commented 1 year ago

Unfortunately the definition MENU_USERAM does not work with the u8g2out library. It causes the LCD output to be garbage.

Here is a comment from a friend who investigated for me:

"There are two types of menu structures: static and dynamic. The static ones are defined and fixed entirely within the code. The dynamic ones are defined in the code but content is fixed at run time based on reading other data (like reading the directory list from an SD card). These do not have a fixed memory allocation. If MENU_USERAM is NOT used then these will be written, like the fixed ones, in the program memory space. With fixed menus that would not be a problem but with dynamic menus it results in the program being corrupted with all the behaviour that we saw.

The dynamic menus are a new feature. He has not implemented that feature in the u8g2 library interface, so it is trying to print only from PROGMEM. If we are using MENU_USERAM this will result in it trying to read the menu data from the wrong place.

Rui himself confessed in March (on the Gitter forum) that he had not tested the dynamic menu stuff with the u8g2 output, or any LCD or OLED; only with the serial monitor output.

I have been looking through the other LCD output libraries parcelled with ArduinoMenu to see if Rui fixed any of them since then, to see what he did. No joy so far.

Here is a quote from the Utils page:

idx_t print_P(Print& s,const char at,idx_t len) print a PROGMEM string, a string that resides on flash memory. On AVR devices you can not print the char because the system will try to read from ram instead. There are some more data residing in flash, be careful when handling pointers to internal data. print_P(Serial,item.getText());

So I think the problem lies with this line (138) in u8g2Out.h:

                                            idx_t printRaw(const char* at,idx_t len) override {
                                                            trace(Serial<<"u8g2Out::printRaw"<<endl);
                                                            trace(Serial<<"["<<at<<"]");
                                                            return print((__FlashStringHelper*)at);
                                              // const char* p=at;
                                              // uint8_t ch;
                                              // for(int n=0;(ch=memByte(at++))&&(len==0||n<len);n++) {
                                                            //            trace(Serial<<"["<<ch<<"]");
                                              //   write(ch);
                                              // }
                                              // return at-p-1;
                                            }

So I think it is that “const char * at” that is causing the problem.

I believe the u8g2Out.h lib is based on the (earlier) Adafruit_gfx library. The data structure (line 19 of u8g2Out.h) reads:

            class u8g2Out:public gfxOut {
                            public:

There is an AdafruitGfxOut.h library, as part of ArduinoMenu. This in turn references Adafruit_GFX.h which is a ‘core’ Arduino library. I think gfxout is defined there. "

hicksan commented 1 year ago

And yes I'm happy to test anything new, but would very much like to fix the u8g2 bug in the existing system first.

In all other respects this is an absolutely superb system. This one bug is very frustrating because it stops me using it!