Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Micropython update #2

Merged
merged 20 commits into from
Jan 24, 2019
Merged

Micropython update #2

merged 20 commits into from
Jan 24, 2019

Conversation

amirgon
Copy link
Collaborator

@amirgon amirgon commented Jan 17, 2019

@kisvegabor The main things we discussed are implemented here:

  • structs support, including colors structs, styles, themes, global fonts, etc.
  • gc support
  • Symbols support
  • Added example. This is actually @rreilink's advanced example which I ported to Micropython. There are still many differences between the Python and Micropython syntax (unfortunately), but it does the same thing.
  • The Makefile now runs gen_mpy.py, generates the bindings and builds them automatically. The commited lv_mpy.c remains as an example, it is not part of the build and doesn't have to be aligned with lv_conf.h. The actual generated lv_mpy.c is generated under build directory.

Still open issues:

  • There is no naming convention to identify "private" functions or structs. So all functions are bound, including many I assume the user should not be exposed to (such as ll_... functions). I suggest adding a single underscore to any "private" function or struct.
  • I don't handle arrays yet (to allow using functions such as lv_line_set_points or updating structs such as lv_chart_series_t). I'll add this in the future, I plan to allow the user pass a native python list and convert it automatically, but I still need to think about it.
  • Macros are not handled. Except from Symbols which I implemented, we still need to handle color macros. I suggest adding inline functions in addition to macros, both functions for converting colors and functions for the colors themselves. Something like static inline lv_color_t lv_get_color(int color_id). The advantage of using inline functions is that the compiler optimizes them down to a constant when possible, so they cost same as using the color macros directly (same in program-memory, data-ram and performance terms)
  • It's not really tested, except from some very simple examples. I guess there are multiple issues that are still to be discovered, I hope to get inputs from users once this goes to a formal release.

So - in order to release this on the v5.3, what do you need? Should I merge the changes to v5.3 branch and send you a PR to lvgl repo?

@embeddedt
Copy link
Member

@amirgon I think you should rename the sample lv_mpy.c to lv_mpy_example.c. Otherwise a user may become confused and think that they can modify that lv_mpy.c to do what they want.

@amirgon
Copy link
Collaborator Author

amirgon commented Jan 19, 2019

@embeddedt Renamed.
@kisvegabor I made some changes in lv_colors.h to expose OPA macros and LV_COLOR_HEX macros to Micropython.

@kisvegabor
Copy link
Member

Thank you for the update.

There is no naming convention to identify "private" functions or structs. So all functions are bound, including many I assume the user should not be exposed to (such as ll_... functions). I suggest adding a single underscore to any "private" function or struct.

It's quite reasonable, great idea!

I don't handle arrays yet (to allow using functions such as lv_line_set_points or updating structs such as lv_chart_series_t). I'll add this in the future, I plan to allow the user pass a native python list and convert it automatically, but I still need to think about it.

No problem, it's still an experimental version.

It's not really tested, except from some very simple examples. I guess there are multiple issues that are still to be discovered, I hope to get inputs from users once this goes to a formal release.

Yes, we will see. After the release let"s open a an issue for Micropython binding feedback.

So - in order to release this on the v5.3, what do you need? Should I merge the changes to v5.3 branch and send you a PR to lvgl repo?

Yes, please!

@amirgon
Copy link
Collaborator Author

amirgon commented Jan 21, 2019

It's quite reasonable, great idea!

@kisvegabor I would like to rename private functions/structs, but I'm not sure which should really be private (never allow user access) and which should still be available for the user in some cases.
Could you help listing the private functions/structs? If you do I'll rename them (underscore prefix) in my PR.

@kisvegabor
Copy link
Member

kisvegabor commented Jan 21, 2019

Basically, everything in the header files is public. As I see now only the global symbols (things form the header files) are added to the binding.

Before renaming let's merge the current version to see these updates in action. In addition, the renaming might cause conflicts so better to have a "stable base".

@kisvegabor
Copy link
Member

In littlevgl organization, we created separate repos for each project. I will move this one there too once this PR is merged.

So first we should merge the updates for lvgl and then this PR with the updated submodule.

…rely on lvgl.h instead of including the objects directly. This adds themes to the generated binding
…E_LV_TILEVIEW USE_LV_TABLE USE_LV_SPINBOX USE_LV_CANVAS because there are unimplemented functions there which causes lv_mpy.c (which wrap them) to fail linking
…cropython bindings and lvgl submodule (Micropython bindings was split from lvgl).
@kisvegabor kisvegabor merged commit 396b68a into lvgl:master Jan 24, 2019
embeddedt pushed a commit that referenced this pull request Aug 7, 2019
amirgon pushed a commit to amirgon/lv_micropython that referenced this pull request Jun 15, 2021
asan considers that memcmp(p, q, N) is permitted to access N bytes at each
of p and q, even for values of p and q that have a difference earlier.
Accessing additional values is frequently done in practice, reading 4 or
more bytes from each input at a time for efficiency, so when completing
"non_exist<TAB>" in the repl, this causes a diagnostic:

    ==16938==ERROR: AddressSanitizer: global-buffer-overflow on
    address 0x555555cd8dc8 at pc 0x7ffff726457b bp 0x7fffffffda20 sp 0x7fff
    READ of size 9 at 0x555555cd8dc8 thread T0
        #0 0x7ffff726457a  (/usr/lib/x86_64-linux-gnu/libasan.so.5+0xb857a)
        lvgl#1 0x555555b0e82a in mp_repl_autocomplete ../../py/repl.c:301
        lvgl#2 0x555555c89585 in readline_process_char ../../lib/mp-readline/re
        lvgl#3 0x555555c8ac6e in readline ../../lib/mp-readline/readline.c:513
        lvgl#4 0x555555b8dcbd in do_repl /home/jepler/src/micropython/ports/uni
        lvgl#5 0x555555b90859 in main_ /home/jepler/src/micropython/ports/unix/
        lvgl#6 0x555555b90a3a in main /home/jepler/src/micropython/ports/unix/m
        lvgl#7 0x7ffff619a09a in __libc_start_main ../csu/libc-start.c:308
        lvgl#8 0x55555595fd69 in _start (/home/jepler/src/micropython/ports/uni

    0x555555cd8dc8 is located 0 bytes to the right of global variable
    'import_str' defined in '../../py/repl.c:285:23' (0x555555cd8dc0) of
    size 8
      'import_str' is ascii string 'import '

Signed-off-by: Jeff Epler <jepler@gmail.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants