Skip to content
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

Utility to generate user config and modules file #905

Closed
wants to merge 1 commit into from

Conversation

TerryE
Copy link
Collaborator

@TerryE TerryE commented Jan 5, 2016

I use this Lua script a lot in testing typically wrapped around my own nodemcumake shell script. This takes an optional parameter XXX and if I am building three module list variants XXX, YYY, ZZZ in
testing then I just do anodemcumake XXX which generates the corresponding user_config.h and user_modules.h even if I've swapped git branches, because these files haven't been added to git.

Likewise when I do a commit I can ignore the modified versions so that the commit has the correct (stupid) defaults.

A very useful and entirely optional nice to have, and IMO a must-have for contributors.

I use this Lua script a lot in testing typically wrapped around
my own nodemcumake shell script.  This takes an optional parameter XXX
and if I am building three module list variants XXX, YYY, ZZZ in
testing then I just do and "nodemcumake XXX" which generates the
corresponding user_config.h and user_modules.h even if I've swapped
git branches, because these files haven't been added to git.

Likewise when I do a commit I can ignore the modified versions so
that the commit has the correct (stupid) defaults.

A very useful and entirely optional nice to have.
@jmattsson
Copy link
Member

Useful perhaps, but that's now one additional place to update whenever an option gets added to user_config.h. I'm not in favour of that. Not that I'm necessarily in favour of user_config.h itself, especially in conjunction with the cloud builder.

If we had an easily parseable format for config options (e.g. standardised macro names like CFGOPT_xyz_ENABLE, CFGOPT_xyz_VALUE 123), or an external config format, so the tool could rip through the existing config and "simply" override specified settings but leave unknown ones alone, I'd be more in favour. Of course, a macro approach would probably come at a cost of reduced flexibility to #ifdef stuff, but that might not necessarily be a bad thing. Looking at user_config.h I see a bunch of stuff which doesn't really belong there imo.

Considering the popularity and usefulness of the cloud builder, what I think I'd like to see is an approach which plays into the cloud builder where it's easy to configure both options (booleans, integers) and included modules. I guess that points in the direction of a cfg text file. Then a small CLI tool to hook into the Makefile to generate the necessary defines. Ideally with a way to stash multiple configs and be able to say make CONF=standard or make CONF="standard_nossl", similar to the FreeBSD kernel approach. The config files would be both easily human-editable as well as cloud-builder constructable. There'd probably need to be a master file listing all the available options and modules somewhere.

@TerryE
Copy link
Collaborator Author

TerryE commented Jan 6, 2016

@jmattsson Johny, I did this for my own benefit really, but I once I had it is just so much easier to configure the make, especially if you are a firmware developer, because this gets around the fundamental flaw in the current approach that the localisation of the configuration, something that in conventional open source packages is left to a configure script, is in git.

It also underlies that the approach to configuration is extremely inconsistent across the source base. I had to have four different rules for processing options.

AllI I suggest is try using this and you won't go back.

Your point about denormalising configuration is true, but we do this in spades with the cloud builder menu. Having a standard cfg format that you could upload would really simplify this.

@jmattsson
Copy link
Member

I haven't had a chance to sit down and take this for a good spin yet, and tomorrow it's back to work, so... May main point is that having another (partial) copy of the user_config.h that may need to be kept in sync whenever an option is added to user_config.h is adding pain. If someone else reckons the overall is a net positive even after that, I'm not opposed to merging this, especially if the longer view is to overhaul user_config.h (and probably user_modules.h) as well.

@TerryE
Copy link
Collaborator Author

TerryE commented Jan 10, 2016

The real task is to completely overhaul the naming and value conventions for user_config.h -- except that the concept of this file is also embedded in the SDK and if therefore only partially in our control.

This is just a pragmatic and entirely optional step in the right direction. Developers don't need to know about it or use it, but if you are regularly swapping git branches and / or module configurations, then it's really useful.

I don't mind if its not committed; I'll still use my copy. It's just that I thought other developers might also find it useful.

@marcelstoer
Copy link
Member

How about putting this into a GitHub gist or NodeMCU wiki and link to it from CONTRIBUTING.md?

@TerryE
Copy link
Collaborator Author

TerryE commented Jan 12, 2016

@marcelstoer, I am happy with either of these alternatives. I have no strong view about insisting its part of the distro. It's just a really useful utility for developers who often pull / fetch / push / make using the git library or a fork.

@TerryE
Copy link
Collaborator Author

TerryE commented Jan 19, 2016

What I suggest that we do is to leave this utility as a gist and include a link to it somewhere in the documentation. I'll then close this PR.

@marcelstoer
Copy link
Member

+1

How about https://github.com/nodemcu/nodemcu-firmware/blob/dev/docs/en/build.md as a home for that link?

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants