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

How best to generate configurations for the firmware build process? #1223

Closed
pjsg opened this issue Apr 3, 2016 · 20 comments
Closed

How best to generate configurations for the firmware build process? #1223

pjsg opened this issue Apr 3, 2016 · 20 comments

Comments

@pjsg
Copy link
Member

pjsg commented Apr 3, 2016

I have a couple of NodeMCU projects under way. These have specific configurations of modules and fonts that are needed (and this is more than the default).

The documented approach for this is to edit user_config,h and u8g_config.h. The problem with this approach is that when these files are changed in dev. problems often arise. Further, changes to these files can get accidentally committed -- note that the user_config.h needs to be modified to test / workon most of the modules.

Various people have their own solutions to this problem. I suggest that one of these solutions be made official and merged into codebase.

@TerryE
Copy link
Collaborator

TerryE commented Apr 4, 2016

Philip, see #905. I was wrong: it was a PR and not an issue, but I limped away licking my wounds. 😆 and thanks for posting this issue.

At twenty 20,000ft, I think that whatever the mechanism we have to accept that the code uses the user_*.h files and we don't want to change that.

Whatever the local configuration is it is by definition local so IMO we should have a local or site subfolder with a modules and an includes subdirectory which are ignored by git. The local/includes folder should be ahead of the app/includes so the user always has the option of having a personal version of these standard includes files and of course there is nothing to stop the user nesting includes in this local version.

I will stick to my generate script, but I would prefer to output to a local directory.

@pjsg
Copy link
Member Author

pjsg commented Apr 4, 2016

I'm actually testing a different approach which only involves a small change to the Dockerfile for the build image that just allows passing through some make parameters. I already have a script which wraps the docker run command, and I can pass in the extra stuff that I want there.

@TerryE
Copy link
Collaborator

TerryE commented Apr 5, 2016

Yup, but real men don't use Docker ;-? Seriously, I just keep my entire dev environment for nodemcu on its own VM; nice and clean. We can't have a solution that only works on Docker.

@pjsg
Copy link
Member Author

pjsg commented Apr 6, 2016

You should be able to go

make "INCLUDES=-include pwd/user_override.h"

and this will include user_override.h (from the root directory of the tree) into every file before compiling.

Real men may not use docker, but they do know how to write Fortran in any language!

@TerryE
Copy link
Collaborator

TerryE commented Apr 6, 2016

I wrote my first Fortan program 48 years ago. Frightening!!

My concern about the optional user_override method is that we would have to restructure our whole user_*.h approach. As you can see in my utility in gist, it's a mess. For example you include a module by defining a constant. A compile line forced include is inserted as a prologue, so if the default user_modules.h includes module X then you still need to edit the user_modules.h.

I feel that using the path list is the simplest approach: if you want to override on of the default config header files but don't want to change the standard directories, then you put your own custom copy in the local/includes directory, and there is no reason stopping you having #include "user_overrides.h" as the first line of this local copy.

@marcelstoer
Copy link
Member

I wrote my first Fortan program 48 years ago.

Woah, I wasn't even born then - did I miss something?

My opinion hasn't changed. I support Terry's idea of a simple key=value file format* from which the .h files are generated except that I'd like the generator script to be shell or Python rather than Lua.

*The sample Terry posted earlier elsewhere:

flash_size       = 512K
bit_rate         = 115200
client_ssl       = disable
gpio_interrupt   = enable
spiffs_cache     = enable
sha2             = disable
builtin = string,table,coroutine,math,debug_minimal
modules = node,file,gpio,wifi,net,i2c,tmr,uart,ow,bit

@pjsg
Copy link
Member Author

pjsg commented Apr 6, 2016

You got me beat -- my first fortran program was probably 43 years ago (on paper tape). Curiously enough there was an article about this on the BBC just the other day: http://www.bbc.com/news/education-35890450 I did my programming for my Computer Science A-Level. I also used a GEC-Elliott 903

I could certainly support the tag value file format above. Combined with a bit or recorganization in the user_modules.h (and maybe user_config.h), we could then use the include path trick to include the generated output from the file above.

@TerryE
Copy link
Collaborator

TerryE commented Apr 6, 2016

Woah, I wasn't even born then - did I miss something?

That's bloody obvious from the lack of wrinkles on your smiling visage -- but that's just jealousy talking!

I'd like the generator script to be shell or Python rather than Lua.

Pick. Bash would be slightly more challenging. Though, it's about 3 years since I wrote my last Python code, so I'll have to dust off the cobwebs and they collect really fast at my age!!

Philip, for some reason, I though that your were US based. A-levels and Eliot-903 is UK stuff.

As far as the tag file format and its extension, how about we code up a standard Python script to take its input from a command file arg line arg (and default local/user.cfg) and output two files local/include/user_config.h and local/include/user_modules.h with the include search list having this directory ahead of the app/include directory?

@pjsg
Copy link
Member Author

pjsg commented Apr 7, 2016

I'm English, born in London at the start of the space age, but left for the US in '91 -- been here ever since.

For the file, I'd also want to be able to specify the fonts added as well.

@marcelstoer
Copy link
Member

I'd also want to be able to specify the fonts added as well.

👍 u8g_config.h and ucg_config.h settings should be supported.

@TerryE
Copy link
Collaborator

TerryE commented Apr 7, 2016

OK

@pjsg
Copy link
Member Author

pjsg commented Apr 11, 2016

❤️ -- Just testing to see if we could use ❤️ as a marker for issues, and then sort by the number of hearts.

Ah -- it seems that you need to add the heart as a reaction using the little '+ smiley' symbol in the header of each comment.

Ah (again) -- you need to add the heart to the issue itself, rather than to any of the comments on it.

@jmattsson
Copy link
Member

So having been at loggerheads with Terry over this in the past, I think I have to admit that the time has probably come to do something like this. Constantly fighting against changing in-tree defaults is wearing me down to the point of acquiescing. I'm still of the view that the underlying mechanics will need to be driven by #defines though, so having a generator tool which composes the include files I can get behind.

While I personally really am not a fan of Python, I think it is probably the best choice for the job here considering we're already relying on esptool.py, and for the Windows crowd there's py2exe for ease of use.

As Marcel already mentioned, I think my preference too would be to make this new config file the One True Config where everything can be controlled from (save for doing things like enabling NODE_DBG etc, and for that I'd use make arguments). I don't think it would be necessary, but keep in mind that with gcc there is the #include_next directive which we could use to chain our user_config.h file. Only do that as a last resort though.

I'm guessing we'd need or more template files for the config - one (everything-plus-dog.cfg) which documents every module and option available (which means we need good comment support in the config file format), and one (blank.cfg) which is an empty/minimal config that can be used as a starting point.

Marcel, do you have any input on what would be good for the cloud builder here? I'm thinking that if we do this well enough, the cloud builder might be able to automatically generate its modulesoption page dynamically directly from the everything-plus-dog.cfg file thus having new modules and options become usable immediately without intervention.

@marcelstoer
Copy link
Member

Marcel, do you have any input on what would be good for the cloud builder here?

Thanks for asking but no, I don't have specific requirements.

@stale
Copy link

stale bot commented Jun 7, 2019

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 stale bot added the stale label Jun 7, 2019
@TerryE
Copy link
Collaborator

TerryE commented Jun 8, 2019

I separately proposed a Python script that I used to do these changes for a time, but I've since abandoned -- too much hassle having to update it every time the format of the base file changed. One option here for the user_modules.h file would be to have a footer in the include:

#ifdef USER_MODULE_OVERRIDES_H
#include USER_MODULE_OVERRIDES_H
#endif

And now you can use this non-git file to set and unset module defines. The make could do an existence check on the file to set the -D option.

@stale stale bot removed the stale label Jun 8, 2019
@marcelstoer
Copy link
Member

@stale
Copy link

stale bot commented Jun 2, 2020

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 stale bot added the stale label Jun 2, 2020
@TerryE
Copy link
Collaborator

TerryE commented Jun 2, 2020

Is #3138 replacing this? If so then we should close this.

@stale stale bot removed the stale label Jun 2, 2020
@marcelstoer
Copy link
Member

Oh right, but it's actually #3130 (the issue) that replaces this.

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

No branches or pull requests

4 participants