lvgl / lv_binding_micropython

LVGL binding for MicroPython
MIT License
237 stars 156 forks source link

fix(script): take forwarded declaration into consideration #308

Closed XuNeo closed 5 months ago

XuNeo commented 5 months ago

Forward declared struct has a None in type field. It's actually defined later and sorted to dict structs_without_typedef. We pick those typedefs and set correct type.

kisvegabor commented 5 months ago

@kdschlosser, please review it.

kdschlosser commented 5 months ago

I am testing it now. Need to see the exact changes that are made to the generated C file

kdschlosser commented 5 months ago

There is no change at all made to the output C source file. when I compare a generated source file with the change to one that was generated without the change they are 100% identical.

So I am not 100% sure what this is supposed to be doing.

XuNeo commented 5 months ago

Hi @kisvegabor The script update is to solve issues met in https://github.com/lvgl/lvgl/pull/5161. Basically, add two lines to lv_types.h like below will trigger the script bug.

struct _lv_obj_class_t;
typedef struct _lv_obj_class_t lv_obj_class_t;
kdschlosser commented 5 months ago

I see what is going on. It's dependent on a change to LVGL that has not yet occurred.

I gotta touch base on the whole hiding structures in private header files. If it is using in a public function or in a public structure or union it should not be made private. This is because not all bindings are written in low level code. This is one of the problems I have been banging my head against the wall with in the CPython binding. The binding is written in Python not in C code. It compiles LVGL as a shared library and accesses it using CFFI. CFFI needs to know the size of structures and there is no way for it to determine that if it's made private. Low level languages will have less of an issue with this because of how everything gets linked together. There is no linking made between a high level language and the underlying shared library.

XuNeo commented 5 months ago

Yes. Hide the struct will make the binding not work. But if it's in private header, then there's no need to export to binding.

So this fix only tries to detect correct struct definition in case of having forward declared struct. We shall see how the private headers go on later.

kdschlosser commented 5 months ago

so this change does nothing without the forward declarations being put into place.

XuNeo commented 5 months ago

Yes. Only this fix is merged, can LVGL CI pass. I also checked the original ./lib/lv_bindings/tests/run.sh && echo $? and confirmed it's running.

kdschlosser commented 5 months ago

@kisvegabor

This is good to go if your intention is to merge that other commit in LVGL. it's doesn't cause any issue with the current build design.