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

drivers: add ATWINC15x0 WiFi netdev driver #13754

Merged
merged 9 commits into from
Jun 26, 2020

Conversation

gschorcht
Copy link
Contributor

@gschorcht gschorcht commented Mar 30, 2020

Contribution description

This PR adds a netdev driver for the Microchip ATWINC15x0 WiFi module.

Background

The ATWINC15x0 WiFi module is used on several boards and extension boarsd, for example:

Furthermore, the ATWINC15x0 WiFi module is the only module supported by the standard Arduino WiFi101 library. Therefore, there are a number of Arduino shields with the ATWINC15x0 WiFi module, e.g. Adafruit WINC1500 WiFi Shield for Arduino.

Using this netdev driver together with an ATWINC15x0 WiFi module enables the connection via WiFi in RIOT. Thus, ATWINC15x0 WiFi module can be used as netdev for GNRC as well as lwIP netifs.

Current Status

The driver works out of the box for the following boards that have the ATWINC15x0 on-board:

It was also tested with an Adafruit WINC1500 WiFi Shield for Arduino for ESP32 and STM32 Nucleo-F411RE.

Arduino AVR boards unfortunately have too less RAM.

The driver supports following WiFi security modes:

  • WPA2 Personal mode (PSK)
  • Open System.

WPA2 enterprise mode with IEEE 802.1X/EAP authentication is not supported for the following reasons:

  • At least ATWINC15x0 firmware version 19.6.1 is required, but most available modules still have a firmware version lower than 19.5.0. Older firmware versions support EAP-TTLSv0/MSCHAPv2 but use an invalid SSL/TLS version.
  • At least version 19.6.1 of the ATWINC15x0 vendor driver is required. The driver version used by this netdev driver, which is available as open source, is version 19.5.2.

Future Work

At the moment, only station mode is implemented. It might be interesting to support AP mode so that it could be used in a border router.

Testing procedure

It should work with examples/gnrc_networking

USEMODULE='atwinc15x0' \
CFLAGS='-DWIFI_SSID=\"ssid\" -DWIFI_PASS=\"pass\" \
        -DATWINC15X0_PARAM_SSN_PIN=GPIO_PIN\(1,6\) \
        -DATWINC15X0_PARAM_RESET_PIN=GPIO_PIN\(1,4\) \
        -DATWINC15X0_PARAM_IRQ_PIN=GPIO_PIN\(0,8\)' \
make BOARD=... -C examples/gnrc_networking flash term

Issues/PRs references

@gschorcht gschorcht requested review from aabadie and benpicco and removed request for haukepetersen, miri64, cgundogan and PeterKietzmann March 30, 2020 13:26
@gschorcht gschorcht added Area: drivers Area: Device drivers Area: network Area: Networking CI: ready for build If set, CI server will compile all applications for all available boards for the labeled PR State: WIP State: The PR is still work-in-progress and its code is not in its final presentable form yet Type: new feature The issue requests / The PR implemements a new feature for RIOT and removed CI: ready for build If set, CI server will compile all applications for all available boards for the labeled PR labels Mar 30, 2020
@gschorcht gschorcht changed the title drivers: add ATWINC15x0 netdev driver drivers: add ATWINC15x0 WiFi netdev driver Mar 30, 2020
@aabadie
Copy link
Contributor

aabadie commented Mar 30, 2020

Is it a take-over from #7433 ?

Otherwise thanks for providing this. I'm be able to test this PR on arduino-mkr1000.

@gschorcht
Copy link
Contributor Author

Is it a take-over from #7433 ?

No, I didn't know that. Even though I searched for all combinations winc, atwinc, atwinc1500, winc1500, atwinc15x0, winc15x0, ... in issues and PRs to avoid duplicates, I didn't find it. Probably because I didn't search in closed issues and PRs.

I'm be able to test this PR on arduino-mkr1000.

Sounds good.

@aabadie
Copy link
Contributor

aabadie commented Mar 30, 2020

Even though I searched for all combinations winc, atwinc, atwinc1500, winc1500, atwinc15x0, winc15x0, ... in issues and PRs to avoid duplicates, I didn't find it. Probably because I didn't search in closed issues and PRs.

You can blame the stale bot ;)

@gschorcht
Copy link
Contributor Author

I'm be able to test this PR on arduino-mkr1000.

@aabadie Thanks for pointing out this board. I found such a board, which I bought 3 years ago, unused in a box. I will try the driver with this board to refine the default parameters of the driver a little bit.

@gschorcht
Copy link
Contributor Author

You can blame the stale bot ;)

But at that time there was also no reaction from the author for over a year. So closing the PR was correct.

@aabadie
Copy link
Contributor

aabadie commented Mar 30, 2020

But at that time there was also no reaction from the author for over a year. So closing the PR was correct.

I know, it was a joke :)

@gschorcht
Copy link
Contributor Author

@aabadie My first test with mkr1000 did not work, I have to check whether I used the right GPIOs.

@gschorcht
Copy link
Contributor Author

gschorcht commented Mar 30, 2020

@aabadie Good news. Worked for me for mkr1000 with following configuration. I had to use SPI_DEV(1) instead of SPI_DEV(0).

USEMODULE='atwinc15x0' \
CFLAGS='-DGNRC_IPV6_NIB_CONF_SLAAC=1 -DDEBUG_ASSERT_VERBOSE \
        -DWIFI_SSID=\"<ssid>\" -DWIFI_PASS=\"<pass>\" \
        -DATWINC15X0_PARAM_SPI=SPI_DEV\(1\) \
        -DATWINC15X0_PARAM_SSN_PIN=GPIO_PIN\(0,14\) \
        -DATWINC15X0_PARAM_RESET_PIN=GPIO_PIN\(0,27\) 
        -DATWINC15X0_PARAM_IRQ_PIN=GPIO_PIN\(1,9\) \
        -DATWINC15X0_PARAM_CHIP_EN_PIN=GPIO_PIN\(0,28\) \
        -DATWINC15X0_PARAM_WAKE_PIN=GPIO_PIN\(1,8\)' \
make BOARD=arduino-mkr1000 -C examples/gnrc_networking flash
> ifconfig
Iface  7  HWaddr: F8:F0:05:F5:E1:50  Link: up 
L2-PDU:1500 MTU:1440  HL:255  RTR  
RTR_ADV  
Source address length: 6
Link type: wireless
inet6 addr: fe80::faf0:5ff:fef5:e150  scope: link  VAL
inet6 addr: 2003:c1:e703:555:faf0:5ff:fef5:e150  scope: global  VAL
inet6 group: ff02::2
inet6 group: ff02::1
inet6 group: ff02::1:fff5:e150

          Statistics for Layer 2
          Statistics for IPv6

I don't know why the output of `ifconfig' is not correctly indented. On ESP32 the indentations are correct.

Anyway, the driver works with mkr1000 and we could enable WiFi in board definition once the driver has been finished and is merged.

@aabadie
Copy link
Contributor

aabadie commented Mar 30, 2020

Well done @gschorcht!

I don't know why the output of `ifconfig' is not correctly indented

I think this problem comes from the stdio over USB layer. For some reasons, the \t character is dropped.

@benpicco
Copy link
Contributor

@aabadie Good news. Worked for me for mkr1000 with following configuration

#define ATWINC15X0_PARAM_SPI         SPI_DEV(1)
#define ATWINC15X0_PARAM_SSN_PIN     GPIO_PIN(0,14) 
#define ATWINC15X0_PARAM_RESET_PIN   GPIO_PIN(0,27) 
#define ATWINC15X0_PARAM_IRQ_PIN     GPIO_PIN(1,9)
#define ATWINC15X0_PARAM_CHIP_EN_PIN GPIO_PIN(0,28)
#define ATWINC15X0_PARAM_WAKE_PIN    GPIO_PIN(1,8)

better add that to the board's board.h then

@gschorcht
Copy link
Contributor Author

I think this problem comes from the stdio over USB layer. For some reasons, the \t character is dropped.

It seem that it donsn't like spaces directly after the newline. The following function is responsible for the line break and the indentation:

static unsigned _newline(unsigned threshold, unsigned line_thresh)
{
if (line_thresh > threshold) {
printf("\n ");
line_thresh = 0U;
}
return line_thresh;
}

@gschorcht
Copy link
Contributor Author

#define ATWINC15X0_PARAM_SPI         SPI_DEV(1)
#define ATWINC15X0_PARAM_SSN_PIN     GPIO_PIN(0,14) 
#define ATWINC15X0_PARAM_RESET_PIN   GPIO_PIN(0,27) 
#define ATWINC15X0_PARAM_IRQ_PIN     GPIO_PIN(1,9)
#define ATWINC15X0_PARAM_CHIP_EN_PIN GPIO_PIN(0,28)
#define ATWINC15X0_PARAM_WAKE_PIN    GPIO_PIN(1,8)

better add that to the board's board.h then

That was my intention, of course. I just wanted to try out the settings on the command line first. I also suppose the ATWINC15x0 netdev should be defined netdev_default device.

@gschorcht
Copy link
Contributor Author

#define ATWINC15X0_PARAM_SPI         SPI_DEV(1)
#define ATWINC15X0_PARAM_SSN_PIN     GPIO_PIN(0,14) 
#define ATWINC15X0_PARAM_RESET_PIN   GPIO_PIN(0,27) 
#define ATWINC15X0_PARAM_IRQ_PIN     GPIO_PIN(1,9)
#define ATWINC15X0_PARAM_CHIP_EN_PIN GPIO_PIN(0,28)
#define ATWINC15X0_PARAM_WAKE_PIN    GPIO_PIN(1,8)

better add that to the board's board.h then

And I will do the same for the Adafruit feather M0 WiFi once I have tested.

@benpicco BTW, now it's time to introduce WIFI_SSID and WIFI_PATH as common configurations 😉

@gschorcht
Copy link
Contributor Author

And for drivers and pkgs, this happens way after:

RIOT/Makefile.include

Lines 452 to 455 in c0d171a

include $(RIOTBASE)/drivers/Makefile.include
# include Makefile.includes for packages in $(USEPKG)
-include $(USEPKG:%=$(RIOTPKG)/%/Makefile.include)

Changing that order would affect too many things and I don't think that doable.

That's clear.

But I have found another trick by adding the include path to CFLAGS in pkg/atwin15x0/Makefile.

CFLAGS += -I$(PKG_BUILDDIR)/src

With this trick the driver's package include path is the first one and the compilation of the driver package works.

I know, it is tricky and breaks the usual handling of include paths, but it helps. What do you think?

@aabadie
Copy link
Contributor

aabadie commented Jun 25, 2020

What do you think?

Since this is only done for the package itself it shouldn't have too much impact. So I think that's ok.

@gschorcht
Copy link
Contributor Author

@aabadie I had to change the dependencies and the include order of the driver header files a bit to make LLVM happy. The reason was a forward type declaration that requires C11.

I'm a bit surprised that the compilation worked with -std=c99 for all platforms but not for LLVM.
I'm wondering why we still use -std=c99 by default even though all our platform toolchains support C11? Wasn't there a PR which switches to C11 by default (@maribu)?

@aabadie
Copy link
Contributor

aabadie commented Jun 26, 2020

all our platform toolchains support C11?

I don't think that's the case for AVR.

@gschorcht gschorcht 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 Jun 26, 2020
@gschorcht
Copy link
Contributor Author

@aabadie Compilation succeed now. May I squash?

@aabadie
Copy link
Contributor

aabadie commented Jun 26, 2020

May I squash?

Yes please !

@gschorcht gschorcht force-pushed the drivers/atwinc15x0 branch from cceb6dc to 009daf7 Compare June 26, 2020 08:48
@gschorcht
Copy link
Contributor Author

@aabadie Should I add proactively a driver/atwinc15x0/Makefile.include regarding PR #14369? It shouldn't have any effect before PR #14369 is merged but it will avoid conflicts if this PR is merged befor PR #14369.

@aabadie
Copy link
Contributor

aabadie commented Jun 26, 2020

Should I add proactively a driver/atwinc15x0/Makefile.include regarding PR #14369?

Don't worry about #14369, I can rebase it later :)

Comment on lines 18 to 20
"$(MAKE)" MODULE=driver_atwinc15x0 -C $(PKG_BUILDDIR)/src/driver/source -f $(RIOTBASE)/Makefile.base
"$(MAKE)" MODULE=driver_atwinc15x0_common -C $(PKG_BUILDDIR)/src/common/source -f $(RIOTBASE)/Makefile.base
"$(MAKE)" MODULE=driver_atwinc15x0_spi_flash -C $(PKG_BUILDDIR)/src/spi_flash/source -f $(RIOTBASE)/Makefile.base
Copy link
Contributor

Choose a reason for hiding this comment

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

#14289 was merged so this Makefile must be adapted once again:

Suggested change
"$(MAKE)" MODULE=driver_atwinc15x0 -C $(PKG_BUILDDIR)/src/driver/source -f $(RIOTBASE)/Makefile.base
"$(MAKE)" MODULE=driver_atwinc15x0_common -C $(PKG_BUILDDIR)/src/common/source -f $(RIOTBASE)/Makefile.base
"$(MAKE)" MODULE=driver_atwinc15x0_spi_flash -C $(PKG_BUILDDIR)/src/spi_flash/source -f $(RIOTBASE)/Makefile.base
"$(MAKE)" -C $(PKG_SOURCE_DIR)/src/driver/source -f $(RIOTBASE)/Makefile.base MODULE=driver_atwinc15x0
"$(MAKE)" -C $(PKG_SOURCE_DIR)/src/common/source -f $(RIOTBASE)/Makefile.base MODULE=driver_atwinc15x0_common
"$(MAKE)" -C $(PKG_SOURCE_DIR)/src/spi_flash/source -f $(RIOTBASE)/Makefile.base MODULE=driver_atwinc15x0_spi_flash

(I also propose to move the MODULE=xxx at the end of the line, it's purely a cosmetic change regarding the build output)

Copy link
Contributor Author

Choose a reason for hiding this comment

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

OK

Copy link
Contributor Author

Choose a reason for hiding this comment

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

May I squash these changes directly to avoid an extra loop in the long, long CI compilation queue?

Copy link
Contributor

Choose a reason for hiding this comment

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

May I squash these changes directly to avoid an extra loop in the long, long CI compilation queue?

Sure, let's save some CI time.

@gschorcht gschorcht force-pushed the drivers/atwinc15x0 branch from 009daf7 to cc2277a Compare June 26, 2020 10:18
@gschorcht gschorcht force-pushed the drivers/atwinc15x0 branch from cc2277a to 4b38d37 Compare June 26, 2020 10:39
Copy link
Contributor

@aabadie aabadie left a comment

Choose a reason for hiding this comment

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

Tested again on arduino-mkr1000 and it still works.

Code changes are now good enough to be merged.

ACK

Thanks for the awesome work @gschorcht !

@aabadie aabadie merged commit 81b7709 into RIOT-OS:master Jun 26, 2020
@gschorcht
Copy link
Contributor Author

@aabadie Thanks for you patience, the reviewing, testing and merging.

Now that we have more than one WiFi network dev, we should pick up the ball again to expose the WiFi configuration via Kconfig, as started in #13135 (@leandrolanzieri).

@gschorcht gschorcht deleted the drivers/atwinc15x0 branch June 26, 2020 13:56
@miri64 miri64 added this to the Release 2020.07 milestone Jun 29, 2020
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Area: drivers Area: Device drivers Area: network Area: Networking CI: ready for build If set, CI server will compile all applications for all available boards for the labeled PR Type: new feature The issue requests / The PR implemements a new feature for RIOT
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants