nodemcu-firmware icon indicating copy to clipboard operation
nodemcu-firmware copied to clipboard

Lua 5.1 to 5.3 Realignment Phase 4

Open TerryE opened this issue 5 years ago • 10 comments

Summary

Now that we have merged #3075 unto dev, I want will close the historic issues which largely scoped the work completed in this PR, and to use this issue as a general discussion thread to track the general points that we can clean up with any following commits. Where any point requires detailed discussion it its own right then we can track this with its own issue and reference it here.

  1. Lua C API realignment. All C code in app/modulesand the separate app successfully compiles using both the (default) LUA=51 and the new LUA=53 targets. Testers and Modules maintainers are encourage to check the modules that maintain or use against the Lua53 environment and raise any issues that are found separately. I have also done a review of the lua.h, lauxlib.h and lnodemcu.h headers and my incorporate some further internal tidying in a subsequent PR.

  2. Extra Documentation. #2808 and my Lua 53 Whitepaper are WIP documentation of this change.

    • IMO we need to have some form of Lua 5.3 Whitepaper in the core document set, but this could probably drop all of the internals and historic discussion and focus solely on the how and what for Lua application developers.
    • We also need a C modules developers guide.
    • Possibly ditto an LFS provisioning guide.
  3. ECG review and adjustment. Replaces #2881. See #2783

  4. Review of SPIFFS use and spiffsimg. spiffsimg should generate loadable images. SPIFFS mounting should be bypassed if the SPIFFS partition size is zero (LFS-based deep-sleep applications might wish to do this to reduce startup times.)

  5. debug.debug() should work as expected on NodeMCU builds. See standard Lua documentation. Calling this from a panic exception handler is really good way to debug panics.

  6. Lua ESP test harness. Though touched on in #2145 which discusses a generic framework for use with new developed modules, this is very much more a narrow one of regressing a subset of the Standard Lua Test Suite for on ESP execution within the NodeMCU environment.

  7. Rename and restructure app directory hierarchy to align this to the esp32 hierarchy. As well as the split of modules into the core, common and esp8266 modules directories, most of this work is Makefile magic and file moves. This initial version will focus on changes to the core modules.

Future enhancements

  • Add second LFS region and ability to create LFS on ESP only See #2917.

TerryE avatar Apr 27 '20 15:04 TerryE

#3193 should address the first 3 items above. We need to drop the debug.debug() idea for now as there are a few architectural issues to sort if we want to do this and I am not sure that this is worth the effort.

@nwf @HHHartmann @marcelstoer, I would appreciate your thoughts on what I should tackle next post #3193. Possible options are:

  • Update Lua test harness to work both on host and on ESP
  • Investigation into SPIFSS instability and updates, though my current thinking is that I should leave one of the other committers to do this drill-down.
  • Add second LFS region support and on ESP generation of LFS images.

TerryE avatar Jul 08 '20 15:07 TerryE

I'd go for the second LFS image. Allthough I still think that a linktime addition of a fixed LFS part containing at least the Lua part of our (yet to come) hybrid modules would be at least as nice. We got this great LFS technology and should use it to allow Lua modules to be first class citizens in our firmware, just as C modules. Now we always need a second step of installing the LFS image.

HHHartmann avatar Jul 09 '20 05:07 HHHartmann

As the person perhaps most noisily agitating for hybrid modules, I agree with @HHHartmann's sense of priority. :)

The SPIFFS thing seems like it's going to lead to an infinite well of sadness, and while I love testing, I think having a baseline design for the New World Order takes priority, given our very limited resources (which is to say, mostly you, @TerryE).

nwf avatar Jul 09 '20 19:07 nwf

Re: the SPIFFS issue see my comment https://github.com/nodemcu/nodemcu-firmware/issues/3148#issuecomment-656122729. Hopefully @pjsg Philip will take a look at this. I'll get started on the LFS stuff when stuck in review cycles on #3193.

TerryE avatar Jul 10 '20 10:07 TerryE

@nwf, Did you prefer the 2nd LFS partition or the LFS builtin to a dev image? @TerryE or will we be able to include one LFS image in the build already?

HHHartmann avatar Jul 12 '20 08:07 HHHartmann

@HHHartmann, we already have defines for two LFS partitions and nodemcu-partition.py includes the code to read/process the RCR. The startup PT is just an RCR record that can be written to. Ditto any startup option. We can also load any partitions the same way. So we just need to extend this Python code a little.

An alternative would be to stay in the file world (ignoring esptool.py entirely) and generate a single combined .bin file that includes the RCR suitably configured, and any LFS / SPIFFS partitions needed. This single file could then be downloaded to the ESP by esptool.py. Note that esptool downloads a bootstrap which expands gzip format, and so any content sent to the ESP is in GZIP format, and it it doesn't really if there are large gaps in the content. It would be nice if esptool understood when a file was already gzipped so that esptool.py write_flash 0x00000 bin/build.img.gz would just work, but that's a nit.

TerryE avatar Jul 12 '20 16:07 TerryE

Honestly, I think I prefer a story with one LFS partition with the ability to build LFS images from .lc files on the device SPIFFS, as I outlined in https://github.com/nodemcu/nodemcu-firmware/issues/3128#issuecomment-638571513 . If people want to add support for two LFS partitions, I won't stop them, but I am not sure I understand its necessity and it seems like it would add complexity to allow either image to be replaced. (If there's hierarchy, so that the "first" image being updated also deletes the second, then this seems simpler.)

ETA: I see that @TerrryE has written at length in https://github.com/nodemcu/nodemcu-firmware/issues/3206 . I should read and understand that and see if it changes my understanding.

nwf avatar Jul 12 '20 16:07 nwf

OK, it seems the dual-LFS story is well underway (sorry, I've been buried in a new day job, so this is perhaps more news to me than it should have been). Given that story, I think we can have our build infrastructure build in the "sysLFS" into the .bin it generates and, so long as the default LFS updated by flashreload and its replacement is the "appLFS", I think more or less everything will just work as it does now. Nice.

nwf avatar Jul 12 '20 16:07 nwf

This issue has been automatically marked as stale because it has not had recent activity. It will be closed if no further activity occurs. Thank you for your contributions.

stale[bot] avatar Jul 08 '21 01:07 stale[bot]

I will kill or cure this one. In the meantime it remains an open issue.

TerryE avatar Sep 27 '21 00:09 TerryE