-
Notifications
You must be signed in to change notification settings - Fork 231
feat: add w5500 driver #788
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
base: dev
Are you sure you want to change the base?
Conversation
w5500/io.go
Outdated
| waitTime := 500 * time.Microsecond | ||
| for { | ||
| if !deadline.IsZero() && time.Now().After(deadline) { | ||
| if debugging(debugDetail) { |
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.
Is this necessary? I'd suggest dropping all debug code, adds dependency from fmt -- and this is huge.
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.
Removed. I am still using fmt for Errorf, but I can refactor this out if you need.
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.
Yes, I'd prefer you remove all fmt usage from driver.
Look, I did this simple change in your example to trigger an error:
IP: netip.AddrFrom4([4]byte{192, 168, 1, 2}),
SubnetMask: netip.AddrFrom4([4]byte{255, 255, 255, 0}),
Gateway: netip.AddrFrom4([4]byte{192, 168, 1, 1}),
+ MaxSockets: 20,
})
netdev.UseNetdev(eth)
And the result is this:
$ tinygo build -target=pico -size=full ./examples/w5500
code rodata data bss | flash ram | package
------------------------------- | --------------- | -------
0 69 3 2 | 72 5 | (padding)
1571 0 9 22 | 1580 31 | (unknown)
240 0 0 0 | 240 0 | /Users/ysoldak/code/tinygo/targets
1862 0 0 0 | 1862 0 | C compiler-rt
0 168 0 0 | 168 0 | C interrupt vector
80 0 0 0 | 80 0 | C picolibc
0 0 0 4096 | 0 4096 | C stack
340 0 0 0 | 340 0 | Go interface assert
534 0 0 0 | 534 0 | Go interface method
0 2012 0 0 | 2012 0 | Go types
426 0 0 0 | 426 0 | cmp
0 0 44 0 | 44 44 | context
78 0 0 0 | 78 0 | device/arm
72 0 0 0 | 72 0 | device/rp
18 0 0 0 | 18 0 | errors
8998 221 28 0 | 9247 28 | fmt
24 0 0 0 | 24 0 | internal/binary
44 0 0 0 | 44 0 | internal/byteorder
754 21 0 0 | 775 0 | internal/fmtsort
140 2 0 0 | 142 0 | internal/itoa
5580 772 48 0 | 6400 48 | internal/reflectlite
560 86 0 0 | 646 0 | internal/task
2152 79 4 330 | 2235 334 | machine
488 0 0 7 | 488 7 | machine/usb/cdc
0 0 93 0 | 93 93 | machine/usb/descriptor
120 25 0 0 | 145 0 | main
294 0 0 0 | 294 0 | math/bits
6 32 0 8 | 38 8 | net
1174 52 0 8 | 1226 8 | net/netip
0 0 0 0 | 0 0 | os
24 0 0 0 | 24 0 | reflect
7266 788 9 771 | 8063 780 | runtime
980 0 0 0 | 980 0 | runtime/volatile
2942 0 0 0 | 2942 0 | slices
7884 15957 1338 0 | 25179 1338 | strconv
116 0 0 0 | 116 0 | sync
158 0 0 0 | 158 0 | sync/atomic
0 11 0 0 | 11 0 | tinygo.org/x/drivers/netdev
1092 86 0 0 | 1178 0 | tinygo.org/x/drivers/w5500
870 288 0 0 | 1158 0 | unicode/utf8
220 0 0 12 | 220 12 | unique
------------------------------- | --------------- | -------
47107 20669 1576 5256 | 69352 6832 | total
Compare with compile result before change:
$ tinygo build -target=pico -size=full ./examples/w5500
code rodata data bss | flash ram | package
------------------------------- | --------------- | -------
0 44 1 2 | 45 3 | (padding)
1295 0 11 22 | 1306 33 | (unknown)
240 0 0 0 | 240 0 | /Users/ysoldak/code/tinygo/targets
1484 0 0 0 | 1484 0 | C compiler-rt
0 168 0 0 | 168 0 | C interrupt vector
80 0 0 0 | 80 0 | C picolibc
0 0 0 4096 | 0 4096 | C stack
264 0 0 0 | 264 0 | Go interface assert
204 0 0 0 | 204 0 | Go interface method
0 1556 0 0 | 1556 0 | Go types
0 0 44 0 | 44 44 | context
78 0 0 0 | 78 0 | device/arm
74 0 0 0 | 74 0 | device/rp
24 0 0 0 | 24 0 | internal/binary
44 0 0 0 | 44 0 | internal/byteorder
140 2 0 0 | 142 0 | internal/itoa
4162 576 48 0 | 4786 48 | internal/reflectlite
560 86 0 0 | 646 0 | internal/task
2192 79 4 330 | 2275 334 | machine
488 0 0 7 | 488 7 | machine/usb/cdc
0 0 93 0 | 93 93 | machine/usb/descriptor
118 25 0 0 | 143 0 | main
8 0 0 0 | 8 0 | math/bits
6 32 0 8 | 38 8 | net
1154 36 0 8 | 1190 8 | net/netip
0 0 0 0 | 0 0 | os
7064 800 9 771 | 7873 780 | runtime
978 0 0 0 | 978 0 | runtime/volatile
1100 2062 1338 0 | 4500 1338 | strconv
158 0 0 0 | 158 0 | sync/atomic
0 11 0 0 | 11 0 | tinygo.org/x/drivers/netdev
1190 0 0 0 | 1190 0 | tinygo.org/x/drivers/w5500
396 288 0 0 | 684 0 | unicode/utf8
178 0 0 12 | 178 12 | unique
------------------------------- | --------------- | -------
23679 5765 1548 5256 | 30992 6804 | total
compare flash sizes, check fmt and strconv sizes.
w5500/netdev.go
Outdated
| if err := d.listen(sockn); err != nil { | ||
| return errors.New("could not set socket to listen: " + err.Error()) | ||
| } | ||
| break |
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.
Sorry, missed this one. Otherwise LGTM from me and ✅
| break |
ysoldak
left a comment
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.
LGTM, one tiny nit.
|
@deadprogram @soypat shall be fine to merge, what you think, guys? |
soypat
left a comment
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.
Wow! Looks much better than the first time I looked at it. Good job with the guidance @ysoldak ! Lets get it merged!
| IP: netip.AddrFrom4([4]byte{192, 168, 1, 2}), | ||
| SubnetMask: netip.AddrFrom4([4]byte{255, 255, 255, 0}), | ||
| Gateway: netip.AddrFrom4([4]byte{192, 168, 1, 1}), |
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.
Very nice! netip is the best!
This adds a driver for the w5500 ethernet chip:
https://docs.wiznet.io/img/products/w5500/W5500_ds_v110e.pdf
While it uses interrupts, it does not use the interrupt pin as there is currently no way to implement a timed wait (something akin to a Futex). For this reason it is not as fast as it could be.
It also does not attempt to tackle DNS, just copping out and allowing the user to set a resolver when they configure the device.
I tried to abide by your Driver guide, let me know if anything is amiss.