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

tests/netstat_l2: workaround for esp32 boards #12756

Closed

Conversation

gschorcht
Copy link
Contributor

@gschorcht gschorcht commented Nov 20, 2019

Contribution description

Since ESP32 boards only have network interfaces if the esp_wifi, esp_now and esp_eth modules are explicitly enabled, they are removed from the list of boards that provide network interfaces.

Instead, the variable BOARD_PROVIDES_NETIF is made adjustable in the environment. Thus the test can be used for ESP32 with and without network interfaces.

Fixes the test tests/netstat_l2 for ESP32.

This PR belongs to the series of PRs, each with very small changes that fix automatic tests on ESP32 boards.

Testing procedure

Make, flash and test with

BOARD_PROVIDES_NETIF=esp32-wroom-32 USEMODULE=esp_wifi make BOARD=esp32-wroom-32 -C tests/netstats_l2 flash test

and without network interfaces.

make BOARD=esp32-wroom-32 -C tests/netstats_l2

Issues/PRs references

Requires #12752

Since ESP32 boards only have network interfaces if the `esp_wifi`, `esp_now` and `esp_eth` modules are explicitly enabled, they are removed from the list of boards that provide network interfaces.
To be able to use the test for network interfaces that have to be enabled explicitly, the variable BOARD_PROVIDES_NETIF has to be settable in the environment.
@gschorcht gschorcht added Area: cpu Area: CPU/MCU ports 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) labels Nov 20, 2019
@benpicco
Copy link
Contributor

I was always wondering why you don't just do

ifneq (,$(filter gnrc_netdev_default netdev_default,$(USEMODULE)))
  USEMODULE += esp_wifi
endif

for esp32/esp8266 since WiFi is pretty much the defining feature of this MCU.

@gschorcht
Copy link
Contributor Author

I was always wondering why you don't just do

ifneq (,$(filter gnrc_netdev_default netdev_default,$(USEMODULE)))
  USEMODULE += esp_wifi
endif

for esp32/esp8266 since WiFi is pretty much the defining feature of this MCU.

Because you have different options to use the WiFi interface, esp_wifi and esp_now. ESP32 also provides an Ethernet interface. Which one should be the default? So I decided to let the user define it.

@benpicco
Copy link
Contributor

benpicco commented Nov 20, 2019

You could do ESP_NETDEV_DEFAULT ?= esp_wifi (or esp_now, afaik that doesn't need any additional config), then you could do

ifneq (,$(filter gnrc_netdev_default netdev_default,$(USEMODULE)))
  USEMODULE += $(ESP_NETDEV_DEFAULT)
endif

@miri64
Copy link
Member

miri64 commented Nov 20, 2019

What to do if #11676 is merged then?

@@ -1,10 +1,8 @@
include ../Makefile.tests_common

BOARD_PROVIDES_NETIF := airfy-beacon fox iotlab-m3 mulle native nrf51dk nrf51dongle \
BOARD_PROVIDES_NETIF ?= airfy-beacon fox iotlab-m3 mulle native nrf51dk nrf51dongle \
Copy link
Contributor

Choose a reason for hiding this comment

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

Maybe use the following:

Suggested change
BOARD_PROVIDES_NETIF ?= airfy-beacon fox iotlab-m3 mulle native nrf51dk nrf51dongle \
BOARD_PROVIDES_NETIF += airfy-beacon fox iotlab-m3 mulle native nrf51dk nrf51dongle \

Not sure if it's useful or not (if not you can ignore).

Copy link
Contributor Author

Choose a reason for hiding this comment

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

It should not make any difference if you specify this variable explicitly for its use with a single board. BTW, netstat_l2 could be used with ESP8266 boards in same way.

@aabadie
Copy link
Contributor

aabadie commented Nov 20, 2019

What to do if #11676 is merged then?

That won't happen in its current state. I think an adaption of @cladmi's approach in #12427 would be better (this is in my plans).

@gschorcht
Copy link
Contributor Author

You could do ESP_NETDEV_DEFAULT ?= esp_wifi (or esp_now, afaik that doesn't need any additional config), then you could do

ifneq (,$(filter gnrc_netdev_default netdev_default,$(USEMODULE)))
  USEMODULE += $(ESP_NETDEV_DEFAULT)
endif

Should be possible in that way. But I wouldn't do that with this PR if you don't insist on this change because it changes the documented way on how to enable and select the network interface for ESPs.

@gschorcht gschorcht changed the title tests/netstat: l2/fix esp32 boards tests/netstat_l2: workaround for esp32 boards Nov 21, 2019
@gschorcht
Copy link
Contributor Author

@benpicco How do we continue with this PR?

@benpicco
Copy link
Contributor

I mean if you were to make a PR to add a default netdev for esp*, this PR would be obsolete.
IMHO this would be the better solution, also from a user perspective.

@gschorcht
Copy link
Contributor Author

Ok, I can try to find a usefull configuration. esp_now might be better as default since it doesn't require additional setting. But it might become a problem to activate esp_wifi which is probably used more often. I have to test.

@benpicco
Copy link
Contributor

esp_netdev_default could be a pseudomodule that depends on esp_now if no other esp_wifi|esp_now|esp_ethernet module is used.

@gschorcht
Copy link
Contributor Author

gschorcht commented Nov 22, 2019

Hm, sounds good. I will try. BTW, esp_wifi and esp_now can also be used together, e.g., to realize a border router.

@benpicco
Copy link
Contributor

esp_wifi and esp_now can also be used together, e.g., to realize a border router.

Wow, I didn't know - this is neat!

@gschorcht
Copy link
Contributor Author

Closed in favor of PR #12787.

@gschorcht gschorcht closed this Nov 22, 2019
@gschorcht gschorcht deleted the tests/netstat_l2/fix_esp32_boards branch December 6, 2019 12:51
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Area: cpu Area: CPU/MCU ports 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