ESP32: External modules

[jpeletier]

• This PR is for the dev-esp32 branch rather than for master.
• This PR is compliant with the other contributing guidelines as well (if not, please describe why).
• I have thoroughly tested my contribution.
• The code changes are reflected in the documentation at docs/*.

External modules. Plugging your own C modules.

In order to make the most of NodeMCU, you will undoubtedly have to connect your ESP device to many different hardware modules, which will require you to write driver code in C and expose a Lua interface.

To make this easy, we have come up with the concept of "external modules". External modules allow you to refer to an external git repository containing the module, which will be downloaded/updated and built along the firmware. It is similar to git submodules, but without the complexity and having to alter the parent repository, while also adapted to the compilation requirements of NodeMCU and Lua.

This also allows to reduce the size of the monolithic NodeMCU repository, which won't scale every time someone wants to add a new module to support some obscure hardware. We think it is better to decentralize the module maintenance responsibility and make it easy to have a permissionless module ecosystem around NodeMCU.

Here is a sample external module repository: https://github.com/espore-ide/nodemcu-module-helloworld.

If this is merged, I will then withdraw #3023 and #3090 since these make more sense as external modules. We can then review all existing modules and determine whether these should be "core" modules that tie directly to internal ESP32 hardware, such as gpio, adc, file... or they are external-hardware specific, like dht or ws2812 and potentially should be external.

How to use external modules:

To use external modules, simply create an extmods.ini file in the repository root, with the following syntax:

[lua_mount_point]
url=<git repo URL>
ref=<branch name, tag or commit id>
...

Where:

• lua_mount_point: Name you want the referenced module to have in Lua.
• url: Repository URL where to fetch the code from
• ref: branch, tag or commit hash of the version of the module you want to pull in.
• disabled: (optional) Whether or not to actually compile the module (see below)

For example:

[helloworld]
url[email protected]:espore-ide/nodemcu-module-helloworld.git
ref=master

You can add further sections to extmods.ini, one for each module you want to add. Once this file is ready, run the update command:

make extmod-update

This will download or update the modules to the external modules directory, components/modules/external.

You can now compile the firmware with make as usual. The build system will find your external modules and compile them along the core modules.

After this is flashed to the device, the module in the example will be available in lua as helloworld.

Updating to the latest code

If your external module entry in extmods.ini points to a branch, e.g., master, you can update your local version to the latest code anytime by simply running make extmod-update.

Temporarily disabling an external module

If you want to stop compiling and including a module in the build for any reason, without having to remove the entry from extmods.ini, simply add a disabled=true entry in the module section and run make extmod-update.

Example:

[helloworld]
url=https://github.com/espore-ide/nodemcu-module-helloworld.git
ref=master
disabled=true

Mounting different module versions

Provided the module is well written (no global variables, etc), it is even possible to easily mount different versions of the same module simultaneously for testing:

[helloworld]
url=https://github.com/espore-ide/nodemcu-module-helloworld.git
ref=master

[helloworld_dev]
url=https://github.com/espore-ide/nodemcu-module-helloworld.git
ref=dev

Note that the second one points to a different branch, dev. Both modules will be visible in Lua under helloworld and helloworld_dev respectively.

How to write external modules:

To write your own external module do the following:

1. Create an empty repository in your favorite remote, GitHub, BitBucket, GitLab, etc, or fork the helloworld example.
2. Create an entry in extmods.ini as explained above. For modules that you author, it is recommended to use an updateable git URL in SSH format, such as [email protected]:espore-ide/nodemcu-module-helloworld.git.
3. Run make extmod-update

You can now change to components/modules/external/your_module and begin work. Since that is your own repository, you can work normally, commit, create branches, etc.

External module scaffolding

External modules must follow a specific structure to declare the module in C. Please refer to the helloworld.c example, or use it as a template. In particular:

1. Include module.h
2. Define a Module Function Map with name module
3. Define a module_init function
4. Include the module lua entries by adding a call to the NODEMCU_MODULE_STD macro

Here is a bare minimum module:

#include "module.h"

// Module function map
LROT_BEGIN(module)
/* module-level functions go here*/
LROT_END(module, NULL, 0)

// module_init is invoked on device startup
static int module_init(lua_State* L) {
    // initialize your module, register metatables, etc
    return 0;
}

NODEMCU_MODULE_STD();  // define Lua entries

For a full example module boilerplate, check the helloworld.c file.

Special makefile or compilation requirements

If your module has special makefile requirements, you can create a module.mk file at the root of your module repository. This will be executed during compilation.

Further work:

• Support for per-module menuconfig (Kconfig). This is actually possible already, but need to work around potential config variable collisions in case two module authors happen to choose the same variable names.
• Module registry: Create an official repository of known external modules.
• Move all non-essential and specific hardware-related C modules currently in the NodeMCU repository to external modules, each in their own repository.
• Create the necessary scaffolding to facilitate writing modules that will work both in ESP8266 and ESP32.
• Port this work to the ESP8266 branch. Mostly, the scripts that make this possible could work in both branches directly.

[pjsg]

My concern with this approach is that, while I trust the authors of nodemcu-firmware, I'm not sure that I trust the authors of all the related modules. This means that I get nervous when building the firmware in case one of the modules starts doing something nasty in its make file.... This is probably very serious for the cloud builder. I'm not sure how to address that in a secure way. [This isn't just a theoretical attack, it happens in the python, browser extension and node worlds -- typically a commonly used module gets taken over by a bad guy and this causes havoc]

[jpeletier]

My concern with this approach is that, while I trust the authors of nodemcu-firmware, I'm not sure that I trust the authors of all the related modules.

With the approach in this PR, if you don't trust the author of a particular module, do not use their module.

This means that I get nervous when building the firmware in case one of the modules starts doing something nasty in its make file....

With this approach, you only have to review module.mk in the module's respository root. 99% of the time, it should be an empty file, meaning that the author did not attempt to add any potentially harmful script.

This said, I can add a switch to disable custom makefile when importing external modules, so you would then be safe.

This is probably very serious for the cloud builder.

The cloud builder can lock to specific vetted versions (via commit hash and code hash) that they know are good.

[This isn't just a theoretical attack, it happens in the python, browser extension and node worlds -- typically a commonly used module gets taken over by a bad guy and this causes havoc]

I know, it is very possible, but this does not stop the NodeJS and Python communities from thriving and fixing these issues.

[pjsg]

I would be very happy with having a default setting that disabled custom make files.

My point about these other communities having problems was to make sure that, before we adopt the same approach, that we think about the issue -- and, indeed, having a setting that you describe would mitigate those issues.

I also missed the part about the C modules being in nodemcu-firmware. I'm still a little unclear on how you envisage the branching to work -- are all the references from the nodemcu-firmware repo (mast & dev branches) to specific commit hashes? I would think that master references would have to be to specific commits (otherwise you wouldn't be able to build a master branch from the past). I'm less clear about dev, but I suspect that it also has to be to specific commits otherwise you can't be sure when merging to master which commits to point to (i.e. which commits were tested). However, this means that, in order to update a module, you now have to do two PRs, one in the module repo and one in the nodemcu-firmware repo (to update the commit pointer).

Maybe I am missing something.....

[jpeletier]

Hi @pjsg Thank you for your comments.

I would be very happy with having a default setting that disabled custom make files.

This is easy to add, if this PR gets traction I'll add it before it is merged.

Maybe I am missing something.....

yes 😉 ! read on:

I also missed the part about the C modules being in nodemcu-firmware. [...]

The way this works, the nodemcu-firmware repo would not reference the external modules in any way. Rather, when you are building firmware for yourself, you edit extmods.ini (which is analogous to requirements.txt in Python or package.json in NodeJS). In there, you just define what external modules to pull and build into your image for your specific application. This file is not checked into nodemcu-firmware, like sdkconfig, which contains the build config for your specific application.

With this in place, people can author modules and publish them in their own repo. Here is an example of one such repo with a helloworld example: https://github.com/espore-ide/nodemcu-module-helloworld. Notice how clean it feels to have an entire repo for a specific module: Documentation, place for their own PRs, code organization...

If something is wrong with this module, you can open a PR to the author of this module, but you don't need to open a PR to nodemcu-firmware at all. In fact, this external module could even be a private closed-source module you or your company owns to drive some proprietary hardware.

Note that this discussion is actually a PR with a working implementation of this approach, you can go ahead and test it out following the guide above in the PR (it is only for ESP32 for now, though).

The objective here is not to add dependencies to nodemcu-firmware, but rather prevent the piling up of modules that drive external hardware. Also, facilitate the creation of new modules by avoiding people having to PR nodemcu-firmware for their module to have a home, increasing the size of the repo and the legacy.

I am motivated by the fact that 2 of my PRs (#3090, #3023) and other PRs(#2767, #2766, #2261, #2255, #2250, #2249 ...) are sitting in there and I see it will be very difficult for them to make much progress. And if they do, they will just make the repo bigger and more complicated to maintain. For example, if I want to use now a HTU21D sensor (PR #2250, opened in February 2018) I have no choice but do something dirty like merge it into my branch, fix the problems and it will be very difficult to contribute my fixes back. The guy who wrote that PR, @s-pw probably has given up already after two years and left the community.

Instead, we should have tools to make sure new module contributions can be used an tested as soon as possible.

[tomsci]

I like the idea but a few significant downsides come to mind:

• No longer being able to find device support by just browsing make menuconfig and/or nodemcu.readthedocs.io. A solid registry of 3rd party modules would offset this somewhat, but there's a lot to be said for keeping popular modules in tree, just for convenience and to reduce fragmentation.
• Modules accepted into the nodemcu source have been code reviewed and must supply documentation. There is a quality bar that's set by the PR and review process which is a nice thing to have. I can't count the number of times I've been prodded to document something (such as by the PR criteria) only to find that the act of documenting leads me to fix a corner case in the actual code. And I can have a reasonable level of confidence that code accepted into the nodemcu repo is going to work and not do something unpleasant because it has gone through that code review process.
• In-tree modules are guaranteed to at least compile against latest source. A major benefit of being in-tree is that global refactors (such as the changes to use LROT_BEGIN/LROT_END for module declarations) are easy to do and often don't need to involve the module maintainer at all. If I find an external module for what I want, but it doesn't build or work with the version of nodemcu I'm using, is it easy to find a version that does? If there are incompatible changes made to say dev-esp32 branch, is there a way of ensuring you can get the right version of the module for the version of nodemcu you're using?

I understand that every convenience benefit to the end user is a maintenance burden for the maintainers, but I think there's still a place for in-tree modules, at least for widely-used hardware. As as way to unblock PRs that are basically stalled at the "well, we don't really want to take on support for this thing none of us use or have hardware for, and maybe the code quality isn't as good as we'd like" stage, I think it's a useful idea - but only if that outweighs all the downsides.

The other interesting question this brings up, is, do we actually need C modules for all of these, and should we be encouraging people to write C modules if it can be avoided? A bunch of those PRs look like they could have been implemented in pure Lua using existing gpio, i2c etc modules. For the ones that can't right now (eg lack of gpio.pulse in esp32?), should we try and address that instead? I think everyone would agree maintaining a Lua module is easier than a C one as there's a greater level of abstraction and the APIs are under our control (ie not subject to something changing in the espressif SDK for example). If some of those stalled PR were written as Lua modules, would it be easier to accept them into the tree?

One possible problem there is I found it actually pretty painful to get set up with LFS and integrating a new C module is definitely simpler, even if it's harder to write the actual code. If the ROM build process supported directly including a .lua file as if it were a C module (ie taking care of running luac.cross, embedding it directly in the main ROM ELF), would that reduce the temptation to implement hardware modules in C when they don't really need to be, and thereby reducing the number of C modules the NodeMCU maintainers have to maintain?

Taking a brief look at some of those stalled PRs you mention:

#3090 - definitely looks like a candidate if we can come up with a good generic API for rmt (There's also #3094 which is a similar bit of pulse counting)
#3023 - This seems fundamentally complicated enough to definitely require its own C code, no arguments there :)
#2261 - seems like it's just an i2c wrapper, not sure why this couldn't just be a Lua module?
#2255 - is rfswitch just a glorified GPIO twiddler? Could you implement this whole module in Lua with gpio.pulse? Are there other current C modules which significantly overlap in functionality (I suspect there probably are...)?
#2249 / #2250 - Another i2c wrapper?
#2766 - I admit I don't really get this one. A device model sounds cool but I couldn't immediately see if this helped one write new modules in Lua or not, or whether it's something most people really need or not.

Sorry this has become a very long comment but I think this PR raises some really interesting questions!

[jpeletier]

Hello @tomsci, thank you for your comments.

No longer being able to find device support by just browsing make menuconfig and/or nodemcu.readthedocs.io. A solid registry of 3rd party modules would offset this somewhat, but there's a lot to be said for keeping popular modules in tree, just for convenience and to reduce fragmentation.

This is nice, but does not scale ad infinitum. A registry for modules like with other platforms will be a follow up to this one.

Note that I am only adding the possiblity of using external modules, not forcing everyone to use them. After this PR, the way the repo works today (make menuconfig, etc) is unaffected.

Modules accepted into the nodemcu source have been code reviewed and must supply documentation. There is a quality bar that's set by the PR and review process which is a nice thing to have.

This PR keeps this as-is. When someone authors a new module, they can choose whether they consider it worth going through this process or whether it makes sense to keep as external, experimental or too specific for the general community. See below for a proposed future policy we could discuss to decide if a new incoming module should be external or not.

I can't count the number of times I've been prodded to document something (such as by the PR criteria) only to find that the act of documenting leads me to fix a corner case in the actual code.

I have also gone through this feeling of documenting something and then realizing either the API was unexplainable or that there were corner cases I was not taking into account and had to go back to the code. In my case, this is why I learned to extensively document my code, not because there is a PR process here. Therefore, this should come from self-discipline and not because there is a PR process in this particular repo and not in another (do you follow the law only when there is a policeman nearby? 😉). This also does not prevent you from inviting people to peer-review and test your module, that is always a good practice.

And I can have a reasonable level of confidence that code accepted into the nodemcu repo is going to work and not do something unpleasant because it has gone through that code review process.

Indeed, and I don't expect this level of review to be reduced. I just want to allow people the freedom of plugging modules in their project easily, without asking for permission of the maintainers of this repo, who have very little time and other important issues to dedicate time to. The alternative to this now is to maintain your own fork, and that sucks.

In-tree modules are guaranteed to at least compile against latest source. A major benefit of being in-tree is that global refactors (such as the changes to use LROT_BEGIN/LROT_END for module declarations) are easy to do and often don't need to involve the module maintainer at all.

Yes, this is at the expense of having the nodemcu maintainers drag these modules across refactors with increasing effort as the legacy increases, without visibility of which ones are important or not.

This is counter-productive as all this refactoring work the nodemcu maintainers have had to do was very difficult to prioritize, since it is difficult to know what modules actually are being used in the community and should be worth the refactor.

In the extreme case that all the modules were external and there is a breaking API change like the LROT_BEGIN/END one you mention, active maintainers of modules actually in use would have hurried to get up to date. If they were not aware, people would open issues or PRs to their repos. Garbage modules would then be left behind.

If I find an external module for what I want, but it doesn't build or work with the version of nodemcu I'm using, is it easy to find a version that does? If there are incompatible changes made to say dev-esp32 branch, is there a way of ensuring you can get the right version of the module for the version of nodemcu you're using?

Yes, this should be part of the release process of the external module maintainer. The extmods.ini file format in this PR allows you to specify either the code branch, the exact commit that works for you or a release version tag. Therefore, if you have chosen a specific NodeMCU version, you can link to a frozen release of the module that works for you. This is analogous to package.json in NodeJS or requirements.txt in Python, where you specify the exact version that works for you.

This is a PR and the implementation is working. If you have time, check it out and try!

I understand that every convenience benefit to the end user is a maintenance burden for the maintainers, but I think there's still a place for in-tree modules, at least for widely-used hardware.

I agree. I would follow this policy:

1. Modules that expose internal hardware. In essence, anything anyone can test with the typical dev board and a few LEDs: IN-TREE.
2. Software modules that everyone needs, such as mqtt, http, etc: IN-TREE
3. Existing modules within the tree that drive very common external hardware such as an SD card or a typical i2c device, etc: move to EXTERNAL at some point, but within the nodemcu GitHub org.
4. Newly created modules that drive any other hardware: EXTERNAL, in the module owner's repo.
5. Newly created software-only modules that implement some application-specific protocol, modules that offload Lua for a specific application, modules that implement proprietary algorithms/communication protocols, etc: EXTERNAL.
6. Closed-source code for company projects, etc: EXTERNAL, potentially will be in a private repository, etc.

I would set up a registry in a repository where we can quickly accept PRs for people wanting to list their modules. We can do a quick review to make sure they have good enough documentation. This registry would be kickstarted with modules described in (3) above.

The other interesting question this brings up, is, do we actually need C modules for all of these, and should we be encouraging people to write C modules if it can be avoided?

This should be the user's choice, not us. Who are we to judge?. My general guideline is to write C for driving hardware and using Lua for business logic, but people should do what they believe is best for their application.

A bunch of those PRs look like they could have been implemented in pure Lua using existing gpio, i2c etc modules. For the ones that can't right now (eg lack of gpio.pulse in esp32?), should we try and address that instead? I think everyone would agree maintaining a Lua module is easier than a C one as there's a greater level of abstraction and the APIs are under our control (ie not subject to something changing in the espressif SDK for example).

Again, I have my guideline above to separate hardware drivers vs. Lua, but people should have the freedom of whether they want to go for a full C module or a Lua one.

If some of those stalled PR were written as Lua modules, would it be easier to accept them into the tree?

This is the same problem. We can't have an evergrowing repo of all the possible modules, even if Lua ones.

[...] If the ROM build process supported directly including a .lua file as if it were a C module (ie taking care of running luac.cross, embedding it directly in the main ROM ELF), would that reduce the temptation to implement hardware modules in C when they don't really need to be, and thereby reducing the number of C modules the NodeMCU maintainers have to maintain?

I think people would use lua more, I am sure. However, in my case I like to drive hardware directly in C. It is more efficient and allows me to access and reuse code from other non-Lua ESP8266/ESP32 communities (Arduino, ESP32 FreeRTOS, PlatformIO ...)

Out of the PRs:

#3090 and #3023 are mine. I will withdraw these and convert them to external modules hosted in my repo if this PR is merged so they can have their own space for documentation, PRs, and release schedule. About #3090 which uses rmt, I understand that a generic rmt API for Lua could be nice, but right now it does not exist and I like the module the way it is. In fact, I don't want to have to ask anyone to merge it or like it. I should be able to publish it without permission and let the community decide if it is useful or not. If my module sucks or is obsoleted by a rmt API + Lua code in the future, perfect. If not, we'll have my module. Without external modules we will never know what approach works better! (or maybe we'll discover that both are valid for different individuals and projects).

My module in #3090 for example, has a very clear advantage over the rmt API + Lua. It it is available now to use. It would be much easier to use it if it was an external module.

The other cited PRs: If these were not PRs and were external modules, they'd sit in their own repo and people could have at least had the chance to try them out. If useless or poorly written, people would have abandoned them. As of today, these haven't even had a chance. Also, it gives a poor image to have open PRs sitting in there for 2 years. This discourages future contributors. This PR would get rid of those PRs existing.

We're not to judge what is useful or not, we cannot know the exact reasons someone decided to do another i2c wrapper or why someone wrote rfswitch. People should have the freedom to create what they want, just don't pollute the NodeMCU repo with that code or PRs for obscure modules we don't know how many people actually use.

Also, this PR for external modules has a very strong use case for private modules. That is, modules for internal company projects or those that drive proprietary hardware.

If you got to this point, thanks for reading!. As you say, this is a very interesting discussion. I believe this approach helps decentralize creativity, reduces the PR review bottleneck and helps the repo and the maintainers to scale while allowing a permssionless ecosystem to emerge around NodeMCU. The stronger the community, the better the platform will work and the more resources we'll have.

[tomsci]

Thanks for taking the time to respond so thoroughly! I didn't intend to denigrate those specific PRs in any way, I was only wanting to take them as "case studies" to see what if anything could be changed to make coders' lives easier. I agree absolutely that it is the developer's choice how to implement a module, the chain of thought it triggered however is that perhaps the project steers people towards writing C modules a little too aggressively.

C is harder to write than Lua (especially with lots of Lua C API usage, getting things like stack slots right is tricky), so I'll always try to write the maximum amount in Lua and the minimum in C, if I possibly can. C modules are also harder to maintain and port, look at the huge differences in underlying APIs between esp8266 and esp32, for example. So I wonder if something we should also be doing is encouraging people to write more device support code in Lua rather than C, since it'll be easier to maintain for whoever the maintainer is, be it in-tree or out.

(As an aside the biggest problem I had with implementing low-level hardware supporting code in Lua is the EGC which absolutely cripples performance, and makes it appear like you have to write that stuff in C to get the performance. A lot of the time that just isn't the case and I've had zero problems so far with memory fragmentation problems, running with EGC switched off. YMMV of course, but it's an aggressive default that leads people towards writing C modules and the increased maintenance overhead that entails, rather than something people could switch on only if they had memory problems which are easier to spot than insidious performance issues).

allows you to specify either the code branch, the exact commit that works for you or a release version tag. Therefore, if you have chosen a specific NodeMCU version, you can link to a frozen release of the module that works for you.

Great, thanks for clarifying that.

it gives a poor image to have open PRs sitting in there for 2 years. This discourages future contributors. This PR would get rid of those PRs existing.

Agree 100%. A couple of things I've contributed and am considering contributing would definitely fit better as external modules, qrcodegen for example probably doesn't get much use in most NodeMCU projects (although it's not a device support module, and I admit I have no idea how many people are using it except me).

As I said, I'm in broadly in favour of this PR, so long as we don't go too slash and burn on existing modules :) And setting some sort of directory up is I think really important to ensure useful stuff can be found, even if it's just a wiki page somewhere. I should probably look at the actual code of the PR at some point! ;)

[jpeletier]

Thank you @tomsci

@marcelstoer @TerryE, @HHHartmann @jmattsson if you can, please weigh in to see if we can go ahead and merge this.

If this is merged, I'll follow up with this:

• Equivalent PR to support ESP8266 (I will put the common code that deals with module updating/downloading in a git submodule, as most of the scripts that make this work are compatible with both branches)
• Module registry. It would be kickstarted at least with two of my modules + the hello world example.
• Convenient tool to select external modules off the registry (to have a similar effect as make menuconfig)

[marcelstoer]

Thank you Javier for bring this up AND for providing an actual implementation demonstrating your approach!

I have been mulling this over a couple of times in the last few days. The issue this PR attempts to address is almost as old as this project itself. We (i.e. the maintainers) discussed this topic a couple of times in the past five years. However, we've never had an actual implementation proposal to reason about. So, thanks for going that extra mile.

It is very clear that our current approach does not scale well at all. The theoretical limit of the number of C modules we can host and maintain here is infinity but what's the realistic practical limit? 200, 100, or are we already beyond that limit?

If we take a few steps back and look at what this change/feature/opportunity (take your pick) would do to how this project works wrt to the interaction between the project and its users one thing becomes clear. It would mean a fundamental shift of responsibility away from the project towards the user. I don't judge that; it's just a fact.

With that responsibility shift the project also significantly lowers its promised offering. The central authority to verify that individual modules fit together is no longer the NodeMCU project but the user. So far, you could rely on the project to keep all modules in sync with the Espressif SDKs and the firmware core. I argue that this is a major contributor to the (relative) attractiveness of this project; the others being script vs. C-like compiling and low development round trip times.

Honestly, I am absolutely split over whether the obvious benefits of your new approach outweigh the downsides. I want this - or something similar - to work but I don't see yet how to address its shortcomings.

Before you read on please consider my role and my motivation in this and other projects: my primary concern is to make technology accessible to as many people as possible. All of those actively participating in discussions here are probably a small minority in the NodeMCU universe. Most of them have the expertise to find the way around our config files, source code and setup requirements. I am working for all those who don't.

This is probably very serious for the cloud builder.

The cloud builder can lock to specific vetted versions (via commit
hash and code hash) that they know are good.

Sorry, this is wishful thinking. We have to be very clear that moving to externally hosted modules will be the end of the cloud build as we know it. I will shut it down as I have neither the motivation nor the capacity to verify a certain version of a module will play nice with a certain version of the firmware.
The only modus operandi I can currently imagine is that users must provide their extmods.ini and if the build fails it's their problem. However, it's a challenge to build this in a secure manner limiting all possible attack vectors.
The only reason I managed to keep the keep service up for so long is because it is so stable and works in self-service mode. If I had to spend even 1h per week for "maintenance" that would be a no-go.

[This isn't just a theoretical attack, it happens in the python,
browser extension and node worlds -- typically a commonly used module
gets taken over by a bad guy and this causes havoc]

I know, it is very possible, but this does not stop the NodeJS and
Python communities from thriving and fixing these issues.

It makes sense to look to those communities for inspiration. I am somewhat familiar with both of them but my home turf is the Java ecosystem. What sets all three of them, and others, apart from our project is the level of complexity involved to set up a local development environment to build your application - or firmware in our case. Unless you go with Docker setting up a NodeMCU build environment is just a lot more involved that let's installing Node, Python or Java.

I am motivated by the fact that 2 of my PRs (#3090, #3023) and other
PRs(#2767, #2766, #2261, #2255, #2250, #2249 ...) are sitting in there
and I see it will be very difficult for them to make much progress.

#3023 has review feedback not yet addressed 😉 Many of the others have as well. It's exactly because we are hosting all the modules that "we" are sometimes a bit anal about contributions. This can be both good and bad. #2854 is a great example of a contributor (thanks @vsky279)
thoroughly interested in improving personal skills and and reviewers (thanks @TerryE & @nwf) motivated to provided detailed feedback and to teach.

Yes, this is at the expense of having the nodemcu maintainers drag
these modules across refactors with increasing effort as the legacy
increases, without visibility of which ones are important or not.

It is difficult to define "important" objectively. I sometimes take a look at what modules a large segment of our user base actually uses - through the stats of the cloud builder. Numbers don't lie but they don't represent the whole truth either, I'm aware of that.

3. Existing modules within the tree that drive very common external
   hardware such as an SD card or a typical i2c device, etc: move to
   EXTERNAL at some point, but within the nodemcu GitHub org.

In the context of your approach this makes a lot of sense, absolutely. However, a 2nd repository, let alone one for each such module, significantly increases the maintenance effort for the project team. We would have to look after multiple issue/PR lists, CI setups and general GitHub repository setups. The group of people willing to contribute in one or the other way and to stick around for more than just a couple of months has always been too small to image this would work well. I am
afraid this would break our neck - but I would like to be proven wrong 😜 .

A topic we haven't touched yet is documentation. I see value in being able to publish a consistent set of documents documenting our API. Again, it's about making technology accessible to people. With modules being external each module developer is free to choose form, style, structure and content of the module documentation. This again, might make our project less attractive for the majority of our users. We could probably put measures in place to mitigate this but it will likely be a tough nut to crack.

Sigh, it's difficult. I hope I managed to give you an idea where I stand on this topic.

[jpeletier]

Hello @marcelstoer and thank you for your thoughtful response.

After reading all the comments, I think that although this PR has triggered a broader discussion (and I also got carried away as well), it is true that this PR did not move modules or documentation away and did not break the Cloud Builder or otherwise changed any existing process. Therefore, I'd like to take a couple of steps back, and for the purposes of this PR, lets consider external modules an advanced optional feature, a way to hook into the build process to integrate external components specific to the user's application.

This said, in the future we can discuss the "old issue" at length, and this external modules feature may help address part of it or not, but that is certainly a bigger discussion I'd like to take out of this PR, otherwise we won't make progress as this PR does not and can't solve all the problems.

It would mean a fundamental shift of responsibility away from the project towards the user. I don't judge that; it's just a fact.

Using external modules is not compulsory. It is an option. People can still PR their changes in so their module becomes a core module, as always.

What it does, though, is enable faster and permissionless development lifecycles. People can create external modules and accept PRs to their repos, quickly developing a new experimental feature. Perhaps if the module becomes very popular, the author can then "promote" it to core module by PRing NodeMCU. We should enable this ecosystem as it can only make NodeMCU stronger.

[...] Sorry, this is wishful thinking. We have to be very clear that moving to externally hosted modules will be the end of the cloud build as we know it

You don't need to support external modules in the Cloud Builder. As you say, it targets users who just want to give NodeMCU a quick try, or have less experience. They won't need this advanced feature.

Unless you go with Docker setting up a NodeMCU build environment is just a lot more involved that let's installing Node, Python or Java.

The external modules feature targets people that can use at least Docker. It does not target people who just want to download a .bin file and get going.

#3023 has review feedback not yet addressed ;-)

😇, sorry I didn't respond yet. I greatly appreciate your review work, your testing and your newsletter post!. I am waiting to see what happens with this PR to see which path I go, but I will implement your suggestions in any case.

In the context of your approach this makes a lot of sense, absolutely. However, a 2nd repository, let alone one for each such module, significantly increases the maintenance effort for the project team.

This step is not included in this PR. The suggestion (part of a broader discussion) of moving some existing core modules out does not have to be accepted to accept this PR.

A topic we haven't touched yet is documentation. I see value in being able to publish a consistent set of documents documenting our API

I agree. Having a single place to go to see all docs is nice. This does not disappear with this PR. All core modules will stay the same.

Please note also that this PR covers the use case for private modules or application-specific modules. The former are closed source and the latter are only useful for the application writer and thus do not make sense to publish or PR the NodeMCU repo. Currently the only option for these users is to maintain an entire fork. This PR solves this problem.

[TerryE]

Javier, balancing the needs of the diaspora of NodeMCU IoT developers has been a source of constant debate amongst the core project members. Getting this right is hard because of the wide range of skills of this IoT developer community. I consider myself a power Linux developer with decades of experience and I have to constantly remind myself that I am very much not a typical NodeMCU user. The C notes and bolts guys tend to go down the Arduino route. This Lua environment with its local SPIFFS is just so much more novice friendly.

I also feel that a majority of IoT developers using NodeMCU still use the ESP8266 branches. This is partly because the features and libraries for the ESP32 are behind those for the ESP8266 modules, and also partly because the barriers to entry are far smaller in the ESP8266 world because you can get a firmware build from Cloudbuilder, build your LFS images through my services, and so all you need are to install on your PC is ESPlorer, PuTTy and Marcel flashing tool.

Yes, power users get into IDFs, Linux environments or subsystems such as docker, but these remain a minority.

Whilst @devsaurus, @chilipeppr ànd @pjsg have all make important incremental contributions to the ESP32 branch, this has been intermittent and mostly driven by their meeting their personal needs, and then sharing these features back to the community.

However the reality is that the ESP32 branch wouldn't exist without the work of @jmattsson and DiUS funding the vast build of this effort. Johny did the initial port and he also lead the project to move the new LFS functionality onto the branch. He continues to make significant burst contributions sometimes DiUS funded and sometimes for a hobby.

The overall consensus of the project members -- at least those who have the bandwidth to support the ESP32 branch is -- that it would really be better to align the two code bases so that as much of the code is truly shared between the two platforms rather than having to maintain an arbitrary fork.
@jmattsson and I have this as a medium term goal (that is before the end of 2020).

The trade-off the we need to make in any introducing new features to the ESP32 build architecture is that we need to balance the added utility to the ESP32 developer community as a whole against the potential risks and costs to the unification project.

I don't think that there is an easy answer to this one and I would welcome the views of:

• other ESP32 developers who would wont to use this feature, and
• the other team members that will be helping in this unification work.

[marcelstoer]

the barriers to entry are far smaller in the ESP8266 world because you can get a firmware build from Cloudbuilder,

The cloud build has been supporting ESP32 since early February 2020, hurray 😄

all you need are to install on your PC is ESPlorer, PuTTy and Marcel flashing tool.

We're even better(?) than that; PuTTy is optional.

[TerryE]

@marcelstoer thanks, I will take a look at your nodemcu-custom-build/ESP32 as this encapsulates some great know-how for me. I also see that idf.py include miniterm. Thanks. I need to get up to speed here with esp32 dev cycles and that's the next big job after I've sorted these outstanding ESP8266 "lua53" issues. I would really want @jmattsson to be engaged though, because without him I feel that we would struggle to resource what needs to be done here.

[marcelstoer]

@jpeletier many of my comments and concerns relate to what I got out of your statements in #3100 (comment).

If you are no longer suggesting we externalize existing modules, as seems to be the case as per your closing remark below, then I'm ok.

Please note also that this PR covers the use case for private modules or application-specific modules. The former are closed source and the latter are only useful for the application writer and thus do not make sense to publish or PR the NodeMCU repo.

Allowing for private or application-specific modules would certainly be a useful feature. Of course there's a slight risk that developers become less interested in sharing (in PR form) modules they developed. However, as long as we can add value* to their modules if hosted with us we should be fine.

* things like review, greater exposure (cloud builder & documentation), help with maintenance, (bulk-)migration to newer SDKs

[jmattsson]

I've only skimmed this thread as I'm currently quite snowed under due to the COVID-19 crisis. Good discussion points all around. In general my feelings are that optional pluggable module support are probably a net win. Good how-to docs critical. Hopefully the scheme will work across both branches and not cause more headaches when we try to merge them (which is very much on my todo list still, I know).

[jpeletier]

Any further comments? can we merge this?

Thanks,
Javier

[marcelstoer]

Can we do a quick poll among the maintainers here? Please edit this comment and replace the ? next to your handle with a 👍 or 👎.

👍 @marcelstoer given that: A) also ported to ESP8266 B) clearly intended for private or application-specific modules
? @TerryE
? @pjsg
👍@HHHartmann as long as we don't start moving our modules in another repo. Just as Marcel said
👍 @nwf
? @jmattsson

[nwf]

Forgive what might be a foolish question, but: rather than a .ini file, would it be possible to use Kconfig directly to specify the modules? I don't know if the Kconfig language supports lists in quite the way you'd want (and if it doesn't, the .ini isn't a terrible solution) but it'd be kind of nice to move towards keeping all the configuration state in one file.

That aside, I think this is a great idea.

[TerryE]

Overall I feel that the broad concept that @jpeletier is proposing is a good one. My concern here is that some aspects of the NodeMCU project suit bottom-up enhancement which is Javier is doing here.

My view is that we should really flesh out our approach to unifying the ESP8266 and ESP32 code bases first, and then developing an external module approach constrained by the non-functional requirements that unification imposes. In other words top down is better in this case.

This being said, we are going to have to re-engineer a lot of architectural aspects to unify these two platforms, and this is going to be just one more. If other ESP32 developers want to use this now and accept the fist of future breaking changing then I would not want to block this, but if in reality Javier is going to the only user in the short term, then perhaps we should get our unification approach sorted first.

[jpeletier]

Forgive what might be a foolish question, but: rather than a .ini file, would it be possible to use Kconfig directly to specify the modules?

Think of the .ini file as more analogous to sdkconfig than to Kconfig. This .ini file is tuned to your particular project, whereas a Kconfig file describes how you may use a component. In fact, an external module may contain a Kconfig describing how that submodule may be configured.

As for @TerryE's concerns, I would settle as follows: Mark this feature as "beta", so no guarantees given on future compatibility. I am pushing a commit to reflect this in the documentation.

I would take a step by step approach. After this PR is merged, my plan is:

1. Do the same in ESP8266
2. Establish a common pattern so the approach is valid for both ESP32 and ESP8266. At this point, I will submit a PR for you to review the progress.
3. Move external module management scripts (new tools/extmod/*) to a git submodule within the NodeMCU GitHub org, so these scripts are common code among the two branches (to make future unification straightforward). After this, there will be two PRs, one for each branch, that should be reviewed in parallel.

If this succeeds, then the feature will be in both branches with the common code driving it in a submodule.

Once both branches are unified, the scripts would move in to the unified repo rather than staying in a submodule.

[nwf]

Sorry, I think I was imprecise in my question. Rephrasing: "Could Kconfig files be created so that the things you are putting in extmods.ini instead end up in the .config file that the Kconfig system manages?"

[jpeletier]

@nwf No. Kconfig is not flexible enough to allow multiple entries. Also, its objective is to produce a key-value list that can be used to generate #defines, so it would not be suitable.

Don't worry too much about this file format, it does not have to be the final one. The idea is first to prove whether this is a useful feature.

[nwf]

I'm not worried about the format, per se, I'm more worried about a proliferation of configuration files; I rather liked the idea of git checkout ... && cp ... .config being all one needed to communicate the origin of a nodemcu build. But since Kconfig won't work, I think extmods.ini is perfectly reasonable.

We should probably write down a list of "these are the bits of the tree we expect you to modify and that you might want to rescue before removing your checkout" somewhere in the docs.