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

netdev_default: migrate to netif_dev_default #19052

Open
wants to merge 3 commits into
base: master
Choose a base branch
from

Conversation

jia200x
Copy link
Member

@jia200x jia200x commented Dec 14, 2022

Contribution description

This PR renames netdev_default to netif_dev_default due to the following reasons:

  1. With the on-going migration to non-netdev devices (e.g Radio HAL), netdev_default is misleading.
  2. From an IoT OS perspective, IMO it makes sense to bring "default network interfaces" instead of network devices, as the network devices are not necessarily complete (e.g a MAC layer may be missing).

At the same time, this PR allows to define granularity for the kind of network interfaces (e.g select only IEEE 802.15.4 devices/interfaces). This is specially useful for test applications such as ieee802154_hal, which still require any IEEE 802.15.4 device but does not require other embedded network devices (e.g WiFi, Ethernet).

So far, this PR only defines the MODULE_NETIF_DEV_IEEE802154_DEFAULT granularity, but we should add more on demand.

Testing procedure

Compile tests should be enough. Make sure TEST_KCONFIG=1 and TEST_KCONFIG=0 builds converge.

Issues/PRs references

It will simplify merging #18472.

@github-actions github-actions bot added Area: boards Area: Board ports Area: build system Area: Build system Area: cpu Area: CPU/MCU ports Area: drivers Area: Device drivers Area: Kconfig Area: Kconfig integration Area: network Area: Networking Area: sys Area: System Area: tests Area: tests and testing framework Platform: ARM Platform: This PR/issue effects ARM-based platforms labels Dec 14, 2022
@jia200x jia200x marked this pull request as ready for review December 15, 2022 10:32
@jia200x
Copy link
Member Author

jia200x commented Dec 15, 2022

no idea why it was marked as a draft. This is ready for review

@jia200x jia200x requested a review from maribu December 15, 2022 10:32
@benpicco benpicco added the CI: ready for build If set, CI server will compile all applications for all available boards for the labeled PR label Dec 15, 2022
@riot-ci
Copy link

riot-ci commented Dec 15, 2022

Murdock results

FAILED

3bdab84 fixup! netdev_default: migrate to netif_dev_default

Success Failures Total Runtime
1526 0 6882 03m:05s

Artifacts

@github-actions github-actions bot added the Area: examples Area: Example Applications label Dec 15, 2022
@jia200x jia200x 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, 2023
@github-actions github-actions bot added Area: pkg Area: External package ports Platform: AVR Platform: This PR/issue effects AVR-based platforms Platform: ESP Platform: This PR/issue effects ESP-based platforms Platform: native Platform: This PR/issue effects the native platform labels Jan 20, 2023
@@ -46,12 +46,12 @@ ifneq (,$(filter gnrc_lorawan_1_1,$(USEMODULE)))
FEATURES_REQUIRED += periph_flashpage
endif

ifneq (,$(filter gnrc_netdev_default,$(USEMODULE)))
ifneq (,$(filter gnrc_netif_dev_default,$(USEMODULE)))
Copy link
Contributor

@benpicco benpicco Jan 20, 2023

Choose a reason for hiding this comment

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

This should probably remain gnrc_netdev_default

Copy link
Member Author

Choose a reason for hiding this comment

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

uh, that's a result of sed. I will revert it.

@benpicco
Copy link
Contributor

Is there no fallback to include netif_dev_default if netdev_default is selected by an external application?

Do I understand it correctly that netif_dev_default would now select netif_dev_ieee802154_default, so that applications that don't care about the type of network interface will get whatever the board provides, while 6lo specific apps would select netif_dev_ieee802154_default?

(Although those would also work on esp_now, nrfmin, cc110x or nrf24l01p_ng - so should we have another netif_dev_6lo_default that then can select netif_dev_ieee802154_default?

@jia200x
Copy link
Member Author

jia200x commented Jan 24, 2023

Is there no fallback to include netif_dev_default if netdev_default is selected by an external application?

Yes, it was there, but it got replaced by sed. I will fix that immediately.

Do I understand it correctly that netif_dev_default would now select netif_dev_ieee802154_default, so that applications that don't care about the type of network interface will get whatever the board provides, while 6lo specific apps would select netif_dev_ieee802154_default?

Exactly.

(Although those would also work on esp_now, nrfmin, cc110x or nrf24l01p_ng - so should we have another netif_dev_6lo_default that then can select netif_dev_ieee802154_default?

That would make sense. But since there are too many link layers, I just wanted to start with the baseline + IEEE 802.15.4. We could then migrate apps and tests as needed.

@jia200x
Copy link
Member Author

jia200x commented Jan 24, 2023

done!

@jia200x
Copy link
Member Author

jia200x commented Apr 13, 2023

ping. It seems this will be required for #19015

@benpicco
Copy link
Contributor

Kconfig is not happy still

@benpicco
Copy link
Contributor

This needs a rebase and Kconfig needs to be fixed.
What do you think of introducing netif_dev_6lo_default? I'm thinking of the border router in particular where we support more than just IEEE 802.15.4 for the 6lo part.

@jia200x
Copy link
Member Author

jia200x commented May 31, 2023

What do you think of introducing netif_dev_6lo_default? I'm thinking of the border router in particular where we support more than just IEEE 802.15.4 for the 6lo part.

Good idea. I can add that while rebase

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Area: boards Area: Board ports Area: build system Area: Build system Area: cpu Area: CPU/MCU ports Area: drivers Area: Device drivers Area: examples Area: Example Applications Area: Kconfig Area: Kconfig integration Area: network Area: Networking Area: pkg Area: External package ports Area: sys Area: System Area: tests Area: tests and testing framework CI: ready for build If set, CI server will compile all applications for all available boards for the labeled PR Platform: ARM Platform: This PR/issue effects ARM-based platforms Platform: AVR Platform: This PR/issue effects AVR-based platforms Platform: ESP Platform: This PR/issue effects ESP-based platforms Platform: native Platform: This PR/issue effects the native platform
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants