-
-
Notifications
You must be signed in to change notification settings - Fork 8.2k
Framework for coherent eth/wlan drivers in MicroPython #876
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
Comments
Commmon stuff you'll need to be able to set for each device: 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. |
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:
|
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. |
+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. |
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.
The basics of this are now implemented, in the network and usocket modules in stmhal. Some renaming and cleanup still to do. |
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. |
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. |
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). |
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 :)
wifi.scan() I guess. For ESP, |
Yep, that was the inspiration.
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. |
Yes, a JSON-inspired data type seems natural. Or how about a namedtuple? |
Hmm, that's idea too, but access to "extra" fields would be more awkward (hasattr/getattr) |
Btw, I keep forgetting that print being a function gives benefit of its usability in lambdas ;-) |
How so? You just create a different namedtuple t
8000
hat 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:
That's what you get with a consistent and well defined language :) |
Compare:
and
I also remember that hasattr pushes NLR context ;-). That's relatively minor of course. I'll try to implement named tuple later. |
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? |
@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. |
Hence #1174 :) |
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
|
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? |
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 ;-). |
So, namedtuple (and tuple as a downgradde) will be. Is the ordering and naming ok? |
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:
Based on the above, there would be following to set/query parameters: .info(param) - return value of a parameter On top of that, there will be standard actions: .connect(...) .isconnected() can be kept for convenience, but really would duplicate .info(NW_STATUS) call. That's it. |
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 ;-)
I agree, it's super ugly ;-)
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 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... |
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.
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.
My latest proposal in previous comment has single method to query any param. Yes, one by one.
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.
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.
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.
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).
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. |
Do you mean this?
I am OK with it, but I'd prefer to have kwargs in it ;-)
It is :-) |
Here is my take:
Note on efficiency: having keyword arguments like 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:
|
I agree. Having the init method as well also improves the consistency of the API across peripherals. |
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). |
If so, why bother to spend bytes on that, if users can call it themselves. If the only reason is:
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.
"All the info" == thousands of items. Potentially.
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.
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).
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, no, but formally, it's clear benefit ;-). Anyway, I know it's not going to win, so I vote it to just have pluralism ;-).
Now that's what I call ugly. We'll face similar issue when discussing #1225 (comment)
So yep, the way to have it would be:
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. |
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 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.
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 F438 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). |
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.
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). |
No it doesn't. qstrdefs[port].h is central and all additions are screened. |
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. |
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. |
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.
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.
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.
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).
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. |
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. |
I finally got time to implement the new proposed constructor for 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
Thoughts? |
@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. |
@pfalcon thanks. I guess you think it's OK, but it is still missing something. Feel free to propose changes. EDIT: |
@danicampora : Well, for WiPy, it should be ok. For MicroPython as a whole, I gave my proposals above. |
Yes, and having cleaner/easier naming of functions to get/set, and fewer such get/set functions. |
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)
There is now a relatively mature "network" module with a definition of its methods given by the docs. |
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
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:
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:Comments, thoughts, suggestions?
The text was updated successfully, but these errors were encountered: