Contributing to NodeMCU

[TerryE]

This is a general discussion about the current CONTRIBUTING.md with the aim of exploring the contributor consensus before suggesting any amendments. I feel that this needs some further enhancements, as there are a number of classes of contribution, and rules for contribution vary for these classes:

1. Bugfixes and corrections to the Documentation. Trivial ones can be made with a simple PR as per the process described, but the comment should clearly explain the reason for the change, preferably with an example of the failure mode.

Non-trivial bugfixes should first be raised as an issue, together with an explanation of the bug and at least one reproducible test case which demonstrates the bug. If the originator is proposing to implement bugfix, then this should be made clear in the post, and possibly including a quick sketch of the proposed solution. This will allow more experienced contributors to comment before the poster carries out potentially wasted effort.
2. New library modules to support extra hardware, etc. We need a new construction guideline covering these (which I discuss below), but in general any developed library which conforms to this guideline will be accepted by the project, so long as:

• The module achieves a materiality test: that is the module meets a need that many ESP8266 Lua developers will find useful. It is recommend that the originator raise an issue describing the reasoning behind the module and implementation approach.
• The originator has provided evidence that the module has been adequately tested as a category (4) person module.
• At least one other committer or regular contributor has put the module to use (e.g. with the target hardware). My reasoning for this test is that this gives us some independent verification of the resilience of the module, and if no one volunteers to do this then it really fails the materiality test.

3. Modifications to the core firmware. This includes the Lua system, it's platform abstraction and any core modules that are commonly used in builds, including string, table, coroutine, math, debug, node, file, tmr, wifi, net, gpio, uart, pmw, ow, i2c, spi. Other than specifically scoped bufixes, this is not something that we should consider lightly and therefore unless there is a demonstrated benefit. An issue must first be raised to discuss the potential advantages; describe the functional change; provide the scope of the modification; define the test strategy; and determine any potential backwards incompatibility. If the propose is not from a committer or establish contributor, then one of these must volunteer to work with the originator during the development process.
4. Local modules and customisation. As this is an Open source project, any user is free to add modules and modifications to meet their own needs. But such modules are not the business of this project, and any construction guidelines or materiality tests are only informative for the developer.

I would like to explore some of the issues relating to specific categories in later comments, but I just want to test the consensus to this analysis so far, first.

[ABonner]

I've seen other projects put modules which are not widely used yet into a contrib/ or community/ folder so that it is clear they are not maintained by the core team and won't be regression tested like core modules. That can help with the chicken-and-egg problem of not having other coders with the hardware because the hardware has zero support for it.

[pjsg]

Having different categories of modules is a good idea. I don't know how many modules could find multiple people to test them while waiting to be merged -- especially those that require specific hardware.

Having some tooling around knowing the build configuration from a built image would be useful. This could be as simple as have node.buildconfig evaluate to a json string containing enough information to repeat the build -- e.g. the commit hash and any overrides in terms of modules etc.

[TerryE]

Having different categories of modules is a good idea. I don't know how many modules could find multiple people to test them while waiting to be merged -- especially those that require specific hardware.

As @ABonner Adam suggests if we had a community (and perhaps local) folder separate modules then fair enough, but should we really be including a module in our distribution that has only been used by one developer and we can't find a second willing to test it?

I've already got that build script -- OK, it's in Lua -- and I have already suggested we should include something like it in the distribution, but there was no consensus support for the idea, so it's only something I use locally.

[ABonner]

@TerryE In other projects, I've seen the community section working as marketing and SEO - it centralizes the community, and other people who are looking for a "How do I use hardware X with ESP8266" will find this project instead of another option. In that model, modules could get "promoted" to core status once they have multiple people using them.

[hvegh]

There is a problem with categorizing modules on the basis of number of people using them:
It will shift the focus away from the discussion: is the quality good and is there a beneficial use case.

I'd rather see a well structured high quality module with only one user. Than bad quality code run by lots of users. Guess witch code will require the most support.... ;)

But maybe the core of the problem is: Yes we want to include code, but as a project we don't want to take ownership or any of the responsibility that comes with it.

In that case the worst you can do is opt for a "community" branch. Because if you load all the "stuff" in there with some "relaxed quality" checking. You might be forced to support code anyway once it will get more widespread usage....

Be careful what you wish for :)

[TerryE]

@hvegh

I'd rather see a well structured high quality module

But this is a quality / value judgement which can't validly be made by the OP unless that OP has a good level of karma on this repository. If not, then such contributions have to be moderated by one or more people with with karma. The committers trust each other and there are maybe half a dozen other regular contributors that we also respect and trust. We are trying to build confidence in the community in our firmware. I just think that "stack it high and sell it cheap" isn't the way to do this.

[hvegh]

I am not a native British speaker, had to look 'stack it high sell cheap' up in the dictionary. But even then I have trouble figuring out what you mean.

Does it translate to: don't include "cheap" code in order to make everyone happy?

You make it sound like we have opposing views but I think on this point we mean exactly the same :)

[hvegh]

BTW I like the device, and I have big admiration for the nodemcu firmware, and the amount of effort that you guys put into this!

[ABonner]

+1 from me for all the hard work everyone is putting in. Great firmware and has made working with the ESP8266 a real joy. I'll support whatever approach you think has the best benefits to the project.

[TerryE]

I am not a native British speaker, had to look 'stack it high sell cheap' up in the dictionary. But even then I have trouble figuring out what you mean.

@hvegh, Henk forgive the English idiom -- and my lack of courtesy to non-native English speakers. This is a form of selling where the target is high-volume, but low-margin (and low-quality) sales as opposed to higher quality, higher margin but therefore lower volume sales.

I will let other contributors and committers have their say. 😄

[hvegh]

Thanks for the clarification...

The only point I was trying to make:
There is a problem with categorizing modules on the basis of number of people using them:
It will shift the focus away from the discussion: is the quality good and is there a beneficial use case.

[pjsg]

If the nodemcu firmware builder automatically offered all these modules, then it would become easy to see when something gets significant usage.

I'd be happy with two levels: "core" and "community" (though it might be better to find two words that didn't start with "co").

[devyte]

How about three levels:
"core": modules like file, wifi, uart, etc.
"utils": modules like crypto, http, ringbuffer, etc.
"drivers": modules for specific peripherals, like LCDs and sensors (many users don't need these)

Maybe one last one:
"universe" or "user" or "misc": modules which have a very small user base, but have been accepted because they're good or useful. This last means, of course, that there is agreement that the module is not of the "stack it high sell it cheap" variety :)

Anyways, just an idea.

[TerryE]

How about three levels

My vote would be to keep the core modules in app/lua and app/modules and have three other app subfolders. (Note that drivers has a specific platform related connotation and its file are Lua agnostic.)

• modules would continue to contain all core modules supported by the core team of committers and contributors
• utilis would contain extra modules that are capable of being executed on standard ESP H/W (e.g. crypto etc.
• hardware would contain community contributed hardware-related modules
• local an emply folder but can be populated with custom application / site modules

[karrots]

This new Github feature may be of interest to this discussion.

https://github.com/blog/2111-issue-and-pull-request-templates

[marcelstoer]

@karrots Thanks. To me that's unrelated or only very loosely related to this discussion. Hence, I created a separate issue, see #1077.

[devsaurus]

Terry, your separation into these 4 categories adds an interesting dimension with respect to quality:

• As far as I understand, code in modules and utils provides self-contained functionality, "executable on standard ESP H/W". If we ever install something like (semi-)automated regression testing, then these would be the candidates. Whichever way, automated or manual, the target is: ensure their functional quality.
• Modules in hardware are most likely too specific for automation and would only see the build regression as we have it already with Travis. Target: ensure build/code quality.

This relates to my thoughts on your initial post.

1. New library modules
   At least one other committer or regular contributor has put the module to use

Maintaining the functional quality of hardware modules will sometimes fully rely on the support of the originator. I doubt that the current contributors & collaborators are able to cover the plethora of potential hardware extensions. There is an inherent limit for what the few of us can support in the light of such a strict rule - and I think we've already crossed that border.

[pjsg]

There isn't much that does useful stuff on a naked board. For example, i2c, gpio, pwm, adc, etc. These can all be made useful with minimal hardware, whereas testing the control of a long string of ws2812 leds is much more tricky.

To me, the important thing is the level of support. I expect that there is a clear set of modules which are actively supported. This means that if I find a bug, it is likely that someone will take an interest and it will get fixed. The other modules are of the community maintained variety -- if I find a bug, I had better be prepared to try and fix it myself.

I'm also wondering if the local/community directory should be multiple level -- so that I can clone one or more other repos into it. This would provide a nice way to support publishing modules -- you just put them in a repo at the top level. Then a user just does a git submodule add to include them in their repo.

app/local/pjsg/rotary-switch.c
app/local/pjsg/whiz-bang.c
app/local/terrye/pulse-sensor.c

etc

[TerryE]

@pjsg for your idea of using submodules to work the local/FRED would need to be complete, which means that we would need a config tool which the user would then run to integrate the local config elements into the master set in app/include.