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

make: add snippets support #4598

Closed
wants to merge 4 commits into from

Conversation

kaspar030
Copy link
Contributor

First try at #4290.

Whenever a module dir contains "NNname.c.snip" (with NN a decimal), the makefile concatenates that file with all other "NNname.c.snip" of other modules into $(BINDIR)/snippets/name.c and compiles all combined files into a "snippets" module.

For all but the application module, the snippets get enclosed by #ifdef MODULE_X ... #endif

That way, static array definitions, functions, ... can be spread over multiple files in all module directories.

Imagine shell commands, auto_init, saul parameters, device driver parameters, coap endpoints, .. can be defined by just adding a snippet where it belongs.

You've hooked up another radio (same as onboard) to your board? Just drop a file "01at86rf2xx_params.c.snip" into your application folder, containing only the array entry for the added device.

Your module auto_inits? Just drop a 01auto_init.c.snip into it's folder, no more rebase conflicts in sys/auto_init/auto_init.c.

What do you guys think?

@kaspar030 kaspar030 added Type: enhancement The issue suggests enhanceable parts / The PR enhances parts of the codebase / documentation State: WIP State: The PR is still work-in-progress and its code is not in its final presentable form yet Area: build system Area: Build system Discussion: RFC The issue/PR is used as a discussion starting point about the item of the issue/PR labels Jan 6, 2016
@kaspar030
Copy link
Contributor Author

I've changed xtimer's auto init to use snippets in order to show how this is supposed to be used.

@jnohlgard
Copy link
Member

Couldn't you just use #include "01snip.c"... and so on?

@kaspar030
Copy link
Contributor Author

@gebart Yes, much simpler.

@DipSwitch
Copy link
Member

Nice one! :)

@kaspar030 kaspar030 added the Impact: major The PR changes a significant part of the code base. It should be reviewed carefully label Jan 8, 2016
@kaspar030
Copy link
Contributor Author

Fun thing to try:

# cd examples/hello-world
# echo 'puts("wooohooo!!");' > 10auto_init.c.snip
# make clean all term

@Kijewski
Copy link
Contributor

Kijewski commented Jan 8, 2016

I like the idea. 👍 Maybe add an underscore after the index for better readability?

@jnohlgard
Copy link
Member

I have some thoughts regarding file names:

.c.snip will confuse many IDEs which set syntax highlighting based on *.c, is it possible to fix without getting them sucked into the normal build rules?

Since the name part of NNname.c.snip is what groups them together, would it make sense to change the pattern to nameNN.c.snip to also get the files grouped together when listing files alphabetically, in case there are multiple snips in the same directory?

@kaspar030
Copy link
Contributor Author

.c.snip will confuse many IDEs which set syntax highlighting based on *.c, is it possible to fix without getting them sucked into the normal build rules?

I thought about that, it might be possible to use make filters filters if they're called something like *.snip.c. Then we'd have to create filter rules for all filetypes, currently this would probably also work with headers.

I'll look into it...

@kaspar030
Copy link
Contributor Author

Since the name part of NNname.c.snip is what groups them together, would it make sense to change the pattern to nameNN.c.snip to also get the files grouped together when listing files alphabetically, in case there are multiple snips in the same directory?

good idea!

@kaspar030
Copy link
Contributor Author

Ok, the snippets are now called <filename>.snipNN.ext.

@Kijewski I tried with a '_' between snip and number, but somehow I liked it better like this. What do you think?

@kaspar030
Copy link
Contributor Author

I'll close this in favor of a linker-script based solution.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Area: build system Area: Build system Discussion: RFC The issue/PR is used as a discussion starting point about the item of the issue/PR Impact: major The PR changes a significant part of the code base. It should be reviewed carefully State: WIP State: The PR is still work-in-progress and its code is not in its final presentable form yet Type: enhancement The issue suggests enhanceable parts / The PR enhances parts of the codebase / documentation
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants