-
Notifications
You must be signed in to change notification settings - Fork 196
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
Netdev #537
Netdev #537
Conversation
Build fails because this PR is dependent on tinygo-org/tinygo#3614. |
Build fails because this PR is dependent on tinygo-org/tinygo#3614. |
I just tested this branch with the code from tinygo-org/tinygo#3704 on my pyportal These examples all worked perfectly, I did not have a chance to test any others yet. http-get
ntpclient
mqttclient
webserver
|
This branch here https://github.com/tinygo-org/drivers/tree/scottfeldman-netdev3 resolvs the merge conflicts in your PR @scottfeldman against the latest I would like to see this PR merged along with tinygo-org/tinygo#3704 for the next TinyGo release. |
|
||
err = client.HandleNext() | ||
if err != nil { | ||
log.Fatal("handle next: ", err) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Maybe this is not an ideal use of HandleNext
as this could block during a conn.Read()
call until a packet is received. This could potentially deadlock if the net.Conn
has no timeout set.
Ideally HandleNext()
is called in its own goroutine. I understand this is limiting for certain microcontrollers like AVR architecture... but those uCs would not have a TCP stack anyways
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
We can add a read deadline on the transport:
+ conn.SetReadDeadline(time.Now().Add(10*time.Second))
err = client.HandleNext()
if err != nil {
log.Fatal("handle next: ", err)
I'm not sure what timeout value to use. 1 second is good enough for my testing, but I see 10secs used for timeout in earlier contexts, so using 10sec.
For testing, I tried setting the deadline to 1 Millisecond. I get the expected timeout i/o failure:
ClientId: tinygo-client-GMWJGSAPGA
Connecting to MQTT broker at test.mosquitto.org:1883
Subscribed to topic cpu/freq
1970/01/01 00:00:14 handle next: read tcp <nil>->91.121.93.94:1883: i/o timeout
Does this solution look good to you?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Hmmm- as a simple example I think it's fine as is. A better alternative would be to leave a link to more complete examples at the repository for natiu: https://github.com/soypat/natiu-mqtt
var ( | ||
ssid string | ||
pass string | ||
broker string = "test.mosquitto.org:1883" | ||
topic string = "cpu/freq" | ||
) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
What are thoughts on defining netdev
here as an interface and setting it in init
functions in their respective board-specific files?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Not sure I follow the question. Can you work up an example?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
changing main.go to:
var (
ssid string
pass string
broker string = "test.mosquitto.org:1883"
topic string = "cpu/freq"
netdev netdev.Netdev // Do we have this interface defined anywhere in drivers?
)
Putting netdev
declaration inside init
function as an assignment (espat.go as an example)
func init() {
netdev = espat.New(&cfg)
}
The above code depends on having the Netdev interface defined somwhere.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I tried this:
diff --git a/examples/net/tcpclient/main.go b/examples/net/tcpclient/main.go
index 76fd562..63b3d92 100644
--- a/examples/net/tcpclient/main.go
+++ b/examples/net/tcpclient/main.go
@@ -14,12 +14,15 @@ import (
"machine"
"net"
"time"
+
+ "tinygo.org/x/drivers"
)
var (
ssid string
pass string
addr string = "10.0.0.100:8080"
+ netdev drivers.netdever
)
var buf = &bytes.Buffer{}
diff --git a/examples/net/tcpclient/rtl8720dn.go b/examples/net/tcpclient/rtl8720dn.go
index 94d5277..fe6b823 100644
--- a/examples/net/tcpclient/rtl8720dn.go
+++ b/examples/net/tcpclient/rtl8720dn.go
@@ -26,4 +26,6 @@ var cfg = rtl8720dn.Config{
WatchdogTimeout: time.Duration(20 * time.Second),
}
-var netdev = rtl8720dn.New(&cfg)
+func init() {
+ netdev = rtl8720dn.New(&cfg)
+}
But it's a no-go because netdever isn't exported by drivers package:
+ tinygo flash -monitor -target wioterminal -stack-size=4KB -size short -ldflags '-X "main.ssid=test" -X "main.pass=testtest"' /home/sfeldma/work/tinygo-drivers/examples/net/tcpclient/
# tinygo.org/x/drivers/examples/net/tcpclient
examples/net/tcpclient/main.go:25:17: netdever not exported by package drivers
examples/net/tcpclient/main.go:34:19: netdev.NetConnect undefined (type drivers.netdever has no field or method NetConnect)
@scottfeldman Heads up: new version v0.5.1 of natiu-mqtt is out with huge bugfixes, new features and minor API changes. |
@soypat Thanks for the heads-up. I pulled and tested v0.5.1 natiu-mqqt against the examples/net/mqttclient/natiu. No issues, looks good. |
Getting MAC before getting FW version was returning garbage for MAC addr. Getting MAC after gives correct MAC addr.
Found while testing Rear/Write deadlines. Some driver where missing checks for deadline. Also, check for expired deadlines by simply testing with time.Now is after deadline, rather than calculating a duration until deadline expires.
Add new MQTT client example using natiu-mqtt
This moves the netdev/netlink namespace out of drivers and into their own packages. Also, defines netdev and netlink as L3/L4 and L2 OSI layers, respectively. Move some L3 functionality from netlink to netdev (GetIPAddr). For netlink, add ConnectParams for NetConnect to pass in L2 connection parameters (ssid, pass, auth_type, etc). Also adds connection mode (STA, AP, etc). For netlink, add SendEth and RecvEthFunc funcs to handle L2 send/recv of Ethernet pkts.
…package Each example/net example had duplicate code to init each supported wifi device. Move that device-specific init code to it's own package (netlink/probe) and let examples call netlink/probe to init the devices. This eliminates a lot of duplicate code, and makes the examples eaiser to follow. netlink/probe does not lock the app into using it; it's there as a conveince, but most will probably use it. The alternate option is to open-code the device-specific init code in the app.
Update the driver to use the new netdev/netlink packages: - Migrate to the new ConnectParams for NetConnect to clean up passing in L2 connection parameters - Add stubbed-out L2 netlink funcs SendEth/RecvEthFunc - Use exports from netdev/netlink package, moved from drivers
Use the new netdev/netlink packages in the examples/net: - Call netlink/probe to load appropriate driver using go:build tags magic - Use new ConnectParams to pass in L2 connect params to driver - go:build tag example for exact targets where example is known to run/pass
This adds three new L2 (intermediate) drivers for bridge/bond/vlan. Theses drivers are incomplete but illustrate how to stack L2 drivers. Here are some examples of how these driver would stack with the cy243439 driver: Bridge: bridge two cyw43 devices together, connecting the two LANs: cyw43_0 := cyw43439.NewDevice(...) cyw43_1 := cyw43439.NewDevice(...) bridge := NewBridge([]netlink.Netlinker{cyw43_0, cyw43_1}) stack := tcpip.NewStack(bridge) netdev.UseNetdev(stack) Bond: bond two cyw43 devices together, creating one logical device. The first physical device is primary, the second is backup (fail-over) device: cyw43_0 := cyw43439.NewDevice(...) // primary cyw43_1 := cyw43439.NewDevice(...) // secondary (backup) bond := NewBond([]netlink.Netlinker{cyw43_0, cyw43_1}) stack := tcpip.NewStack(bond) netdev.UseNetdev(stack) Vlan: add tagged VLAN ID=100 to cyw43: cyw43 := cyw43439.NewDevice(...) vlan100 := NewVlan(100, cyw43) stack := tcpip.NewStack(vlan100) netdev.UseNetdev(stack)
Add basis for TCP/IP stack. The stack implements Netdever interface for the top-end, and calls into a Netlinker interface on the bottom-end. Each TCP/IP stack instance represents a L3/L4 endpoint with an IP address. The Netlinker is bound when creating the stack. E.g.: spi, cs, wlreg, irq := cyw43439.PicoWSpi(0) cyw43 := cyw43439.NewDevice(spi, cs, wlreg, irq, irq) stack := tcpip.NewStack(cyw43) netdev.UseNetdev(stack) Here, the cyw43439 driver is the Netlinker for the stack. The new stack is a Netdever, so we tell the "net" package to use the stack as the netdev. The stack manages L3/L4 socket connections, ultimately calling into the Netlinker to send/recv L2 Ethernet pkts.
Scott, could we hold off on adding the netlink changes until we'e used the interface with the cyw43439? It would also help lessen the cognitive load of this PR, which has become quite hard to parse mentally. Im actually not sure if the netlink interface is final too... |
The only changes in netlink in this PR is the addition of two methods to the interface for Send/Recv Eth pkts. And I have high confidence that those are the right additions. I'd like to see this PR reviewed and merged to drivers:dev sooner than later. It's been here since Feb 15th. Holding it off until cyw43439 is done is the tail wagging the dog. |
I'll back out the netlink changes and push to this PR, tomorrow. |
This tinygo-drivers PR must be coordinated with tinygo and tinygo-net PRs:
|
@scottfeldman can you please test this branch against the latest updates in the tinygo-org/tinygo#3704 to make sure things are still working? |
This is now done. ❇️ |
@deadprogram, thanks for house keeping. Ok, everything looks good except for one minor issue. I tested this PR against the latest from:
example/net/* tests working on wifinina & rtl8720dn devices. The minor issue is net:dev is missing go.mod, go.sum files. You'll hit this when running make on tinygo:net-submodule-netdev3. (After submodule --init). |
@scottfeldman the |
@deadprogram oh, ok. I'm using go workspaces, so maybe I need to remove it from my go.work file? In any case, no problem leaving go.mod out of the |
@scottfeldman please see #613 |
(I'm closing the original PR #523 and submitting this one as a replacement).
This PR adds a network device driver model called netdev. There is a companion PR for TinyGo to update the "net" package here tinygo-org/tinygo#3614. This PR is for the netdev interface definitions, netdev drivers (wifinina, rtl8720dn, espat) and network examples.
See netdev documentation here: README-net.md.
Testing results are here.