[v6.0] Callback conventions

As discussed on #557, #785, #316 and #935, callbacks must have a well defined structure that would allow an automatic script (for [micro]python bindings) find the related user_data of each callback.
A user_data is needed when more than one callback instance can be present for a function callback, and we need to correlate the right callback object of a higher level language with the C callback functions that was just fired.
A convention that we agreed upon was:

The callback will receive as the first parameter the pointer to the structure that contains the user_data
The user_data will be named according to the callback name (<callback>_user_data)

I checked and this convention solves some cases, but not all cases.
Here is a list of callbacks which currently do not follow this convention:
void lv_indev_feedback_t(struct _lv_indev_t *, uint8_t)
int32_t lv_anim_path_t(const struct _lv_anim_t *)
void lv_anim_fp_t(void *, int32_t)
void lv_anim_cb_t(void *)
void lv_group_style_mod_func_t(struct _lv_group_t *, lv_style_t *)
void lv_group_focus_cb_t(struct _lv_group_t *)
lv_res_t lv_img_decoder_info_f_t(const void *src, lv_img_header_t *header)
const uint8_t *lv_img_decoder_open_f_t(const void *src, const lv_style_t *style)
lv_res_t lv_img_decoder_read_line_f_t(lv_coord_t x, lv_coord_t y, lv_coord_t len, uint8_t *buf)
void lv_img_decoder_close_f_t(void)
lv_res_t lv_tileview_action_t(lv_obj_t *, lv_coord_t, lv_coord_t)
void lv_spinbox_value_changed_cb_t(lv_obj_t *spinbox, int32_t new_value)
void mem_blend_cb(lv_color_t *dest, const lv_color_t *src, uint32_t length, lv_opa_t opa)
void mem_fill_cb(lv_color_t *dest_buf, const lv_area_t *dest_area, const lv_area_t *fill_area, lv_color_t color)
void task(void *)
const uint8_t *get_bitmap(const struct _lv_font_struct *, uint32_t)
int16_t get_width(const struct _lv_font_struct *, uint32_t)
void cb(lv_obj_t *) // used on lv_obj_animate
void f(lv_log_level_t, const char *, uint32_t, const char *)

I would like to ask your opinion:

Which functions do you think are important for [micro]python bindings? My opinion is that we should support as much as possible, including images, animations etc.
Do we need user_data for each of these functions? In some cases (like logging) a single global callback is enough so we don't need a user_data per callback (or we can manage it globally, internally in the binding code)
Where do you think we can add the missing user_data on each of these callbacks? Can we use the existing convention (and pass a struct as first parameter) or do we need some new convention (like passing the user_data itself to the callback)?

I think lv_spinbox_value_changed_cb_t and lv_tileview_action_t should be updated to use the event system.

(cc @kisvegabor)

I'm a little late to this party, but I've read up on the prior discussions referenced above.
Apologies in advance for being a dissenting voice, but it feels like there is the potential for a lot of complexity,  overhead, and constraints to be added on the LVGL side in order to support automated code generation for bindings and python/micropython in general.
When I see suggestions, such as adding macros to reference count micropython objects, requiring fixed struct member order and naming, adding different user_data depending on the callback, it gives me pause. All of these changes restrict the code, and make future development more difficult and constrained. Some will also increase the resource load.
I just wanted to raise this because while those that are developing the bindings are understandably vocal in their requirements and @kisvegabor has been extremely accommodating in making changes, there are likely a lot of current C users of LVGL that don't follow these issues and are thus silent in these discussions.
There are both less and more resource intensives GUI libraries out there, but LGVL sits in a somewhat unique position, running on MCUs with very limited resources, and is very cleanly structured and documented. We need to remember that the gradual creep of too much complexity and overhead has the potential to push it out of that niche.
Apologies again for the intrusion. This is not the place for an extended discussion, nor would I like to start one. Just wish to remind everyone to remain mindful of these possibilities.

@canardos As a C developer myself, I can definitely understand where you're coming from.

adding macros to reference count Micropython objects

This was actually for Python itself, not Micropython. Micropython doesn't need that complexity.

Requiring fixed struct member order

Can you point me to that suggestion? I don't see why that would be necessary for a high-level language binding.

This was actually for Python itself, not Micropython

Got it, but I think the point remains. Regarding the struct ordering, I don't recall exactly where I saw it mentioned. There have been a lot of discussions and it's possible I'm misremembering function/param naming/param order, but as mentioned, I don't really want to get into a discussion around individual changes, just suggest that we remain mindful of striking the right balance between flexibility and added complexity/overhead.
I'll bow out of this discussion here. Keep up the good work all :)

@canardos I just want to mention that LittlevGL version v5.3 already supports micropython. It required very small modifications to LittlevGL sources in order to add this support, and all these modifications were backward compatible and invisible to the user.
I would like to make the discticntion between Python and MicroPython requirements, I think it's important.

There are both less and more resource intensives GUI libraries out there, but LGVL sits in a somewhat unique position, running on MCUs with very limited resources, and is very cleanly structured and documented. We need to remember that the gradual creep of too much complexity and overhead has the potential to push it out of that niche.

I agree with your point, and still I think that micropython fits LittlevGL well. It is targeted to the same audience (MCUs with very limited resources) as LVGL.
You are right that there was a lot of discussion about adding conventions and changing the API prototypes, but most of them were related to the Python binding, not micropython.  Examples are reference counting and special naming conventions to identify who allocates memory (caller or callee), all of which are not needed for micropython.
The only exception, and really the only big feature that is planned for micropython on LVGL v6.0 is the Generic Callbacks support. It would add a lot of flexibility for micropython and we'll make sure that the cost on C is minimal - this is why I opened this issue in the first place!
I would like to identify which callbacks are important for micropython. Those which are not, will not change. For those which are important, I hope the change will be minimal, and will not add penalty in resources or performance. Regarding the C style, if anything, it will make it more clear as we would need to use a clear and consistent convention for callback definitions.
In general, I think user_data is instrumental for callbacks in general, not only in the micropython bindings context. For example, if you ever want to use LVGL with C++ in a proper way and pass over the object pointer to callbacks, user_data is as useful.

@amirgon Thank you for this list. It's extremely useful. I fixed some the issue but some need to be discussed.

void lv_indev_feedback_t(struct _lv_indev_t *, uint8_t)

I moved feedback_cb to the driver, similarly to read_cb.

void mem_blend_cb(lv_color_t *dest, const lv_color_t *src, uint32_t length, lv_opa_t opa)
void mem_fill_cb(lv_color_t *dest_buf, const lv_area_t *dest_area, const lv_area_t *fill_area, lv_color_t color)

I updated them.

int32_t lv_anim_path_t(const struct _lv_anim_t *)

I added user data to lv_anim_t.

void lv_group_style_mod_func_t(struct _lv_group_t *, lv_style_t *)
void lv_group_focus_cb_t(struct _lv_group_t *)

Groups already have user data. What was the problem here?

const uint8_t *get_bitmap(const struct _lv_font_struct *, uint32_t)
int16_t get_width(const struct _lv_font_struct *, uint32_t)

No user data in the fonts right now. The fonts are being reworked and will take care of adding a user data.

void lv_anim_fp_t(void *, int32_t)

It's a generic prototype compatible with the majority of lv_xxx_set_yyy() functions. Should be fine this way.

void lv_anim_cb_t(void *)
void cb(lv_obj_t *) // used on lv_obj_animate

void * means the "thing" being animated. However, it would more versatile to pass lv_anim_t. I updated this way.

lv_res_t lv_img_decoder_info_f_t(const void *src, lv_img_header_t *header)
const uint8_t *lv_img_decoder_open_f_t(const void *src, const lv_style_t *style)
lv_res_t lv_img_decoder_read_line_f_t(lv_coord_t x, lv_coord_t y, lv_coord_t len, uint8_t *buf)
void lv_img_decoder_close_f_t(void)

User data might be used in case of open, read_line and close. I'm sure it's required.

lv_res_t lv_tileview_action_t(lv_obj_t *, lv_coord_t, lv_coord_t)
void lv_spinbox_value_changed_cb_t(lv_obj_t *spinbox, int32_t new_value)

These are obsolete. I deleted them.

void task(void *)

void * is a pointer defined by the user in lv_task_create. It's like a user data. It's a common practice to have void * user_data parameter in task create functions so I think it's fine this way.

void f(lv_log_level_t, const char *, uint32_t, const char *)

A global user data should be fine as discussed earlier.

@canardos I agree with your concerns. Care must be taken to keep the library readable and easy to contribute. Please join the conversation in #935 about the Python binding. It might introduce some inconveniences so any ideas are welcome!

@kisvegabor

void mem_blend_cb(lv_color_t *dest, const lv_color_t *src, uint32_t length, lv_opa_t opa)
void mem_fill_cb(lv_color_t *dest_buf, const lv_area_t *dest_area, const lv_area_t *fill_area, lv_color_t color)

I updated them.

I can't find mem_blend_user_data and mem_fill_user_data. Did you add them?

int32_t lv_anim_path_t(const struct _lv_anim_t *)

I added user data to lv_anim_t.

What about typedef void (*lv_anim_exec_cb_t)(void *, int32_t); ?

void lv_group_style_mod_func_t(struct _lv_group_t *, lv_style_t *)
void lv_group_focus_cb_t(struct _lv_group_t *)

Groups already have user data. What was the problem here?

These callbacks are defined like this:
typedef struct _lv_group_t
{
    ...
    lv_group_style_mod_func_t style_mod;
    lv_group_style_mod_func_t style_mod_edit;

Can we name them style_mod_cb and style_mod_edit_cb, as other callbacks?

void task(void *)

void * is a pointer defined by the user in lv_task_create. It's like a user data. It's a common practice to have void * user_data parameter in task create functions so I think it's fine this way.

That's fine, but if we want to support this, we need to define a new convention that would identify these cases.
In which cases can the script assume that a void * passed to a callback function is the user_data?

Addition problem:
lv_event_cb_t is used like this:
void lv_obj_set_event_cb(lv_obj_t * obj, lv_event_cb_t cb);

The user_data name is derived from the parameter name cb and not the type lv_event_cb_t.
This is what we were doing until now. For example, when the script looks for the user_data for lv_event_cb_t event_cb; member, it look for event_user_data based on event_cb member name. We can't really use the type name to find the corresponding user_data because same type may appear more than once on the same struct (with different user_data for each), for example, style_mod and style_mod_edit.
The problem in this case is that the parameter name is cb and there is no user_data called cb_user_data...
So I suggest changing the callback function parameter name on functions that receive function pointers, to be consistent with the corresponding user_data name.
The parameter is not named correctly on:

lv_obj_set_event_cb
lv_obj_set_signal_cb
lv_obj_set_design_cb
lv_group_set_style_mod_cb
lv_group_set_style_mod_edit_cb

Another option is to change the convention to allow the script find the user_data according to the type name in certain cases, but I think it's less favorable because it's better to keep the callback conventions as simple as possible and not allow multiple "options" that could cause confusion.

@amirgon
All seems doable. I'll fix them later today.

I hope I fixed all the mentioned things. See 514e2ba

hope I fixed all the mentioned things. See 514e2ba

@kisvegabor Going over the changes, I can see many of these issues are resolved. Thanks!
Remaining open issues are:

img_decoder callbacks
lv_anim_exec_cb_t
get_bitmap/get_width - waiting for reworked fonts
Task callback convention.
How can the script know that void * passed to a callback is meant to be used as the user_data itself?

We can either pass a pointer to lv_task_t and add user_data to lv_task_t
Or we can leave it as is with void* parameter, but extend the user_data convention definition: Name the parameter user_data and define a new convention that a whenever a callback receives void *user_data parameter, it will use it as the user_data.
This would require naming the parameter user_data in such cases. For example,

    void (*task)(void *user_data);

img_decoder callbacks

I still need to examine it.

lv_anim_exec_cb_t

It should be compatible with LittlevGL API functions so we can't add user data here.
Shall we rename this function to avoid confusion? E.g. exec_cb -> animator_func.

We can either pass a pointer to lv_task_t and add user_data to lv_task_t

I like this approach. With this, the pointer to the created task (lv_task_t * task = lv_task_craete(...)) doesn't have to be stored because it's available in the task itself.  I update dev-6.0 with this.

@kisvegabor

lv_anim_exec_cb_t

It should be compatible with LittlevGL API functions so we can't add user data here.
Shall we rename this function to avoid confusion? E.g. exec_cb -> animator_func.

I'm not so sure how we can support this on Micropython. Could you explain how it is meant to be used?

We can either pass a pointer to lv_task_t and add user_data to lv_task_t

I like this approach. With this, the pointer to the created task (lv_task_t * task = lv_task_craete(...)) doesn't have to be stored because it's available in the task itself. I update dev-6.0 with this.

A few things were not clear to me, I've added comments on 5243d235a6.

animations
In C it looks like this:
lv_anim_t a;

a.var = obj;
a.start = 10;
a.end = 100;
a.exec_cb = (lv_anim_exec_cb_t)lv_obj_set_y;
a.path_cb = lv_anim_path_linear;
a.ready_cb = NULL;
a.act_time = 0;
a.time = 200;
a.playback = 0;
a.playback_pause = 0;
a.repeat = 0;
a.repeat_pause = 0;
a.user_data = NULL;

lv_anim_create(&a);

However, I don't how it can be used in MicroPython. Shall we improve something here?

A few things were not clear to me, I've added comments on 5243d23.

I replayed in the commit.

However, I don't how it can be used in MicroPython. Shall we improve something here?

@kisvegabor Is there a strict naming convention for those functions that are assigned to exec_cb? Something like lv_obj_set_<var>?
If there is, we can support this in Micropython by providing access to the addresses of each of these functions from Micropython.
I provide two ways for assigning a callback in Micropython:

One is by providing a python callable object such as a python function. In such case we need a user_data to store the pointer to the micropython callable object.
The other is by directly providing the address of a C function. This is useful when the callback is already implemented in C and we have access to the callback function pointer in Micropython, for example in case of a display driver that is implemented in C.

What we are missing in this case is a list of pointers to functions that can be assigned to exec_cb.
If we could identify all the relevant functions that could be assigned to exec_cb, we could write something like this in Micropython:
a.exec_cb = lv.obj_set.y

Where obj_set is a class that contains the constant values of the pointers to each of these C functions.

Not only lv_xxx_set_yyy() functions can be used as callback but any custom, user-defined function.
So the two cases:

Use built-in functions directly: lv_anim_exec_cb_t should have 2 parameters (pointer to something to animate + value). This way it's easy to create animations to change some basic properties.
Use custom functions: the first parameter of lv_anim_exec_cb_t should have a pointer to lv_anim_t to get the user data.

I don't see a good solution right now.

I don't see a good solution right now. Do you have any ideas?

@kisvegabor Yes, I think this is solvable.

Use built-in functions directly: lv_anim_exec_cb_t should have 2 parameters (pointer to something to animate + value). This way it's easy to create animations to change some basic properties.

For the built in functions, their names should obey some naming convention.
For example lv_<obj>_set_<var>(void *, int). Is this the case already?
Could you list all the built-in functions that can be used with exec_cb?
Once the script can identify them, we can let the user assign them to exec_cb in the python script.
In the same way, we would also need a list of built-in functions that can be assigned to path_cb.
Another option, instead of a naming convention, is using an enum in the same manner we solved the Symbols issue. (an enum that would list all functions that could be used with exec_cb and path_cb). The disadvantage of an enum is that it would need to be maintained when built-in functions are added/removed, so I like the naming-convention option better.

Use custom functions: the first parameter of lv_anim_exec_cb_t should have a pointer to lv_anim_t to get the user data.

We can simply add another inline function like this:
typedef void (*lv_anim_custom_exec_cb_t)(lv_anim_t *, int32_t);

inline lv_anim_set_custom_exec_cb(lv_anim_t *a, lv_anim_custom_exec_cb_t exec_cb)
{
  a->var = NULL;
  a->exec_cb = (lv_anim_exec_cb_t)exec_cb;
}

In this case, the script will identify exec_user_data as the relevant user_data on lv_anim_t struct, and will update a->exec_user_data before calling lv_anim_set_custom_exec_cb.
(Question: What should be assigned to a->var in this case? I assumed that NULL)
What do you think?

Another problem
https://github.com/littlevgl/lvgl/blob/9f216a55be65ec05c477ac55f73a6efad50f7680/src/lv_core/lv_obj.h#L624-L625
lv_obj_animate does not obey the callback convention: It receives a callback ready_cb, and it looks for the user_data on the first function parameter - which is obj.
However, ready_cb is defined on lv_anim_t and not on lv_obj_t so the binding code cannot find it.

Based on your idea with the inline callback we can add functions for all parameters of lv_anim_t. Probably it would worth to group the related ones. For example:
lv_anim_init(&a);  //Set default values

lv_anim_set_var(&a, var);
lv_anim_set_values(&a, start_val, end_val);
lv_anim_set_time(&a, duration, delay);

lv_anim_set_exec_cb(&a, exec_cb);
lv_anim_set_path_cb(&a, path_cb);
lv_anim_set_ready_cb(&a, ready_cb);

lv_anim_set_playback(&a, enable, wait_time);
lv_anim_set_repeat(&a, enable, wait_time);

lv_anim_set_user_data(&a, user_data;

Would it make things easier?

lv_obj_animate
I see. I can imagine 2 solutions:

Remove ready_cb. I don't like it because ready_cb might be important to synchronize the animations or delete something when it is faded out.
Remove the whole lv_obj_animate. I think it's quite limited and can not be used very well. Actually, the currently supported FLOAT and GROW animation can be easily created without lv_obj_animation as well. The above proposed lv_anim_... functions also help here.

We can add functions for all parameters of lv_anim_t.

Would they all be static inline functions in a header file, or actual functions in the source files?

Remove ready_cb. I don't like it because ready_cb might be important to synchronize the animations or delete something when it is faded out.

I agree. ready_cb greatly simplifies things when it comes to chaining animations together. We should definitely keep it.

Would it make things easier?

@kisvegabor It makes sense, although for the binding I only need lv_anim_set_exec_cb, because of the mismatch we have today between lv_anim_t * (where the user_data is defined), and void * var provided to lv_anim_exec_cb_t callback.
To solve the issue we need to handle the exec_cb issue, and make a decision about naming convention for the built-in modifier functions as I explained above.

lv_obj_animate
I see. I can imagine 2 solutions:

Any of these solutions is good for me.

Would they all be static inline functions in a header file, or actual functions in the source files?

@embeddedt they can be static inline functions. The script parses them the same way as any other function. (The problem is only with constructs that are consumed by the preprocessor, like defines and comments)

I agree. ready_cb greatly simplifies things when it comes to chaining animations together. We should definitely keep it.

@embeddedt Do you agree to @kisvegabor's second solution, to remove the whole lv_obj_animate?

It's fine to remove lv_obj_animate.

In summary, I will add these lv_anim_... functions and remove lv_obj_animate().
@amirgon

To solve the issue we need to handle the exec_cb issue, and make a decision about naming convention for the built-in modifier functions as I explained above.

Using the user_data the built-in functions, (like lv.obj_set.y) can be used similarly as a custom function, right?

Using the user_data the built-in functions, (like lv.obj_set.y) can be used similarly as a custom function, right?

@kisvegabor No. If we only have user_data, we can only use custom functions, but not assign a function directly. That's because for function assignment we need to expose the C function pointer in micropython instead of performing an actual call.
That's the distinction I made above between lv.obj.set_y and lv.obj_set.y. The first is a function you can call, the second is a function pointer you can assign.
If we want to support assigning built-in functions in python (like in C), we would need a naming convention for the built-in modifier functions, so we could identify them and expose their function pointers.
If we don't provide a built-in function support, the user would always have to define a custom function to delegate the call.  That's why I suggested that we have a naming convention for built-in functions - to support assigning them directly to exec_cb (and path_cb) instead of delegating them.
Example of doing the same thing with/without delegation:

Using a built in function:

a.exec_cb = lv.obj_set.y

Using a custom function:

def my_cb(obj, new_y):
    obj.set_y(new_y)

a.exec_cb = my_cb

Another option is to provide a way to extract a C function pointer from every micropython wrapper function. It may be possible, but I'm not sure yet (it may require more memory, for example). This approach is an overkill because we really need to be able to assign only a very small number of functions. Most functions we only call, not assign.

Got it, thank you.

That's the distinction I made above between lv.obj.set_y and lv.obj_set.y. The first is a function you can call, the second is a function pointer you can assign.

In general the "lv_" + <obj> + "_set_" <somthing> convention can work. For simplicity and clearance what if we make it mandatory to use custom functions for animations? Would it be too inconvenient?

In general the "lv_" + <obj> + "_set_" <somthing> convention can work. For simplicity and clearance what if we make it mandatory to use custom functions for animations? Would it be too inconvenient?

@kisvegabor It's possible to only use custom functions for animations.
The same way with C. Instead of providing a built-in function, you can always create a custom C function that calls another function. The motivation on both C and Micropython to use built-in functions instead of custom function is convenience.
Having a strict naming convention for the built in functions is enough to allow us support this, if we want.

That's the distinction I made above between lv.obj.set_y and lv.obj_set.y

I'm afraid of these similar names might cause confusion for the users because if you look at them you can't simply and surely decide which is which.

I agree with @kisvegabor. Having both of those two functions is confusing even if you know the difference.

I'm afraid of these similar names might cause confusion for the users because if you look at them you can't simply and surely decide which is which.

@kisvegabor I agree. We can name them differently. With some effort, I can even convert it from a function to a callable object that can be converted to a pointer, thus allowing the use of the same name for both (lv.obj.set_y could be used as both callable python function, or convertible to a C function pointer).
But I need to know which functions are the built-in functions because of the extra memory it would consume (so it would apply to a small number of functions, not to all functions).
That is why I asked about the naming convention.

With some effort, I can even convert it from a function to a callable object that can be converted to a pointer, thus allowing the use of the same name for both (lv.obj.set_y could be used as both callable python function, or convertible to a C function pointer).

That would be awesome. can you estimate the memory overhead? (Just the magnitude, 1k, 10k?)

But I need to know which functions are the built-in functions because of the extra memory it would consume (so it would apply to a small number of functions, not to all functions).
That is why I asked about the naming convention.

The naming convention is "lv_" + <obj> + "_set_" <somthing>. However the type ad number of arguments should be chacked as well: one pointer + a not-pointer (can be lv_coord_t, uint16_t, lv_opa_t, etc.)
@rreilink
In Python is it more trivial to solve? Or do you need some convention or other things to set the animations' "animator" function?

That would be awesome. can you estimate the memory overhead? (Just the magnitude, 1k, 10k?)

@kisvegabor The overhead is per function. In principle, we just need to keep the function pointer value as part of the micropython callable object (function) that needs to be both callable and convertible to a pointer. The additional size for each function is small, so it's no big deal for a few functions.
What I would like to avoid is enabling this for all functions, because the total number of generated functions is already very large. My plan was to enable this only for these functions which match a naming convention ("lv_" + <obj> + "_set_" <somthing>).

The naming convention is "lv_" + <obj> + "_set_" <somthing>. However the type ad number of arguments should be chacked as well: one pointer + a not-pointer (can be lv_coord_t, uint16_t, lv_opa_t, etc.)

It's easy to check this on the script, but even if we match a larger group it won't hurt, only add a few functions that are convertible to pointers.
This check would become important if we add syntax checking to make sure the assigned function has the right prototype, but I don't check this for now.

@kisvegabor On Cortex-M/most 32-bit architectures, I would expect the overhead to be roughly 4 bytes per function assuming that it just involves adding another variable to this "callable object". But I don't know exactly how Micropython handles things.

After starting the implementation of Generic Callbacks for v6.0, I gave some more thought to the user_data-per-callback vs. single-user_data (per object/struct) strategy.
In previous posts I explained why we must have user_data-per-callback.
This is still true - I need to keep a python object for every callback. But one way to implement this is to keep a single user_data on the lvgl struct/object that points to a micropython object, and keep a dict in that micropython object with a user_data per callback. This way we actually "hide" the user_data-per-callback inside the micropython bindings, while on lvgl header files we only have a single user-data per object/struct.
This suggested single-user_data strategy would require creating an extra dict per object/struct to hold the callbacks, which will cost more memory per object. (It also means more work for me to implement this)
The extra RAM cost would be another 32bit per object/struct. For structs/objects with registered callbacks the cost would be even higher, because we would need to allocate a dict and add the callback objects to it. But since usually only a few structs/objs will hold callbacks (a callback registration function would only be called for a few structs/objs) it may still be worth it because we save RAM by using a single user_data in lvgl objects.
Single-user_data strategy would also cost more cycles when registering and when calling a callback. But I assume this is less critical vs. RAM consumption.

single user_data
per-callback user_data

RAM cost per obj/struct without registered callbacks
1 ptr for user_data + 1 ptr for dict (points to NULL)
num_callbacks ptrs

RAM cost per obj/struct with registered callbacks
1 ptr for user_data + 1 ptr for dict (points to a dict object) + dict_object_size + num_callbacks X dict_entry_size
num_callbacks ptrs

So there are some trade-offs here. I'm still not sure which strategy is better.

Function pointers
It looks to me the function pointer question is clear. Now it's memory effective an transparent to the user. Are there anything else to discuss about it?
Single/Multi user data
I suppose, but I'm not sure this approach will work in C++ too, right?
At first look, it seems we can save some memory here. Now lv_obj has 3 callback user data but usually, none of them are used.
In case of "with registered callbacks" and "single user data" it should be:
1 ptr for user_data + 1 ptr for dict (points to a dict object) + dict_object_size + current_ num_callbacks X dict_entry_size
So only the registered callbacks consumes memory in the dictionary.
I can imagine having a general user_data typedef (instead of lv_obj/group/anim_user_data_t) where you can typedef a struct with an additional
void * _dict element.

Function pointers
It looks to me the function pointer question is clear. Now it's memory effective an transparent to the user. Are there anything else to discuss about it?

I think it's clear.

I suppose, but I'm not sure this approach will work in C++ too, right?

I believe such approach could work for C++ or any other binding for lvgl. (instead of dict, C++  could use std::map or even a list/vector)

In case of "with registered callbacks" and "single user data" it should be:
1 ptr for user_data + 1 ptr for dict (points to a dict object) + dict_object_size + current_ num_callbacks X dict_entry_size
So only the registered callbacks consumes memory in the dictionary.

If there are no current callbacks for an object, we can avoid allocating the dict itself and so consume only 1 ptr for the user_data and 1 ptr for the dict (points to NULL). That's 2 ptrs which is less than 3 ptrs we need today (for callbacks-user_data).
So it saves 1 ptr per obj when no callbacks are registered, but consumes more RAM when callbacks are used, and requires additional cycles to manage this every time a callback is registered or called.

I can imagine having a general user_data typedef (instead of lv_obj/group/anim_user_data_t) which you can typedef as a struct with an additional
void * _dict element.

It's possible, but if we anyway need to keep track of the python object that represents the lvgl object, we could leave the default void * typedef for user_data and keep the per-callback-user_data in that python object.
That's not a bad thing to keep a separate user_data typedef for lv_obj/group/anim, because it adds flexibility and could be useful for some users, but I think that for Micropython we can leave the default void * for all.

@amirgon
So if I understood well we can delete all the LV_USE_USER_DATA_MULTI callbacks?

@amirgon
So if I understood well we can delete all the LV_USE_USER_DATA_MULTI callbacks?

Maybe.
I understand it would make lvgl source code simpler if we completely remove the userdata-per-callback (LV_USE_USER_DATA_MULTI) option.
Please let me try implementing it with single user_data and I'll let you know if I see more problems with the extra complexity on the micropython binding side.

@kisvegabor new problems:
 * lv_task_t *lv_task_create(void (*task)(lv_task_t *), uint32_t period, lv_task_prio_t prio, void *user_data)

Here task is a callback. The script doesn't know, according to our convention, where to find user_data.
How about spitting this to several (inline) functions, to first create the task then set its callback?
This will make it work similar to other callbacks that we set to other lvgl objects, and will work with the existing callback conventions.
lv_res_t lv_event_send_func(lv_event_cb_t event_cb, lv_obj_t *obj, lv_event_t event, const void *data)

Could you explain why this function is needed?
Again, the script can't find the user_data for event_cb here, and existing calling convention does not work for this one.

task
As the callback is the most important part of a task (and it doesn't make any sense without it) I think we should leave it in the create function.
What about creating a function to set the user data?
lv_event_send_func
I added this function to make possible to manually call an event function. It might be useful if you have a general event callback (e.g. to increment a variable on click and refresh a label) and you want to call that function manually. Here lv_event_send(obj, LV_EVENT_..., user_data) is not working because you might not know the object only the event function to call. Simply calling the function might work but as you can see in lv_event_send_func it does a lot more things than simply call the function.
So this function doesn't bind any callbacks, just calls a function directly. The name of event_cb parameter causes the confusion? If yes, I can rename it.

task
As the callback is the most important part of a task (and it doesn't make any sense without it) I think we should leave it in the create function.

@kisvegabor Which do you prefer then?

Not support tasks in Micropython
Create new callback convention that could be applied to tasks
Change the task creation function to comply with the existing convention (I understand that you want to leave it as is, so this option is less preferred for you)
Some other suggestion?

The existing convention assumes that when a function receives a callback, it would receive a struct pointer as the first parameter, which contains the user_data for that callback. This is true for most cases (lv_obj_set_event_cb etc.) but not for this case.

lv_event_send_func
I added this function to make possible to manually call an event function. It might be useful if you have a general event callback (e.g. to increment a variable on click and refresh a label) and you want to call that function manually. Here lv_event_send(obj, LV_EVENT_..., user_data) is not working because you might not know the object only the event function to call. Simply calling the function might work but as you can see in lv_event_send_func it does a lot more things than simply call the function.
So this function doesn't bind any callbacks, just calls a function directly. The name of event_cb parameter causes the confusion? If yes, I can rename it.

The problem is not the name, but the fact the the function receives a function pointer. Every time a function receives a function pointer, we must know the associated user_data in order to bind a micropython function. It doesn't matter if we register a callback or call the function directly, for that matter.
In this case, as the previous case, the first parameter of the function does not contain the user_data as we expect by convention.
Again - Do you prefer to 1) not support it in micropython 2) define a new convention 3) change the way it's defined ?

exec_cb
void lv_anim_exec_cb_t(void *, lv_anim_value_t)

We discussed this, the problem here is the same - the first parameter is not a struct with user_data.
I suggested above:
typedef void (*lv_anim_custom_exec_cb_t)(lv_anim_t *, int32_t);

inline void lv_anim_set_custom_exec_cb(lv_anim_t *a, lv_anim_custom_exec_cb_t exec_cb)
{
  a->var = NULL;
  a->exec_cb = (lv_anim_exec_cb_t)exec_cb;
}

This would solve the problem since lv_anim_t contains user_data, which is accessible from both lv_anim_custom_exec_cb_t callback and from lv_anim_set_custom_exec_cb registration function.
What do you think?

tasks
Is there a reasonable way to replace lv_tasks in Micropython. E.g. how would you blink a text? (without animations and timer interrupts)
lv_event_send_func
The obj parameter can be NULL for this function to just call the event_cb. In this case you won't be able to retrieve the user data of the callback.
For this function, IMO, it's better if do not support it.
Actually what does "not support" means? Won't have a "pair" in micropython? If so, how to tell to the parser that this function should be skipped? Simply don't generate binding for functions which are not compatible with the "callback conventions"?
anim_exec_cb

This would solve the problem since lv_anim_t contains user_data, which is accessible from both lv_anim_custom_exec_cb_t callback and from lv_anim_set_custom_exec_cb registration function.
What do you think?

If we add inline functions to the animations we can have lv_anim_set_custom_exec_cb too. But two questions:

Why did you set  a->var = NULL;?
How will you skip the "original" lv_anim_set_exec_cb? Skip incompatible function as I asked for lv_event_send_func?

@kisvegabor

tasks
Is there a reasonable way to replace lv_tasks in Micropython. E.g. how would you blink a text? (without animations and timer interrupts)

That depends.
In pure micropython you could just loop and sleep.
On specific platforms (pyboad, esp32) there are ways to schedule a callback, for example by using a timer callback that is called after a timeout expires. This class usually employs a timer interrupt under the hoods.
I still think that if we want to provide a generic clean way to do it (which is not platform dependent), your lv_task is a good solution and it would be nice to expose it to the micropython user.

lv_event_send_func
The obj parameter can be NULL for this function to just call the event_cb. In this case you won't be able to retrieve the user data of the callback.
For this function, IMO, it's better if do not support it.
Actually what does "not support" means? Won't have a "pair" in micropython? If so, how to tell to the parser that this function should be skipped? Simply don't generate binding for functions which are not compatible with the "callback conventions"?

Yes. When the script identifies a function pointer, it checks to see if it complies with the "callback convention" (if it can find user_data where it expects etc.)
If it does not comply with the callback convention - we can decide what to do, either print an error or just skip (ignore) this function. In this case, as you said, there will not be a micropython "pair" function since we will not generate binding for the incompatible function.

anim_exec_cb
If we add inline functions to the animations we can have lv_anim_set_custom_exec_cb too. But two questions:

Why did you set  a->var = NULL;?

What should it be then? If the function is a custom function that knows which var to change, what is the use of a->var?

How will you skip the "original" lv_anim_set_exec_cb? Skip incompatible function as I asked for lv_event_send_func?

Yes. Incompatible functions (that do not comply with the "callback convention") can be skipped if we decide this is the desired behavior.
The disadvantage of "skipping" incompatible functions is that if, in the future, someone adds a new callback registration function that mistakenly doesn't comply with the callback convention, it could be harder to know that there is a problem since that function would just be skipped without any error.

@amirgon

The disadvantage of "skipping" incompatible functions is that if, in the future, someone adds a new callback registration function that mistakenly doesn't comply with the callback convention, it could be harder to know that there is a problem since that function would just be skipped without any error.

In that case, I would suggest printing a warning instead. Users can ignore the warning if they wish but we would still be aware of any unsupported functions.

In that case, I would suggest printing a warning instead. Users can ignore the warning if they wish but we would still be aware of any unsupported functions.

@embeddedt In general, I thinks warnings are bad practice.
If they indicate a problem that should be fixed - they should be errors, not warnings.

With your suggestion, the script will always emit warnings for the functions it can't generate. Will anyone notice another warning added to the list when that new erroneous callback registration function is added?

If they indicate a problem that should be fixed - they should be errors, not warnings.

It's not a problem that should be fixed - it's a problem that can be fixed.

With your suggestion, the script will always emit warnings for the functions it can't generate. Will anyone notice another warning added to the list when that new erroneous callback registration function is added?

We could add a filter list that would hide the warning for functions that we, the maintainers, are aware would never need to be generated.

@kisvegabor
Update
As discussed above, I changed Micropython binding implementation to use a single user_data per struct/object.
To simplify lvgl code, you can remove all LV_USE_USER_DATA_MULTI macros and all per-callback-user_data members, if you want.
I recommend keeping the LV_USE_USER_DATA_SINGLE  macro to allow enabling/disabling user_data (can probably rename it to something like LV_USE_USER_DATA). On Micropython this macro would have to be defined as 1.
I've pushed my updates and now the unix port of lv_micropython branch dev-v6.0  works with lvgl dev-v6.0 branch with generic callbacks supported, except from the open cases we are still discussing.
These include:

img_decoder callback
log callback (we discussed this but I didn't implement global callbacks yet. I'm not sure how to differentiate this case from other cases where callbacks should not be generated due to convention compliance)
task callback
anim exec and ready callback
lv_event_send_func
get_bitmap and get_width callbacks of font
style mod and mod_edit callbacks

To simplify lvgl code, you can remove all LV_USE_USER_DATA_MULTI macros and all per-callback-user_data members, if you want.

One thing just came to my mind. If you are using the user_data to point to a Micropython object what will happen if the user calls lv_obj_set_user_data(obj, x). Will it overwrite the Micropython object? If so we can have 2 user data: one public (accessible through the API) and an internal. Both can be enabled in lv_conf.h

I've pushed my updates and now the unix port of lv_micropython branch dev-v6.0 works with lvgl dev-v6.0 branch with generic callbacks supported

Awesome!

img_decoder callback

Now it look like this:
void lv_img_decoder_set_custom(lv_img_decoder_info_f_t info_fp, lv_img_decoder_open_f_t open_fp,
                               lv_img_decoder_read_line_f_t read_fp,
                               lv_img_decoder_close_f_t close_fp);

I can update is like this:
typedef struct {
    lv_img_decoder_info_f_t info_cb;
    lv_img_decoder_open_f_t open_cb;
    lv_img_decoder_read_line_f_t read_line_cb;
    lv_img_decoder_close_f_t close_cb;

    lv_img_decoder_user_data_t user_data;
}lv_img_decoder_t;

lv_img_decoder_t * lv_img_decoder_create(void);

void lv_img_decoder_set_info_cb(lv_img_decoder_t decoder, lv_img_decoder_info_f_t info_cb);
void lv_img_decoder_set_open_cb(lv_img_decoder_t decoder, lv_img_decoder_info_f_t open_cb);
void lv_img_decoder_set_read_line_cb(lv_img_decoder_t decoder, lv_img_decoder_info_f_t rad_line_cb);
void lv_img_decoder_set_close_cb(lv_img_decoder_t decoder, lv_img_decoder_info_f_t close_cb);

log callback

The current prototype is:
void lv_log_register_print(void f(lv_log_level_t, const char *, uint32_t, const char *))

What if you use a "global user data" if no object is present to assign the callback?

task callback

Having two task create would solve it (the names are still stupid):
lv_task_t * lv_task_create_minimal(void);
lv_task_t * lv_task_create_full(cb, period, pririty, user_data);

In addition

lv_task_set_cb(task, cb)

anim exec and ready callback

What should it be then? If the function is a custom function that knows which var to change, what is the use of a->var?

It really can know, but why should it know? E.g. you can have my_special_width_anim(anim, value) and the function can get all parameters from anim. Solv_anim_set_custom_exec_cb should just overwrite exec_cb. In addition, the lv_anim system prevents to have the same animation twice in the same time on a var. So var should be known.
What's the problem with ready_cb? Its first parameter is lv_anm_t *.

lv_event_send_func

I vote for skipping but some convention would be nice to explicitly express it. If you agree with above written things only this function will be skipped. It'd solve the "warning question" too.

get_bitmap and get_width callbacks of font

I design the new fonts with this in mind.

theme group mod and mod_edit callbacks

I suppose the problem is the missing user_data in themes and the missing lv_theme_set_style_mod_cb(theme, cb) functions. If so I can add them.

@kisvegabor

One thing just came to my mind. If you are using the user_data to point to a Micropython object what will happen if the user calls lv_obj_set_user_data(obj, x). Will it overwrite the Micropython object? If so we can have 2 user data: one public (accessible through the API) and an internal. Both can be enabled in lv_conf.h

I think having 2 user data would be a waste of RAM. If the user wants to associate more data with his object he can already do that by extending his object (inheriting from it). That would be the natural way of doing it in python (or any other Object Oriented language).
So, from the user perspective, obj.set_user_data(x) is redundant. Not only that - since it receives void* argument, the user can't really provide a meaningful user_data to this function, because there is no automatic conversion between micropython object and void*.
I suggest leaving set_user_data as it is, for flexibility, or removing set_user_data binding.

img_decoder callback

Now it look like this:
void lv_img_decoder_set_custom(lv_img_decoder_info_f_t info_fp, lv_img_decoder_open_f_t open_fp,
                               lv_img_decoder_read_line_f_t read_fp,
                               lv_img_decoder_close_f_t close_fp);

I can update is like this:
typedef struct {
    lv_img_decoder_info_f_t info_cb;
    lv_img_decoder_open_f_t open_cb;
    lv_img_decoder_read_line_f_t read_line_cb;
    lv_img_decoder_close_f_t close_cb;

    lv_img_decoder_user_data_t user_data;
}lv_img_decoder_t;

lv_img_decoder_t * lv_img_decoder_create(void);

void lv_img_decoder_set_info_cb(lv_img_decoder_t decoder, lv_img_decoder_info_f_t info_cb);
void lv_img_decoder_set_open_cb(lv_img_decoder_t decoder, lv_img_decoder_info_f_t open_cb);
void lv_img_decoder_set_read_line_cb(lv_img_decoder_t decoder, lv_img_decoder_info_f_t rad_line_cb);
void lv_img_decoder_set_close_cb(lv_img_decoder_t decoder, lv_img_decoder_info_f_t close_cb);

Yes, but the callbacks themselves (lv_img_decoder_[info|open|read_line|close]_f_t) should also receive lv_img_decoder_t as their first argument. This is not the case today.

log callback

The current prototype is:
void lv_log_register_print(void f(lv_log_level_t, const char *, uint32_t, const char *))

What if you use a "global user data" if no object is present to assign the callback?

Yes it's possible, but then, how can I differentiate between registration of global callback vs. malformed registration of callback?  (when callback convention is not followed)
When the script looks for user_data and can't find it, there are two options:

It could use the global user_data
It could print an error/warning/comment in the code, and not create any binding for that callback.

How would the script know which option to choose?

task callback

Having two task create would solve it (the names are still stupid):
lv_task_t * lv_task_create_minimal(void);
lv_task_t * lv_task_create_full(cb, period, pririty, user_data);

In addition

lv_task_set_cb(task, cb)

This should work.
Actually, I was even able to work around this with today's code by writing:
t = lv.task_create(None, 5000, lv.TASK_PRIO.LOW, None)
t.task_cb = my_cb

But your suggestion is cleaner.

anim exec and ready callback

What should it be then? If the function is a custom function that knows which var to change, what is the use of a->var?

It really can know, but why should it know? E.g. you can have my_special_width_anim(anim, value) and the function can get all parameters from anim. Solv_anim_set_custom_exec_cb should just overwrite exec_cb. In addition, the lv_anim system prevents to have the same animation twice in the same time on a var. So var should be known.

I can think of the following reasons why it should know which var to change, and not rely on var:

Isn't it redundant and wastes RAM to have both void * var and void * user_data? Why not combine them together? It looks to me like var is actually used as some kind of user_data for the callback.
In Micropython, the user can't really use a void * in a natural way. What should the user set var to, and how would he later use var in his callback? In C you would cast it to some other type, but in Micropython we should really try to avoid that.

What's the problem with ready_cb? Its first parameter is lv_anm_t *.

You are right, I'm not sure what is the problem. I'll check that.

lv_event_send_func

I vote for skipping but some convention would be nice to explicitly express it. If you agree with above written things only this function will be skipped. It'd solve the "warning question" too.

Ok, what convention do you suggest for excluding callback functions?

theme group mod and mod_edit callbacks

I suppose the problem is the missing user_data in themes and the missing lv_theme_set_style_mod_cb(theme, cb) functions. If so I can add them.

There is some mismatch here: style_mod_cb and style_mod_edit_cb are defined on lv_theme_t, but these callbacks receive struct _lv_group_t * as their first parameter, and not lv_theme_t.
So setting user_data on lv_theme_t won't work, because when the callbacks are called, the user_data of
lv_group_t would be used instead.

Isn't it redundant and wastes RAM to have both void * var and void * user_data? Why not combine them together? It looks to me like var is actually used as some kind of user_data for the callback.

Unless something changed recently in the v6.0 API, var is always passed as the first parameter to the callback. For example, if you wanted to animate the width of an object, you could set lv_obj_set_width as the callback and set var to your object. That would ensure that the first parameter is the lv_obj_t * you want animated, and the second parameter would be the width.
I assume user_data is intended for more advanced scenarios, where you have a custom callback that needs to know more information.

I assume user_data is intended for more advanced scenarios, where you have a custom callback that needs to know more information.

The exec_cb callback don't receive lv_anim_t so it can't access user_data. This is where it's incompatible with the callback convention.
Could you give an example of such advanced scenario where lv_anim_t user_data is used? (when you can't access it from the callback)

The exec_cb callback don't receive lv_anim_t so it can't access user_data.

I wasn't aware of that. At first glance it seems like an oversight.

What's the problem with ready_cb? Its first parameter is lv_anm_t *.

You are right, I'm not sure what is the problem. I'll check that.

@kisvegabor I found what is the problem with ready_cb.
lv_anim_ready_cb_t ready_cb is a member of lv_style_anim_t.
However lv_anim_ready_cb_t is defined like this:
typedef void (*lv_anim_ready_cb_t)(struct _lv_anim_t *);

The problem is a mismatch between struct lv_style_anim_t which contains the callback and user_data, and struct lv_anim_t received as the first argument of the callback. It should be the same struct.
The same problem happens with style_mod_cb and style_mod_edit_cb, as I mentioned above.

We really can't use the user data this way.  However, with a lv_anim_get_act()(not exists now) the current animation can be accessed from exec_cb.
@amirgon
I started to rework the image decoder interface and will add functions to animations to solve the mentioned issues. Hopefully, I will be ready in a few days.

image decoder
I updated the image decoder interface: https://github.com/littlevgl/lvgl/blob/imgdec/src/lv_draw/lv_img_decoder.h
Here is how it works:

Create a new decoder with lv_img_decoder_t * decoder = lv_img_decoder_create()
Set callbacks with lv_img_decoder_set_info/open/read_line/close_cb(decoder, my_xxx_cb)
To use the decoder declare lv_img_decoder_dsc_t dsc;
Use the decoders as lv_img_decoder_open(dsc, src, style)

The prototype of the decoder functions are:
func (struct _lv_img_decoder * decoder, struct _lv_img_decoder_dsc * dsc, <other_params>)

It should work in Micropython because decoder has user_data to store the callback data. However, dsc contains a pointer to decoder to fully describe the decoding session. So the first decoder parameter is redundant because it is also available as dsc->decoder but it won't be compatible with Micropython binding. So do you have any idea about how to eliminate it decoder parameter?
@manison
It also should solve your issue to run multiple decoders in parallel manually.
single user data
@amirgon
You mentioned that besides user_data (for the user with public API) a sys_data (for internal use) is not required because the user can simply store its own data using Python's "features" as btn1.my_data = 3. However, in C++ it's not the case because you can't dynamically extend an object. So do you agree to introduce void * sys_data (can be enabled in lv_conf.h)?
animations
Inline functions are added:

lv_anim: https://github.com/littlevgl/lvgl/blob/anim/src/lv_misc/lv_anim.h#L108
lv_style_anim: https://github.com/littlevgl/lvgl/blob/anim/src/lv_core/lv_style.h#L152

@amirgon pointed out that we set callbacks for lv_style_anim_t but e.g. ready_cb gets lv_anim_t paramter. It's becasue lv_style_anim_t is just a temporary structure which is turned to  lv_anim_t in lv_style_anim_create(). Is it working if I copy the style_anim->user_data to anim->user_data in lv_style_anim_create()?

@kisvegabor

So do you have any idea about how to eliminate it decoder parameter?

Should we? What is the cost of passing another parameter to a function?
And why does lv_img_decoder_dsc contain a pointer to the decoder? This is more costly than passing a parameter in terms of RAM consumption, because parameter is short lived (to the lifetime to the function), and a struct member is long lived (to the lifetime of the  lv_img_decoder_dsc struct instance).

You mentioned that besides user_data (for the user with public API) a sys_data (for internal use) is not required because the user can simply store its own data using Python's "features" as btn1.my_data = 3. However, in C++ it's not the case because you can't dynamically extend an object. So do you agree to introduce void * sys_data (can be enabled in lv_conf.h)?

Actually, no.
You can't add a property such as btn1.my_data = 3 to a built-in class, only to a user defined class, so you can't do that with micropython either (all lvgl classes are considered built-in classes, and as such they consume only flash and not RAM until they are instantiated).
My suggestion was that, for both [micro]python and C++, the user could use inheritance to derive his own class from lvgl class, and add his data there. It would look something like:
class my_btn (btn):
...
my_btn.my_data = 3

This should work with both [micro]python and C++, as well as any other Object Oriented language.
You can add sys_data if you want, as long as we could disable it, but I think it would be redundant and would not be used.

Is it working if I copy the style_anim->user_data to anim->user_data in lv_style_anim_create()?

Currently we expect the type of the struct that contains the user_data to be the same when registering the callback and when the callback is called.
I think it's dangerous to allow different types, because it will require whoever defines the callback to remember copying the user_data.
Currently you define:
https://github.com/littlevgl/lvgl/blob/5af101a1eb01249fa1aa67047e53557aa6da4fd4/src/lv_misc/lv_anim.h#L39-L40
It's still a problem for the binding because it cannot know which type is the first parameter (void *).
Also, could you explain why you need an lv_style_anim_t temporary structure which is turned to lv_anim_t later?

So do you have any idea about how to eliminate it decoder parameter?

Should we? What is the cost of passing another parameter to a function?

Not a big problem to leave it but would be cleaner with only the dsc.

And why does lv_img_decoder_dsc contain a pointer to the decoder?

The most simple decoding process looks like this:
lv_img_decoder_dsc_t dsc;
image_data = lv_img_decoder_open(&dsc, src, style);
lv_img_decoder_close(&dsc);

Hence lv_img_decoder_close needs to know which decoder's close_cb to call.

You can't add a property such as btn1.my_data = 3 to a built-in class, only to a user defined class, so you can't do that with micropython either (all lvgl classes are considered built-in classes, and as such they consume only flash and not RAM until they are instantiated).

I see, thank you for the explanation.

You can add sys_data if you want, as long as we could disable it, but I think it would be redundant and would not be used.

I think it's cleaner to keep user_data for the user. Creating a new class really should work in C++ too, however,  it's an extra effort which we can save. So would it be a problem for you to use void * sys_data instead and disable user_data?

typedef void (*lv_anim_exec_cb_t)(void *, lv_anim_value_t);
It's still a problem for the binding because it cannot know which type is the first parameter (void *).

I see the problem but can't see a solution. I understand that you need typedef void (*lv_anim_exec_cb_t)(lv_anim_t *, lv_anim_value_t); but the void * enable direct assignment of lv_xxx_set function. Maybe we can add a common animator function (with lv_anim_t * parameter) which somehow calls the real exec_cb (with void* parameter) but I still can't imagine how it could work.

Also, could you explain why you need an lv_style_anim_t temporary structure which is turned to lv_anim_t later?

When lv_style_anim_t is turned to lv_anim_t some elements of lv_anaim_t is set to special, "style anim specific" values. See this.
However, I've just realized that I can make that hard-coded initialization in lv_style_anim_init too so lv_style_anim_t can be dropped.

I think it's cleaner to keep user_data for the user. Creating a new class really should work in C++ too, however, it's an extra effort which we can save. So would it be a problem for you to use void * sys_data instead and disable user_data?

@kisvegabor  Do you suggest adding sys_data to all objects and structs where we have user_data today?
I don't care how we call it, user_data or sys_data, but I don't want to have both of them because it's a waste of RAM. So if each of them can be enabled/disabled individually using a single macro on lv_conf.h, that would be fine.
But my opinion is that it's still extra complexity to lvgl to add both, and I think they would always be used exclusively (only one and not the other) so it does not make sense to keep two variables. There's also the risk that someone adds a new struct with user_data but without sys_data and create a bug for the binding.
So I don't see the benefit in keeping them both, and if it was my decision I would only keep one of them.

I see the problem but can't see a solution. I understand that you need typedef void (*lv_anim_exec_cb_t)(lv_anim_t *, lv_anim_value_t); but the void * enable direct assignment of lv_xxx_set function.

I think there is a solution, as I already suggested in previous post.
All we need to do is to provide a custom function for registering the callback, and there, instead of passing the original lv_anim_exec_cb_t as the callback type, we would pass lv_anim_custom_exec_cb_t which receives lv_anim_t * instead of void * as the first parameter.
This custom registeration function would be a static inline function so it does not cost anything, and frankly I think it would make more sense to use it even when using lvgl with C, as it makes it clear what the callback is actually receiving as a parameter.

Creating a new class really should work in C++ too, however, it's an extra effort which we can save.

It is a little bit of extra work, but it's the proper way of doing things in C++. Part of the whole philosophy in that language is to avoid using generic pointers like void *, as they make it difficult or impossible to enforce type safety.
I think it would be much cleaner to just extend a class in C++.

Animations
I successfully removed lv_style_anim_t. Now both normal and style animations work with lv_anim_t.
sys_data
Ok, I don't insist on it.
LV_USE_USER_DATA_MULTI has been removed and LV_USE_USER_DATA_SINGLE has been renamed to LV_USE_USER_DATA.
I merged the image decoder and animation updates to dev-6.0

@kisvegabor
image decoder
https://github.com/littlevgl/lvgl/blob/322211b1edbf2a7db9a407a41da9bc7c49b0ce4b/src/lv_draw/lv_draw_img.h#L60-L62
This function registers callbacks, but does not receive an lv_img_decoder_t as the first parameter, as it should according to the convention.
anim
https://github.com/littlevgl/lvgl/blob/322211b1edbf2a7db9a407a41da9bc7c49b0ce4b/src/lv_misc/lv_anim.h#L264
This function looks like it's registering a callback, although it is actually deleting one. If we want it to work the same with binding, we would need an lv_anim_custom_del that receives lv_anim_t as the first parameter. This would allow passing a callback the same way, and prevent the need for casing the anim object to void*.
tasks
You had a good suggestion in a previous post, to define lv_task_create_minimal and lv_task_set_cb(task, cb).
If you want, we can even skip defining lv_task_create_minimal and pass in micropython None (NULL) as the first parameter to lv_task_create instead.
But I still think having at least lv_task_set_cb(task, cb) is a good idea.

Image decoder
It was the old declaration and left there by accident. I removed it.
anim
I added lv_anim_custom_del.
task
I added lv_task_set_cb() and lv_task_create_basic() (Fianlly I found "basic" a better name than "minimal")

@kisvegabor
Thank you for these fixes! We are really making progress. Very few problems remain with callbacks :+1:
Global user_data
https://github.com/littlevgl/lvgl/blob/eb1e28e1f59b40065131fe1f32ec93ff32625055/src/lv_misc/lv_log.h#L52
As we discussed, in this case the callback is global and the correct behavior would be to access a global user_data.
However - there is a problem.
How can the script differentiate between registration of global callback (like this one) vs. malformed registration of a callback? (when callback convention is not followed)
Malformed registrations of callback are present in the code anyway. For example, lv_anim_set_exec_cb is malformed (but this is ok because we can use lv_anim_set_custom_exec_cb instead).
When the script sees a malformed registration of callback it has two options:

It could use the global user_data. The problem with this option is that it misleads the user: If the user calls lv_anim_set_custom_exec_cb it would register with the local user_data. But if the user calls lv_anim_set_exec_cb, it would register with the global user_data - and cause bugs (registrations will overrun each other). Because really we should not use the global user_data for lv_anim_set_exec_cb because there is a local user_data somewhere, we just can't access it.
It could skip the malformed registration - print a warning/comment in the code, and not create any binding for that callback. The problem with this option is that no callback will be generated for lv_log_register_print because it looks like a malformed registration of a callback.

So, to summarize,  the problem here is how to identify when it makes sense to use the global user_data (for example in lv_log_register_print), vs. when it does not make sense (for example in lv_anim_set_exec_cb).
I can suggest one of the following solutions:

Use some special naming convention (lv_log_register_print_globalcb) but it's a bit ugly
Not support global user_data at all. Currently (I think) it means only lv_log_register_print will not be available for the binding, but maybe more functions in the future.
Some other ideas?

@amirgon

Thank you for these fixes! We are really making progress. Very few problems remain with callbacks +1

I also thank you! Meanwhile, some part of the library became clear too.
global user data
What about marking it the variable name? It won't disturb the user.
E..g.
lv_log_register_print(lv_log_print_cb_t print_g_cb);

What about marking it the variable name? It won't disturb the user.
E..g.
lv_log_register_print(lv_log_print_cb_t print_g_cb);

@kisvegabor  That could work.
Another option is to mark the callback prototype:
lv_log_register_print(lv_log_print_g_cb_t print_cb);

This is useful when the same callback prototype is used by multiple registration functions.
What do you think?

Another option is to mark the callback prototype:

That's even better. Updated accordingly: https://github.com/littlevgl/lvgl/commit/234fa342547034c491675e5d481073cec1a0e526
According to the list in a previous comment only two things remained

[x] img_decoder callback
[x] log callback (we discussed this but I didn't implement global callbacks yet. I'm not sure how to differentiate this case from other cases where callbacks should not be generated due to convention compliance)
[x] task callback
[x] anim exec and ready callback
[x] lv_event_send_func
[ ] get_bitmap and get_width callbacks of font
[ ] theme group mod and mod_edit callbacks

The fonts are in progress but  "theme - group_style_mod_cb" question is still not clear.
The problem here is that** group related functions pointers are stored in the themes** which are copied to the groups when a theme is applied. As I see it's very different from the concepts we used so far and I see no way to make it compatible with the binding generator. Do you have any idea?  If no, IMO it's not a  critical issue because it's not a must-have feature.

theme group mod and mod_edit callbacks

The problem here is that group related functions pointers are stored in the themes which are copied to the groups when a theme is applied. As I see it's very different from the concepts we used so far and I see no way to make it compatible with the binding generator. Do you have any idea?

@kisvegabor a few questions:

What is the use case of these callbacks? (style_mod_cb and style_mod_edit_cb). How are these related to groups? (groups are used only for input device, right? What is the relation between input device and switching themes?) Why are you copying callbacks between styles and groups when themes are applied?
When you copy callbacks, can you also copy the user_data? If you can - we could support this.
The basic assumption is that user_data is the same user_data when registering a callback and when the callback is called. If this assumption holds, we could add (static inline) functions to overcome naming and type discrepancies, like we did for other callbacks.

anim exec
@kisvegabor I just realized there is still a problem with lv_anim_set_custom_exec_cb.
It's first argument is lv_anim_t *a, and its user_data can be updated. However when the callback is called, it receives as the first argument var, and not the lv_anim_t *a we updated when registering the callback in lv_anim_set_custom_exec_cb.
This breaks the assumptions that the same user_data is exposed on both the registration and the callback.
So here is my suggestion:

var field of lv_anim_t will be allowed to remain uninitialized (NULL).
When the callback is called, if var is NULL, then instead of var pass the anim struct as the first argument of the callback.

This will work correctly in both cases, when using lv_anim_set_exec_cb with an existing lv_obj function, or when using lv_anim_set_custom_exec_cb with custom callback (in which case var will remain NULL).
It's not only for Micropython. It makes sense to allow the user access the user_data (and the entire anim struct) when the user provides a custom callback.
What do you think?

When the callback is called, if var is NULL, then instead of var pass the anim struct as the first argument of the callback.

This is dangerous in my opinion. I agree with the general principle, but we need to find a safer way of handling that. People passing NULL into var would expect NULL to come out on the other end of the callback, not the lv_anim_t * struct. It seems unintuitive to me.
A better method, in my opinion, is to introduce a third lv_anim_t * pointer to lv_anim_exec_cb_t.

better method, in my opinion, is to introduce a third lv_anim_t * pointer to lv_anim_exec_cb_t.

@embeddedt I think @kisvegabor wants to keep exec_cb prototype compatible with functions like lv_set_x etc. So we can't simply pass other arguments to that callback.
The other option would be to support two "types" of exec_cb callbacks. One is for using built-in functions as callbacks, and the other is for using custom callbacks.
In my suggestion I tried to merge these two options and keep a single callback.

style_mod_cb

What is the use case of these callbacks? (style_mod_cb and style_mod_edit_cb). How are these related to groups? (groups are used only for input device, right? What is the relation between input device and switching themes?) Why are you copying callbacks between styles and groups when themes are applied?

These callbacks modify the style of an object when it is focused by a keyboard or encoder. You can try it here. E.g. the default style_mod_cb mixes some orange color to the original colors of the style. However, as this is related to the appearance of the objects, the themes also have their own style_mod_(edit)_cb. But something like this seems possible in lv_group_mod_style:
if(there_is_active_theme && group->style_mod_cb == NULL) {
  //Use the style_mod from the theme
} else {
  //Use style_mod fro mteh group
}

When you copy callbacks, can you also copy the user_data? If you can - we could support this.
The basic assumption is that user_data is the same user_data when registering a callback and when the callback is called. If this assumption holds, we could add (static inline) functions to overcome naming and type discrepancies, like we did for other callbacks.

Let's see if the above-proposed solution work. It seems clearer than coping user data.
anim_exec_cb

This is dangerous in my opinion. I agree with the general principle, but we need to find a safer way of handling that. People passing NULL into var would expect NULL to come out on the other end of the callback, not the lv_anim_t * struct. It seems unintuitive to me.

I agree.

keep exec_cb prototype compatible with functions like lv_set_x etc. So we can't simply pass other arguments to that callback.

The other option would be to support two "types" of exec_cb callbacks. One is for using built-in functions as callbacks, and the other is for using custom callbacks.

It can work but I have an other idea too. In lv_anim_set_custom_exec_cb set a flag like a->is_custom = 1 and in this case pass the lv_anim_t* as first argument. Only a more descriptive name should be found instead of custom. What do you think?

style_mod_cb

if(there_is_active_theme && group->style_mod_cb == NULL) {
//Use the style_mod from the theme
} else {
//Use style_mod from the group
}

@kisvegabor I'im not sure I understand how this would solve the problem.
When you register the callback on themes, you provide it with theme object and the binding will update theme's user_data. But when you call the style_mod_cb callback, you pass the lv_group as the first argument, right?
In that case we would see on the callback a different user_data (group's user_data) than the one we saw on the registration (style's user_data).
It's a problem to pass different structs on registration and callback, and not only for Micropython.
The user, unrelated to Micropython, can't access his user_data which he set on the registration.
Copying user_data by lvgl might be dangerous. The user might have already updated group's user_data, and copying user_data automatically by lvgl would overrun the user's user_data.
anim_exec_cb

It can work but I have an other idea too. In lv_anim_set_custom_exec_cb set a flag like a->is_custom = 1 and in this case pass the lv_anim_t* as first argument. Only a more descriptive name should be found instead of custom. What do you think?

Having a flag to differentiate "custom" exec callback vs "builtin" exec callback could definitely work.
I have yet  another idea:
It looks to me like the var and the exec_cb are tightly coupled. You can't set the one without the other and the value of var is important to the selection of exec_cb.
Why not set them together? It would make things more intuitive to the user, and require less changes in lvgl code:
// remove lv_anim_set_var function

static inline void lv_anim_set_exec_cb(lv_anim_t * a, lv_anim_exec_cb_t exec_cb, void * var)
{
    a->exec_cb = exec_cb;
    a->var = var;
}

static inline void lv_anim_set_custom_exec_cb(lv_anim_t * a, lv_anim_custom_exec_cb_t exec_cb)
{
    a->exec_cb = (lv_anim_exec_cb_t)exec_cb;
    a->var = a;
}

The user would provide var only for the lv_anim_set_exec_cb.
In case of lv_anim_set_custom_exec_cb, var would be set to a, such that the custom callback function would receive a as the first argument.
lv_anim_set_var would be removed, so the user can't make a mistake when setting var - it would be set correctly according to his selection of exec_cb registration function.
What do you think?

@kisvegabor I'im not sure I understand how this would solve the problem.
When you register the callback on themes, you provide it with theme object and the binding will update theme's user_data. But when you call the style_mod_cb callback, you pass the lv_group as the first argument, right?

Ups, really, I missed it.

looks to me like the var and the exec_cb are tightly coupled. You can't set the one without the other and the value of var is important to the selection of exec_cb.

Let's say you have a custom function like my_set_x(). Without a->var you can use this function with only one object which is hardcoded in it. With a->var my_set_x() can be used for any object.

Ups, really, I missed it.

@kisvegabor  Is there a way to design these callbacks in such way that the same struct is used on both registration and callback? For example, provide the groups in registration, or provide the styles on the callback?

Let's say you have a custom function like my_set_x(). Without a->var you can use this function with only one object which is hardcoded in it. With a->var my_set_x() can be used for any object.

This is exactly why we need user_data associated with a callback.
For the C user, he would use a->user_data to access any object he wants.
For the Micorpython user (or any other OO binding) it would even be simpler. Since the binding manages the user_data, the user could use any data associated with the callback itself (as in closure).
For example:
lv.anim_set_custom_exec_cb(a, lambda a, val, self=self: self.my_set_x(val))

In this example self object is a user custom object which holds the information the user needs during the callback.
In any way, I suggest keeping lv_anim_set_exec_cb as well, so if the C user wants he could use it instead of the custom version.
So,

What do you think of my idea of coupling var and exec_cb for lv_anim_set_exec_cb?
Do you agree lv_anim_set_custom_exec_cb would set var to a?

I think it makes things clearer on the API, and it would work well for Micropython.
You can see an example of how it looks like on this micropython example.
(In this example I had to use lv.anim_set_var(self, self) to make it work correctly, since lv_anim_set_custom_exec_cb doesn't set var to a in case of custom callback)

For the C user, he would use a->user_data to access any object he wants.

Not really. The user data can help to reuse an animator function with different parameters. For example. float_in_radial can float-in objects with an arbitrary angle. The angle can be passed as user data. An animation like this can float in teh buttons a numpad, with different dierection for each. Without user data:

Ten animations would be required.
Some data needed to be stored in the animated objects.

What do you think of my idea of coupling var and exec_cb for lv_anim_set_exec_cb?

I agree with that. They are really used together.

Do you agree lv_anim_set_custom_exec_cb would set var to a?

As I wrote above having user_data and var separately can increase flexibility. So I think it's better to keep them and use a->is_custom or similar solution. Or do you have an idea to easily customize the animations (like in the example above) without user_data?

As I wrote above having user_data and var separately can increase flexibility. So I think it's better to keep them and use a->is_custom or similar solution. Or do you have an idea to easily customize the animations (like in the example above) without user_data?

@kisvegabor lv_anim_set_custom_exec_cb was added because of micropython binding needs right?
Do you think lv_anim_set_custom_exec_cb would also be used by the C users?
If it won't, then micropython binding (and I think other bindings as well) do not need and cannot use both user_data and var. user_data is enough to associate any additional data to an animation, as well as multiple values (such as both "object" and "angle", in your float_in_radial example above).
From my point of view, having both var and user_data, is the same as having user_data1 and user_data2 in a struct. It doesn't make sense - you could always make a single user_data pointer point to a struct with two (or any number of) pointers, if you want, instead of always keeping multiple pointers in the anim object.
Anyway, if you insist on having both var and user_data, it can work for the micropython binding.
Just please make sure the custom callback receives the anim object, such that user_data is accessible.
In such case the micropython binding will always ignore var (will not set it and will not use it).

I'M convinced I forget that in user data you can set structs too which can contain var.
I update the code accordingly: LINK
Just a better name needs to be found instead of lv_anim_set_var_and_cb. Do you have any idea?

@kisvegabor Since var is just a parameter that is passed to exec_cb, I think you can just call it lv_anim_set_exec_cb (and it would receive and set both the callback and var).
Another small problem:
https://github.com/littlevgl/lvgl/blob/b6a86f31d7cd0e7add31328f0bbfb2bf275a9978/src/lv_misc/lv_anim.h#L147-L151
Could you either:

Change a->var = NULL to a->var = a
or pass to the callback a instead of var if var is NULL

This is important for the callback convention, that expects the struct that holds the user_data to be the first argument of the callback. Otherwise lv_anim_custom_exec_cb_t would always receive NULL as its first argument.

Since var is just a parameter that is passed to exec_cb, I think you can just call it lv_anim_set_exec_cb (and it would receive and set both the callback and var).

Ok, let's do this way.

Change a->var = NULL to a->var = a

It really sounds better. Updated in 5ebed4f

@amirgon
I added user_data to the fonts. It's in the font branch which will be merged soon.
Unfortunately, I haven't found a good solution to the theme - group issue. I suggest skipping it in the binding for now. Do you agree?

I added user_data to the fonts. It's in the font branch which will be merged soon.

@kisvegabor Please let me know after it's merged into v6.0 and I'll check it then.

@amirgon
I merged the font update.

@kisvegabor fonts callbacks look fine, I think.
So we are left with:

 lv_event_send_func
style_mod_cb_callback and style_mod_edit_cb_callback
Did I forget anything?

You suggested we will not support these callbacks in the binding.
Another thing we should be aware of, is that the script will not stop with an error if it cannot generate a callback.
This is because there are known cases where it cannot generate a callback, in case we don't support certain callback or we have a special binding version for the callback (such as lv_anim_set_custom_exec_cb), so the "regular" version of the callback will not be generated (lv_anim_set_exec_cb).
This will make it a bit more difficult to identify new callbacks that don't comply with the convention. The script will just run, and not provide the callback. In such case I add a C comment about this in the generated binding C file.

This will make it a bit more difficult to identify new callbacks that don't comply with the convention. The script will just run, and not provide the callback. In such case I add a C comment about this in the generated binding C file.

That's a good idea. We were talking about a --verbose (or similar) option to print warnings. What do you think about it?

We were talking about a --verbose (or similar) option to print warnings. What do you think about it?

@kisvegabor I don't see a benefit in adding a --verbose flag.
Adding comments in the generated code is just as good, and actually better than that, since it's visible without adding any special command line flag.
It's the same problem for any case that requires the user inspecting a log and trying to figure out if the warnings that are printed in it are ok or not. I think it's best to avoid this, if possible.
A true solution would be to mark these non-compliant functions in some way, and report an error for any non-compliant callback that is not marked that way. A mark could be something like the _g_cb we added to the global callback.
That way, when someone adds a new non-compliant callback which does not have this special mark, the script would stop with an error.
What do you think?

I don't see a benefit in adding a --verbose flag.
Adding comments in the generated code is just as good, and actually better than that, since it's visible without adding any special command line flag.
It's the same problem for any case that requires the user inspecting a log and trying to figure out if the warnings that are printed in it are ok or not. I think it's best to avoid this, if possible.

I'd prefer both the log and comments but it's your project. If you feel the comments are more reasonable then it's also fine from my side.

A true solution would be to mark these non-compliant functions in some way, and report an error for any non-compliant callback that is not marked that way. A mark could be something like the _g_cb we added to the global callback.

Is it possible to mark the variable name? E.g.:
void lv_anim_set_exec_cb(lv_anim_t * a, void * var, lv_anim_exec_cb_t exec_cb_XXX)

If so, I'm happy with that because it doesn't bother anything. Just good generic name should be found for it.

Is it possible to mark the variable name? E.g.:
void lv_anim_set_exec_cb(lv_anim_t * a, void * var, lv_anim_exec_cb_t exec_cb_XXX)

@kisvegabor This could work for functions that register callbacks, but not for structs that contain a callback pointer.
The script expects to find user_data member on such structs, and expects the callback to receive the same struct pointer as the first parameter.

This could work for functions that register callbacks, but not for structs that contain a callback pointer.

By updating the animations, if I don't miss anything, every structure is set/initialized by functions. So there are no direct assignments for struct fields by the user, only via functions, right?
In case of theme-group question, the lv_group_style_mod_cb_t function pointer are still valid in the context of lv_gorup_... function and invalid only for themes. It also shows that, if possible,  it would be better to mark-up the variable name to get different behavior in different contexts.

So there are no direct assignments for struct fields by the user, only via functions, right?

@kisvegabor actually on lv_font_t we've got get_glyph_dsc and get_glyph_bitmap, on disp drv we have flush_cb and probably there are other cases where the user needs to update the callback directly on the struct.
And that's fine, the binding supports this as long as conventions are followed, and now that's the case in most cases.
Even if we add a setter function for each of the cases the convention is not followed, I think we should keep the possibility to update the callback directly on the struct for future flexibility.
So - in the rare cases where the convention is not followed, for example in case of exec_cb field of lv_anim_t struct, do you think we can find a way to mark it? (either on the struct itself or on the callback prototype, not only on lv_anim_set_exec_cb as you suggested)

actually on lv_font_t we've got get_glyph_dsc and get_glyph_bitmap, on disp drv we have flush_cb and probably there are other cases where the user needs to update the callback directly on the struct.

True. However in these cases, e.g. lv_anim_t a.exec_cb = my_func, it's the user responsibility to set an exec_cb function with the correct type.
So marking the parameter name in lv_anim_set_exec_cb and lv_event_send_func should work, right? Here really only prototype should be marked because there are valid use cases of lv_anim_exec_cb_t and lv_event_cb_t. In contrast theme style mod is invalid in all cases so here we can mark the names in the struct directly.
So in summary

Mark the parameter name in the API if the callback is valid generally, but that API function is tricky somehow (lv_event_send_func)
Mark the variable name in the struct if the callback is in a "wrong place", e.g. theme style_mod_cb.

What do you think?

@kisvegabor

True. However in these cases, e.g. lv_anim_t a.exec_cb = my_func, it's the user responsibility to set an exec_cb function with the correct type.

But the binding needs to know if it's ok or not that callback convention is not followed.
When callback is a struct field (such as exec_cb or flush_cb) the binding script identifies this as a callback and checks if the callback conventions are followed.
In this case, it recognizes that flush_cb  follows the convention, but lv_anim_exec_cb_t exec_cb doesn't.
Now we need to tell it that it's ok that exec_cb doesn't follow the convention, but flush_cb should still follow it.
What about adding the naming convention in the callback prototype itself? Something like:
typedef void (*lv_anim_exec_cb_t)(void *bad_cb, lv_anim_value_t);

(you can change bad_cb to any other naming convention, that marks that it's not an error that this callback cannot be supported by the binding)
The advantage of adding the convention in the callback prototype is that the same prototype is used in both cases - when registering a callback with a function call and when just assigning a function pointer on a struct.

I see. In light of that, it should work:

anim_exec_cb: use bad_cb in the typedef because it's an "invalid" type
lv_event_send_func: mark the paramter name because this function is an exception, lv_event_cb_t is still valid
theme-group: mark the theme->group_style_mod_bad_cb becasue in this context the style mod functions are invalid.

Do you agree?

@kisvegabor I wish there was a simpler "single" way to represent this, instead of 3 different ways depending on the situation.
But if there isn't, this is probably good enough.

@amirgon
I also think that these solutions are good enough as they are not really visible for the user.
I added the updates here bceb46b
How is it working?

@kisvegabor I haven't looked at it until now, but what about lv_fs callbacks? I'm not sure they follow the callback conventions, and I don't remember if we discussed this.

I'm not sure they follow the callback conventions, and I don't remember if we discussed this.

They are not following the convention now but it seems it's easy to fix it. I'll do it soon.
How about the previous updates? Are they working?

How about the previous updates? Are they working?

@kisvegabor I didn't have the chance to check it yet. It requires some changes in the binding script and I'm working on something else right now.
I'll check that later and let you know.

@amirgon
I update the fs callbacks: 760359f

I update the fs callbacks: 760359f

@kisvegabor We would also need user_data on lv_fs_drv_t for the callback convention.

Done: 51878e6

Done: 51878e6

Thanks!

Me too!
I've still added _cb suffix to the callbacks: abae15a

@kisvegabor There is still some problems with marking exclusions (as you added in bceb46b)

lv_task_create receives lv_task_cb_t task_cb, which does not follow the convention but is not marked with xcb
lv_event_send_func receives lv_event_cb_t event_cb does not follow the convention but is not marked with xcb

The others seems ok.

I fixed them here d84e977

@kisvegabor
I've discovered a new problem, related to littlevgl/lv_binding_micropython#25.
https://github.com/littlevgl/lvgl/blob/ca1f21ad04ed3da251369a199dd6ec619c35c881/src/lv_objx/lv_list.c#L177
https://github.com/littlevgl/lvgl/blob/ca1f21ad04ed3da251369a199dd6ec619c35c881/src/lv_objx/lv_list.c#L195
lv_list_add receives event_cb and registers it on liste list.
The problem is that Micropython binding assumes that every event registration also sets user_data to the Micropython callable object.
In this case an event is registered, but the associated user_data is not set. When the callback is called,  Micropython binding cannot find the callable object in user_data.
The problem is not limited to list.
I searched for calls to lv_obj_set_event_cb and found this:
./src/lv_objx/lv_list.c:    lv_obj_set_event_cb(liste, event_cb);
./src/lv_objx/lv_tabview.c:        lv_obj_set_event_cb(ext->btns, tab_btnm_event_cb);
./src/lv_objx/lv_tileview.c:        lv_obj_set_event_cb(ext->page.scrl, tileview_scrl_event_cb);
./src/lv_objx/lv_win.c:    lv_obj_set_event_cb(btn, event_cb);
./src/lv_objx/lv_kb.c:        lv_obj_set_event_cb(new_kb, lv_kb_def_event_cb);
./src/lv_objx/lv_mbox.c:        lv_obj_set_event_cb(new_mbox, lv_mbox_default_event_cb);
./src/lv_objx/lv_page.c:        lv_obj_set_event_cb(ext->scrl, scrl_def_event_cb); /*Propagate some event to the background

A possible solution is to set user_data of the internal object to the user_data of the external object.
For example, in the lv_list_add case:
    lv_obj_set_event_cb(liste, event_cb);
    lv_obj_set_user_data(liste, list->user_data);

Is this a valid solution? Can we apply this to all the cases above?

A possible solution is to set user_data of the internal object to the user_data of the external object.

Thinking about this again... there is a problem with my suggestion:
If one "external" object contains multiple "internal" objects, my suggestion would cause all the internal objects to have the same user_data. This means their micropython callbacks would overrun each other and only the last callback registered will be invoked for all internal objects.
So we probably need to think about some other solution to this problem.

The problem is not limited to lv_list.

Does really every element of the list is problematic? Most of them seems valid to me. If lv_list_add is a special case we can figure out a workaround for it.

Does really every element of the list is problematic? Most of them seems valid to me. If lv_list_add is a special case we can figure out a workaround for it.

@kisvegabor You are right, it's problematic only when the callback is delegated from the user.
If the callback is internal to lvgl, it's not a problem.
So going over the list I think that we have a problem at:

lv_list_add
lv_win_add_btn

In both these cases, the callback is given as a parameter, and then passed on to lv_obj_set_event_cb.
But I noticed that in both those cases, the delegating function (lv_list_add, lv_win_add_btn) is returning the internal object, so a workaround could be to set the event on the returned object.
something like:
btn_close = list1.add(lv.SYMBOL.CLOSE, "Close", None)
btn_close.set_event_cb(my_close_callback);

How about requiring the user to always do it like this? just remove the callback parameter from the delegating function.

I created the "complex" functions because usually the only thing you want to set for a list element or window control button is its event function. I'd like to keep these functions at least in C.
I think it's the same case than lv_task_create and lv_task_create_basic.
However lv_list_add_basic would look ugly because lv_list_add is not that complex either.
So IMO the solution would be to have two functions but still don't know how to name them.
@embeddedt
Do you have any a good name idea for lv_list_add_with_event and lv_list_add_without_event?

I think it's better to just remove the callback parameter. It's not that much extra typing to type lv_obj_set_event_cb immediately below. Plus, it's more consistent with other objects.

I think it's better to just remove the callback parameter. It's not that much extra typing to type lv_obj_set_event_cb immediately below. Plus, it's more consistent with other objects.

@kisvegabor, I agree with @embeddedt .
If, however, you decide to have both functions, please add the "xcb" marker to the with_event versions, because we would like to prevent Micropython users from using the with_event versions.

I think it's better to just remove the callback parameter. It's not that much extra typing to type lv_obj_set_event_cb immediately below. Plus, it's more consistent with other objects.

I also don't have a better idea, in lack of good names. So I removed the event parameters. I also reneamed lv_list_add to lv_list_add_btn for consistency.

@amirgon
I there anything else with the callbacks?

@amirgon
I there anything else with the callbacks?

@kisvegabor At the moment, I don't see any additional problems! :smile:
I'm closing for now. Thank you for the efforts!

Awesome! Hopefully, no API breaking issues will turn out.