Added documentation for the configuration system

[bogdanm]

No description provided.

[bogdanm]

@screamerbg

[screamerbg]

Targets already provide base configuration for generic macros, labels, features and device peripheral configuration (via device_has).

If this is the case then we need to turn ALL targets in targets.json into configuration data and I don't think that this is sensible or needed. I'd rather expect that the targets.json already implement common data which is well documented.

-1

[bogdanm]

If this is the case then we need to turn ALL targets in targets.json into configuration data and I don't think that this is sensible or needed.

We definitely don't need to do that. Targets have their own legacy configuration, which is there before the configuration system existed. The configuration system doesn't try to change that, it just adds the mechanisms to add more specific configuration in a structured way (which means that there are clear rules for overriding configuration data up to the application layer). There is some overlap between configuation data and the previous mechanism, as you say, but it doesn't mean in any way that we need to convert them.

Also, if you want to -1 something, it makes much more sense to -1 the implementation, not the associated docs (especially since this was already reviewed and approved elsewhere).

[screamerbg]

-1

See comments above. Either this has to be consistent, e.g. ALL targets are turned into config data or shouldn't be introduced/encouraged at all.

[bogdanm]

See comments above. Either this has to be consistent, e.g. ALL targets are turned into config data or shouldn't be introduced/encouraged at all.

No. Some attributes are simply target attributes (data associated with the target, set in stone and not meant to be overriden) and others are configuration parameters (data that has a default value, but it is well understood that this value is meant to be overridable if needed). Think about the core attribute of a target: why would you ever need to override that? It's a constant.

[screamerbg]

The configuration system doesn't try to change that, it just adds the mechanisms to add more specific configuration in a structured way (which means that there are clear rules for overriding configuration data up to the application layer).

There is no difference in the defined macros between

    "K64F": {
        "macros": ["CPU_MK64FN1M0VMD12", "FSL_RTOS_MBED", "MBEDTLS_ENTROPY_HARDWARE_ALT", 
"STORAGE_DRIVER_CONFIG_HARDWARE_MTD_ASYNC_OPS=1"]
            }
        }
},

and

    "K64F": {
        "macros": ["CPU_MK64FN1M0VMD12", "FSL_RTOS_MBED", "MBEDTLS_ENTROPY_HARDWARE_ALT"],
        "config": {
            "storage_driver_mode": {
                "help": "Configuration parameter to select flash storage driver mode. 1 => async operation, 0 => sync operation",
                "macro_name": "STORAGE_DRIVER_CONFIG_HARDWARE_MTD_ASYNC_OPS",
                "value": 1
            }
        }
},

I would expect that:

1. ASYNC_STORAGE is the default behavior and is a requirement for every platform.
2. The application/user can define non-async/blocking behavior which is done via mbed_app.json, not targets.json
3. If for some reason it's HAS to be macro it should live alongside macros, see "MBEDTLS_ENTROPY_HARDWARE_ALT", e.g. "ASYNC_STORAGE=1". This is the consistency I was referring to
4. If something is common/requirement for all supported boards then it should be defined elsewhere - like in Target, not the individual targets. But then the question is why it's defined at all given that it's a requirement/common behavior??

We already have established Target attributes to handle this:

• Use Target.device_has if it's an MCU or mbed API capability (e.g. STORAGE_ASYNC belongs here, similar to SERIAL_ASYNC)
• Use Target.features if it's a ARM mbed platform capability or software stack (e.g. UVISOR, BLE, IPV4, IPV6)
• Use Target.macros if it's a subset of feature that requires the flag to achieve the default behavior for the ARM mbed platform

And last but not least, the fact that a code allows certain behavior doesn't mean we should encourage it. targets.json is intended to provide common/default configuration across all targets for same ARM architecture. The proposed documentation encourages target/platform specific configuration that is moving away from compatibility across all targets and rather encourages platform specific behavior of the ARM mbed as IOT platform.

-1

[bogdanm]

There is no difference in the defined macros between.

There is a big difference. Configuration parameters are structured, namespaced, overridable and self-explanatory if provided with a help field. Macros are something that anyone can use pretty much how they please, which leads to confusion and possible name clashes.

ASYNC_STORAGE is the default behavior and is a requirement for every platform.

I don't believe that's the case. It is my understanding that not all platforms are capable of async storage operations.

The application/user can define non-async/blocking behavior which is done via mbed_app.json, not targets.json.

Why, if the target is the one that knows its hardware capabilities, thus can hint about the value of this parameter by giving it a sensible default value?

If for some reason it's HAS to be macro it should live alongside macros,

Following this logic, the configuration system shouldn't exist at all, since it eventually compiles into macro definitions that can be declared directly as macros in the first place.

If something is common/requirement for all supported boards then it should be defined elsewhere - like in Target, not the individual targets

Completeley agreed with this one. However, right now, it is my understanding ASYNC_STORAGE is not applicable to a lot of targets, thus it doesn't belong in Target (yet).
(Also, this PR is about the documentation of the configuration system, not about ASYNC_STORAGE).

We already have established Target attributes to handle this:

Again, two different things here:

• target attributes, which are (generally) immutable descriptions of the target. I've given the example of the core attribute before.
• target configuration parameters, which are menat to be overriden if needed, and don't generally refer directly to the target's hardware, but are semantically related to its hardware capabilities (for example the default serial baud rate, or the fact that it has enough resources to support async storage operations).

targets.json is intended to provide common/default configuration across all targets for same ARM architecture.

And it does precisely that.

[screamerbg]

Again, two different things here:

• target attributes, which are (generally) immutable descriptions of the target. I've given the example of the core attribute before.
• target configuration parameters, which are menat to be overriden if needed, and don't generally refer directly to the target's hardware, but are semantically related to its hardware capabilities (for example the default serial baud rate, or the fact that it has enough resources to support async storage operations).

See the comprehensive post I added above. Configuration parameters belong to the applications and the library, not the target. The configuration system allows removing and adding of either "macros", "device_has" or "features" from the application and libraries. The targets are already preconfigured for everyone, and if a feature is not common across targets (and in fact is supported on just 1 target) then it shouldn't be enabled by default.

As an example If I'm working on a prototype with mbed, then I would expect that I can change the target at any time (one of the mbed core features!). And I don't have to worry that I might have to rewrite my own application or libraries, just because the default configuration of the board/platform I'm using has an isolated feature turned on.

[bogdanm]

Configuration parameters belong to the applications and the library, not the target.

I have yet to see a good reason why this should be the case.

The configuration system allows removing and adding of either "macros", "device_has" or "features" from the application and libraries

As far as targets are concerned, that's already happening automatically using targets.json. You don't need the configuration system for that.

and if a feature is not common across targets (and in fact is supported on just 1 target) then it shouldn't be enabled by default.

Why on the earth would I not want to use some feature of my hardware if it is there ?! All hardware is not created equal.

if a feature is not common across targets (and in fact is supported on just 1 target) then it shouldn't be enabled by default.

How is "default UART speed" a feature? It is configuration data. The feature (or however you want to call it) is the hardware UART implementation, and it's always there. It doesn't mean that the feature itself doesn't have any configuration of its own. As it happens, some of this configuration might well be target-specific, so it belongs in the target.

And I don't have to worry that I might have to rewrite my own application or libraries, just because the default configuration of the board/platform I'm using has an isolated feature turned on.

Now I'm completely lost:

• features and the configuration system are separate things as far as the targets are concerned.
• not targets are created equal. It is perfectly possible for a target to have different features/configurations than another target. How exactly is that wrong?

[screamerbg]

How is "default UART speed" a feature? It is configuration data. The feature (or however you want to call it) is the hardware UART implementation, and it's always there. It doesn't mean that the feature itself doesn't have any configuration of its own. As it happens, some of this configuration might well be target-specific, so it belongs in the target.

All platforms on mbed use default UART baud rate of 9600. Let's say an application uses a library that produces a lot of data over UART (perhaps a trace library doing lots of print/info/debug) then the application should define suitable UART values. We don't want to encourage different defaults for UART on different targets, especially knowing that all targets support all speeds and this is requirement of the mbed Enabled program (https://developer.mbed.org/handbook/mbed-Enabled).

The examples you gave above (both UART and the whole PR documentation) encourage target specific configurations, which negate common and well adopted defaults. Again, these should be configurable through the application, not the target.

To put this in perspective - do you expect me to check the documentation of every platform regarding the default virtual com port/UART speed? Thought mbed provides common defaults across all targets ?? And FYI, there are established defaults, see https://developer.mbed.org/handbook/Serial ("9600 8N1, and this is common notation for Serial port settings.")

Now I'm completely lost:

• features and the configuration system are separate things as far as the targets are concerned.

Not at all. "labels", "features", "device_has" and "macros" are all parts of one and same configuration system (config system is not just config.py), which emits macros to the code base that is built. Not sure why you're separating these?

not targets are created equal. It is perfectly possible for a target to have different features/configurations than another target. How exactly is that wrong?

Different features is one thing, different configuration is another. For one and same mbed feature, like UART, STORAGE, etc, the default configuration should be the same across all targets, which then can be overridden by the application. See the UART example above.

[bogdanm]

All platforms on mbed use default UART baud rate of 9600.

They do now. Now assume that at some point we change that to 115200 (as we've been asked many times before) and one of the targets can't provide that speed, because its only clock source is a 32khz oscillator and the UART implementation can't generate that that baud rate, for example. This is a hardware limitation that only the target knows about. If the target has any kind of hardware limitation that prevents it from achiving the default serial speed, the only way to make your printf("Hello World!\n") program work without additional changes to the code or the configuration is if you define this kind of parameter in the target. This was also discussed elsewhere.

encourage target specific configurations, which negate common and well adopted defaults.

They don't encourage target specific configurations, they just give you the means to do that in a well defined way. Defining common configuration parameters and their default values in a base target definition (such as Target) is still the way to do. Also, you can still do this kind of target specific configuration right now with macros using macros_add and macros_remove, the difference being that doing it with macros is way more confusing.

do you expect me to check the documentation of every platform regarding the default virtual com port/UART speed?

Not for every platform, but I do expect a platform that can't comply with the default for whatever reason to specify that explicitly in its documentation page.

For one and same mbed feature, like UART, STORAGE, etc, the default configuration should be the same across all targets, which then can be overridden by the application.

This is a dangerous assumption. It implies that you can always find a good enough default that'll work equally well on all targets. I agree we should strive to do that, but it might not be always possible. For UART, that's indeed quite simple (although not guaranteed). For other "features", it might not be that easy, and that's when this kind of mechanism can be really useful.

[screamerbg]

I agree we should strive to do that, but it might not be always possible. For UART, that's indeed quite simple (although not guaranteed). For other "features", it might not be that easy, and that's when this kind of mechanism can be really useful.

Glad that you agree on striving for common OS parameters. I'm happy for us to discuss this on case by case basis. For now I don't think we should encourage different OS configurations based targets, especially for common settings like UART or enforcing ASYNC/SYNC modes through compile time configuration.

[bogdanm]

For now I don't think we should encourage different OS configurations based targets, especially for common settings like UART or enforcing ASYNC/SYNC modes through compile time configuration.

We aren't encouraging it. We're providing the mechanisms to make it happen, if needed. Read the docs that you -1'ed, you might see that they provide a complete story for the configuration, front (application) to back (targets). What I don't buy is you rejecting a mechanism just because you think it can be used to "encourage" bad behaviour. I can make the same reasoning about various pieces of code in our code base, yet they seem to work fine in practice. All things considered, we have control over what gets in the tree or not, so we can control this kind of behaviour if it proves that it leads to bad behaviours indeed.

[bridadan]

We could really use some docs on the config system as evidenced by #2072. Can these changes be brought in soon? @bogdanm @screamerbg

[bogdanm]

Can these changes be brought in soon?

Obviously, I don't have a problem with that :)

[screamerbg]

+1
@bogdanm please think of the discussion above