Circuitpython/nickzoic/703 wiznet 5500 native #1236

nickzoic · 2018-10-04T12:32:05Z

This is an implementation of #703 ... porting WizNet 5200/5500 support across from MicroPython.
Tested on Metro M0 Express and Metro M4 Express with WizNet 5500 FeatherWing.

There's no particular reason this shouldn't be adaptable for other architectures which support SPI.

This particular version doesn't use LWIP as that used up too much memory for use with M0. Instead it uses the WizNet native mode which imposes some limitations but reduces demand on the main CPU.

Example code (updated for dhcp and connected property):

import board
import busio
import wiznet

spi = busio.SPI(clock=board.SCK, MOSI=board.MOSI, MISO=board.MISO)
eth = wiznet.WIZNET5K(spi, board.D10, board.D11)
eth.connected

host = 'example.com'
import socket
fam, typ, pro, nam, sockaddr = socket.getaddrinfo(host, 80)[0]
ss = socket.socket(fam, typ, pro)
ss.connect(sockaddr)
ss.send("GET / HTTP/1.0\nHost: %s\n\n" % host)
print(ss.recv(1024))
ss.close()

tannewt

Thanks for this! I'm excited to see the progress.

Looks like a docs pass is need in addition to a bit of refactoring. You can test the docs locally with:

pip install 'Sphinx<1.8.0' sphinx-rtd-theme recommonmark
sphinx-build -E -W -b html . _build/html

You don't need a new PR to push new changes. Any changes to the PRs branch will be reflect on the PR.

Thanks!

tannewt · 2018-10-04T20:57:30Z

shared-bindings/wiznet/__init__.c

+}
+
+/******************************************************************************/
+// MicroPython bindings


Only the argument parsing done below should be in this file. The above should go in shared-module. Please also document everything below with the //| rst style so it appears in sphinx.

tannewt · 2018-10-04T20:58:04Z

shared-bindings/socket/__init__.c

+//|   :synopsis: TCP, UDP and RAW sockets
+//|   :platform: SAMD21, SAMD51
+//|
+//| XXX TODO Write Docs.


Now is the time. :-)

tannewt · 2018-10-04T21:01:16Z

shared-bindings/network/__init__.h

+        } u_param;
+        mp_uint_t u_state;
+    };
+} mod_network_socket_obj_t;


These structs should go into shared-module as well. This header should only extern it. It should also declare any functions called.

This is done to mirror the common-hal structure where each port actually implements the struct and methods.

tannewt · 2018-10-04T21:04:56Z

shared-bindings/network/__init__.c

+}
+
+void network_module_deinit(void) {
+}


These go in shared-module as well because they are C API.

tannewt · 2018-10-04T21:06:52Z

shared-bindings/network/__init__.c

+
+STATIC const mp_rom_map_elem_t mp_module_network_globals_table[] = {
+    { MP_ROM_QSTR(MP_QSTR___name__), MP_ROM_QSTR(MP_QSTR_network) },
+    { MP_ROM_QSTR(MP_QSTR_route), MP_ROM_PTR(&network_route_obj) },


What is this useful for? Can we omit the network module for now?

The network module maintains a registry of nics, and when you configure a nic (eg: construct it with wiznet.WIZNET5K()) it calls network_module_register_nic to add that nic to the list. When you call socket.socket() it calls network_module_find_nic to find an appropriate nic for this socket.

This functionality could be merged into the socket module I suppose.

... in MicroPython, the network module also collects all the network device classes in one place (they're not all compiled in, you can control that with various MICROPY_PY_WHATEVER flags)

I've split WIZNET5K out into its own 'wiznet' module, but I'm not sure that once we have a few of these we wouldn't be better collecting them into 'network' after all ...

I think network is the way to go. I don't want extra stuff in socket.

Is it useful from Python though? Maybe it should be an internal only concept for now.

Yeah, in MicroPython it exposes the nic classes. In CircuitPython all it has left is 'network.route' which lets you enumerate all the configured nics, eg:

>>> network.route()[0].ifconfig() ('10.107.1.123', '255.255.255.0', '10.107.1.1', '0.0.0.0')

not necessarily very useful

How about making it internal only for now?

tannewt · 2018-10-04T21:14:21Z

shared-bindings/wiznet/__init__.c

+    uint8_t sn_size[16] = {2, 2, 2, 2, 2, 2, 2, 2, 2, 2, 2, 2, 2, 2, 2, 2}; // 2k buffer for each socket
+    ctlwizchip(CW_INIT_WIZCHIP, sn_size);
+
+    // set some sensible default values; they are configurable using ifconfig method


Can we default to DHCP instead? My guess is that will be much more common. Maybe take ip, netmask, gateway and dns as kwargs instead in case static allocation is wanted instead.

Yeah, if you'd rather.

tannewt · 2018-10-04T21:15:14Z

shared-bindings/wiznet/__init__.c

+}
+STATIC MP_DEFINE_CONST_FUN_OBJ_1(wiznet5k_regs_obj, wiznet5k_regs);
+
+STATIC mp_obj_t wiznet5k_isconnected(mp_obj_t self_in) {


Lets have a .connected property instead since its state.

tannewt · 2018-10-04T21:17:21Z

shared-bindings/wiznet/__init__.c

+    { MP_ROM_QSTR(MP_QSTR_regs), MP_ROM_PTR(&wiznet5k_regs_obj) },
+    { MP_ROM_QSTR(MP_QSTR_ifconfig), MP_ROM_PTR(&wiznet5k_ifconfig_obj) },
+    { MP_ROM_QSTR(MP_QSTR_isconnected), MP_ROM_PTR(&wiznet5k_isconnected_obj) },
+};


Class related functions should go into a separate file (WIZNET5K.c in this case) to make it easier to find.

tannewt · 2018-10-04T21:21:22Z

shared-bindings/wiznet/__init__.c

+
+/// \method ifconfig([(ip, subnet, gateway, dns)])
+/// Get/set IP address, subnet mask, gateway and DNS.
+STATIC mp_obj_t wiznet5k_ifconfig(size_t n_args, const mp_obj_t *args) {


How about adding properties for each of these four attributes? Do they all need to be set at the once? If not, they can be read/write. Otherwise they can be read-only.

If they all need to be set at once, can we switch to a method that takes kwargs so each ip is labeled with what it is?

Yeah, they're all set together. It'd make sense for these to be kwargs I agree, it does make the code more readable.

nickzoic · 2018-10-09T12:04:44Z

(note that while the docs build, they're not actually filled in yet. Also, we need to exclude the smallest M0 boards from using this option ... not sure how you want to handle that.

tannewt · 2018-10-09T19:28:04Z

I think we'll need to enable MICROPY_PY_NETWORK and MICROPY_PY_WIZNET5K on a board-by-board basis in mpconfigboard.mk since the express builds with lots frozen in are failing. Any idea how much extra space it takes? Maybe we can get them to fit too.

nickzoic · 2018-10-11T01:46:50Z

I think we'll need to enable MICROPY_PY_NETWORK and MICROPY_PY_WIZNET5K on a board-by-board basis in mpconfigboard.mk since the express builds with lots frozen in are failing. Any idea how much extra space it takes? Maybe we can get them to fit too.

metro_m4_express build without:

276436 bytes free in flash out of 499712 bytes ( 488.0 kb ).
180088 bytes free in ram for stack out of 196608 bytes ( 192.0 kb ).

metro_m4_express build with:

265492 bytes free in flash out of 499712 bytes ( 488.0 kb ).
179892 bytes free in ram for stack out of 196608 bytes ( 192.0 kb ).

hallowing_m0_express build without:

3724 bytes free in flash out of 253696 bytes ( 247.75 kb ).
24216 bytes free in ram for stack out of 32768 bytes ( 32.0 kb ).

hallowing_m0_express build with network:

region `FLASH' overflowed by 5772 bytes

... in general, I suspect this module will be useful to only a few people ... and if we also add support for, say, the ATWINC1500 and similar then that's even more ... I think it'd be worth considering how we could have a more flexible build system so that these modules are more easily optional.

…oic/703-wiznet-5500-native

nickzoic · 2018-10-11T23:16:13Z

Sure, we can literally just remove the python API parts in shared- bindings and the shared-module part will still work as normal. Personally I think we'd be better off having the network device classes all neatly under module network as per MicroPython, but it's your call. It's an area where there is no CPython API to follow so I think it makes sense for MicroPython and CircuitPython to stay the same.

tannewt · 2018-10-12T19:31:49Z

My worry with having all of them under one module is that we won't be able to pick and choose which to have in particular builds. We've already set a guideline that feature availability is done at the module level instead of the class level. If we have it under network then we'd need to have them all there.

I think we'll add more under network in the future so we could also just leave it.

tannewt · 2018-10-12T19:32:02Z

My worry with having all of them under one module is that we won't be able to pick and choose which to have in particular builds. We've already set a guideline that feature availability is done at the module level instead of the class level. If we have it under network then we'd need to have them all there.

I think we'll add more under network in the future so we could also just leave it.

nickzoic · 2018-10-12T21:53:22Z

On Sat, Oct 13, 2018, at 06:32, Scott Shawcroft wrote: My worry with having all of them under one module is that we won't be able to pick and choose which to have in particular builds. We've already set a guideline that feature availability is done at the module level instead of the class level. If we have it under network then we'd need to have them all there.

Ah, right. Yeah, how I would have done it would be to have different classes appear under network (network.WIZNET5K, network.ATWINC1500, network.SLIP etc) depending on what you built with. It seems pretty sensible to me, but if that's not the way you want to go then I suppose separate modules is what it has to be.

I think we'll add more under network in the future so we could also just leave it.

No worries. It doesn't have to be imported if you don't want to use `network.route()` ... the registry works even if the module is not imported.

tannewt · 2018-10-16T01:53:42Z

shared-module/wiznet/wiznet5k.c

+        DHCP_time_handler();
+        int dhcp_state = DHCP_run();
+        if (dhcp_state == DHCP_IP_LEASED || dhcp_state == DHCP_IP_CHANGED) break;
+        mp_hal_delay_ms(1000);


This makes the construction of the object really slow when not plugged in. Can we do one try in the constructor and one each time the other APIs are called?

nickzoic · 2018-10-16T03:20:22Z

That's a good point. I meant to replace that with a proper background timer from run_background_tasks but never got around to it.

tannewt · 2018-10-16T21:41:22Z

Do you mind doing it now?

If not, can the loop at least check for a CTRL-C? I noticed it when trying to ctrl-c and it not responding.

nickzoic · 2018-10-17T00:30:06Z

It's already a branch, if you're happy with the thinking behind that I'll merge it into the PR branch.

tannewt · 2018-10-17T19:05:50Z

Yup! Using the background tasks to drive dhcp makes sense to me. Please merge it in.

nickzoic · 2018-10-22T11:23:20Z

(this has been held up by my day job, for reasons which are incomprehensible to anyone not following Australian politics. I'll get on with it soon.)

…o circuitpython/nickzoic/703-wiznet-5500-native

…oic/703-wiznet-5500-native

tannewt

Thanks!!

gvcp · 2019-03-19T22:52:08Z

tried to use the W5500 feather wing with a Feather M4 Express (great combination, pin compatible and allow piggypacking) with CP4 beta 5 but only with partial success:
connected, ifconfig(), socket.socket, connect seems to work, get a connection to a server socket, but when I try to send something it results in I/O errors, close doesn´t result in an error but socket isn´t really closed.

notro · 2019-03-19T23:57:32Z

I had a similar problem (which I gave up on): #1500 (comment)

nickzoic · 2019-03-20T01:07:53Z

On Wed, 20 Mar 2019, at 10:57, Noralf Trønnes wrote: I had a similar problem (which I gave up on): #1500 (comment) <#1500 (comment)>

Hmmm, I've tried a couple of times to replicate these ones but somehow my module is cursed to work. I'll go through the reports again and see if I can find some common element, perhaps my hardware is slightly differently configured.

…

-----Nick

gvcp · 2019-04-15T15:42:11Z

is there any chance that the wiznet ethernet wing will work with the feather M4 express in the final CP4 release ? the last time I tried it was with CP4 beta6 which still gave an IO error on send when done from REPL. when executing the sequence from a file it hanged and couldn´t be aborted via ctrl C.

dhalbert · 2019-04-15T15:44:14Z

@gvcp Please open an issue if something about Ethernet isn't working. Include your program and other details. Thanks!

nickzoic added 8 commits October 4, 2018 21:13

Copy wiznet drivers over from MicroPython 1.9.4

38d99b1

Fixups for wiznet 5500 driver

8670215

Copy wiznet module across from MicroPython

3c32d04

Modify wiznet module for circuitpython

27fc84a

Copy modnetwork and modusocket across from MicroPython

f4c3213

modify modnetwork and modusocket for circuitpython

d33f621

Makefile & mpconfigport for atmel-samd with wiznet

f9bda0f

change initialization method + mod_network names

15b59be

nickzoic requested a review from tannewt October 4, 2018 12:32

nickzoic mentioned this pull request Oct 4, 2018

Support for #703 ... Ethernet Featherwing on M0 Express and M4 Express platforms #1164

Closed

tannewt requested changes Oct 4, 2018

View reviewed changes

tannewt added the atmel-samd label Oct 4, 2018

tannewt added this to the Long term milestone Oct 4, 2018

nickzoic added 6 commits October 9, 2018 12:24

Change WIZNET5K.isconnected to connected property

4b767ff

Split wiznet.WIZNET5K off into its own file

6e624b9

move random mac address function into network module

9b36d33

Get DHCP working ...

a60700b

Documentation builds (though has no real content yet)

6d9c2c5

Exclude .direnv from sphinx-build

3879844

nickzoic added 8 commits October 11, 2018 13:42

Changed to only build wiznet for {feather|metro}_m[04]_express

8d75c3d

network module c api into shared-module

823ff77

update translations?

09e6127

Merge remote-tracking branch 'origin/master' into circuitpython/nickz…

aab2874

…oic/703-wiznet-5500-native

Split wiznet driver into shared-bindings vs shared-module

bb23905

update locale info

a580e87

fix doc comments, translations again

a4a0cf8

update documentation and translations again

5bb1279

tannewt reviewed Oct 16, 2018

View reviewed changes

nickzoic added 4 commits October 16, 2018 23:07

header file cleanup for wiznet

1f760bd

add mechanism for timer ticks in NICs

a15f336

fixup

4597497

timer ticks for DHCP state machine for wiznet

06894be

nickzoic added 5 commits October 25, 2018 11:29

Add "dhcp" property to turn DHCP on and off

b714f5d

Merge branch 'circuitpython/nickzoic/703-wiznet-5500-native-dhcp' int…

9c90435

…o circuitpython/nickzoic/703-wiznet-5500-native

Merge remote-tracking branch 'origin/master' into circuitpython/nickz…

f648269

…oic/703-wiznet-5500-native

Update translations

c9279e8

Fix build without network code

bbf8334

tannewt approved these changes Oct 25, 2018

View reviewed changes

tannewt merged commit fd89cdd into adafruit:master Oct 25, 2018

Circuitpython/nickzoic/703 wiznet 5500 native #1236

Circuitpython/nickzoic/703 wiznet 5500 native #1236

Uh oh!

Conversation

nickzoic commented Oct 4, 2018 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

tannewt left a comment

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

nickzoic commented Oct 9, 2018

Uh oh!

tannewt commented Oct 9, 2018

Uh oh!

nickzoic commented Oct 11, 2018 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

nickzoic commented Oct 11, 2018 via email

Uh oh!

tannewt commented Oct 12, 2018

Uh oh!

tannewt commented Oct 12, 2018

Uh oh!

nickzoic commented Oct 12, 2018 via email

Uh oh!

Choose a reason for hiding this comment

Uh oh!

nickzoic commented Oct 16, 2018 via email

Uh oh!

tannewt commented Oct 16, 2018

Uh oh!

nickzoic commented Oct 17, 2018 via email

Uh oh!

tannewt commented Oct 17, 2018

Uh oh!

nickzoic commented Oct 22, 2018

Uh oh!

tannewt left a comment

Choose a reason for hiding this comment

Uh oh!

gvcp commented Mar 19, 2019

Uh oh!

notro commented Mar 19, 2019

Uh oh!

nickzoic commented Mar 20, 2019 via email

Uh oh!

nickzoic commented Oct 4, 2018 •

edited

Loading

nickzoic commented Oct 11, 2018 •

edited

Loading