search

lvgl-micropython / lvgl_micropython

LVGL module for MicroPython
MIT License
61 stars 19 forks source link

SPI.Bus weirdness #83

Open dcmcshan opened 1 month ago

dcmcshan commented 1 month ago

SPI.Bus initialization has some weird things for me...

The code below

I've had issues with SPI bus retaining state after soft_resets before, and suspect it's something similar. But this is pretty weird. It's like the kernel itself is getting corrupted?

Is there a way to reset/release the SPI bus?


        print(f"PIN_MOSI = {type(PIN_MOSI)}")
        print(f"PIN_MISO = {type(PIN_MISO)}")
        print(f"PIN_SCK = {type(PIN_SCK)}")

        # Initialize the SPI bus
        self.spi_bus = SPI.Bus(
            host=1,
            mosi=PIN_MOSI,
            miso=PIN_MISO,
            sck=PIN_SCK
        )

        print(f"spi_bus = {self.spi_bus}")
dcmcshan commented 1 month ago

And now, just to be interesting, 

MPY: soft reboot
PIN_MOSI = <class 'int'>
PIN_MISO = <class 'int'>
PIN_SCK = <class 'int'>
Traceback (most recent call last):
  File "<stdin>", line 302, in <module>
  File "<stdin>", line 262, in main
  File "<stdin>", line 71, in __init__
TypeError: can't convert float to int
kdschlosser commented 1 month ago

That last error is an error in your main.py file on line 71. in your __init__ function.

Without seeing all of the code I am not going to be able to offer assistance with this...

dcmcshan commented 1 month ago

It's not actually an error, is my point. It's weirdness that happens after soft reboot.

Another below. What I'm wondering is whether with this new SPI architecture if there is a way to reset the SPI bus before we initialize it.

MPY: soft reboot PIN_MOSI = <class 'int'> PIN_MISO = <class 'int'> PIN_SCK = <class 'int'> spi_bus = Traceback (most recent call last): File "", line 124, in File "MCT.py", line 67, in init File "MCT.py", line 124, in init_display File "display_driver_framework.py", line 169, in init MemoryError: memory allocation failed, allocating 335946543 bytes

kdschlosser commented 1 month ago

unless I see all of the code that is being run I cannot determine what is happening.

kdschlosser commented 1 month ago

also what is the purpose of doing a soft reboot? if you have to do a soft reboot then you also have the ability to shut down the display driver and the SPI bus gracefully before the reset takes place so it is a non issue.


th.disable()
th.deinit()
del th

fb1 = display_driver._frame_buffer1
fb2 = display_driver._frame_buffer2
del display_driver

display_bus.free_framebuffer(fb1)
if fb2 is not None:
    display_bus.free_framebuffer(fb1)

del fb1
del fb2

del display_bus
spi_bus.deinit()
del spi_bus()

reboot()
kdschlosser commented 1 month ago

and I would actually call a gc.collect() just before the reboot happens.

dcmcshan commented 1 month ago

I suppose it's just an IDE quirk. Thonny "soft reboots" when you click the "stop/restart backend" button.

If I change code in a module and I need it to reimport it, it's easiest to just click that button and start fresh. Only, of course, it doesn't start fresh...

dcmcshan commented 1 month ago

I'd be interested to know better development habits...

kdschlosser commented 1 month ago

Oh lord. Don't use Thonny. It doesn't play nice with Micropython and LVGL. You are better to just write the firmware using esptool and then use putty to connect to the MCU. I have a modified version of ampy to send files to the ESP32. It doesn't have any issues with connecting. I fixed those.

I also have boot script you can load onto your ESP so that you will not be limited to 115200 when sending files. You can jack it up to almost 1 mbit and the files will transfer without any issues.

Carglglz commented 1 month ago

also what is the purpose of doing a soft reboot? if you have to do a soft reboot then you also have the ability to shut down the display driver and the SPI bus gracefully before the reset takes place so it is a non issue.

@kdschlosser there is a bug in lv_binding_micropython in init/deinit so right now soft reset is not supported see https://github.com/lvgl/lv_binding_micropython/issues/343, also I think this is the same as #8, #66 and #74, it will raise a memory error or crash.

test this, soft-reset and run it again...

import lvgl as lv
print(f"LVGL INIT STATE: {lv.is_initialized()}")
if not lv.is_initialized():
     lv.init()
     print(f"LVGL INIT STATE: {lv.is_initialized()}")

print(f"LVGL INIT STATE: {lv.is_initialized()}")
lv_display = lv.display_create(320, 240)

lv.deinit()

print(f"LVGL INIT STATE: {lv.is_initialized()}")

also what is the purpose of doing a soft reboot?

This allows to run multiple lvgl-micropython tests on devices with the micropython test suite. 👍🏼

kdschlosser commented 1 month ago

This issue only exists when people use Thonny. That seems the be the current commonality between them. Solution. Stop using Thonny.

Carglglz commented 1 month ago

This issue only exists when people use Thonny

No, it is the soft reset, Thonny seems to use soft reset between runs, so that's why it happens there. Again check https://github.com/lvgl/lv_binding_micropython/issues/343 and the script above, something is wrong with the root pointers...

kdschlosser commented 1 month ago

I do not know of any reason why someone would want to perform a soft reset with an ESP32 it's either being put to sleep or it's running.

It states in the micropython documentation that the software reset is to ONLY restart the python interpreter. It does NOT clear memory that is used and it does NOT reset the hardware. In other words it restarts the REPL. There are things that micropython does where it stored information in the global namespace in C code so resetting the interpreter is not going to clear those things out. micropython has the SPI done in this manner as well as Pins and other hardware related things.

There is a hardware reset and that is what should be used if wanting to restart the device.

This is not a bug it is by design. The software reset only has the purpose of stopping the running code on the device. So if there is a loop that gets stuck that kind of thing. It is so the connection between a PC and the ESP doesn't get broken... That is the ONLY purpose it has.

This is under the REPL section of the MicroPython Documentation

https://docs.micropython.org/en/latest/reference/repl.html#soft-reset

A soft reset will reset the python interpreter, but tries not to reset the method by which you’re connected to the MicroPython board (USB-serial, or Wifi).

Things like frame buffers are created in C code and the pointers to them are also stored in C code. Those pointers gets passed into the interpreter so if the interpreter gets reset only the addresses that are stored in the interpreter gets deleted and not the actual buffer. The buffer still resides in memory and it remains allocated. A software reset is not going to clear that information. Most everything that is done hardware wise is going to persist and that in and of itself is going to cause issues. software resetting the interpreter is not supported.

Carglglz commented 1 month ago

I do not know of any reason why someone would want to perform a soft reset with an ESP32 it's either being put to sleep or it's running.

Testing, e.g. micropython test suite soft resets between tests.

But ignoring the use case,

It states in the micropython documentation that the software reset is to ONLY restart the python interpreter. It does NOT clear memory that is used and it does NOT reset the hardware. In other words it restarts the REPL. There are things that micropython does where it stored information in the global namespace in C code so resetting the interpreter is not going to clear those things out.

I'm not sure about the memory bit, see

ports/esp32/main.c

soft_reset_exit:

    #if MICROPY_BLUETOOTH_NIMBLE
    mp_bluetooth_deinit();
    #endif

    #if MICROPY_PY_ESPNOW
    espnow_deinit(mp_const_none);
    MP_STATE_PORT(espnow_singleton) = NULL;
    #endif

    machine_timer_deinit_all();

    #if MICROPY_PY_THREAD
    mp_thread_deinit();
    #endif

    // Free any native code pointers that point to iRAM.
    if (MP_STATE_PORT(native_code_pointers) != MP_OBJ_NULL) {
        size_t len;
        mp_obj_t *items;
        mp_obj_list_get(MP_STATE_PORT(native_code_pointers), &len, &items);
        for (size_t i = 0; i < len; ++i) {
            heap_caps_free(MP_OBJ_TO_PTR(items[i]));
        }
    }

    gc_sweep_all();

    mp_hal_stdout_tx_str("MPY: soft reboot\r\n");

    // deinitialise peripherals
    machine_pwm_deinit_all();
    // TODO: machine_rmt_deinit_all();
    machine_pins_deinit();
    machine_deinit();
    #if MICROPY_PY_SOCKET_EVENTS
    socket_events_deinit();
    #endif

    mp_deinit();
    fflush(stdout);
    goto soft_reset;
}

Most everything that is done hardware wise is going to persist and that in and of itself is going to cause issues. software resetting the interpreter is not supported.

Well the script above doesn't set any hardware nor allocates any frame buffer, yet it fails so I still believe there is a bug in init/deinit code (especially when it can be fixed by https://github.com/lvgl/lv_binding_micropython/issues/343#issuecomment-2185427218)

kdschlosser commented 1 month ago

are you shutting down the SPI? are you shutting down the display bus? are you shitting down the display driver? are you shutting down the touch driver? are you shitting down the I2C or SPI for the touch? are you freeing the frame buffer(s)?

I am willing to bet NO to every single one of those things.

The reason why I am willing to bet that is because Thonny is passing an escape sequence to the REPL terminal that does the restart. It is NOT calling a function in your code that would shut these things down. There is NO way for me to determine that the interpreter is being restarted and because there is no way of knowing that I am not able to perform a cleanup of anything. You are more than welcome to reach out to the developers of MicroPython and ask them to add the ability to register callback functions for a soft restart this way if the interpreter is restarted by something like Thonny then the callback will get called and you can perform the proper shutdown of the display and associated parts and pieces. That is going to be a larger code change than I am willing to make for it to only support a single use case. Yes I said single use case read below to find out why.

There are ways to perform testing without needing to use the tests that are built into MicroPython. Those tests are written to test MicroPython and only MicroPython. MicroPython is a dependency of this project and the tests that are built into that dependency are not going to be written to work with the code in this library. There are ways of testing to make sure that it is working properly and it doesn't do any soft resets. See here...

https://github.com/lvgl/lvgl/tree/master/tests/micropy_test

I am the one that wrote that testing package and it works and is used each and every single time someone makes a PR or a commit gets pushed to LVGL.

kdschlosser commented 1 month ago

and to your statement of no memory allocation...

https://github.com/micropython/micropython/blob/6007f3e2062cc65fc8416f241c682e37eb956c11/ports/esp32/machine_hw_spi.c#L136

That right there. A variable set in the global namespace that is tasked with holding all of the SPI instances that have been started, Memory IS allocated for the objects that are stored in there.

That allocation occurs right here...

https://github.com/micropython/micropython/blob/6007f3e2062cc65fc8416f241c682e37eb956c11/ports/esp32/machine_hw_spi.c#L275

the freeing of those resources will ONLY occur is deinit gets called. MicroPython doesn't call deinit when soft restarting. That is the responsibility of the user. Unfortunately the authors of MicroPython never provided a way to do that if an external program that is connected to the MCU passes an escape sequence to perform the soft reset.

The ability to release everything and to free buffers has been coded into this repo. If there was some kind of a notification a soft reset happening then it could be handled. So unless that ability gets added to micropython there is nothing I can do about it without having to change a decent size piece of the core code in MicroPython. I am just not interested in making that time investment. You would be better off to ask the developers of Thonny to fix their code and to use esptool to perform a hard reset instead of doing a soft reset. That would solve the problem as well and it would actually be a better solution.

kdschlosser commented 1 month ago

If the ability to get a notification on soft reset existed what I would end up doing is performing a hard reset once the notification was received. In the end that is what needs to be done if you want to perform a reset.

This is a 100% gurantee that all memory and hardware settings will be cleaned out for the program to restart into a clean slate and not have any issues.

Carglglz commented 1 month ago

you can perform the proper shutdown of the display and associated parts and pieces. the freeing of those resources will ONLY occur is deinit gets called. That is the responsibility of the user.

What I'm saying is that even in the case you can clean and free those resources (not even setting hardware, buffers, etc) before doing a soft-reset , after a soft-reset lvgl.init()/lvgl.deinit() won't work and will raise a memory error or crash.

Have you tested this script, soft-reset and run again?


import lvgl as lv
print(f"LVGL INIT STATE: {lv.is_initialized()}")
if not lv.is_initialized():
     lv.init()
     print(f"LVGL INIT STATE: {lv.is_initialized()}")

print(f"LVGL INIT STATE: {lv.is_initialized()}")
lv_display = lv.display_create(320, 240)

lv.deinit()

print(f"LVGL INIT STATE: {lv.is_initialized()}")

https://github.com/lvgl/lvgl/tree/master/tests/micropy_test

I'm aware of this but AFAIK this isn't intended to run on devices, only unix port...

There are ways to perform testing without needing to use the tests that are built into MicroPython. Those tests are written to test MicroPython and only MicroPython

Not exactly, it can run any micropython/test code and compare stdout output with an expected output, and can run on devices too ( and it's nice to have tests that can be run both on unix and on devices...)

kdschlosser commented 1 month ago

As I have stated in my previous comments. The soft reset is ONLY for the purposes of restarting the REPL. It is not intended to be used as a "restart" that is what a hard reset is for. There are things that are not going to be able to get cleaned up properly unless a hard reset is performed.

The test program that comes with MicroPython is written for the purpose of testing MicroPython. It has not been written for testing anything other than MicroPython. The fact that you can use it in SOME cases to test user code is because the user code isn't going anything that is really advanced. It is not going to work with ALL user code and nor should it. The test program is written for the sole purpose of testing the internals of MicroPython and nothing else.

And this is the output I get from running that code...

LVGL INIT STATE: False
LVGL INIT STATE: True
LVGL INIT STATE: True
LVGL INIT STATE: False