-
Notifications
You must be signed in to change notification settings - Fork 3.1k
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
LFS support for ESP32 NodeMCU #2801
Conversation
The default partition tables now also include 64k LFS.
@jmattsson @jasaw, I will take a detailed look at this in a day or so. I am keen that we continue to converge the two architectural variants rather than diverge them. I also have a deeper understanding of the Lua VM now than when I started this journey, and I am using this to sort out some the worse botches introduced in the eLua patch original and the original NodeMCU port. If you guys are actively working on the firmware, then I'd rather keep you in the loop than let you play catch-up. |
Plus add static asserts to fail hard if we get this wrong in the future.
Alright, I've added the necessary tweaks for this PR to build cleanly again after the IDF 3.3 upgrade over the weekend. I believe the alignment issue this uncovered could also be triggered on plain |
Technically, we only need a single LONG(0) in there as we only check the first field for null to determine the end-of-list, but better safe than sorry. Arguably we should drop an end-of symbol and only iterate to that.
This snuck in from the esp8266 side it seems.
Due to the change in the LOCK_IN_SECTION macro, our system event callbacks were, well, not called back.
@jmattsson Johny, I find it hard to track the logic and rationale behind why DiUS is making some of these changes, as you don't seem to openly document this in any form off issue / whitepaper / design paper ahead of time. Part of this is clearly also my unfamiliarity with the constraints of how the ESP32 IDF integrates with all of this. An example here is provisioning. On the ESP8266, I have fixed the location of the PT within the image, this allows me to use a post-link Python script to assemble the image from its components (the firmware, any SPIFFS and LFS images), and if necessary even move and resize the various partitions within the image. As part of this, the LFS image format was slightly changed to make compression more efficient and also to make it easy for a Python script to do LFS image relocation (see Given this, I couldn't see any point in having the cross compiler support the At the moment I feel that the two platforms seems to drifting apart because of our different drivers: I am trying to keep the ESP8266 variant have the fastest performance and maximum RAM and being able to work within a CloudBuilder / Docker environment really optimised for developers who do small scale development, whilst the ESP32 direction is primarily driven by the needs of DiUS. Surely we should be documenting and debating these non-functional drivers ahead of implementation so that we can converge the platform implementations rather than let them drift further apart? If you do produce this sort of design documentation ahead of implementation, then I would be more than happy to review and discuss this even offline. If you want me to roll up the various issue debates into an aggregate architecture document that I will try to do that. Time for Skype chat maybe? 😄 |
The ESP8266 implementation already has this functionality, in that we have a Python script that can assemble an image and relocate / resize partitions and load the LFS and SPIFFS partitions with their respective image files. I am not sure what the drivers are for making the LFS image optionally embedded within the firmware partition. Perhaps you can explain the rationale?
This default size is already a setting on
Making the default LFS init script called So from my PoV what we have really done is chose to reimplement three existing functions differently, and this just gives use more convergence work to do in the future. I am not saying that way these are implemented in the ESP8266 variant are better and perhpas we should move ESP8266 to the same implementation, but surely if this is the case then we should have to explicit reasons why the existing implementation is deficient and merits changing. |
On the ESP32, firmware images are both checksummed, and optionally signed (and encrypted). The IDF OTA support only handles application images. Thus, if you need to be able to distribute self-contained firmware (C & Lua), the LFS component must be linked into the application itself, before checksumming, or the system will not boot. This is an advanced use case, which is why the default is to look for the LFS in the designated partition, where you can employ the same approach as on the esp8266 - i.e. remote LFS upgrades only. I never got enough buy-in to PR the full OTA support we're using at DiUS, though it is available in one of the DiUS branches but doing full firmware upgrades on the 8266 is very much not straight forward and again an advanced use case. I feel that we're talking past each other in many areas here Terry - on the ESP32 there is no such thing as a |
That's a huge one :) Not sure how to review that...
It seems that the build system tries to compile for 32bit but I only have x84_64 glibc headers installed. Guess I can pull in 32bit glibc, but why doesn't it compile for the native host architecture? |
@devsaurus There are pointers in the Proto struct. ESP32 expects those pointers to be 32-bits, which is why we have to compile luac.cross as 32-bit. |
@jasaw, sorry but this is incorrect. I haven't looked at this part of your port, but the ESP8266 also uses a 32 bit size_t and luac.cross works fine in both 32 and 64 bit versions. It was designed to do this. That's what I did suggest to Jonny that I am happy to chat on swap emails 1-1 if you needed any b/g context etc. My commits have my email addr and we can chat using Skype or WhatsApp if that works better for you. |
Including updated otaupgrade registration.
The 32 and 64 bit builds generate identical outcomes; whatever prompted the use of 32 bit builds must've been something else.
The output from the 64bit and 32bit cross compilers is identical (save for the build timestamp). I've removed the forced 32bit build of it. Also updated PR branch to build cleanly against latest dev-esp32. Terry, I diffed the |
Aside from LFS, the big performance updates that I've made are:
But confession time, I started the lua53 stuff and forgot to publish the PR for these last two 😟 and because of all of the interdependencies, I decided to leave doing this one until later. |
Alrighty, if nobody yells loudly Real Soon(tm) I'm going to merge this to close the gap and prep for #2836. Definitely playing catchup on the '32 side, but one step at a time and we'll get there :D We've been loving the LFS stuff here at $work. While not as necessary on the esp32, it's definitely allowed us to write Lua as if we were on a desktop rather than spending time trimming things to make them fit. |
Hi there @jmattsson Issue: #2843 Error log:
|
@jpeletier That looks like your host doesn't have the |
What @nwf said. Try a |
@TerryE @jmattsson after excute the command below then excute at last, reboot!!! (sometimes, it's ok, try one more time.)
|
LFS support for ESP32 NodeMCU (nodemcu#2801)
@jmattsson @TerryE rst:0x1 (POWERON_RESET),boot:0x13 (SPI_FAST_FLASH_BOOT) I (178) esp_image: segment 3: paddr=0x00048020 vaddr=0x40080400 size=0x07ff0 ( 32752) load I (468) esp_image: segment 5: paddr=0x001322ec vaddr=0x400883f0 size=0x0e308 ( 58120) load I (489) esp_image: segment 6: paddr=0x001405fc vaddr=0x400c0000 size=0x0008c ( 140) load I (0) cpu_start: App cpu up. A2 : 0x00000018 A3 : 0x00000060 A4 : 0x00000000 A5 : 0x00000000 ELF file SHA256: 25c6e226706b171ddad237c0b448eb1ef94e2e275bcdac55378b647d7738e928 Backtrace: |
@nwf @jmattsson Thanks! This fixed the issue. |
@o07sai if you suspect a stack problem then increase the stacksize If the problem persists then please open a new issue with steps to reproduce (sdkconfig, Lua code, etc.). |
@devsaurus Thank you. The problem has been solved. |
dev
branch rather than formaster
.docs/*
.This is a port of @TerryE's Lua Flash Store support (and an Lua VM resync) from the esp8266 branch done by my colleague @jasaw. By necessity it also includes the change to the module registration syntax, which blows out the size of this PR substantially.
Differences from the esp8266 version:
Also included, naturally, is the new luac.cross compiler for building LFS images.
A small number of bugs in the LFS internals were corrected as part of this journey. Terry, I'd suggest doing a diff between the two lflash.c files - I believe a few things apply on the 8266 side as well. If I find time I'll try to do it for you and PR that branch too, but right now I suspect you're the one with time! ;)
There are undoubtedly things that could be made prettier, options to be added, etc. I believe this should however be a good starting point. With some luck we won't need to prettify too much before we've managed to join the 8266 and 32 trees :) At this point we haven't carried over the docs yet, as we haven't had time to check them for 8266-specifics.
We've tested this internally, but would love to have others give it a bash too before merge (though it should not have an impact when not used, other than the partition entry).
Oh, and Terry, I really wish you'd included a marker in the header for whether the debug fields were compiled in or not - that mismatch between host & esp was a near endless source of weird behaviour :(