boards: examples: tests: refactor the way boards providing netif is defined

[aabadie]

Contribution description

This PR is RFC because I'm not 100% sure of the approach but the idea behind is to get rid of the BOARDS_PROVIDES_NETIF variables in examples/default and tests/netstat_l2 applications by using a specific feature for boards providing an interface (using FEATURES_REQUIRED += <something>). The <something> should be generic enough to cover 802154, ble, lora, wifi, ethernet, etc and must be provided at board level, similar to other features.
Using netif was the best choice but I realized it was already defined as a pseudomodule by the build system. And after looking a bit closer I realized it was not really useful.
So the 3 first commits of this PR are just removing the existing netif pseudomodule.

The rest of the commits are providing the netif to boards with an interface. The last 2 commits are just dropping the BOARDS_PROVIDES_NETIF variables from the related application and use FEATURES_REQUIRED += netif instead.

This is simpler to maintain and IMHO way cleaner than the actual design.

Testing procedure

• A green Murdock
• Verify that boards providing the netif feature are listed in the results of (note that some new ones were added and some were removed):

$ make --quite -C examples/default info-boards-supported
$ make --quite -C tests/netstats_l2 info-boards-supported

Issues/PRs references

None

[aabadie]

Discussing this @fjmolinas offline and this is changing the previous behaviour: this application is not built for boards not providing a netif which was the case previously. So we still need BOARD_PROVIDES_NETIF here...

[cladmi]

Going for FEATURES for these kind of things is something I am interested in.
Same for the peripherals like radios/sensors that are available on a board.

However for the issue in example/default it goes into the pattern of optionally include modules on condition which is not currently supported.

It is something I am also interested in but that I keep for after #9913

[cladmi]

Is not the 'FEATURES_REQUIRED' supposed to be in a dependency of one of the used module ?

[aabadie]

Maybe but which one ? ;)

I wanted to put this as a dependency of gnrc_netif or netdev_default but the problem then is that it will be difficult to build an application for a board where one wires manually an external radio. That's why I left it at the application level.

[cladmi]

Maybe but which one ? ;)

I would say netdev_default as it is the one that pulls the radio in boards/board/Makefile.dep but maybe gnrc_netif is better suited. @miri64 knows better than me.

I wanted to put this as a dependency of gnrc_netif or netdev_default but the problem then is that it will be difficult to build an application for a board where one wires manually an external radio. That's why I left it at the application level.

If you wire an external radio, it is for me a "different" board. And you need to write you external radio configuration too

boards/hamilton/include/board.h: * @name AT86RF233 configuration
boards/hamilton/include/board.h:#define AT86RF2XX_PARAM_SPI      SPI_DEV(0)
boards/hamilton/include/board.h:#define AT86RF2XX_PARAM_SPI_CLK  SPI_CLK_5MHZ
boards/hamilton/include/board.h:#define AT86RF2XX_PARAM_CS       GPIO_PIN(PB, 31)
boards/hamilton/include/board.h:#define AT86RF2XX_PARAM_INT      GPIO_PIN(PB, 0)
boards/hamilton/include/board.h:#define AT86RF2XX_PARAM_SLEEP    GPIO_PIN(PA, 20)
boards/hamilton/include/board.h:#define AT86RF2XX_PARAM_RESET    GPIO_PIN(PB, 15)

So would need to have some files adapted.
However, for me the re-use of a board or modifying a board is its own topic. Something I would like but has lots of dependencies #11674

Is it possible to "modify" a board from an application Makefile ? not entirely as it does not define Makefile.include and Makefile.dep that are parsed with the others.

That last point could be addressed. To include $(APPDIR)/Makefile.features, $(APPDIR)/Makefile.include and $(APPDIR)/Makefile.dep in they exist.
However it would not currently entirely work currently due to the hacks:

RIOT/boards/common/nrf52/Makefile.include

Lines 12 to 23 in 761530c

	# The following configuration is dependencies specific
	# but they are resolved later
	# Hack to know now if 'nordic_softdevice_ble' is used
	include $(RIOTBOARD)/$(BOARD)/Makefile.dep
	PROGRAMMER ?= jlink
	ifeq (jlink,$(PROGRAMMER))
	# setup JLink for flashing
	export JLINK_DEVICE := nrf52
	# special options when using SoftDevice
	ifneq (,$(filter nordic_softdevice_ble,$(USEPKG)))

The hacks will be able to be removed after #9913 if we can move forward with #11478

[aabadie]

Same for the peripherals like radios/sensors that are available on a board.

I started with radio_802154, radio_ble, radio_lora, etc but it was too specific and not available in the application Makefile. But in any case, it would be interesting to this level of dependency as well.

[cladmi]

Same for the peripherals like radios/sensors that are available on a board.

I started with radio_802154, radio_ble, radio_lora, etc but it was too specific and not available in the application Makefile. But in any case, it would be interesting to this level of dependency as well.

The not available could be solved by doing -include $(APPDIR)/Makefile.include, -include $(APPDIR)/Makefile.dep and maybe -include $(APPDIR)/Makefile.features. However I would rather merge it after #9913 is addressed.
Doing an exploratory branch to see if it could be done already before could be interesting.
Maybe the issue with the dependency being available in board/Makefile.include are not a problem for the current use cases and so caveat could be documented.

[cladmi]

Question, if netif is required by gnrc_netif or netdev_default it should have the consequence that that including one of this module gets the radio driver.

However nrf52dk includes nordic_softdevice_ble only if gnrc_netdev_default is included. So requesting netdev_default does not pull the radio dependency which does not match with the previous statement. It may be outdated and could require cleanup to be done before.

RIOT/boards/nrf52dk/Makefile.dep

Lines 4 to 6 in 761530c

	ifneq (,$(filter gnrc_netdev_default,$(USEMODULE)))
	USEPKG += nordic_softdevice_ble
	endif

[aabadie]

I think the best would be to put the FEATURES_PROVIDED += netif (or radio_xxx) at a lower level than board level. I would say in a Makefile.features at driver level, but these files doesn't exist (yet).
This way when a driver module is loaded, the build system knows the features provided by this driver. If it's at86rf2xx, it would be netif, radio_802154 for example.
Like this, it will then be possible to add PERIPH_REQUIRED += netif as a dependency of netdev_default or gnrc_netif without having to touch the boards.

[aabadie]

@cladmi, I've reworked a bit this PR to add a mecanism where drivers can provide high level features. Now only networking related features (netif, radio_802154, radio_lora) are provided.
It seems to work well without too many changes in boards.

I'm not sure about the integration in the build system though. I know there's room for improvements but I'd like to have your opinion if possible.

[aabadie]

Interestingly, the number of jobs has dropped from 38860 to 34538. This is because now applications requiring gnrc_netif are only build for the subset of boards that pulls by default a driver providing feature netif: for example, now we get an error message when building examples/gnrc_networking for nucleo-l073rz. But if we add the at86rf231 module to the build, the error message goes away.

[cladmi]

If it is "features" it should be included in Makefile.features.

[aabadie]

The features provided by (radio) drivers must already know the driver modules used in the application. Because of this Makefile.features has to be included after Makefile.dep. I don't know if we want this or not.
If I put the drivers/Makefile.features file in the global Makefile.features, I have to change a few things in Makefile.include that break the build. This is still unclear for me why.

[cladmi]

Indeed, I found why it does not work. I will explain in the main comments.

[cladmi]

I slept on the concept and found what was the issue for me.

A module cannot provide a feature.

Providing a feature means that there is:

• BSP support (== hardware/toolchain)
• hardware specific "configuration"
• Code to use it (== the module)

If on an arduino-mega2560, it is not because I use the at86rf2xx module that I magically have the radio.

I cannot enable riotboot without having the toolchain adapted, the hardware description to build with offset, the flasher ready, and without the code working for my architecture.

Which means it is he role of the bsp/board to say "HEATURES_PROVIDED +=".

And then, when the features is used it must select which implementation to use and which configuration (usually already defined in the board headers).
For the periph the code inclusion is done automatically globally

RIOT/Makefile.dep

Lines 880 to 881 in d5bdb5d

	# all periph features correspond to a periph submodule
	USEMODULE += $(filter periph_%,$(FEATURES_USED))

But here, it may need to be done by the board.

[aabadie]

I think we need to clarify a bit the concept of feature.

A module cannot provide a feature.

Why not if the module is about a driver. A driver could also provide the filesystem feature (SD card SPI), the saul feature, etc

Providing a feature means that there is:

BSP support (== hardware/toolchain)
hardware specific "configuration"
Code to use it (== the module)

Drivers provides hardware support, a specific (but overridable) configuration and modules to use them.

If on an arduino-mega2560, it is not because I use the at86rf2xx module that I magically have the radio.

But the setup potentially have it.
If you wire an at86rf2xx to your board and provide the right at86rf2xx_params.h in your application directory, then you have the radio.

I admit it's not as automatic as for CPU peripherals with boards but I think it's worth having this mecanism.

[cladmi]

But the setup potentially have it.
If you wire an at86rf2xx to your board and provide the right at86rf2xx_params.h in your application directory, then you have the radio.

By doing this, "wiring" and "putting params" you add the radio to your board and you somehow modify your board.
You provide the features to your BSP and you can then add the FEATURES_PROVIDED +=.

A module cannot provide a feature.

Why not if the module is about a driver. A driver could also provide the filesystem feature (SD card SPI), the saul feature, etc

If the module is a driver, without the hardware it does nothing.
The board provides the filesystem feature through its SD card SPI hardware.

For the saul feature, the board would also need to have described all the hardware visible by saul.

I know there may be cases where it could indeed be working "features rng" that you can provide as software but it is not the default case.

[cladmi]

The fact that it is possible or not to do it from the application directory (or outside of riot) is a different topic from what means feature.
It is solvable but it is a different topic.

[aabadie]

By doing this, "wiring" and "putting params" you add the radio to your board and you somehow modify your board.

Any nucleo 64 board can be become a lora device by just plugging on it an mbed Lora shield. This is the same with x-nucleo-xxxx sensor shields, etc.
You can also just think of Arduino shields, like the wireless shield: just plug an xbee on it and you are ready.

It's not a different board, it's a different setup.

Providing features from drivers takes advantages of the flexibility of build system modules to enable netif without changing a line of code in this case.

[cladmi]

It's not a different board, it's a different setup.

That is what I call a different "board" in RIOT.
In linux-arm, it would require a different device tree (or a really adapted device tree for the shields, like the beaglebone).

If you may need to have a device mapping, I call it a different board as we store this in the board/periph_conf
And if your board can use the default one, you must have checked the default works for your setup and not configure a different one.

Interestingly, the number of jobs has dropped from 38860 to 34538. This is because now applications requiring gnrc_netif are only build for the subset of boards that pulls by default a driver providing feature netif: for example, now we get an error message when building examples/gnrc_networking for nucleo-l073rz. But if we add the at86rf231 module to the build, the error message goes away.

Is this really wanted ?

I do not fully understood yet what netif, gnrc_netdev_default, netdev_default are.
Changing netif from a module to a feature is somehow an API change and should update some documentation too.

This PR is RFC because I'm not 100% sure of the approach but the idea behind is to get rid of the BOARDS_PROVIDES_NETIF variables in examples/default and tests/netstat_l2

This is currently not removed, so the refactoring does not get to the the goal yet.

The fact that provided features depends on used modules really changes the way we currently handle features, and I do not like the direction.

BOARD=iotlab-m3 make --no-print-directory -C examples/hello-world/ info-debug-variable-FEATURES_PROVIDED
periph_i2c periph_rtt periph_spi periph_timer periph_uart riotboot periph_dma periph_flashpage periph_flashpage_raw periph_cpuid periph_gpio periph_gpio_irq puf_sram periph_uart_modecfg periph_pm cpp cpu_check_address

Something I would prefer is that the iotlab-m3 provides at86rf2xx. And that if at86rf2xx is provided, provide the netif feature too.

The build_system_sanity_check currently forbids checking FEATURES_PROVIDED but it is because it was used instead of FEATURES_USED to add modules, and so could be adapted.

I would even try to use a specific pattern that does not use immediate evaluation, and maybe be declarative completely.
FEATURES_PROVIDED_at86rf2xx ?= netif
But would need some attention as it would be a circular dependency with the current way of defining.

At least I would prefer staying on the provided level without usemodule as it is breaking our model.

One other issue then, would be to be clear how should one include the radio module.
As requiring netif would not pull at86rf2xx. You would still need to require the top module.

[cladmi]

I would even try to use a specific pattern that does not use immediate evaluation, and maybe be declarative completely.
FEATURES_PROVIDED_at86rf2xx ?= netif
But would need some attention as it would be a circular dependency with the current way of defining.

I guess somehow we should then say, with maybe a helper function

FEATURES_PROVIDED += feature_name $(FEATURES_PROVIDED_feature_name)

or

FEATURES_PROVIDED_feature_name += feature_name $(FEATURES_PROVIDED_other_ones)

Or do the recursive resolution at the end to another variable with a recursive resolve_deps.

FEATURES_PROVIDED_RESOLVED = $(call resolve_deps,$(FEATURES_PROVIDED)) 

[maribu]

The Murdock build succeeded before I restarted it. I did restart Murdock for a random PR it previously succeed to verify that Murdock is currently having issues, which it apparently has :-/

[aabadie]

• Moved features management in individual drivers directories (just for netif and related radio technologies)
• Adapted Makefile.include (please forgive my limited Makefile skills)
• info-board-supported is broken but I can't find/understand why (due to my limited Makefile skills).

As you may see, I have to parse dependencies before parsing the features, but also after. Which is quite ugly...

[fjmolinas]

As you may see, I have to parse dependencies before parsing the features, but also after. Which is quite ugly...

Did you look at #12427?

waiting for author

[stale]

This issue has been automatically marked as stale because it has not had recent activity. It will be closed if no further activity occurs. If you want me to ignore this issue, please mark it with the "State: don't stale" label. Thank you for your contributions.

[miri64]

Ping @aabadie?

[aabadie]

Once module dependencies and provided features will be handled in Kconfig, I think it will be simpler to declare the availability of high-level features (or API features). So I'm closing this PR.