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

Framework for coherent eth/wlan drivers in MicroPython #876

Closed
dpgeorge opened this issue Sep 26, 2014 · 52 comments
Closed

Framework for coherent eth/wlan drivers in MicroPython #876

dpgeorge opened this issue Sep 26, 2014 · 52 comments

Comments

@dpgeorge
Copy link
Member

We need a standard way of using internet modules on the pyboard that is powerful, extensible and easy to use. Powerful means being able to attach an ethernet and wifi module at the same time (and possibly multiple ones of these); extensible means we should be able to add new drivers without changing existing ones; easy to use means it should behave as expected for someone familiar with the Python socket module. Note that higher level abstractions (like http servers) can be built upon a proper functioning socket interface.

Let's first briefly review how things are done in Python and on a POSIX system. Python has the socket module which allows you to make socket objects, and do things like gethostbyname:

import socket
ip = socket.gethostbyname('micropython.org')
s = socket.socket(socket.AF_INET)

This is just a wrapper for the underlying POSIX calls.

The above calls don't know anything about the underlying internet device (be it eth0 or wlan0 for example). That, and the configuration of an IP address, is left up to the operating system.

On the pyboard, uPy is the operating system, and so we need a way (in Python) to initialise and configure the eth/wlan drivers, and also a way to select which one we want to use for, eg, a gethostbyname call.

A scenario might be: I have a pyboard with a CC3000 (wifi) and a WIZnet (eth) module connected. I need to initialise them on the correct pins, then perhaps run DHCP on the CC3000, but set a static IP for the WIZnet. I want the CC3000 to be a server and the WIZ to be a client, and I need to open sockets on both.

My proposed solution is to have a "meta socket" module which contains a class for each driver that is available. To initialise a driver, you make a new instance of the associated class. The instance object then behaves like a CPython socket module, and calling socket on this class makes a socket associated with that driver. Each driver has its own gethostbytname, etc.

Initialised drivers also register themselves with the "meta socket" module, so, for example, you can get a list of all initialised internet drivers (something like ifconfig on linux).

I thought about splitting into eth and wlan modules, but I think it makes sense to put all drivers into one module, since the application shouldn't care if a connection is wired or wireless.

Adding a new driver would be as simple as writing a new class and putting it into the "meta socket" module.

I propose calling the "meta socket" module network. An example use would be:

import network
cc3k = network.CC3k(pyb.SPI(1), pyb.Pin.board.Y5, pyb.Pin.borad.Y6)
cc3k.init('myssid', key='secret')
# cc3k now acts like a socket module
cc3k.gethostbyname('micropython.org')
s = cc3k.socket(cc3k.AF_INET, cc3k.SOCK_STREAM)

wiz = network.WIZnet5k(...)
wiz.init(...)
# wiz also acts like a socket module

network.iflist() # will return [('wlan0', cc3k), ('eth0', wiz)]

Comments, thoughts, suggestions?

@dhylands
Copy link
Contributor

Commmon stuff you'll need to be able to set for each device:
ip address, mask, gateway, and metric (all needed for static IP)

gethostbyname isn't normally a per-interface thing. It's a per-machine thing. You would normally check your local cache, check /etc/hosts, and then check with the DNS server using whatever the primary interface is (normally each interface also has a metric and the one with the lowest metric is usually considered to be the "primary" one. So while you may have a DNS server per interface, you only use the one from the primary interface.

Things that you'll need to be able to do with wlan versus eth are things like setting the SSID to connect to, encryption & password, scanning for APs.

@stinos
Copy link
Contributor

stinos commented Sep 27, 2014

I'd consider sicking with the standard sockets api and keep the network stuff seperately for a number of reasons: it's a defacto standard, has been in use for years, is well-known even to beginners and allows to simply copy-paste socket samples, or applied to uPy: write your socket stuff on unix and then move it to the board without having to change code, just add network configuration. It maintains proper seperation of underlying platform-specific configuration and the platform-agnostic socket api. Im not saying that in general because something has been done for ages it is the best thing to do, but in this case this does apply imo.

The implementation disadvantage is of course that for client connections (using connect()) you cannot specify which of the network adapters to use using sockets (for server connections() the implementation of bind() should take care of that). On most userland OS this is implemented by checking the destination address of the connections vs the routing table and choose the most suitable interface. Possible solutions for pyBoard: IIRC calling bind() defore connect() doesn't hurt, just has no effect on typical OS, so pyBoard could implement it for client sockets in the same way it does for server sockets. Might be somewhat confusing. Another option is specifying a sort of routing table in the network module: before connecting, the code can tell the network module 'when connecting to this address (or range), use this interface'. Easily allows specifying defaults, comes closest to other OS. Another option, a bit messy though, is setting the 'active' NIC for the socket calls following it. These 3 options in pseudocode:

cc3k = network.CC3k(pyb.SPI(1), pyb.Pin.board.Y5, pyb.Pin.borad.Y6)
wiz = network.WIZnet5k(...)

#bind specifies address
s = socket.socket(socket.AF_INET, socket.SOCK_STREAM)
s.bind((localAddress, ignoredLocalPort))
s.connect.. #uses NIC which has address equal to localAddress

#routing table style
network.routingTable[ default ] = cc3k;
network.routingTable[ destinationAddress1 ] = cc3k;
network.routingTable[ destinationAddress2 ] = wiz;

s.connect((destinationAddress1, port)) #uses cc3k

#active NIC syle
network.setActive( cc3k )
s = socket.socket..... #uses cc3k

network.setActive( wiz )
s = socket.socket..... #uses wiz

@dpgeorge
Copy link
Member Author

Thanks for the feedback.

The point I made about making each driver (cc3k, wiznet, ...) behave like the socket module was so that you could use existing Python code (and knowledge of socket calls). But you'd need to make 1 change, namely use the driver object instead of importing socket. I guess even this 1 change is asking too much, and we want 100% compatibility with Python in this respect.

In than case I'd vote +1 for the routing table style.

@pfalcon
Copy link
Contributor

pfalcon commented Sep 30, 2014

+1 from me for routing-table style. Then network drivers will have a C-level API, and all multiplexing (routing) among them will be handled by socket (usocket) module. But what type of this C API should be is a bit hard to tell out of top of mind. As I already mentioned, it would nice to consider drivers for both "builtin stack" (e.g. wiznet) and "raw packet" (slip would be simplest) devices.

dpgeorge added a commit that referenced this issue Sep 30, 2014
As per issue #876, the network module is used to configure NICs
(hardware modules) and configure routing.  The usocket module is
supposed to implement the normal Python socket module and selects the
underlying NIC using routing logic.

Right now the routing logic is brain dead: first-initialised,
first-used.  And the routing table is just a list of registered NICs.

cc3k and wiznet5k work, but not at the same time due to C name clashes
(to be fixed).

Note that the usocket module has alias socket, so that one can import
socket and it works as normal.  But you can also override socket with
your own module, using usocket at the backend.
@dpgeorge
Copy link
Member Author

The basics of this are now implemented, in the network and usocket modules in stmhal. Some renaming and cleanup still to do.

@dpgeorge
Copy link
Member Author

This has been implemented in stmhal/ port as above, but it has some design flaws.

You need to know the IP address for connect/bind/sendto before opening a socket, in order that you can select the right NIC from the routing table. But what about setsockopt? Eg:

import socket
s = socket.socket() # can't select NIC yet because don't know anything about the socket
s.setsockopt(...) # still can't select NIC, so what to do with options??
s.connect(('192.168.0.1', 80)) # now we can select NIC, but then options are not applied

Yes, we could store all options and apply them after the socket is opened, but that seems very wasteful in RAM and leads to very messy code.

@dpgeorge dpgeorge reopened this Jan 15, 2015
@pfalcon
Copy link
Contributor

pfalcon commented Jan 16, 2015

Can you show an example when setsockopt would need to be set up on fresh socket?

POSIXy way to resolve that would be bind() before calling .setsockopt(). Address to bind to would be injected from outside (e.g. specified by user). As that's not always possible, some alternative solution may be needed, like creating socket objects from adapter object, as you proposed in starter message.

@pfalcon
Copy link
Contributor

pfalcon commented Jan 17, 2015

Let's discuss how to name function to scan for WiFi APs. In wifi terminology that's usually called "scan", but maybe "list_ap", or "list_aps", or "scan_aps" are better names?

And then maybe what interface it should have? (Though for esp8266 it will take callback for starters).

@dpgeorge
Copy link
Member Author

Let's discuss how to name function to scan for WiFi APs. In wifi terminology that's usually called "scan", but maybe "list_ap", or "list_aps", or "scan_aps" are better names?

On linux I do "iwlist wlan0 scan". There is also an I2C.scan() method on pyboard to scan the I2C bus for device addresses. So "scan" keeps it short and simple, and if you have a wifi object it looks like "wifi.scan()" which is pretty obvious what it does :)

And then maybe what interface it should have? (Though for esp8266 it will take callback for starters).

wifi.scan() I guess. For ESP, wifi.scan(callback=lambda ap:print(ap)).

@pfalcon
Copy link
Contributor

pfalcon commented Jan 22, 2015

On linux I do "iwlist wlan0 scan"

Yep, that was the inspiration.

So "scan" keeps it short and simple,

Yes, it's also more reusable to technologies which don't use "access point" technology, like you point out with i2c. So, let's go with it.

We'll also need to agree on basic data structure for results. Implementation for esp8266 I have surely uses tuple, but that doesn't scale (different devices will supply different amounts and types of info), so apparently that should be dict with some basic keys defined.

@dpgeorge
Copy link
Member Author

so apparently that should be dict with some basic keys defined.

Yes, a JSON-inspired data type seems natural. Or how about a namedtuple?

@pfalcon
Copy link
Contributor

pfalcon commented Jan 22, 2015

Hmm, that's idea too, but access to "extra" fields would be more awkward (hasattr/getattr)

@pfalcon
Copy link
Contributor

pfalcon commented Jan 22, 2015

wifi.scan(callback=lambda ap:print(ap))

Btw, I keep forgetting that print being a function gives benefit of its usability in lambdas ;-)

@dpgeorge
Copy link
Member Author

Hmm, that's idea too, but access to "extra" fields would be more awkward (hasattr/getattr)

How so? You just create a different namedtuple that has more fields. For a given wifi NIC, the namedtuple will be fixed, I guess. But if not, and if you want real flexibility, then better go with a dict. namedtuple is a bit neater though: ap.ssid vs ap['ssid'].

Btw, I keep forgetting that print being a function gives benefit of its usability in lambdas ;-)

That's what you get with a consistent and well defined language :)

@pfalcon
Copy link
Contributor

pfalcon commented Jan 22, 2015

How so?

Compare:

if "rssi" in d:
    print(d["rssi"])

and

if hasattr(d, "rssi"):
    print(d.rssi)

I also remember that hasattr pushes NLR context ;-).

That's relatively minor of course. I'll try to implement named tuple later.

@danicampora
Copy link
Member

modwlan from the CC3200/CC3100 returns a list of tuples with the following info:

('ssid', 'bssid', security_type, rssi)

but it might be better to return a named tuple like:

(ssid='name', bssid='mac_addr', security=2, rssi=-58)

The BSSID is nice to have in case you want your device to connect to a specific AP, since you could have several APs with the same SSID.

@pfalcon is the above approach also OK for the ESP8266?

@pfalcon
Copy link
Contributor

pfalcon commented Apr 1, 2015

@danicampora: First of all, note that realistically namedtuple will take more space (even if flash) than dict - that's because namedtuple requires definition of new type, which is pretty lengthy structure.

Otherwise, we should define standard fields which should be available, at the given numeric positions, for each port/device. Then, the rest can go as port-specific extensions, to be always accessed by name.

@dpgeorge
Copy link
Member Author

dpgeorge commented Apr 1, 2015

that's because namedtuple requires definition of new type, which is pretty lengthy structure.

Hence #1174 :)

@danicampora
Copy link
Member

OK, I'll go for whatever uses less memory. But I think a dictionary is better than a non named tuple here.

Sent from my iPhone

On Apr 1, 2015, at 10:58 PM, Paul Sokolovsky notifications@github.com wrote:

@danicampora: First of all, note that realistically namedtuple will take more space (even if flash) than dict - that's because namedtuple requires definition of new type, which is pretty lengthy structure.

Otherwise, we should define standard fields which should be available, at the given numeric positions, for each port/device. Then, the rest can go as port-specific extensions, to be always accessed by name.


Reply to this email directly or view it on GitHub.

@dpgeorge
Copy link
Member Author

dpgeorge commented Apr 1, 2015

Otherwise, we should define standard fields which should be available, at the given numeric positions, for each port/device

If you want numeric positions you need to use a tuple (ie can't use a dict). Then if you also want port-specific extras to be named, then it must be a namedtuple, no?

@pfalcon
Copy link
Contributor

pfalcon commented Apr 1, 2015

Yes, bare tuple with numeric positions is nice downgrade option for really constrained ports. namedtuple is apparently the way to go, even if it'll require a bit more work and thought to implement ;-).

@danicampora
Copy link
Member

So, namedtuple (and tuple as a downgradde) will be. Is the ordering and naming ok?
(ssid='name', bssid='mac_addr', security=2, rssi=-58)

@pfalcon
Copy link
Contributor

pfalcon commented Jun 24, 2015

Ok, so we had further discussion spread over so many tickets that it's not hard to collect references. There were so many ideas, and the problem that their implementation is backlogged, then new use- or corner-cases were raised, so now it's all pretty messy. The only way to resolve it is push thru it, but as I start looking into implementing it myself, I got more "bright" ideas. So, I'll try to present them here in the hope to find "ultimate" solution.

The scope of the proposal is "network" module. It offers classes to instantiate for each network interface (or group of network interfaces). In #1319 we already established that constructors of these classes should accept only static hardware-related params (like pin numbers to which hardware is connected). This proposal further discusses interface of these classes.

Few basic premises/requirements:

  1. It's hard to predict what communication technology will be used by a particular "network" interface. It seems that common ground is at least IP, but there're two IP's - IPv4 & IPv6. IPv6 is still a novelty for many people, and yet sooner or later IPv4 will get clear "legacy" moniker and will be optimized out. Bluetooth doesn't have IP. I mean old good BT, now with 4.2 there exactly appears to be a case of IPv6 support without legacy stuff. Conclusions: any adhoc way to configure networking parameters will be just that - adhoc, and we'll need to return to the same tangled discussion again and again.
  2. @roger- in one of the tickets expressed concern "if I want to get just 1 param, why to query them all?". But you really can't win this issue. If underlyingly you query each param one by one, then it's inefficient to return them all in one go. But vice-versa, if underlyingly you have a call to get structure with lot of params, it's inefficient to query each one by one. There're 2 ways to deal with it: 1) remember our "Micro" nature is just say that we query each param on its own, saving memory for tuples in the process, and don't care about any thing else. 2) do the same, but optionally allow to query several params at once for optimization freaks.
  3. While querying params one by one is arguably ok, setting them in groups is a musthave feature - simply because it may be the case that underlying tech requires setting few at once.
  4. Then remaining question is how to encode param types. Here it's another "can't win it all" case. My personal preference is to use integers, and optional symbolic constants (can go in a different module, like implemented in Python). But then "multiple configuration" call from point 3. would be something like .config(NW_IP, "192.168.3.1", NW_NETMASK, "255.255.255.0"). Not exactly Pythonic, using keywords would be better. But the "info" call from p.2 would use strings, like .info("netmask"), and I find that not strict-typed (aka C-like) enough. What can I say? My vote is to use integer and symbolic constants - that's also the most efficient way (small ports can be void of symbolic constants and still work). But if many people thing that using kwargs + string is better, well, I'm ready to agree.

Based on the above, there would be following to set/query parameters:

.info(param) - return value of a parameter
.config(param1, value1, param2, value2, ...)

On top of that, there will be standard actions:

.connect(...)
.disconnect()

.isconnected() can be kept for convenience, but really would duplicate .info(NW_STATUS) call.

That's it.

@danicampora
Copy link
Member

@roger- in one of the tickets expressed concern "if I want to get just 1 param, why to query them all?

What's the problem with that? Think about it, how often in your software are you going to check the current configuration of the nic? once, twice (if ever)? So, it's better, IMHO to have 1 method to query several config values that aren't used very often, than to have separate methods for every param. The first is more efficient in terms of ROM, the second approach is more efficient at runtime. Anyway, the proposal was to return a named tuple which is nice, pythonic and also easier to add more fields when/if needed in the future. If people is so worried about cpu cycles then they should reconsider the idea of using an interpreted language ;-)

.config(NW_IP, "192.168.3.1", NW_NETMASK, "255.255.255.0"). Not exactly Pythonic

I agree, it's super ugly ;-)

It's hard to predict what communication technology will be used by a particular "network" interface. It seems that common ground is at least IP, but there're two IP's - IPv4 & IPv6. IPv6 is still a novelty for many people, and yet sooner or later IPv4 will get clear "legacy" moniker and will be optimized out. Bluetooth doesn't have IP. I mean old good BT, now with 4.2 there exactly appears to be a case of IPv6 support without legacy stuff. Conclusions: any adhoc way to configure networking parameters will be just that - adhoc, and we'll need to return to the same tangled discussion again and again.

ipv6 has been around for some years already, yet it's still not mainstream, and I think it will take several more years to make ipv4 obsolete. In the case of .ifconfig we could have ifconfig for ipv4 and ifconfig6 for ipv6. How often would you like to change your ifconfig that, for example, you would like to be able to only modify the IP address and leave the rest untouched?

Agreeing on an API takes aaaages, and it all comes down to making compromises. There's no perfect solution to please everyone. There are many options, we just have to choose one and learn to live with the pros and cons. One way to do it is to model it after something known, and I think we were kind of doing that when taking examples from linux (ifconfig, iwconfig for instance). I haven't made the changes on the CC3200 that we agreed upon because I am busy with setting up the integration test setup, and because I have the feeling that if I do it right away, some other ideas will pop up and I'll have to change it again, and again...

@pfalcon
Copy link
Contributor

pfalcon commented Jun 25, 2015

@roger- in one of the tickets expressed concern "if I want to get just 1 param, why to query them all?
What's the problem with that?

The problem is that number of params may be high, so more memory needed, less comforting for user to look at. Another problem is ordering of params. It's wishful thinking that we'll have stable and useful order not just eventually, but soon - alas so far we can't get even stable method names.

Think about it, how often in your software are you going to check the current configuration of the nic?

In my list, that's big of the problem - this stuff is both important, so we can't ignore it, and used rarely enough so gets always backlogged.

than to have separate methods for every param.

My latest proposal in previous comment has single method to query any param. Yes, one by one.

.config(NW_IP, "192.168.3.1", NW_NETMASK, "255.255.255.0"). Not exactly Pythonic
I agree, it's super ugly ;-)

I concur. There's nothing ugly in it, it just doesn't use Python features. That makes it look like written in C, and in mylist, there's nothing wrong with it, as it is more efficient. It's still my preferred solution. But if there will be strong opposition to do it "more Pythonic", I'm ok with it either.

we could have ifconfig for ipv4 and ifconfig6 for ipv6.
How often would you like to change your ifconfig that, for example, you would like to be able to only modify the IP address and leave the rest untouched?

You will need to invent new config function for each new technology, that's the point. And more modest guys will invent new functions for each new hardware module, like happens with esp8266.

There are many options, we just have to choose one and learn to live with the pros and cons.

Let's choose something totally generic then, like the last proposal - at least we will learn its pros and cons once and for all, instead of discovering new implication on each new field addition to some port.

One way to do it is to model it after something known, and I think we were kind of doing that when taking examples from linux (ifconfig, iwconfig for instance).

Yep, we tried, I conclude that doesn't scale for us (e.g. @dpgeorge's cc3100 driver lived happily without iwconfig() by "silently" adding stuff to ifconfig(). Me and you now trying to add iwocnfig(), more examples can be dug out).

I haven't made the changes on the CC3200 that we agreed upon because I am busy with setting up the integration test setup, and because I have the feeling that if I do it right away, some other ideas will pop up and I'll have to change it again, and again...

Makes sense and I understand that you have a lot to do (hope KS is still fun for you ;-P). I would like to resolve this finally as I feel: a) this is in backlog long enough that we forgot older "decisions" and reinvent new; b) maintainers' task is to make contributing straightforward, and with current situation, esp8266 contributors are either inventing their own stuff, or need do go thru maze of our incomplete decisions.

@danicampora
Copy link
Member

Let's choose something totally generic then, like the last proposal - at least we will learn its pros and cons once and for all, instead of discovering new implication on each new field addition to some port.

Do you mean this?

.info(param) - return value of a parameter
.config(param1, value1, param2, value2, ...)

I am OK with it, but I'd prefer to have kwargs in it ;-)

(hope KS is still fun for you ;-P)

It is :-)

@dpgeorge
Copy link
Member Author

Here is my take:

  1. Network class constructors must take hardware-specific config values as their required arguments (positional and/or keyword); this may be zero args. This then creates an instance of the peripheral. Eg wlan = network.WLAN(pin1, pin2).
  2. The constructor allows additional (optional) keyword arguments to also initialise the "soft" parameters of the peripheral. This means you can create and initialise in one line. Eg wlan = network.WLAN(pin1, pin2, mode=X).
  3. Such "soft" parameters can also be set later on using the init() method, which takes keyword arguments only, and takes the same keywords as the constructor. Underlying, the constructor just calls init() if it has additional keywords. Eg wlan.init(mode=X).
  4. Calling init() method with no arguments returns an object which contains all the info about the soft settings. This object can be an attrtuple (namedtuple). If it's really inefficient to get everything at once then this object can be a custom object that supports attr access and caches results. Eg wlan.init().mode.

Note on efficiency: having keyword arguments like init(param1=value, ...) is pretty much the same as having pairs of values like config(param1, value1, ...). They both load 2 values onto the stack for each param/value pair (the former a qstr and value, the latter an integer and value); underlying C code in both cases is very similar, it's just a list of pairs (former in a map's table, latter in an array of args). They both need strings in ROM (former for the qstr keyword, latter for the constant). Realistically I don't think it's worth accommodating the case of a port that doesn't have enough room to store qstrs to config it's peripherals, and hence needing to use integers like config(1, 2, 3, 4).

Having init() with no args return an attrtuple (namedtuple) with the same entries as used in the keywords for init() means that there are no duplicated qstrs. It's also "type safe". Also, it allows a script to dynamically query available settings by using hasattr on the result.

Note that having the constructor and init method behave this way makes network peripherals the same as UART, SPI and I2C in terms of creation and configuration.

Having the same method to query (with no params) and set (with 1 or more params) is how other methods work. Eg pyb.freq, servo.calibration.

I think the above is clean for the end user, Pythonic, and relatively consistent with everything else. It's also fairly efficient and allows general config variables.

Some concessions that could be made:

  1. If init() is not a good name then it can be config or iwconfig or whatever.

  2. If returning an attrtuple of all settings is really too much then could possibly support the following:

    a. Single string argument to init gets a setting, eg init("mode").

    b. Keywords with special value do a get, eg init(mode=QUERY). This would be "type safe" and extend to getting multiple values at the same time (returning a tuple).

@danicampora
Copy link
Member

Note that having the constructor and init method behave this way makes network peripherals the same as UART, SPI and I2C in terms of creation and configuration.

I agree. Having the init method as well also improves the consistency of the API across peripherals.

@danicampora
Copy link
Member

I pretty much agree with everything that god almighty just said ;-) (in part because I just want us to agree on a API and move on).

@pfalcon
Copy link
Contributor

pfalcon commented Jun 25, 2015

2.The constructor allows additional (optional) keyword arguments to also initialise the "soft" parameters of the peripheral.

Underlying, the constructor just calls init() if it has additional keywords. Eg wlan.init(mode=X).

If so, why bother to spend bytes on that, if users can call it themselves.

If the only reason is:

Note that having the constructor and init method behave this way makes network peripherals the same as UART, SPI and I2C in terms of creation and configuration.

Then well, obvious objection is that UART, SPI is not the same thing as a network interface. The latter is more complex, so separating stuff out is helpful. Also, SPI doesn't take name of memory chip attached to it or similar high-level params.

  1. Calling init() method with no arguments returns an object which contains all the info about the soft settings.

"All the info" == thousands of items. Potentially.

This object can be an attrtuple (namedtuple).

Ordering problem mentioned above. For example, I really considered dropping esp8266's .status() call and moving it to this init-which-is-actually-info() method. But status is kind of "head" information about interface state, so should go 1st, and then I suddenly need to move every driver's attrtuple.

If it's really inefficient to get everything at once then this object can be a custom object that supports attr access and caches results. Eg wlan.init().mode.

Few, complicated. Compare that with returning single value via .info(IP), multiple in pythonic way using [.info(x) for in in (IP, MASK, GW)], or letting driver optimize that via .info(IP, MASK, GW).

They both need strings in ROM

No, having params coded as integers doesn't require any strings in ROM, but rather a separate .py file with "IP = const(1)", etc. (Yup, those who load it now spend RAM, not ROM).

Realistically I don't think it's worth accommodating the case of a port that doesn't have enough room to store qstrs to config it's peripherals, and hence needing to use integers like config(1, 2, 3, 4).

Realistically, no, but formally, it's clear benefit ;-). Anyway, I know it's not going to win, so I vote it to just have pluralism ;-).

b. Keywords with special value do a get, eg init(mode=QUERY). This would be "type safe" and extend to getting multiple values at the same time (returning a tuple).

Now that's what I call ugly. We'll face similar issue when discussing #1225 (comment)

a. Single string argument to init gets a setting, eg init("mode").

So yep, the way to have it would be:

nic.config(mode=FOO)
mode = nic.info("mode")

That's not exactly stellar syntax-wise, but I'm growing to accept, knowing that underlyingly it's the same qstr.

But above still shows this point - as we now reduced config/query interface to simple rules, there's no need to stuff set/get actions into single method, can go into 2 for clarity. (info() can be called query() too).

Summing up: I propose to drop attrtuple requirement. (No worries it was added in vain - it's already used by os.uname() and the recommended way to return .scan() results). Then, not ovecomplicate constructor and not overcompress get/set method.

@dpgeorge
Copy link
Member Author

"All the info" == thousands of items. Potentially.

How can there realistically be 1000's of items? If there are, and each one takes order 50-100 bytes to configure, then the driver takes 50-100k! If you are saying that there might be 1000 registers then ok, but just bunch them up into a single memoryview object.

For the most case there will be only a handful of items. Maybe we need to see some actual examples from cc3200 and esp8266.

Ordering problem mentioned above. For example, I really considered dropping esp8266's .status() call and moving it to this init-which-is-actually-info() method. But status is kind of "head" information about interface state, so should go 1st, and then I suddenly need to move every driver's attrtuple.

Ordering doesn't matter if it's an attrtuple because you just access it by name only. Then it's wlan.init().status. Not being able to access the tuple reliably via index is a small concesion to make given the alternatives discussed here.

No, having params coded as integers doesn't require any strings in ROM, but rather a separate .py file with "IP = const(1)", etc. (Yup, those who load it now spend RAM, not ROM).

This is not a realistic scenario: everyone will need to configure the peripheral, and everyone will use the predefined constants, so better to put them in ROM than force users to waste RAM.

I really don't see how you can argue for something like init(PARAM, value, ...). Someone looking from afar will ask "isn't that just like a key-value pair?", yes, "then why isn't it using the builtin language feature, ie keywords?". We've put in a lot of effort to make keywords efficient, to make qstrs work, and so we should use them to make the API dead obvious.

Note that having such constants only gives you a false sense of "type safety", since you could use a different constant name that yields a valid config number, and then you'll be setting the wrong value; eg init(SOME_WRONG_BUT_SEEMINGLY_CORRECT_NAME, 123).

@pfalcon
Copy link
Contributor

pfalcon commented Jun 25, 2015

Note that having such constants only gives you a false sense of "type safety",

Well, let me put it this way: if we have integer values, we also have central registry of parameters in a header file, so any additions going thru this central registry and peer review, so hopefully there's low chance of having different names for the same param, etc. But strings are, well, - strings, everyone feels relaxed about them and mess ensues. Anyway, let's consider matter decided, majority wants keywords ;-).

I feel stronger about avoiding attrtuples though.

Ordering doesn't matter if it's an attrtuple because you just access it by name only. Not being able to access the tuple reliably via index is a small concesion to make given the alternatives discussed here.

That's not logic originally applied there IIRC. Original logic is that attributes are optional, and used as a fallback to provide "additional" fields beyond standard. So, it turns out that coming up with these standard is not so easy, and in future we may see more cases which might have been done better (e.g. missed similarity between different modules). Having all that in a structure vs just as a set of scalar values definitely adds extra dimension to complication of how it should be done right.

E.g. we already have 2 ways of encoding mac addresses - as a text representation and as bytes object. If we have precise and specific way to get mac address, e.g. .info("mac"), it's easy to see that we don't need 2 ways to encode them, the most general should be chosen, and description of that parameter should just have notice added "if you want to get textual representation of MAC address, run result thru ubinascii.hexlify(mac, ":")". But if it will be part of structure, there easily can be "other-dimensional" excuses like "but one structure is "ip, mac", another is "mac, ip", so they're already different and no problem to make them even more different". Being pointed that mac is always ,ac, there can "Ah ok, let me rename that to mac_addr." Those are "rightful" excuses to do the mess, but the biggest problem is that it will be just harder to spot such cases (and using qstrs instead of specific registry already complicates review as pointed above).

@dpgeorge
Copy link
Member Author

and using qstrs instead of specific registry already complicates review as pointed above

No it doesn't. qstrdefs[port].h is central and all additions are screened.

@pfalcon
Copy link
Contributor

pfalcon commented Jun 25, 2015

qstrdefsport.h is central place for everything port (and only single port!), and as all strings are, again, look like strings, it's easy to miss whether a particular string is just a string or actually an enumerated value of a specific type used for a specific purpose.

Anyway, what I propose is based on my assessment of current state of "network" module implementations, and to do that assessment, I over last ~month about a dozen times performed following procedure: dig out links to rendered docs for different ports, compared them pair-wise, compaired sources to the docs, compared sources to sources, compared sources to discussions to see if anything was propagated, etc. Conclusion: we have just a handful of drivers, but already have mess, and it takes "extraordinary effort" to even discover that we have a mess (for example, you likely review all commits, did you know about the case of MAC addresses above?). Based on that conclusion I came up with suggestions how to spend less effort while achieving better consistency. If all that doesn't ring a bell, well, it's sad. I'll just mark that in this direction we reached complexity we can't handle, where it's just gotta be messy from now on.

@dpgeorge
Copy link
Member Author

For consistency, I think it's not so important how the API is implemented but rather that devs actually stick to the agreed upon convention. We should decouple these things: design the API so it's Pythonic and obvious for the user, etc; then implement it consistently across ports. However the API is designed we'll need to put some effort in to making sure devs don't use "mac" here and "mac_addr" there. And that effort only needs to be spent once.

The issue is we don't have a reference to follow because the wifi drivers have evolved on their own so far. What do we actually need to configure? How about we design a mock API for wipy and esp8266 independently, with all the required config values that they need/want, and then see where we stand.

@pfalcon
Copy link
Contributor

pfalcon commented Jun 27, 2015

that devs actually stick to the agreed upon convention.

Yes, and that's how I hoped that it worked all this time (not one month by now) - that we'll discuss it and come to conclusions, then someone on the forefront of this work will implement it, to serve as actual code template for other ports. In my list, WiPy development is on this forefront, but I don't see much much of what discussed being implemented there. And I don't say that for finger-pointing, it's perfevtly clear that @danicampora has a lot of other important things to do for his port. In the meantime, we get submissions for esp8266 which either need to be ignored, rejected, or reworked. I finally take a step to lay good base for esp8266 by implementing all the latest ideas myself, take a final peek to compare WiPy and pyboard drivers, and see that not only WiPy doesn't implement latest ideas (which is know), but that it's drivers based on older ideas is inconsistent with your original implementation of pyboard drivers (which are kinda news).

So, "important [...] that devs actually stick to the agreed upon convention" doesn't work. And I don't blame devs, I blame conventions, which apparently too tangled and vague, and propose soemthing which can easily fits onto one head, so it will be easy to say what's right and what's not going forward.

We should decouple these things: design the API so it's Pythonic and obvious for the user

I don't see anything unpythonic in what I propose (again: query params one by one as a baseline), and that can be only more obvious to user than potentially long structures.

And that effort only needs to be spent once.

No, it will need to be spent again and again going forward, for each newly written or contributed drivers. So, the more complicated scheme we have, the more complicated it will be.

What do we actually need to configure?

The most generic answer is "anything", and mys scheme allows to configure anything. For example, adhoc .patch() method in some driver can be replaced with .config(patch=bytes).

How about we design a mock API for wipy and esp8266 independently, with all the required config values that they need/want, and then see where we stand.

We already have mock of API there. We already spent a lot of time discussing how it should be, with ~zero outcome so far. I already spent a whole coding session on that, usual outcome of that should be newly implemented or cleaned up functionality, but this time, outcome if zero (well, yet another proposal on how to get it right; well, final on my side). I don't see all that as sustainable or rewarding, given the context of this API (use once and forget), the fact that I don't consider working on esp8266 too beneficial as it stands, that there're other issues in WIP many of which are more important and interesting, etc.

So, the only way I see is to do this KISS and get over it.

@danicampora
Copy link
Member

In my list, WiPy development is on this forefront, but I don't see much much of what discussed being implemented there.

Apart from the things that we already know, what else do you see in the WiPy that doesn't follow what we have discussed?

On Jun 27, 2015, at 5:08 PM, Paul Sokolovsky notifications@github.com wrote:

that devs actually stick to the agreed upon convention.

Yes, and that's how I hoped that it worked all this time (not one month by now) - that we'll discuss it and come to conclusions, then someone on the forefront of this work will implement it, to serve as actual code template for other ports. In my list, WiPy development is on this forefront, but I don't see much much of what discussed being implemented there. And I don't say that for finger-pointing, it's perfevtly clear that @danicampora has a lot of other important things to do for his port. In the meantime, we get submissions for esp8266 which either need to be ignored, rejected, or reworked. I finally take a step to lay good base for esp8266 by implementing all the latest ideas myself, take a final peek to compare WiPy and pyboard drivers, and see that not only WiPy doesn't implement latest ideas (which is know), but that it's drivers based on older ideas is inconsistent with your original implementation of pyboa rd drivers (which are kinda news).

So, "important [...] that devs actually stick to the agreed upon convention" doesn't work. And I don't blame devs, I blame conventions, which apparently too tangled and vague, and propose soemthing which can easily fits onto one head, so it will be easy to say what's right and what's not going forward.

We should decouple these things: design the API so it's Pythonic and obvious for the user

I don't see anything unpythonic in what I propose (again: query params one by one as a baseline), and that can be only more obvious to user than potentially long structures.

And that effort only needs to be spent once.

No, it will need to be spent again and again going forward, for each newly written or contributed drivers. So, the more complicated scheme we have, the more complicated it will be.

What do we actually need to configure?

The most generic answer is "anything", and mys scheme allows to configure anything. For example, adhoc .patch() method in some driver can be replaced with .config(patch=bytes).

How about we design a mock API for wipy and esp8266 independently, with all the required config values that they need/want, and then see where we stand.

We already have mock of API there. We already spent a lot of time discussing how it should be, with ~zero outcome so far. I already spent a whole coding session on that, usual outcome of that should be newly implemented or cleaned up functionality, but this time, outcome if zero (well, yet another proposal on how to get it right; well, final on my side). I don't see all that as sustainable or rewarding, given the context of this API (use once and forget), the fact that I don't consider working on esp8266 too beneficial as it stands, that there're other issues in WIP many of which are more important and interesting, etc.

So, the only way I see is to do this KISS and get over it.


Reply to this email directly or view it on GitHub.

@pfalcon
Copy link
Contributor

pfalcon commented Jun 27, 2015

Apart from the things that we already know, what else do you see in the WiPy that doesn't follow what we have discussed?

Unfortunately, I don't follow WiPy development too close. When I spot anything worth communicating, I do, like that case with time.sleep() taking milliseconds. And anyway, the point of this discussion is not WiPy specifically, but unbreaking networking interfaces.

@pfalcon pfalcon changed the title Framework for coherent eth/wlan drivers on pyboard Framework for coherent eth/wlan drivers in MicroPython Jul 25, 2015
@danicampora
Copy link
Member

I finally got time to implement the new proposed constructor for WLAN. See this commit:
danicampora@0a99cf8

If no objections, I will push to MicroPython master later. Examples:

from network import WLAN
wifi = WLAN()
wifi.iwconfig(mode=WLAN.STA)
wifi = WLAN(mode=WLAN.AP, security=WLAN.WPA2, key='123456abc', channel=2, antenna=WLAN.INTERNAL)
wifi.iwconfig(channel=4)  # only change the channel, keep the rest as it is.

# get the current config (namedtuple)
wifi.iwconfig()

I also added an extra method to just get the MAC address nic.mac()

nic.ifconfig() remains as it was, it takes a 4-tuple with (ip, netmask, gateway, dns), or 'dhcp`. With no params the 4-tuple with the current config is returned.

Thoughts?

@pfalcon
Copy link
Contributor

pfalcon commented Aug 6, 2015

@danicampora : Well, I guess it's better than was before. As for ultimate solution, it seems to be hanging in the air (at least that how I see it), so doing something is again better than waiting.

@danicampora
Copy link
Member

@pfalcon thanks. I guess you think it's OK, but it is still missing something. Feel free to propose changes.

EDIT:
From what you proposed during the long discussion of this ticket, he only thing that I think my implementation doesn't cover is the option to GET config params independently, they can be SET independently though.

@pfalcon
Copy link
Contributor

pfalcon commented Aug 6, 2015

@danicampora : Well, for WiPy, it should be ok. For MicroPython as a whole, I gave my proposals above.

@pfalcon
Copy link
Contributor

pfalcon commented Aug 6, 2015

From what you proposed during the long discussion of this ticket, he only thing that I think my implementation doesn't cover is the option to GET config params independently, they can be SET independently though.

Yes, and having cleaner/easier naming of functions to get/set, and fewer such get/set functions.

danicampora pushed a commit to danicampora/micropython that referenced this issue Aug 9, 2015
Changes are based on this post:
micropython#876 (comment)

The constructor can optionally take the same params of iwconfig in
order to configure WiFi when creating the object. Params are
keyworkd only. The WiPy accepts:

- mode (int -> WLAN.AP or WLAN.STA)
- ssdi (string)
- security (int -> WLAN.OPEN, WLAN.WEP, WLAN.WPA, WLAN.WPA2)
- key (string)
- channel (int (1-11))
- antenna (int -> WLAN.INTERNAL, WLAN.EXTERNAL)
@danicampora danicampora mentioned this issue Aug 13, 2015
4 tasks
@dpgeorge
Copy link
Member Author

There is now a relatively mature "network" module with a definition of its methods given by the docs.

tannewt added a commit to tannewt/circuitpython that referenced this issue Jun 8, 2018
We now track the last time the background task ran and bail on the
PulseIn if it starves the background work. In practice, this
happens after the numbers from pulsein are no longer accurate.

This also adjusts interrupt priorities so most are the lowest level
except for the tick and USB interrupts.

Fixes micropython#516 and micropython#876
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

5 participants