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

cpu/esp32: fix provided features and Kconfig for esp_eth #18394

Merged

Conversation

gschorcht
Copy link
Contributor

@gschorcht gschorcht commented Aug 3, 2022

Contribution description

This PR fixes Kconfig problems for esp_eth introduced with PR #17739.

In Makefiles.features, periph_eth was defined as provided feature without having a peripheral driver implementation for the ETH interface in cpu/esp32/periph. Instead, the esp_eth netdev driver directly uses the ESP-IDF Ethernet API. This caused compilation problems with Kconfig. Therefore the required feature periph_eth is replaced by the feature esp_eth and HAS_PERIPH_ETH is replaced by HAS_ESP_ETH in Kconfig.

Furthermore, the selection of modules in Kconfig was not correct if the module esp_eth was used. The reason for this is that the dependencies of modules have changed with the migration to ESP-IDF 4.4.

Since no board with the feature esp_eth was compiled with TEST_KCONFIG=1, the problems were not detected by the CI compilation and was only found when investigating the reasons for the failure of the nightly builds. Therefore, esp32-olimex-evb in .murdock is set to TEST_KCONFIG_BOARDS_AVAILABLE.

Testing procedure

Use command

TEST_KCONFIG=1 BOARD=esp32-olimex-evb make -j8 -C tests/driver_esp_eth

to compile. Wihtout this PR, the compilation fails.

Issues/PRs references

In fact the ESP32 has no peripheral driver for the ETH interface. Instead, the `esp_eth` netdev driver directly uses the ESP-IDF Ethernet API. This caused compilation problems with Kconfig. Therefore the required feature `periph_eth` is replaced by the feature `esp_eth`.
@github-actions github-actions bot added Area: boards Area: Board ports Area: CI Area: Continuous Integration of RIOT components Area: cpu Area: CPU/MCU ports Area: Kconfig Area: Kconfig integration Area: tests Area: tests and testing framework Platform: ESP Platform: This PR/issue effects ESP-based platforms labels Aug 3, 2022
@gschorcht gschorcht added Type: bug The issue reports a bug / The PR fixes a bug (including spelling errors) CI: ready for build If set, CI server will compile all applications for all available boards for the labeled PR labels Aug 3, 2022
In fact the ESP32 has no peripheral driver for the ETH interface. Instead, the `esp_eth` netdev driver directly uses the ESP-IDF Ethernet API. This caused compilation problems with Kconfig. Therefore the required feature `periph_eth` is replaced by the feature `esp_eth`.
@gschorcht gschorcht force-pushed the cpu/esp32/fix_kconfig_for_esp_eth branch from 3ad2d58 to 2347e80 Compare August 3, 2022 07:02
@MrKevinWeiss MrKevinWeiss 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 Aug 3, 2022
Copy link
Contributor

@MrKevinWeiss MrKevinWeiss left a comment

Choose a reason for hiding this comment

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

Looks good, we can check murdock output. Works locally. ACK.

@benpicco benpicco merged commit 004ac82 into RIOT-OS:master Aug 5, 2022
@gschorcht
Copy link
Contributor Author

@MrKevinWeiss Thanks for reviewing.

@gschorcht gschorcht deleted the cpu/esp32/fix_kconfig_for_esp_eth branch August 8, 2022 07:35
@maribu maribu added this to the Release 2022.10 milestone Oct 14, 2022
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: CI Area: Continuous Integration of RIOT components Area: cpu Area: CPU/MCU ports Area: Kconfig Area: Kconfig integration 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: ESP Platform: This PR/issue effects ESP-based platforms Type: bug The issue reports a bug / The PR fixes a bug (including spelling errors)
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants