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/esp*: define esp_now as default netdev #12787

Merged
merged 5 commits into from
Nov 23, 2019

Conversation

gschorcht
Copy link
Contributor

@gschorcht gschorcht commented Nov 22, 2019

Contribution description

For ESP32 and ESP8266 boards, various on-board netdevs can be enabled by the esp-now, esp-wifi, esp-eth pseudo modules.

Since esp-wifi requires additional configuration settings and esp-eth is not available on each board, esp-now is defined as default netdev if none of the other netdev modules is enabled explicitly.

It is possible to use multiple netdevs. GNRC_NETIF_NUMOF is determined automatically as long as dynamic approaches in PR #9903 and #12308 are not available.

Testing procedure

Test the following configurations:

  1. esp-now as default netdev

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

    esp-now should be used as default netdev and the statistics should contain a L2-PDU size of 249.

  2. esp-wifi as default netdev

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

    esp-wifi should be used as default netdev and the statistics should contain a L2-PDU size of 1500.

  3. esp-eth as default netdev

    USEMODULE=esp_wifi make BOARD=esp32-olimex-evb -C tests/netstats_l2 flash test
    

    Check with command

    grep MODULE_ESP_ETH tests/netstats_l2/bin/esp32-olimex-evb/riotbuild/riotbuild.h
    

    that module esp_eth is used.

  4. Multiple netdevs

    USEMODULE='esp_wifi esp_now' make BOARD=esp32-wroom-32 -C tests/netstats_l2 flash term
    

    There should be two netdevs with the ifconfig command.

  5. Tests 1, 2 and 3 for esp32-wroom-32 board should also work for esp8266-esp-12x board.

Issues/PRs references

Makes #12756 obsolete

The number of thread priority levels has to be 32 if esp_eth is used.
Since Makefile.dep files are included as last files multiple times to resolve all module dependencies, GNRC_NETIF_NUMOF is handled here.
@gschorcht gschorcht requested a review from benpicco November 22, 2019 17:05
@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: cleanup The issue proposes a clean-up / The PR cleans-up parts of the codebase / documentation labels Nov 22, 2019
@gschorcht gschorcht changed the title cpu/esp32: defines esp_now as default netdev cpu/esp32: define esp_now as default netdev Nov 22, 2019
@benpicco
Copy link
Contributor

Works like a charm!
Any reason not to also include this for esp8266?

This is unrelated to this PR, but the esp_now interface doesn't get a link-local address with the gnrc_networking example:

2019-11-23 08:50:36,049 # Iface  9  HWaddr: 3C:71:BF:9E:13:FD 
2019-11-23 08:50:36,053 #           L2-PDU:249 MTU:1280  HL:64  RTR  
2019-11-23 08:50:36,057 #           6LO  Source address length: 6
2019-11-23 08:50:36,059 #           Link type: wireless
2019-11-23 08:50:36,062 #           inet6 group: ff02::2
2019-11-23 08:50:36,064 #           inet6 group: ff02::1
2019-11-23 08:50:36,067 #           inet6 group: ff02::1a
2019-11-23 08:50:36,068 #           
2019-11-23 08:50:36,071 #           Statistics for Layer 2
2019-11-23 08:50:36,074 #             RX packets 0  bytes 0
2019-11-23 08:50:36,078 #             TX packets 0 (Multicast: 0)  bytes 0
2019-11-23 08:50:36,081 #             TX succeeded 0 errors 0
2019-11-23 08:50:36,084 #           Statistics for IPv6
2019-11-23 08:50:36,087 #             RX packets 0  bytes 0
2019-11-23 08:50:36,091 #             TX packets 3 (Multicast: 3)  bytes 144
2019-11-23 08:50:36,094 #             TX succeeded 3 errors 0

@gschorcht
Copy link
Contributor Author

No reason. Should we rename the PR to cpu/esp* and add the same for ESP8266?

@benpicco
Copy link
Contributor

Sure, go ahead!

@gschorcht gschorcht changed the title cpu/esp32: define esp_now as default netdev cpu/esp*: define esp_now as default netdev Nov 23, 2019
@gschorcht
Copy link
Contributor Author

@benpicco I have added the same for esp8266. Test steps 1, 2, and 4 should also work for any esp8266 board.

Copy link
Contributor

@benpicco benpicco left a comment

Choose a reason for hiding this comment

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

Tested both on esp32 and esp8266, please squash!

Two unrelated things:

  • esp8266 prints scandoneevery 10s which is a bit annoying
  • no link-local address after boot, but they can communicate after manually assigning an address

@gschorcht
Copy link
Contributor Author

  • esp8266 prints scandoneevery 10s which is a bit annoying

Yes. Sinec ESP-NOW is an ad-hoc network, ESP-NOW nodes scan every 10 seconds for new peers. Unfortunately, these messages come from the depth of the vendor binary blobs and I couldn't figure out til now how to disable them in the ESP8266 RTOS SDK. It's the same with the other messages like

add if0
add if1
bcn 100

I will place it on the open issue list in issue #12707.

  • no link-local address after boot, but they can communicate after manually assigning an address

That's clear. tests/netstats_l2 doesn't include IPV6, ICMP and NDP for SLAAC. You
could try

CFLAGS='-DGNRC_IPV6_NIB_CONF_SLAAC=1' make BOARD=esp8266-esp-12x -C examples/gnrc_networking flash test

to get a link local address assigned automatically which works definitely.

If the user or the board definition doesn't enable `esp_wifi` or `esp_eth`, `esp_now` is defined as default netdev.
fixup! cpu/esp32: defines esp_now as default netdev
If the user or the board definition doesn't enable `esp_wifi`, `esp_now` is defined as default netdev.
@gschorcht gschorcht force-pushed the cpu/esp32/netdev_default branch from 8579ff6 to 500c5f4 Compare November 23, 2019 13:26
@benpicco
Copy link
Contributor

That's clear. tests/netstats_l2 doesn't include IPV6, ICMP and NDP for SLAAC.

Yea it took me a bit but then I noticed there was also a message printed about it:

2019-11-23 14:21:47,979 # SLAAC not activated; will not auto-configure IPv6 address for interface 9.
2019-11-23 14:21:47,983 #     Use GNRC_IPV6_NIB_CONF_SLAAC=1 to activate.

And indeed with -DGNRC_IPV6_NIB_CONF_SLAAC=1 I get the address I expected.
Still this is with examples/gnrc_networking so I would expect this to be enabled - @miri64 is this a bug?

@benpicco benpicco merged commit d244b0f into RIOT-OS:master Nov 23, 2019
@gschorcht
Copy link
Contributor Author

Still this is with examples/gnrc_networking so I would expect this to be enabled - @miri64 is this a bug?

ESP-NOW nodes are so-called 6Lo nodes but not 6LN nodes. That is, they use the 6Lo sublayer for fragmentation, but they don't support neighbor discovery according to RFC6775 for the address configuration. The distinction between 6Lo nodes and 6LN nodes was introduced by @miri64 with PR #10499 after a long discussion we had a year ago when esp_now was realized as netdev. As I remember, the reason why ESP-NOW nodes can't be 6LN nodes is that they don't have a unique eui64. But this is something that @miri64 could explain much better than me.

But, indeed the default behavior has changed for ESP-NOW nodes which got a link local address automatically before PR #10499 was merged.

@miri64 Makes it sense to define GNRC_IPV6_NIB_CONF_SLAAC=1 for ESPs if esp_now module is enabled?

@gschorcht gschorcht deleted the cpu/esp32/netdev_default branch November 23, 2019 14:23
@gschorcht
Copy link
Contributor Author

@benpicco Thanks for reviewing, testing and merging.

@fjmolinas fjmolinas added this to the Release 2020.01 milestone Dec 13, 2019
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: cleanup The issue proposes a clean-up / The PR cleans-up parts of the codebase / documentation
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants