drivers/netdev: cleanup dependencies and remove ieee802154_radio_hal pseudomodule#15886
Merged
miri64 merged 7 commits intoRIOT-OS:masterfrom Mar 10, 2021
Merged
Conversation
leandrolanzieri
added
Area: build system
leandrolanzieri
requested review from
MichelRottleuthner,
aabadie,
bergzand,
daniel-k,
haukepetersen and
miri64
as code owners
benpicco
added
the
CI: ready for build
Contributor
Author
|
After an offline discussion with @jia200x we plan to make these dependencies in a different way, so I'll mark it as WIP for now. |
leandrolanzieri
added
State: WIP
leandrolanzieri
force-pushed
the
pr/netdev_remove_radio_hal_pseudomodule
branch
from
12be225 to
c4c2a17
Compare
leandrolanzieri
requested review from
PeterKietzmann and
cgundogan
as code owners
leandrolanzieri
added
State: waiting for other PR
leandrolanzieri
force-pushed
the
pr/netdev_remove_radio_hal_pseudomodule
branch
from
c4c2a17 to
640d755
Compare
Contributor
Author
|
Updated description to new changes. |
leandrolanzieri
force-pushed
the
pr/netdev_remove_radio_hal_pseudomodule
branch
from
640d755 to
50cf8b2
Compare
leandrolanzieri
removed
the
State: waiting for other PR
Contributor
Author
|
#15909 has been merged, so this has no dependencies now |
benpicco
added
the
CI: ready for build
Contributor
Author
|
ping? |
jeandudey
approved these changes
Mar 10, 2021
Contributor
jeandudey
left a comment
jeandudey
left a comment
There was a problem hiding this comment.
LGTM, I'm +1 on having separate pseudomodules for each driver that implements either the netdev directly or the HAL, can be useful when an existing netdev drivers get ported to the HAL and you want to select which driver uses the HAL/netdev directly.
leandrolanzieri
force-pushed
the
pr/netdev_remove_radio_hal_pseudomodule
branch
from
2089a06 to
c54d680
Compare
netdev_default should enable the network devices, which do not necessarily use netdev.
This introduces the cc2538_rf_netdev_legacy pseudomodule that switches to the netdev-based implementation of the cc2538 radio driver.
This introduces the nrf802154_netdev_legacy pseudomodule that switches to the netdev-based implementation of the nrf802154 radio driver.
Contributor
Author
|
Seems something has changed since last build, and CI now is failing. I'll take a look. |
Contributor
Author
|
Seems to be fixed now |
leandrolanzieri
added
the
Release notes: ignored
11 tasks
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
5 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Contribution description
Currently radio drivers that implement the IEEE 802.15.4 radio HAL allow switching to the netdev implementation when
netdev_ieee802154_legacymodule is included. In fact, that module andieee802154_radio_halare mutually exclusive, but can only be present on radio drivers that implement both interfaces. This PR tries to make a cleanup of all these dependencies.<radio>_netdev_legacy) which is now specific to the radio, giving more granularity to the configuration. The check whether the legacy driver is used or not is kept internally in every driver.This PR now depends on #15909, and includes its commits to ease testing.Additionally, the PR avoids pulling
netdevwhennetdev_defaultis used, as actually that pseudomodule should bring the network devices available in the platform, which could potentially not use netdev (e.g. cc2538, nrf52). As an example, see thattests/ieee802154_halapplication does not use anynetdev_%modules (aside fromnetdev_default) anymore, as it only tests the radio HAL.Finally, the PR adds some documentation on default configurations (
saul_defaultandnetdev_default). I can split this part if it makes reviewing easier.Testing procedure
netdev_ieee802154_legacy, but the binaries should not change for themtests/ieee802154_halstill works on the supported boards, but no netdev dependencies should be pulled inIssues/PRs references
Depends on #15909