External C modules & root pointers support

[andrewleech]

I've been working on some minor refactoring of the lvgl library to support building with the standard External C Module system.

The library / module compiled has a number of global variables that are dynamically allocated when running a lvgl_init() function on first use. These are currently stored / tracked as root pointers, now registered with the new #define eg.

MP_REGISTER_ROOT_POINTER(lv_layout_dsc_t *_lv_layout_list);

These then make their way into build/genhdr/root_pointers.h which is then included in the mp_state_vm_t struct in py/mpstate.h

However, this throws compile issues because py/mpstate.h doesn't #include any of the (external c module specific) headers to declare the lv_layout_dsc_t etc types.

For my initial proof of concept PR linked above, I hacked around this with CFLAGS_USERMOD += -include "lvgl/lvgl.h" for stm32 / unix builds, but don't know if/how this is possible with cmake builds - it's not ideal either way.

I've had some thoughts on alternative options, not sure of the best course forward though?

• new define to add includes
  Add a #define MP_REGISTER_ROOT_POINTER_INCLUDE that is parsed by the same root pointers genhdr to create an header file to be #include'd in py/mpstate.h. Not really my preferred approach, seems pretty heavy handed for something that's not been needed before? But perhaps quite flexible?

• register as void*
  Register all the root pointers like MP_REGISTER_ROOT_POINTER(void*_lv_layout_list); to avoid the need of includes, then cast them to the correct type somewhere else in the external C module for use within lvgl.

• don't use root pointers, attach to module itself
  This would really be my preferred approach; the lvgl module that's created allocates these objects during an init function, it would arguably be better if they were just attached to the module itself rather than directly in global root pointers table. However currently, the module is CONST declared in flash so can't have new attributes attached.

Eventually I want to make this module work as a native mpy as well; in this case the module is always dynamically created in the mpy_init() function at which point these pointers could be allocated and attached to the module itself - perfect!

I'm wondering if there's some way we could create a similar mpy_init() function that's run-on-import for compiled-in user C modules that could then be a common system for both native and external c modules?

[jimmo]

Option 2 (void*) seems like a pretty reasonable approach for now (especially with some helper macros). But definitely something that can be improved in the future.

Frustratingly, when you write MP_REGISTER_ROOT_POINTER(lv_layout_dsc_t *_lv_layout_list) the issue isn't that the type isn't available (the compiler doesn't care, it's just a pointer), it's that the lv_layout_dsc_t typedef doesn't exist.

So if the struct had been declared

typedef struct lv_layout_dsc_t {
    ....
} lv_layout_dsc_t;

Then you could have written MP_REGISTER_ROOT_POINTER(struct lv_layout_dsc_t *_lv_layout_list) and it would have been fine. But it's actually declared

typedef struct {
    ...
} lv_layout_dsc_t;

so no dice.

One option would be to do this in your user module C code:

typedef struct lvgl_root_pointers_t {
  lv_layout_dsc_t *_lv_layout_list;
} lvgl_root_pointers_t;

MP_REGISTER_ROOT_POINTER(struct lvgl_root_pointers_t * lvgl);

and then heap-allocate the root pointers struct. This is basically what the NimBLE bindings do. The disadvantage is that you now spend an extra word of .bss, but the advantage is that the RAM for the pointers isn't used unless you use LVGL.

(Note that you must heap-allocate the root pointers struct, it can't be in .bss itself, because the GC won't search it -- it only follows heap pointers)

[andrewleech]

Ah yes, at one point I had wrapped so the lvgl pointers on a new struct (actually a new micropython type) that meant there was only one struct that needed to be registered globally, but then figured that anyone actually compiling lvgl into their codebase is going to be using it, so I shouldn't need to worry about not using ram if it's not used.

Wrapping it in that way to allow cleaner registration (without just void*) is nice though, and better than needing to wrap each pointer in the list with individual casting access macros.
I was thinking of trialling the void* approach just after submitting this discussion but was struggling to think what that would actually look like in practice because these root pointers are all auto-generated with iterating macros already and accessed via a macro so there's a few levels of complexity to work around. The one common containing struct to macro/cast past would be much simpler.

[GeorgeCGV]

@andrewleech @jimmo I followed this discussion but got into a situation where callbacks are still being garbage collected.

I use Zephyr with modified LVGL to expose lvgl_init and lvgl_heap. Micropython (b110266) and adapted lv_bindings based on @andrewleech lvgl/lv_binding_micropython#242 PR. Therefore, MP_REGISTER_ROOT_POINTER(struct lvgl_root_pointers_t * lvgl); is used as well as LV_GC_ROOT(x) MP_STATE_VM(lvgl->x).

The application interprets single script at startup:

import lvgl as lv
import dsys as ds

lv.linit()
scr = lv.obj(None)

label = lv.label(scr)
label.set_text('Hello World!')
label.center()

def timer_callback(event_struct):
    text = label.get_text()
    print(text)
    try:
        label.set_text(str(int(text)+1))
    except ValueError:
        label.set_text("0")

lv.timer_create(timer_callback, 1000, None)

def gc_callback(event_struct):
    print('gc')
    try:
        ds.gc_collect()
    except:
        print('gc failed')

lv.timer_create(gc_callback, 10000, None)

lv.scr_load(scr)

LVGL is initialized with lv.linit() and GC is executed with ds.gc_collect() within standalone Zephyr (C) thread/task that deals with MPY.

The application runs and timers work until first GC execution, then it crashes with bus fault within mp_get_callbacks.
When gc_collect is modified to add lvgl_heap root it works:

void gc_collect(void)
{
	gc_collect_start();
	gc_helper_collect_regs_and_stack();
	gc_collect_root((void**)&lvgl_heap_mem, Z_LVGL_HEAP_BYTES / sizeof(mp_uint_t));
	gc_collect_end();
}

However, shouldn't MP_REGISTER_ROOT_POINTER be enough for MPY to avoid gc_collect modification? Can it be the case that callbacks stored in lvgl objects user_data (allocated within lvgl_heap) that point to MPY objects (allocated in mpy's heap) are not detected by GC? Or do I miss something?

[andrewleech]

I haven't tested my lvgl mr with zephyr, so I'm guessing it's either

• something missing with my integration there, or
• the register root pointer macro/parsing script isn't set up right for zephyr?