ImpulseAdventure / GUIslice

GUIslice drag & drop embedded GUI in C for touchscreen TFT on Arduino, Raspberry Pi, ARM, ESP8266 / ESP32 / M5stack using Adafruit-GFX / TFT_eSPI / UTFT / SDL
https://www.impulseadventure.com/elec/guislice-gui.html
MIT License
1.17k stars 209 forks source link

gslc_SetPageCur does not work reliably #516

Closed ilka-schulz closed 1 year ago

ilka-schulz commented 1 year ago

Describe the bug

gslc_SetPageCur works just fine for the first couple times switching from one page to another. However, after the first couple switches, it shows an array of spurious phenomena:

Device hardware

Checklist to try first

Most display/touch issues can be identified by working through the steps in the Configuring GUIslice guide.

Please confirm whether:

Expected behavior

When switching to another page by calling gslc_SetPageCur the new page is shown and they previous page is not shown again. All page switching succeeds.

Initialization messages

TODO: add this – I currently do not have the device connected via USB.

Additional info

All page switching in my application is done in button callbacks which looks sth. like this:

enum class Page : int16_t {
    E_PG_STARTUP,
    E_PG_UPDATE,
    E_PG_INFOSCREEN,
    E_PG_MAIN,
    E_PG_SETTINGS_ROOT,
    E_PG_SETTINGS_NETWORK,
    E_PG_SETTINGS_NETWORK_ETHERNET,
    E_PG_SETTINGS_NETWORK_WIFI,
    E_PG_SETTINGS_SYSTEM,
    E_POP_KEYPAD,
    MAX_PAGE
};

std::map<gslc_tsElemRef *, Page>
    settings_node_button_pages;  // associates the IDs of buttons for settings
                                 // nodes with their respective settings page
bool on_setting_node_button_click(void *pvGui, void *pvElemRef,
                                  gslc_teTouch eTouch, int16_t nX, int16_t nY) {
    gslc_tsElemRef *pElemRef = (gslc_tsElemRef *)(pvElemRef);

    Page page;
    try {
        page = settings_node_button_pages.at(pElemRef);
    } catch (std::out_of_range &) {
        ESP_LOGE("GUI", "settings button click: button not found");
        page = Page::E_PG_STARTUP;
    }
    ESP_LOGD("GUI", "show page %i", page);
    gslc_SetPageCur(&m_gui, (int16_t)page);

    return true;
}

Adding/removing these compiler flags has no effect on the issue: -DBOARD_HAS_PSRAM -mfix-esp32-psram-cache-issue

Pconti31 commented 1 year ago

@ilka-schulz Sorry, but there is no way guislice gslc_SetPageCur() is failing. Now if you could create a small standalone sample program that fails and has no external libraries needed and you post a zip file here I would be willing to look at it.

But my guess is that you won't be able to create such a sample app. I suspect your app most likely is stepping on memory at some point or simply no longer asking for the correct page.

If this is true you will need to add trace statements or set up for JTAG debugging so you can set breakpoints.

For tracing I would start with setting DEBUG_ERR and uncommenting DBG_LOG inside your config file.

  #define DEBUG_ERR               2   // 1,2 to enable, 0 to disable

 // -----------------------------------------------------------------------------
  // Debug diagnostic modes
  // -----------------------------------------------------------------------------
  // - Uncomment any of the following to enable specific debug modes
  #define DBG_LOG           // Enable debugging log output

Then I would modify GUIslice.c gslc_SetStackPage (the only routine called by gslc_SetPageCur) to add a another trace statement at the very top:

void gslc_SetStackPage(gslc_tsGui* pGui, uint8_t nStackPos, int16_t nPageId)
{ 
#if defined(DEBUG_LOG)
  GSLC_DEBUG_PRINT("gslc_SetStackPage: nStackPos=%u nPageId=%d\n",nStackPos, nPageId);
#endif 

Hopefully, that will give you a starting point for further tracing or debugging.

One word of warning since you choose to create an odd way to define your enums you should know that E_PG_MAIN is always 0 in GUIslice apps and this isn't true in your code. I don't think there is anything in GUIslice that depends upon this but I haven't dug deeply into @ImpulseAdventure Calvin's code in years so...

Paul--

ilka-schulz commented 1 year ago

@Pconti31 Thank you so much for the detailed answer! I got it fixed and will leave a short post-mortem below for other people who stumble across the same problem:

Debugging

I attached a serial interface and found valuable information about the problem: my button callback function is called in unexpected situations. When the app shows a page which has already been shown before, the callback function gets called with some interesting gslc_teTouch values, specifically 2 (GSLC_TOUCH_DOWN_IN), 5 (GSLC_TOUCH_UP_IN) and 6 (GSLC_TOUCH_UP_OUT). Of course, I am only interested int the GSLC_TOUCH_UP_IN touch events.

Fix

I added these two lines to my button click callback function before it can call gslc_SetPageCur:

if (eTouch != gslc_teTouch::GSLC_TOUCH_UP_IN)
    return true;

Other Suspected Causes

Heap

I create my pages dynamically with some recursive function which traverses a tree of pages to create. It is easiest to allocate memory for the gslc_PageAdd call on the heap. simplified version:

struct Node {
    const char *title;
    const gui::Page gui_page;
    const std::vector<std::variant<Node, class Entry>> children;
};
success_t init_settings_page(const settings::Node &node, const Page &parent_page) {
    // create page on heap
    const size_t elem_num = node.children.size() + 5;  // size of GUI element buffer
    gslc_PageAdd(&m_gui, (int16_t)node.gui_page, new gslc_tsElem[elem_num],
                 elem_num, new gslc_tsElemRef[elem_num], elem_num);
    // add elements...
    // <truncated>

    // decent into children of `node`
    for (child : node.children)
        init_settings_page(child, node.gui_page);

    // return
    return success_t::SUCCESS;
}

However, this seems to work fine with GUIslice.

Zero-Initialization

The examples use global variables for the above buffers passed to gslc_PageAdd which means they are all initialized with 0. However, GUIslice works fine if the buffers are not zero-initialized.

Page enum

My Page enum currently looks sth. like this because GUIslice uses int16_t for page index:

enum class Page : int16_t {
    E_PG_STARTUP,
    E_PG_UPDATE,
    E_PG_INFOSCREEN,
    E_PG_MAIN,
    E_PG_SETTINGS_ROOT,
    E_PG_SETTINGS_NETWORK,
    E_PG_SETTINGS_NETWORK_ETHERNET,
    E_PG_SETTINGS_NETWORK_WIFI,
    E_PG_SETTINGS_SYSTEM,
    E_POP_KEYPAD,
    MAX_PAGE
};

GUIslice expects the E_PG_MAIN to have the value 0 but in the above example E_PG_STARTUP which is shown as the first page after boot has the index 0. GUIslice does not bother and works either way.

ilka-schulz commented 1 year ago

@Pconti31 I just saw that this issue is still open. The fix above works and I think this issue can be closed. Thank you again for your help!

Pconti31 commented 1 year ago

@ilka-schulz I'm happy it's working for you now. Here I'm just another user like you. I don't have write access so I can't close issues or merge pull requests myself except inside the GUIslice Builder repository. However, You should be able to close it since you opened it. Paul--