Client driver for WiFiNINA firmware

[bgould]

An alternative to the ESP-AT firmware for ESP8266 and ESP32 is nina-fw from Arduino: https://github.com/arduino/nina-fw

This is the default firmware on ESP chip of the Arduino Nano 33 IOT as well as a number of Adafruit boards and daughterboards, having a driver for it would obviate the need to reflash the ESP chip and simplify the experience of using these boards with TinyGo. Also, this firmware uses 8MHz SPI instead of UART which should be faster could eventually take advantage of DMA.

Currently WIP; code for framing messages to the ESP chip according the firmware's protocol and a number of commands are working. This initial commit has enough functionality implemented to initialize the SPI communication, scan for and connect to wifi networks, and read connection status and IP information.

Next steps will be to get connection and data transfer working with an API that matches espat/net with the goal that eventually users can easily choose between either implementation without having to rewrite their code.

[deadprogram]

This is going to be very useful, thanks for working on it @bgould

[bgould]

I've implemented enough at this point to connect to wifi and open a TCP connection and send data. The code needs some serious cleanup, which I will do, but I wanted to get something pushed in order to get some feedback on how the net package can be implemented. I copied the espat/net package and refactored it so that it relies on a "DeviceDriver" interface with a few methods to make it easier to switch between implementations.

To wit:

https://github.com/tinygo-org/drivers/blob/03adeaccb49a5feef5e69b76bdb3359565b627d0/wifinina/net/device.go
https://github.com/tinygo-org/drivers/blob/03adeaccb49a5feef5e69b76bdb3359565b627d0/wifinina/tcp.go

drivers/examples/wifinina/tcpclient/main.go

Line 66 in 03adeac

net.UseDriver(adaptor.NewDriver())

Do you think this is the correct approach, and if so should the "net" package be moved out of the espat package?

[bgould]

@deadprogram

I think this is starting to get close to being ready for an initial release. Known issues that work in espat but not with wifinina include:

1. TLS connections - these aren't working because I haven't put in any effort to make them work yet. Opening a TLS connection using the nina-fw protocol is only slightly different than regular TCP connections, so I do not anticipate that this will take very long when I get around to it.
2. UDP connections - pretty much the same state as TLS, except that it will probably require a little bit more work because of the differences in the API
3. MQTT subscriptions - the mqttsub example from espat works at first if you replace the espat/* packages with the wifinina/* equivalents, but it seems like the subscription stops working after a little while, and I have not tried to track down what is happening with that yet.

The wifinina/mqtt, wifinina/net, and wifinina/tls packages are copied almost verbatim from the latest version of espat, except for the "DeviceDriver" abstraction I mentioned in my previous comment. @aykevl and I chatted briefly a little while back about this, and I think his proposal would be a good long-term goal, that being the ability to compile an equivalent of the standard net package (https://gophers.slack.com/archives/CDJD3SUP6/p1575055023499200):

We already override some standard library packages in TinyGo (like the os package), the net package should be overridden in a similar way.
My proposal would be:
Make a custom implementation of the net package, similar to how the os and sync packages are re-implemented.
By default, any network operation in the net package will fail with a "no network connection available".
Add a function that sets/adds a network driver, perhaps with a routing prefix (like "send 0.0.0.0/32 to this driver", "send 10.0.3.0/8 to this driver", "send [::] to this driver"). Once registered, network operations will use the appropriate driver for the task.

That means a bit more setup is required, but I think that's unavoidable.
net.AddDriver(driver *net.Driver)

In my opinion, if we are going to do that I think it might make sense to continue to iron out the "net" package in the drivers repo so that we can iterate more quickly and get the net.Driver interface into a stable state before bringing it into the main TinyGo repository. Towards that end, I'd like to propose that we do the following as first steps:

1. Move the drivers/espat/net, drivers/espat/tls, and drivers/espat/mqtt packages to drivers/net, drivers/net/mqtt, and drivers/net/tls (or drivers/crypto/tls, but I'm not totally sure that makes sense here since we only have a very small subset of functionality).
2. Update those packages to use a net.Driver interface instead of *espat.Device directly. The DeviceDriver interface I'm currently using in this PR can be used as a starting point, since *espat.Device already implements it and the wifinina package provides an implementation as well.
3. If backward compatibility is required for existing users of espat/mqtt, espat/net, etc, the existing packages can likely be deprecated but left in place for the time being, which would provide some stability for existing espat applications. I'm not sure if this is necessary or not but it is an option.
4. I also would like to propose that we consider developing a standard API for managing WiFi connections similar in spirit to https://www.arduino.cc/en/Reference/WiFi to make it easier to switch between implementations and check/repair connection status in a standard way.

If we agree on the above steps, I can open a new PR with those changes, and then once merged I can rebase this PR against it (which should reduce the size of this one a lot and I think make it more approachable from a review standpoint).

Please advise about the above approach, thank you.

[deadprogram]

Here is some feedback:

Move the drivers/espat/net, drivers/espat/tls, and drivers/espat/mqtt packages to drivers/net, drivers/net/mqtt, and drivers/net/tls (or drivers/crypto/tls, but I'm not totally sure that makes sense here since we only have a very small subset of functionality).

We for sure want to share as much implementation as possible here. There are yet other WiFi chipsets that we want to support that have similar implementation patterns, such as the ATECC608A family.

Update those packages to use a net.Driver interface instead of *espat.Device directly. The DeviceDriver interface I'm currently using in this PR can be used as a starting point, since *espat.Device already implements it and the wifinina package provides an implementation as well.

This is inevitable implementation detail that we probably cannot avoid.

If backward compatibility is required for existing users of espat/mqtt, espat/net, etc, the existing packages can likely be deprecated but left in place for the time being, which would provide some stability for existing espat applications. I'm not sure if this is necessary or not but it is an option.

Once we declare a 1.0 release, we are going to want to make some backward compatability guarantees, but this package is not at that point yet. What I am saying is that needed API changes are still acceptable, IMO.

I also would like to propose that we consider developing a standard API for managing WiFi connections similar in spirit to https://www.arduino.cc/en/Reference/WiFi to make it easier to switch between implementations and check/repair connection status in a standard way.

We probably want to define an interface, and then implement it using the lower level APIs of whichever WiFi chips we will support.

Anyhow, these are all excellent suggestions, I am happy to help out as well to make it happen!

[tcpipchip]

Hi, plans to have BLE commands access ? Or only wifi ?

[deadprogram]

Now that #106 has been merged, perhaps this PR can be gotten ready to land? @bgould it would be great to get this into the release coming this week... 🙏

[tcpipchip]

Nice! Thank you!

[tcpipchip]

Btw, wifiNina works with Arduino Uno ? (atmega328p)

[bgould]

@tcpipchip

Hi, plans to have BLE commands access ? Or only wifi ?

I didn't have any plans to work on a BLE driver at this time; I looked briefly at the Arduino library that exists for this and it looks pretty different than the protocol for wifi so there might not be a log that can be reused from the work I've already done... I could be wrong about that though

Btw, wifiNina works with Arduino Uno ? (atmega328p)

I haven't tried this, so it probably doesn't work well. In theory there is no reason that it shouldn't work, except that I did not really make the memory buffers fit into the small amount of RAM that chip has. Also I don't think TinyGo on AVR has garbage collection so that could be a problem.

[bgould]

@deadprogram I should be able to get this rebased and ready to merge in the next few days, at least without UDP and possibly not TLS for now. I would like to fix whatever bug there is that is causing MQTT subscriptions to break after a little while, which I think shouldn't take too long. I'll work on that and also any remaining cleanup to get this ready to merge

[deadprogram]

The MQTT subscriptions bug is probably in my implementation, and if you fixed that along the way I certainly would be most grateful! ;)

[bgould]

After rebasing with #106 I'm not noticing the same issue I saw before with MQTT subscriptions being dropped, so I am removing (WIP) from the title and considering this ready to merge. There are still features I'd like to add and I'm sure some things will need to be fixed along the way, but I think this is an ok starting point.

[deadprogram]

This is so very close to landing!

I suggest adding the new examples to the smoke tests here: https://github.com/tinygo-org/drivers/blob/dev/Makefile#L10

Also, add to https://github.com/tinygo-org/drivers/blob/dev/README.md#currently-supported-devices

Then we merge.

[bgould]

Test failure was due to my inclusion of the MQTT examples in the smoke tests; since these have an external dependency it was failing. It could be fixed I think by adding a go get when running the smoke tests, but I just removed them for now rather than mess with that. There are still 2 other wifinina examples to built in smoke tests.

[deadprogram]

@bgould I cannot thank you enough for this contribution. It will make the onboarding process for developers so much easier, and thanks to the SPI interface it should be a lot faster as well.

For developers with a NINA co-processor on the same physical board, in particular those with the WiFiNINA code already installed, it will make a big difference.

Thank you also for all the changes getting this PR ready to merge... which I am going to squash/merge now!