search

nodemcu / nodemcu-firmware

Lua based interactive firmware for ESP8266, ESP8285 and ESP32
https://nodemcu.readthedocs.io
MIT License
7.64k stars 3.12k forks source link

Rework of the lua source hierachy to support a unified apporach to ESP8266 and ESP32 #1661

Open TerryE opened 7 years ago

TerryE commented 7 years ago

Background

The Lua source hierarchy is currently based on the following fork tree:

Issues

Whilst we have arrived at our current NodeMCU Lua code base in a set of logical steps, the reality is that this could be viewed as a dead-end from a maintenance perspective. I believe that we should now take stock and decide together how we approach a scenario where we wish to maintain a common Lua code base for both NodeMCU ESP variants.

More thought and discussion is needed, but my view is that this should be around options to maintain a common code base.

jmattsson commented 3 years ago

Wait, they still have an issue with overriding the linker file?? I raised an issue on that like two years ago I thought!

igrr commented 3 years ago

Hi @pjsg, could you describe the change you had to do in the linker script?

marcelstoer commented 3 years ago

see internal discussion thread 9

@pjsg https://github.com/orgs/nodemcu/teams/firmware/discussions/9

pjsg commented 3 years ago

@igrr In my case, I just wanted to include some LD config directly. I added the single INCLUDE line below. It uses an absolute path as I wasn't sure what the working directory was going to be! Obviously this isn't the right approach:

This is in the esp32-esp-idf/components/esp32c3/ld/esp32c3.project.ld.in file:

  .flash.rodata : ALIGN(0x10)
  {
    _rodata_start = ABSOLUTE(.);

    *(.rodata_desc .rodata_desc.*)               /* Should be the first.  App version info.        DO NOT PUT ANYTHING BEFORE IT! */
    *(.rodata_custom_desc .rodata_custom_desc.*) /* Should be the second. Custom app version info. DO NOT PUT ANYTHING BEFORE IT! */

    INCLUDE /home/philip/eclipse-workspace/nodemcu/components/base_nodemcu/ld/nodemcu_rodata.ld

    mapping[flash_rodata]

    *(.irom1.text) /* catch stray ICACHE_RODATA_ATTR */
    *(.gnu.linkonce.r.*)
    *(.rodata1)
    __XT_EXCEPTION_TABLE_ = ABSOLUTE(.);

The actual contents of the included file are:


    /* Don't change the alignment here without also updating the _Static_assert
     * over in linit.c! */
    . = ALIGN(8);
    /* Link-time arrays containing the defs for the included modules */
    lua_libs_map = ABSOLUTE(.);

    KEEP(*(.lua_libs)) 
    /* Null-terminate the array, 24 bytes */
    LONG(0) LONG(0) LONG(0) LONG(0) LONG(0) LONG(0) 

    /* Don't change the alignment here without also updating the _Static_assert
     * over in linit.c! */
    . = ALIGN(8);
    lua_rotables_map = ABSOLUTE(.);
    KEEP(*(.lua_rotable))
    /* Null-terminate the array, 24 bytes */
    LONG(0) LONG(0) LONG(0) LONG(0) LONG(0) LONG(0)

    /* Don't change the alignment here without also updating the _Static_assert
     * over in nodemcu_esp_event.h! */
    . = ALIGN(4);
    esp_event_cb_table = ABSOLUTE(.);
    KEEP(*(.lua_esp_event_cb_table))
    LONG(0) LONG(0) /* Null-terminate the array, 8 bytes */

    /* ----- End NodeMCU link-time arrays ------- */
igrr commented 3 years ago

I see, thanks. While replacing the IDF linker script with a user-provided one is unlikely to be supported (it is error-prone, especially if IDF gets updated), we can extend the linker script generation mechanism to support such link-time arrays generation.

pjsg commented 3 years ago

@igrr That would be great. Alternatively, allow an include statement as part of the linker mappings.

jmattsson commented 3 years ago

I had a look at old issues I'd raised and it seems I've only imagined raising a feature request for more flexible linker script generation :( Apologies for unfairly cast shade.

projectgus commented 3 years ago

Sorry for the very long delay in responding. A few months back we added linker script generation support for marking output symbls as KEEP and also for marking them with _start and _end symbols, which should allow for this kind of support.

https://docs.espressif.com/projects/esp-idf/en/latest/esp32/api-guides/linker-script-generation.html#types

Can search for "KEEP" and "SURROUND" here (the actual description is a few pages down from the anchor.)

However this doesn't support the "8 terminating words" method, just symbol markers for start & end.

Sample linker.lf linker fragment file:

[mapping:test]
archive: libfreertos.a
entries:
     * (default);
          text -> flash_text KEEP SORT(name, alignment) ALIGN(4, pre, post) SURROUND(test_symbol)

Generates:

.flash.text {
    ...

    . = ALIGN(4)
    _test_symbol_start = ABSOLUTE(.)
    KEEP(*libfreertos.a (SORT(.text) ...))
    . = ALIGN(4)
    _test_symbol_end = ABSOLUTE(.)
    ...
}

If this isn't enough for nodemcu requirements[*] then please consider opening a feature request at https://github.com/espressif/issues/new with the requirements you have for the ESP-IDF linker script, and we'll try to find a way to support it in the build system.

[*] For example: if you need to pull in sections from multiple ESP-IDF component libraries into one array, or if you need the "8 null words" termination instead of looping from ..._start to ..._end

jmattsson commented 3 years ago

Hi @projectgus! I'm actually working on this transition right now (chatter over on #3397). The KEEP/ALIGN/SURROUND should be fine for our needs, and I'm hoping the "embedding binary data" support will see me through the inclusion of pre-generated Lua bytecode. That's on my to-do list for tomorrow, together with working out how to put together the build of the build-host run compiler without having HOST_CC and co at my disposal.

igrr commented 3 years ago

@jmattsson I saw this the other day, assuming you can accept Python as a dependency it might do the trick: https://pypi.org/project/ziglang/ (plus https://andrewkelley.me/post/zig-cc-powerful-drop-in-replacement-gcc-clang.html)

jmattsson commented 3 years ago

While I'm not exactly a Python fan (I am after all working on a Lua project, not embedded Python 😂), I have to admit I'm seriously intrigued by that, @igrr! I might not get that in right off the bat, but it absolutely merits a deeper look. If it lives up to its promise that could be a very nice way to handle cross compilation.

So you're clearly about to add xtensa l106/lx6 support to it as well, right? 😉 One compiler front-end to rule them all...

igrr commented 3 years ago

Just to be clear I meant solving the problem of compiling the host tools when you don't necessarily have HOST_CC; that's how I understood your "how to put together the build of the build-host run compiler" sentence :) Using ziglang-cc to cross-compile for an embedded target is also possible but that's not what I had in mind.

So you're clearly about to add xtensa l106/lx6 support to it as well, right?

Yes, that currently depends on https://github.com/espressif/llvm-project/issues/4 (LLVM backend targeting Xtensa).

jmattsson commented 3 years ago

Yes, that's the correct interpretation, and yes, that is what I took you to mean :) I was then, mostly in jest, thinking that it'd be neat to have it embedded target support in zig as well, not being aware that there was already an ongoing LLVM effort for Xtensa. So my joke fell flat, but in the best possible way!

jmattsson commented 3 years ago

I don't suppose there's someone skilled in cmake who can tell me why my dependencies aren't working as expected?

In https://github.com/DiUS/nodemcu-firmware/blob/dev-esp32-idf4/components/embedded_lfs/CMakeLists.txt I want to ensure that ${BUILD_DIR}/lua.flash.store.reserved gets rebuilt after ${BUILD_DIR}/luac.out has been updated between idf.py build invocations. I've worked around it by having tools/embed_lfs.sh nuke the ${BUILD_DIR}/lua.flash.store.reserved file to force the rebuild, but that's a bit inelegant.

(And before someone tries to use this branch to build and run NodeMCU - please don't; I haven't dealt with the deprecated/removed APIs yet so a few big chunks are still commented out, and I'm not expecting what's built to even start up at this point.)

jmattsson commented 3 years ago

TIL that you can override a symbol entry entirely with -Wl,-defsym=<target>=<source>. And that, thankfully lets me get our link-time arrays get put together sufficiently to make things work. Slowly getting there...

jmattsson commented 3 years ago

For those following along at home, I've spent a couple of days looking at ways to support both the esp32 IDF and the 8266 RTOS IDF in the one branch and as far as I can see that's just not an option for us at this point.

With the 8266 RTOS SDK's tools (kconfig, ldgen) being old in comparison to the esp32 IDF's, I'm hitting no end of incompatibilities. While I have a workaround for the lack of ldgen features, I'm still being bitten by things like missing "rsource" support in the Kconfig files, and if I try to upgrade the IDF (tools dir) alone, then I'm missing dependencies in the RTOS components and hitting linker issues like missing dependencies, and even if I slap in a -Wl,--start-group it dies due to the vector table functions being too far offset from the vector table itself. Obviously this is all technically fixable, but it's looking like a job way too big for me as it would, as far as I can see, effectively be creating a new repo that has only the code from the ESP8266_RTOS_IDF and a completely rewritten component structure together with the lifted idf-v4 from the esp32-idf. Which of course would be a maintenance nightmare if done my anyone other than Espressif themselves.