Makefile.dep: auto_init_% as DEFAULT_MODULES by fjmolinas · Pull Request #13089 · RIOT-OS/RIOT

fjmolinas · 2020-01-11T09:14:31Z

Contribution description

This PR looks to extend the granularity of auto_init MODULES. Currently only auto_init is a DEFAULT_MODULE which means its the only one that can be disabled, which translates into, get everything or nothing. As a user I could want to only deactivate one of this auto_init modules and keep the rest (for examples in our tests).

This PR wan't to achieve this by declaring all curent cases of USEMODULE += auto_init_% as DEFAULT_MODULE += auto_init_%, it also by default declares all auto_init_% as pseudomodules (except for those that are not).

As an example I add an auto_init_xtimer module and changes the gauards in auto_init.c to reflect this. IMO this could be done for all modules in auto_init, but that would mean a lot of changes, so maybe better to introduce when needed?

Testing procedure

green murdock
try disabling a module, I added a #error in:

#ifdef MODULE_AUTO_INIT_XTIMER
    #error
    DEBUG("Auto init xtimer module.\n");
    xtimer_init();
#endif

and compiled with and without disabling DISABLE_MODULE+=auto_init_xtimer make -C tests/xtimer_usleep

Issues/PRs references

Though of this because of #13052

fjmolinas · 2020-01-16T10:16:08Z

I removed now all DISABLE_AUTO_INIT I checked them all and replaced the ones that where needed by specific ones. I only left minimal and unittests. Unitests could probably also be removed by adding disable to every sub-test.

fjmolinas · 2020-01-16T10:19:42Z

Ups I had NO_PSEUDOMODULES += auto_init_usb instead of NO_PSEUDOMODULES += auto_init_usbusm i'll push the fix once the build ends.

fjmolinas · 2020-01-16T10:43:33Z

BTW this should wait for the release to be over.

fjmolinas · 2020-01-16T12:39:04Z

Rebased.

Makefile.dep

fjmolinas · 2020-01-27T11:45:17Z

I'll also add drivers/periph_common/init.c since #11026 mentioned it being a nuisance for modular design, that way it can be disabled.

fjmolinas · 2020-02-11T10:18:56Z

I'll also add drivers/periph_common/init.c since #11026 mentioned it being a nuisance for modular design, that way it can be disabled.

I'll do a separate PR for this, murdock already on this one so it should be ok as is,

fjmolinas · 2020-02-11T10:25:36Z

Makefile.dep

  USEMODULE += random
 endif

+ifneq (,$(filter auto_init,$(USEMODULE)))


Would it make sense to move this to a separate file?

kaspar030 · 2020-02-11T10:27:35Z

Makefile.dep

  USEMODULE += random
 endif

+ifneq (,$(filter auto_init,$(USEMODULE)))


I think it would make sense to turn this into a loop.
Something like

HAS_AUTO_INIT += foo \ bar \ # ifneq (,$(filter auto_init,$(USEMODULE))) DEFAULT_MODULE += $(patsubst %,auto_init_%,$(filter $(HAS_AUTO_INIT),$(USEMODULE)) endif

maybe additionally filter on DISABLED_MODULES

HAS_AUTO_INIT += foo \ bar \

where would this be?

I think in Makefile.dep for now is fine, no?

The alternative would be to have, for every module that uses auto_init, a dependency block:

ifneq (,$(filter foo,$(USEMODULE))) ifneq (,$(filter auto_init,$(USEMODULE))) DEFAULT_MODULE += auto_init_foo endif endif

This way, the information is "closer" to the module's dependency info.

I don't like the HAS_AUTO_INIT because these function do not allways match a module name which makes it kind of weird. These HAS_AUTO_INIT would IMO have to be in the modules MAKEFILE, but some of these MODULES are not pseudomodules, so it isn't a HAS_AUTO_INIT but use this MODULEs init function in a way, and there isn't a MODULE<->auto_init naming match in all cases.

This is what I had initially and changed to a suggestion from @aabadie

hmok, @aabadie, why "later in the file"?

why "later in the file"?

in order to know all modules with potential auto_init support. but since this file is included transitively, this doesn't make much sense.

So which style is prefered @aabadie @kaspar030?

For me,

ifneq (,$(filter foo,$(USEMODULE))) DEFAULT_MODULES += auto_init_foo ... endif

... together with some logic so that if auto_init is disabled, all auto_init_% are also disabled.

fjmolinas · 2020-02-11T10:29:08Z

tests/periph_rtt/Makefile


 FEATURES_REQUIRED = periph_rtt

-DISABLE_MODULE += auto_init


Should be removed this is still called, since init_rtt is not in auto_init

kaspar030 · 2020-02-11T12:29:45Z

I've removed the discussion label, I think there's consensus this is a nice idea.

fjmolinas · 2020-02-12T15:10:38Z

Rebased to fix conflict.

Declaring all auto_init_% modules as pseudomodules will allow using auto_init_% modules as modules that can be disabled. This will give a higher lever of granularity allowing users to not disable the complete auto_init module but only some of them.

Currently default modules resolution is only performed in Makefile.include. This avoids DEFAULT_MODULES being declared in Makefile.dep since they never become USEMODULE. Duplicate at the end of the dependency resolutiion after recursive cach of transitive depdencies since at this stage DEFAULT_MODULES can't and SHOULD NOT trigger depedency resolutions.

Having the modules as DEFAULT_MODULES allows disabling them.

fjmolinas · 2020-02-12T16:19:54Z

@kaspar030 @aabadie can you check if all is OK for you, this one diverges quickly.

kaspar030

ACK. Nice change!

benpicco · 2020-02-13T10:45:10Z

Makefile.dep

+  DEFAULT_MODULE += auto_init_gnrc_sixlowpan
  USEMODULE += sixlowpan
+  DEFAULT_MODULE += auto_init_gnrc_sixlowpan


Is there a reason for adding auto_init_gnrc_sixlowpan twice?

I think those were added twice by accident. This cleans that up. introduced by RIOT-OS#13089

fjmolinas added Type: enhancement The issue suggests enhanceable parts / The PR enhances parts of the codebase / documentation Discussion: RFC The issue/PR is used as a discussion starting point about the item of the issue/PR labels Jan 11, 2020

fjmolinas requested review from aabadie and kaspar030 January 11, 2020 09:14

fjmolinas mentioned this pull request Jan 14, 2020

boards/arduino-mkr: feather-m0: sodaq-*: provide stdio over USB and setup automatic flash with bossa #12304

Merged

kaspar030 mentioned this pull request Jan 14, 2020

tests: broken with stdio_rtt if auto_init is disabled #13120

Open

fjmolinas force-pushed the pr_auto_init_default_modules branch 2 times, most recently from 6a4a3e8 to 4d5ddfb Compare January 16, 2020 12:38

aabadie reviewed Jan 16, 2020

View reviewed changes

Makefile.dep Show resolved Hide resolved

fjmolinas added CI: ready for build If set, CI server will compile all applications for all available boards for the labeled PR and removed CI: ready for build If set, CI server will compile all applications for all available boards for the labeled PR labels Jan 16, 2020

fjmolinas mentioned this pull request Jan 22, 2020

sys/auto_init: Allow to auto-initalize sensors and actuators independ of SAUL #11871

Closed

aabadie mentioned this pull request Jan 24, 2020

board/arduino-nano-33-ble: add initial support #13194

Merged

aabadie mentioned this pull request Feb 4, 2020

boards/adafruit-clue: add initial support #13276

Merged

fjmolinas commented Feb 11, 2020

View reviewed changes

kaspar030 reviewed Feb 11, 2020

View reviewed changes

fjmolinas commented Feb 11, 2020

View reviewed changes

kaspar030 removed the Discussion: RFC The issue/PR is used as a discussion starting point about the item of the issue/PR label Feb 11, 2020

fjmolinas added the CI: ready for build If set, CI server will compile all applications for all available boards for the labeled PR label Feb 12, 2020

fjmolinas force-pushed the pr_auto_init_default_modules branch from aefb068 to ef76fe4 Compare February 12, 2020 15:10

fjmolinas removed the CI: ready for build If set, CI server will compile all applications for all available boards for the labeled PR label Feb 12, 2020

fjmolinas force-pushed the pr_auto_init_default_modules branch from ef76fe4 to c830260 Compare February 12, 2020 15:50

fjmolinas added 9 commits February 12, 2020 16:51

Makefile.dep: add auto_init_xtimer pseudomodule

a5a036b

Makefile.dep: add auto_init_random MODULE

73bf229

Makefile.dep: add auto_init_at86rf2xx MODULE

802216c

Makefile.dep: add auto_init_lwip MODULE

afc43bf

Makefile.dep: add auto_init_gnrc_% MODULEs

e446920

Makefile.dep: auto_init_% as DEFAULT_MODULES

7bfb855

Having the modules as DEFAULT_MODULES allows disabling them.

tests: remove uneeded DISABLE_MODULE+=auto_init

e0855de

fjmolinas force-pushed the pr_auto_init_default_modules branch from c830260 to e0855de Compare February 12, 2020 15:51

fjmolinas added the CI: ready for build If set, CI server will compile all applications for all available boards for the labeled PR label Feb 12, 2020

kaspar030 approved these changes Feb 12, 2020

View reviewed changes

kaspar030 merged commit aa9388b into RIOT-OS:master Feb 12, 2020

fjmolinas deleted the pr_auto_init_default_modules branch February 12, 2020 16:23

fjmolinas mentioned this pull request Feb 12, 2020

boards/mulle: move nvram init into auto_init #13052

Closed

benpicco reviewed Feb 13, 2020

View reviewed changes

benpicco added a commit to benpicco/RIOT that referenced this pull request Feb 13, 2020

Makefile.dep: deduplicate DEFAULT_MODULE

4f8224b

I think those were added twice by accident. This cleans that up. introduced by RIOT-OS#13089

benpicco mentioned this pull request Feb 13, 2020

Makefile.dep: deduplicate DEFAULT_MODULE #13363

Merged

leandrolanzieri mentioned this pull request Feb 18, 2020

Makefile.dep: Fix duplicate modules from USEMODULE #13401

Merged

leandrolanzieri added this to the Release 2020.04 milestone Feb 20, 2020

wosym mentioned this pull request Feb 24, 2020

Tests/candev: Initial version with native support #13360

Merged

fjmolinas mentioned this pull request Feb 28, 2020

drivers/periph_common: add periph_init% modules #13511

Merged

fjmolinas mentioned this pull request Mar 18, 2020

Documentating usage and correct usage of DEFAULT_MODULE #13651

Closed

fjmolinas mentioned this pull request Apr 2, 2020

build system: Restructure dependency resolution #13349

Merged

Conversation

fjmolinas commented Jan 11, 2020

Contribution description

Testing procedure

Issues/PRs references

Uh oh!

fjmolinas commented Jan 16, 2020

Uh oh!

fjmolinas commented Jan 16, 2020

Uh oh!

fjmolinas commented Jan 16, 2020

Uh oh!

fjmolinas commented Jan 16, 2020

Uh oh!

Uh oh!

fjmolinas commented Jan 27, 2020

Uh oh!

fjmolinas commented Feb 11, 2020

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

aabadie Feb 11, 2020 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

kaspar030 Feb 11, 2020 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

kaspar030 commented Feb 11, 2020

Uh oh!

fjmolinas commented Feb 12, 2020

Uh oh!

fjmolinas commented Feb 12, 2020

Uh oh!

kaspar030 left a comment

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

5 participants

aabadie Feb 11, 2020 •

edited

Loading

kaspar030 Feb 11, 2020 •

edited

Loading