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

Config feature cleanup #1917

Merged
merged 6 commits into from
Dec 30, 2014
Merged

Conversation

OlegHahm
Copy link
Member

Fixing some side effects of #1846 and #1900.

@@ -190,7 +190,7 @@ void auto_init_net_if(void)

void auto_init(void)
{
#ifdef MODULE_CONFIG
#if defined MODULE_CONFIG && defined FEATURE_CONFIG
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

why are both necessary?

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The first one says that this module should be used, the latter one that the feature is available on the platform. See #1917 (comment).

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

So, the module works if the feature isn't available? That seems wrong ;)

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

If it's not available it won't be able to link and must therefore be disabled.

@OlegHahm
Copy link
Member Author

@Kijewski, I tried to make use of FEATURES_PROVIDED for filtering out the module config from USEMODULES, instead of these ugly ifdefs, but failed.

I tried with something like

USEMODULE = $(filter-out $(filter-out $(FEATURES_PROVIDED), $(FEATURES_OPTIONAL)), $(sort $(USEMODULE)))

but Make didn't like the recursion here. Do you have an idea?

@OlegHahm OlegHahm added Area: build system Area: Build system Type: cleanup The issue proposes a clean-up / The PR cleans-up parts of the codebase / documentation labels Oct 31, 2014
@OlegHahm OlegHahm force-pushed the config_feature_cleanup branch from daa8acc to 4659333 Compare December 2, 2014 22:54
@OlegHahm
Copy link
Member Author

OlegHahm commented Dec 2, 2014

With USEMODULE := it is so much simpler...

@OlegHahm OlegHahm added this to the Release NEXT MAJOR milestone Dec 2, 2014
@OlegHahm OlegHahm force-pushed the config_feature_cleanup branch from 4659333 to 18cc8af Compare December 3, 2014 19:46
@OlegHahm
Copy link
Member Author

OlegHahm commented Dec 9, 2014

@Kijewski, @LudwigOrtmann, you like?

@Kijewski
Copy link
Contributor

Kijewski commented Dec 9, 2014

Actually, no, sorry. Will elaborate later.

@OlegHahm
Copy link
Member Author

How much later? ;-)

@OlegHahm OlegHahm force-pushed the config_feature_cleanup branch from 18cc8af to 03cca0a Compare December 22, 2014 19:35
@OlegHahm
Copy link
Member Author

Rebased. @Kijewski, please state what's wrong with this now (or forever hold your peace).

@Kijewski
Copy link
Contributor

What does this line do?

USEMODULE := $(filter-out $(filter-out $(FEATURES_PROVIDED), $(FEATURES_OPTIONAL)), $(sort $(USEMODULE)))

@OlegHahm
Copy link
Member Author

$(filter-out $(FEATURES_PROVIDED), $(FEATURES_OPTIONAL)) returns all optional features that are not provided. The outer filter-out rule then removes all optional, but not available modules from the USEMODULE list.

@Kijewski
Copy link
Contributor

OK, ACK. Seems legitimate for this usecase.

@OlegHahm
Copy link
Member Author

Apparently something is wrong with DEFAULT_MODULES. I'm investigating.

@OlegHahm
Copy link
Member Author

Apparently the DEFAULT_MODULES should be added to the USEMODULE after they are collected also from boards and cpu.

@OlegHahm
Copy link
Member Author

@Kijewski, any better idea or does your ACK hold?

@Kijewski
Copy link
Contributor

My ACK holds

OlegHahm added a commit that referenced this pull request Dec 30, 2014
@OlegHahm OlegHahm merged commit e60d25d into RIOT-OS:master Dec 30, 2014
@OlegHahm OlegHahm deleted the config_feature_cleanup branch December 30, 2014 18:15
@OlegHahm OlegHahm modified the milestone: Release 2015.12 Dec 18, 2015
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 Type: cleanup The issue proposes a clean-up / The PR cleans-up parts of the codebase / documentation
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants