-
-
Notifications
You must be signed in to change notification settings - Fork 8.4k
esp8266/network_wlan: Allow enumerating connected stations in AP mode. #16765
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
Conversation
Thanks for this addition, it's good because a few other WLAN interfaces also support Unfortunately those existing implementations don't really follow any standard about what they return! Some return a a list of 1-tuples of MAC, some return a list of IPs. We need to standardise on something and document it. What you have here, namely a list of 2-tuples of MAC and IP, seems like a good choice. The thing with a tuple is that it could be extended in the future to add new entries, and that's backwards compatible. But the first entry in the tuple should just be a bytes object of the MAC. You can easily hexlify this in Python via |
f2bae29
to
b9baf4a
Compare
That makes sense, thanks for letting me know. I admit I haven't looked at what other ports did as I thought that different ports had different WiFi capabilities exposed to user code. I'll look at other ports' code later today to see what information is available for connected stations, and see if a followup PR can be submitted to align the ports on that regard. Those changes, if made, will be put behind |
b9baf4a
to
69d07a0
Compare
Codecov ReportAttention: Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## master #16765 +/- ##
==========================================
- Coverage 98.53% 98.52% -0.02%
==========================================
Files 169 169
Lines 21852 21855 +3
==========================================
Hits 21532 21532
- Misses 320 323 +3 ☔ View full report in Codecov by Sentry. |
Code size report:
|
69d07a0
to
385c4c1
Compare
385c4c1
to
cd53f29
Compare
This commit introduces the ability to obtain a list of stations connected to the device when in soft-AP mode. A new parameter ("stations") to pass to WLAN.status is supported, returning a tuple of (bssid, ipv4) entries, one per connected station. An empty tuple is returned if no stations are connected, and an exception is raised if an error occurred whilst building the python objects to return to the interpreter. Documentation is also updated to cover the new parameter. This fixes micropython#5395. Signed-off-by: Alessandro Gatti <a.gatti@frob.it>
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Looks good, thanks for updating.
I tested on an esp8266 and it works (after having issues with the esp8266 disassociating the STA, I think due to power saving timing problems).
cd53f29
to
4d034f8
Compare
Summary
This commit introduces the ability to obtain a list of stations connected to the device when in soft-AP mode.
A new parameter ("stations") to pass to WLAN.status is supported, returning a tuple of (bssid, ipv4) entries, one per connected station. An empty tuple is returned if no stations are connected, and an exception is raised if an error occurred whilst building the python objects to return to the interpreter.
Documentation is also updated to cover the new parameter.
This fixes #5395.
Testing
I've tested this code as part of a project of mine on a NodeMCU with three stations connected to it, with the expected output being returned by calling
interface.status("stations")
. I doubt this can be put in the CI pipeline or how this can have a test of its own.Trade-offs and Alternatives
This comes with a small footprint increase for the general firmware configuration, however if this is unacceptable maybe I can add a port define that disables AP support in MicroPython.
Also, no idea on how
wifi_softap_get_station_info
behaves with a large number of stations, all possible recoverable error conditions should be handled in this PR.