How to ensure consistent defaults across all pieces of an application

[pabigot]

OK, so I'm confused enough to make this a separate issue from the pull I wanted to submit where I tried to work around it. The basic question: what header file(s) should be included to ensure all application and platform options are applied consistently across all modules, and where in the sequence of include files should it appear? (I do see the discussion around #178 but I didn't get a clear sense of direction from it.)

There's contiki.h and contiki-conf.h and (now) contiki-default-conf.h.

contiki.h seems to be for applications as it includes a bunch of useful sys headers, so I can see that just including "contiki.h" everywhere is probably not what should normally be used in core/* or platform files. It does make sure that contiki-conf.h and contiki-default-conf.h both get included right away, though, so including it first should produce a well-defined configuration.

In other source and header files in core I see some include contiki.h and some just include contiki-conf.h. One includes both directly; others probably do indirectly. Sometimes these files are the first ones included, but some times other core headers are included first, making it possible that they act on the visible defaults before all defaults have been read.

core/net/uip-icmp6.c is the only file other than contiki.h that explicitly includes contiki-default-conf.h, which it maybe doesn't have to because it first includes net/uip-ds6.h which includes net/uip.h which includes net/tcpip.h which includes contiki.h.

There are conditionals based on potentially default values for the same macro in multiple places: contiki-default-conf.h, other headers, and source files. At this time the defaults appear to be consistent, but I'd worry about that remaining true in the future. If contiki-default-conf.h ever does something more than/different from what's done by the individual headers that also assume defaults, source files that include only contiki-conf.h have the potential to have different defaults than ones that also include contiki.h.

I haven't found a clear case where something goes wrong, but it does seem fragile with a risk that different files will get built with different defaults. I guess I just want to raise the issue to see if anybody shares my concern, and more importantly to get advice on what I should do to reduce risk in my own code.

[oliverschmidt]

At this time the defaults appear to be consistent, but I'd worry about that remaining true in the future.

No need to worry - this has already happened in the past ;-)

I guess I just want to raise the issue to see if anybody shares my concern, and more importantly to get advice on what I should do to reduce risk in my own code.

I share your concern. As you know there's no single truth regarding "include styles" - one large "include everything file" vs. "minimal necessary inlude file set". I personally can live with both styles - it just needs to be used consistently...

[pabigot]

Copied from #281:

@pabigot: As you seem to be willing to invest effort in Contiki cleanup: In former times there was the consistent >approach of FEATURE_CONF_FOO being only used at a single spot always like this:

ifdef FEATURE_CONF_FOO

define FEATURE_FOO FEATURE_CONF_FOO

else

define FEATURE_FOO

endif

I'd really like to see that approach being used again consistently - or as an alternative all FEATURE_CONF_FOO >variables remove altogeter and replaced by FEATURE_FOO. I'm just missing consistency here.

Especially as there's not that much great Contiki documentation - and even if there were it would be largely ignored - most newcommers look at the existing code. And if they don't find a consistent approach they'll do however they see fit - which is understandable. So consistency is they only way to keep the quality of a growing codebase...

@oliverschmidt I agree wholeheartedly, and would very much like to see FEATURE_FOO consistently replace inconsistent use of FEATURE_CONF_FOO. I've thought a lot about arguments in support of this, and about how to reliably express and override configuration parameters values at compile time, for specific applications, in platforms, and when nothing else provides them (in decreasing priority). If all goes well with bsp430 support and the warning removal that I've already signed up for, and there isn't another solution already in progress by that point, I'll come back to this.

[oliverschmidt]

@pabigot: I'm not sure how much this relates to what you write above but I always felt that it's a pity the Contiki build system builds the whole Contiki library individually for every application (for the same target) but doesn't provide a mean to take advantage of this by providing means for per-application configuration. Maybe my perspective isn't the Contiki mainstream but I see a matrix of targets and applications and every row/collumn/cell in that matrix should allow for inidividual configuration.

A simple example for application specific configuration (here uIP): A REST server doesn't need DNS and therefore doesn't need UDP, while a REST client (usually) needs both (plus "active open" support).

A simple example for target-and-application specific configuration (again uIP) is the uIP buffer size reduzed just enough to fit into RAM.

Therefore six years ago I introduced with https://github.com/contiki-os/contiki/commit/a5ccd9515d92af78a119f695fbe5221f3b0d9fbb and https://github.com/contiki-os/contiki/commit/125f261b7df9892ea07dae99c567ca8c389d43e3 a very primitive mechanism to allow for target-and-application specific configuration (documented here: https://github.com/contiki-os/contiki/blob/master/cpu/6502/README.md and used i.e. here: https://github.com/contiki-os/contiki/blob/master/examples/webserver/Makefile.c64.defines). I'd love to see this hack replaced with something real (incremental build support, support for all targets, ...).

[malvira]

+1 on that.

I jump through hoops to make a build a RPL_LEAF_NODE or not based on the application:

https://github.com/malvira/th-12/tree/master/targets

I do it with a "fake" platform. So make TARGET=th12 is a "normal" config and make TARGET=th12-lowpower adds in lowpower_conf.h which will set it as a leaf node (or whatever).

-Mar.

On Mon, Jun 24, 2013 at 7:07 AM, Oliver Schmidt notifications@github.comwrote:

@pabigot https://github.com/pabigot: I'm not sure how much this relates to what you write above but I always felt that it's a pity the Contiki build system builds the whole Contiki library individually for every application (for the same target) but doesn't provide a mean to take advantage of this by providing means for per-application configuration. Maybe my perspective isn't the Contiki mainstream but I see a matrix of targets and applications and every row/collumn/cell in that matrix should allow for inidividual configuration.

A simple example for application specific configuration (here uIP): A REST server doesn't need DNS and therefore doesn't need UDP, while a REST client (usually) needs both (plus "active open" support).

A simple example for target-and-application specific configuration (again uIP) is the uIP buffer size reduzed just enough to fit into RAM.

Therefore six years ago I introduced with a5ccd95https://github.com/contiki-os/contiki/commit/a5ccd9515d92af78a119f695fbe5221f3b0d9fbband 125f261https://github.com/contiki-os/contiki/commit/125f261b7df9892ea07dae99c567ca8c389d43e3a very primitive mechanism to allow for target-and-application specific configuration (documented here: https://github.com/contiki-os/contiki/blob/master/cpu/6502/README.md and used i.e. here: https://github.com/contiki-os/contiki/blob/master/examples/webserver/Makefile.c64.defines). I'd love to see this hack replaced with something real (incremental build support, support for all targets, ...).

— Reply to this email directly or view it on GitHubhttps://github.com/contiki-os/contiki/issues/280#issuecomment-19900698 .

[pabigot]

@oliverschmidt Yes, what you describe is part of what I would like to see; not just per-application configuration, but also the ability to affect the configuration of a single build without having to edit source code. Support for a compile-time facility overrides lessens the risk of unintended commits when one doesn't notice that a change made to stress-testing a border case (e.g. setting a really short update interval) got left in the merge inadvertently. A clean configuration from Makefile variables and C macro definitions does enable this.

At this time my Contiki effort is personal, though with a hope of developing some clients who want contracted support. There's a lot that could be done with the build environment, and I could certainly put out some proposals and strawmen, but bringing it to a complete and robust solution is a major effort that I'm unlikely to take on for myself.

[oliverschmidt]

There's a lot that could be done with the build environment, and I could certainly put out some proposals and strawmen, but bringing it to a complete and robust solution is a major effort that I'm unlikely to take on for myself.

I see. I'd say it nevertheless didn't hurt to briefly discuss it here :-)

[pabigot]

Nope, this is as good a place as any to hold discussions.

[pabigot]

In #254 @oliverschmidt commented:

A make is for generating stuff, not for removing stuff ... Conclusion: Do some explicit removal but have the directory removal done by the user.

Taken to its conclusion, this has implications on configuration.

A build process like make necessarily creates derived artifacts. The build process is expected to support a way to re-build an artifact when what it depends on changes. Whether the artifact is a file or directory is mostly irrelevant to this expectation.

make(1) ensures configuration changes are taken into account if the changes are made to a filesystem object that is a dependency of the build process. If configuration changes can be communicated without source changes (e.g., Makefile variables) some other mechanism (e.g. a strong "make clean" infrastructure) is necessary to force a rebuild of artifacts affected by a change.

I'm recording this here because my preference is to be able to override configuration values at compile-time through the build environment without source code changes. To do this with the normal "make" workflow requires that "clean" and its related targets be able to remove all derived artifacts, even if some are directories.

I just want to record the difference in perspective so it can be reviewed/resolved if/when this issue is addressed.

[oliverschmidt]

A make is for generating stuff, not for removing stuff ... Conclusion: Do some explicit removal but have the directory removal done by the user.

Taken to its conclusion, this has implications on configuration.

While my statement above was intentionally somewhat casual you're absolute right with your analysis !

If configuration changes can be communicated without source changes (e.g., Makefile variables) some other mechanism (e.g. a strong "make clean" infrastructure) is necessary to force a rebuild of artifacts affected by a change.

Yes, I fully aggree with you on this conclusion.

I just want to record the difference in perspective so it can be reviewed/resolved if/when this issue is addressed.

Again full ack! It's certainly good to have that difference in mind - it for sure wasn't that clear to me but you carved it out nicely :-)

I think configurations (as described here) should in general fall into one of the following cathegories

• Switched often (like the classic debug / release or the Contiki target) - they should have their own build artifacts directories
• Switched about never - they should be handled by the user by manually removing "everything"
• Switched seldom - they should be reflected in header files (what you in more general terms described as "communicated with source change")

I understand that there are certianly downsides to this perspective regarding corner cases or flexibility in general. However I believe that a build process that relies on a 'strong "make clean" ' as you phrase it is nicely tends to be fragile and rather easily broken by non-insiders - and is therefore not suited to a project like Contiki without that strong governance policies ;-)

Just my two cents, Oliver

[simonduq]

+1 on that discussion This issue seems pretty serious to me, as for example contikimac.c or csma.c never get contiki-default-conf.h included, which means that tuning these protocols from this default conf file will never have any effect. Worse: with some modules, one might end up with a table or struct having different sizes when compiled from different modules because of erroneous file inclusion. A simple fix is needed quick! (higher priority than getting a nice config system with per-app, per-example and per-platform config) One option is to move the inclusion of contiki-conf.h from contiki.h to contiki-default-conf.h, and include contiki-default-conf.h in all files that need config information. Another option is to include contiki-default-conf.h at the bottom of all contiki-conf.h.

/Simon