lvgl / lv_binding_micropython

LVGL binding for MicroPython
MIT License
250 stars 161 forks source link

Missing lvgl.SYMBOL.BATTERY_1 #224

Closed PGNetHun closed 2 years ago

PGNetHun commented 2 years ago

Hi!

Somehow the SYMBOL.BATTERY_1 is gone in this commit: https://github.com/lvgl/lv_micropython/commit/6ebf96a9032170a957dd07c83350480c08b79c78 Related commit in this repo: https://github.com/lvgl/lv_binding_micropython/commit/65304b6825093ff80fc88ed2cad685efa203858a It seems only this symbol is affected.

Ports:

Repro in REPL:

import lvgl as lv
lv.SYMBOL.BATTERY_EMPTY
lv.SYMBOL.BATTERY_1

I still could not find the exact cause of this, maybe you @amirgon find it faster.

Thanks!

PGNetHun commented 2 years ago

Not reproducible after commit https://github.com/lvgl/lv_binding_micropython/commit/341134744b2a919d8a9e7773bba1bdec0fa21b44

amirgon commented 2 years ago

Actually I tried checking out specifically https://github.com/lvgl/lv_micropython/commit/6ebf96a9032170a957dd07c83350480c08b79c78 and the problem still doesn't reproduce.

Please let me know if you see anything like that again, of even if you can reproduce this on an historic commit.

PGNetHun commented 2 years ago

Hi @amirgon,

Sorry, but I can reproduce again on unix with commit https://github.com/lvgl/lv_micropython/commit/0c889ab157b2936c6fef30628c72dadaa5cff53e

Can you please check it?

I've updated git submodules, cleaned the build folders, but getting the problem again.

Make unix port:

make submodules
make VARIANT=dev DEBUG=1

Execute in REPL:

import lvgl as lv
lv.SYMBOL.BATTERY_1
PGNetHun commented 2 years ago

The most interesting in this issue is, that lv.SYMBOL contains BATTERY_1, but somehow MicroPython can't find it. See screenshot.

Code:

image

amirgon commented 2 years ago

@PGNetHun I've tried again now, but I still can't reproduce this on my side. I checked out 0c889ab157b2936c6fef30628c72dadaa5cff53e , updated submodules, cleaned the project and built it just like you did (make VARIANT=dev DEBUG=1), but lv.SYMBOL.BATTERY_1 is still there:

MicroPython v1.19.1-632-g0c889ab15 on 2022-08-01; linux [GCC 9.4.0] version
Use Ctrl-D to exit, Ctrl-E for paste mode
>>> import lvgl as lv
>>> lv.SYMBOL.BATTERY_1
'\uf243'
>>> 

Here are some ideas:

PGNetHun commented 2 years ago

Hi, I've setup a new Ubuntu-22 in VirtualBox, same result, and unix standard variant throws errors. Also created docker file, same errors for standard variant.

Build steps (for standard variant) taken from: https://github.com/lvgl/lv_micropython#unix-linux-port

Dockerfile:

FROM ubuntu:latest
RUN apt-get update -y
RUN apt-get -y install build-essential libreadline-dev libffi-dev git pkg-config libsdl2-2.0-0 libsdl2-dev python3 parallel
WORKDIR /home
RUN git clone https://github.com/lvgl/lv_micropython.git
WORKDIR /home/lv_micropython
RUN git submodule update --init --recursive lib/lv_bindings
RUN make -C mpy-cross
RUN make -C ports/unix submodules
RUN make -C ports/unix
RUN ./ports/unix/micropython

Build docker container: sudo docker build -t micropython-unix -f Dockerfile .

amirgon commented 2 years ago

Also created docker file, same errors for standard variant.

I tried your Dockerfile and build fails for me because of macro redefinitions (with -Werror turned on) It turns out that until now I was always running micropython-dev unix port, but the unix standard port fails for compile.

The fix is very simple:

diff --git a/ports/unix/variants/standard/mpconfigvariant.h b/ports/unix/variants/standard/mpconfigvariant.h
index 1ec46ef92..0f86fbec8 100644
--- a/ports/unix/variants/standard/mpconfigvariant.h
+++ b/ports/unix/variants/standard/mpconfigvariant.h
@@ -31,8 +31,8 @@
 #define MICROPY_OPT_MPZ_BITWISE                 (0)
 #define MICROPY_OPT_MATH_FACTORIAL              (0)
 #define MICROPY_MODULE_ATTR_DELEGATION          (0)
-#define MICROPY_MODULE_BUILTIN_INIT             (0)
-#define MICROPY_ENABLE_SCHEDULER                (0)
+// #define MICROPY_MODULE_BUILTIN_INIT             (0)
+// #define MICROPY_ENABLE_SCHEDULER                (0)
 #define MICROPY_PY_BUILTINS_EXECFILE            (0)
 #define MICROPY_PY_MATH_CONSTANTS               (0)
 #define MICROPY_PY_MATH_FACTORIAL               (0)

I've added this patch to your dockerfile:

COPY fix.patch fix.patch
RUN git apply fix.patch

And now your dockerfile builds and runs as expected, but... the problem is not reproduced:

$ sudo docker run -it --rm micropython-unix ports/unix/micropython
MicroPython v1.19.1-633-g0e2023e50-dirty on 2022-08-04; linux [GCC 11.2.0] version
Use Ctrl-D to exit, Ctrl-E for paste mode
>>> import lvgl as lv
>>> lv.SYMBOL.BATTERY_1
'\uf243'
>>> 

PGNetHun commented 2 years ago

Hi! Unfortunately I could reproduce the BATTERY_1 issue also in Docker for variant dev.

Dockerfile:

FROM ubuntu:latest
RUN apt-get update -y
RUN apt-get -y install build-essential libreadline-dev libffi-dev git pkg-config libsdl2-2.0-0 libsdl2-dev python3 parallel
WORKDIR /home
RUN git clone https://github.com/lvgl/lv_micropython.git
WORKDIR /home/lv_micropython
RUN git submodule update --init --recursive
RUN make -C mpy-cross
RUN make -C ports/unix submodules
RUN make -C ports/unix VARIANT=dev DEBUG=1

PS: all MicroPython submodules are initiated and cloned

Commands: sudo docker build -t micropython-unix -f Dockerfile . sudo docker run -it --rm micropython-unix ports/unix/micropython-dev

MicroPython v1.19.1-633-g0e2023e50 on 2022-08-04; linux [GCC 11.2.0] version
Use Ctrl-D to exit, Ctrl-E for paste mode
>>> import lvgl as lv
>>> lv.SYMBOL.BATTERY_1
Traceback (most recent call last):
  File "<stdin>", line 1, in <module>
AttributeError: type object 'LV_SYMBOL' has no attribute 'BATTERY_1'
>>> lv.SYMBOL.BATTERY_2
'\uf242'

Host OS: Latest Ubuntu 22 - x64 running in Oracle VM VirtualBox

embeddedt commented 2 years ago

I was able to reproduce it with the dev variant using this slightly modified version of the above Dockerfile. The modifications were just to speed up submodule cloning by skipping the unnecessary ones.

FROM ubuntu:latest
RUN apt-get update -y
RUN apt-get -y install build-essential libreadline-dev libffi-dev git pkg-config libsdl2-2.0-0 libsdl2-dev python3 parallel
WORKDIR /home
RUN git clone https://github.com/lvgl/lv_micropython.git
WORKDIR /home/lv_micropython
RUN git submodule update --init --recursive lib/lv_bindings
RUN make -C mpy-cross
RUN make -C ports/unix VARIANT=dev submodules
RUN make -C ports/unix VARIANT=dev DEBUG=1
amirgon commented 2 years ago

Thank you @PGNetHun and @embeddedt ! I finally reproduced this problem on my side!! Couldn't have done this without your help :+1:

It's a bug in https://github.com/micropython/micropython/pull/6896, that only happens in case of hash collision. On ports/unix/build-dev/genhdr/qstrdefs.generated.h:

QDEF1(MP_QSTR_BATTERY_1, 14214, 9, "BATTERY_1")
QDEF1(MP_QSTR_read_line, 14214, 9, "read_line")

So BATTERY_1 and read_line have the same hash and the same length...
When the binary search looks for 14214, if it hits read_line as the lower bound, it misses BATTERY_1 which comes before it. So it's a very rare corner case that was not handled.

The fix is simple, I'm going to submit it soon.

amirgon commented 2 years ago

Fixed on https://github.com/lvgl/lv_micropython/commit/4111f1b5f0f070637c554caf4d21b96523d4451a, and also on https://github.com/micropython/micropython/pull/6896

PGNetHun commented 2 years ago

Thank you @amirgon for the fix!