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

[WIP] automatically set GNRC_NETIF_NUMOF #12308

Closed
wants to merge 2 commits into from

Conversation

benpicco
Copy link
Contributor

Contribution description

Currently, GNRC_NETIF_NUMOF has to be set manually.
This PR changes that so that each network interface will increment GNRC_NETIF_NUMOF at compile-time.

Since GNRC_NETIF_NUMOF now consists of 0 + 0 + sizeof(…) + 0 + sizeof(…) + … the processor can not evaluate it anymore. This is not a problem since the compiler will see the same job if it sees an if (0).
But as a prerequisite, many #if (GNRC_NETIF_NUMOF > 0) had to be changed into if (GNRC_NETIF_NUMOF > 0).

As expected, code size did not increase.

Testing procedure

Check if everything still works, especially examples/tests that define GNRC_NETIF_NUMOF := 2

Issues/PRs references

#11979
#9903

The preprocessor is not able to do arithmetic using the sizeof() operator.
Instead, let the compiler evaluate the constant, the result should be the
same.
@benpicco benpicco added Type: enhancement The issue suggests enhanceable parts / The PR enhances parts of the codebase / documentation Area: network Area: Networking State: WIP State: The PR is still work-in-progress and its code is not in its final presentable form yet CI: ready for build If set, CI server will compile all applications for all available boards for the labeled PR labels Sep 26, 2019
#define STM32_ETH_NUM (0)
#endif

#define GNRC_NETIF_NUMOF (AT86RF2XX_NUM + CC2420_NUM + KW2XRF_NUM + MRF24J40_NUM \
Copy link
Contributor

@gschorcht gschorcht Sep 27, 2019

Choose a reason for hiding this comment

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

What about modules esp_now, esp_wifi and esp_eth? All of them provide a separate netif.

Copy link
Contributor Author

@benpicco benpicco Sep 27, 2019

Choose a reason for hiding this comment

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

Will add those!
Why are they not initialized in sys/auto_init/netif/?

Copy link
Contributor

Choose a reason for hiding this comment

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

The initialization of these netdevs is called as for other drivers in sys/auto_init/auto_init.c

#ifdef MODULE_ESP_ETH
extern void auto_init_esp_eth(void);
auto_init_esp_eth();
#endif
/* don't change the order of auto_init_esp_now and auto_init_esp_wifi */
#ifdef MODULE_ESP_NOW
extern void auto_init_esp_now(void);
auto_init_esp_now();
#endif
/* don't change the order of auto_init_esp_now and auto_init_esp_wifi */
#ifdef MODULE_ESP_WIFI
extern void auto_init_esp_wifi(void);
auto_init_esp_wifi();
#endif

The difference is, that the implementations of the auto_init_* functions are not placed in separate files in sys/auto_init/netif but in cpu/esp*/esp_*/esp_*_netdev.c. The reason is that these netdevs are platform specific and shouldn't be placed in a common place.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I see! But stm32_eth and nrf802154 are platform specific too…

Copy link
Contributor

Choose a reason for hiding this comment

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

Yes. The question is whether we should move the auto_init_esp_* code to sys/auto_init/netif or whether we should leave it as it is.

#define STM32_ETH_NUM (0)
#endif

#define GNRC_NETIF_NUMOF (AT86RF2XX_NUM + CC2420_NUM + KW2XRF_NUM + MRF24J40_NUM \
Copy link
Contributor

Choose a reason for hiding this comment

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

What about defining a generic NETIF_NUMOF here and make GNRC use that? so we could keep this stack agnostic

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Area: network Area: Networking CI: ready for build If set, CI server will compile all applications for all available boards for the labeled PR State: WIP State: The PR is still work-in-progress and its code is not in its final presentable form yet Type: enhancement The issue suggests enhanceable parts / The PR enhances parts of the codebase / documentation
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants