docs/library/network: Enhance AbstractNIC.status to take an optional argument. #3351
Conversation
The argument is optional and if given should be a string naming the status variable to query.
|
Hi Damien, thanks for the heads up :-) It seems like a clear & sensible distinction between 'config' and 'status'. I would add that for both 'status' and 'config', a ValueError should be thrown if the param isn't meaningful for that interface. That way at least you can reliably ask a network interface for its rssi (etc) and know what it'll do. I think it'd also make sense to define the expected return from 'status' with no parameter: perhaps as a human-readable string value. (I'd be just as happy adding this sort of stuff as new attributes / methods, it's arguably more Pythonic ... but I'm happy to run with what we've got ...) |
Yes I agree a ValueError is better than a seg fault. Otherwise there's no way to know (programatically) what is and is not supported on a given port.
On esp8266 the return values are integers and correspond to the constants network.STAT_xxx. These constants were specific to the esp8266 so may need refining to be more general.
Yes that's an alternative, eg |
|
By documenting it as ValueError (which is what esp32 and esp8266 ports currently return from .config) we avoid confusion around ValueError vs NameError vs KeyError and hopefully that'll help dissuade people from catching Exception :-). If the overheads of adding methods are significant then this is the way to go, no problem, I'll bring the micropython/micropython-esp32#186 implementation into line with this as documented. |
|
As a quick comment, an earlier idea (perhaps a workaround) was to reuse .config() for that after all. I'll review thoroughly/comment later. |
Well, it's not configuration parameter, but it's fair to say that the value representing current configuration of the interface, that's why it was found to be acceptable to reuse existing .config() method. That's also rooted in Unix tradition, where ioctl() can do all things. If you think, "status" is also not the best word for that (e.g. "state" would be better), so, I don't see too big distinction between using existing .config() or overloading .status(). But if there's strong desire 8000 for the latter, why not? |
It definitely a good idea to tighten up return values for status(). As was noted, the interface for it is set - it returns integer state IDs. |
We don't have to follow that, we can be a bit more user friendly and use more meaningful names. That way it makes it a bit easier to read the code and understand what it's trying to do. In my scripts I hardly use status() at the moment because isconnected() does pretty much what you need. So adding more functionality to status() makes sense. One other reason to use status() for some things would be to have a separate namespace. One big driver of designing config() as it stands today is because there may be a large number of config options for a given NIC. Separating some of those "dynamic status" values into a separate function (ie into status()) reduces the possibility for a name clash, and also reduces the user error in typing the wrong value (when there are a large number of them). |
|
I think if there's two methods there ...`config` and `status` ... there
should be a clear separation between them so it isn't always a guess as
to which is which.
The split between `config` as stuff you're *sending* to the API
(writeable, otherwise static) and `status` as stuff you're
*receiving* from the API (read-only, dynamic) makes as much sense to
me as anything.
|
Yes, exactly. The criteria for being a status() would be that the parameter is read-only, and that it is expected in the normal course of operation to change asynchronously. RSSI and the list of connected stations satisfies this criteria. It may be possible to blacklist stations, but that's a configuration item, ie updating the blacklist, and would be performed something like: |
|
(long comment moved to where it belongs in micropython/micropython-esp32#186 )
|
I'd say that's more of a risk factor, when similarly named param may mean different things for .status() and .config(), leading to confusion. Anyway, I'm +1 on the idea. |
|
Ok, this was merged in 31550a5 |
This PR aims to further refine the spec of the AbstractNIC class so that it has a clear way to query arbitrary status variables.
In it's current form AbstractNIC.status() is quite vague in what it returns. A general wifi interface can have multiple status values and it's useful to be able to query many of them. So the proposal is to add an optional parameter to status() that can specify which value to query. The parameter is a string naming the status variable.
The existing AbstractNIC.config() can handle querying paramaters, but querying something like the list of connected stations is not really a configuration parameter. It makes more sense to use status() for that.
The distinction between status() and config() would then be:
See #2998 for initial discussion on specifying AbstractNIC, and #2785 for a discussion on being able to query the RSSI.