Version 2: Optimised connection flow and bug fixes#1
Version 2: Optimised connection flow and bug fixes#1jayeheffernan wants to merge 10 commits intomysticpants:masterfrom
Conversation
|
Might be a good idea to allow the user to set whether it should connect in order of the wifiList or in order of signal strength |
|
|
||
| ## Constructor: WifiQueue(*cm[, flags[, logger]]*) | ||
|
|
||
| The WifiQueue class is instantiated with a ConnectionManager object and two optional parameters, wifiList and logs. |
There was a problem hiding this comment.
This statement doesn't match the constructor...
| Flags are boolean options passed as the 3rd argument to the constructor. Multiple flags should be "added"/"stacked" with the binary "or" operator, `|`. All flags are off/`false` by default. | ||
|
|
||
| # setWifiList(wifiList) | ||
| #### WIFIQUEUE\_HIDDEN |
There was a problem hiding this comment.
As discussed, I would keep this but also add the ability to specify individually on each network whether it is hidden or not. I would prioritise connections like this:
If the user has set priority to be based on RSSI:
1. Visible networks in order of RSSI
2. Any network in the list that is set to be hidden
3. Other open networks
If the user has set priority to be based on the list order, then just follow the list.
There was a problem hiding this comment.
I agree with this but am not going to do it right now. If you want you can make an issue to request this as a v3.1.0 and someone will get to it eventually.
|
|
||
| // ----------------------- | ||
| // `cm` is a connection manager instance | ||
| // `wifis` is an [ { ssid, pw } ] |
There was a problem hiding this comment.
This isn't actually a param in the constructor...
| _wifis = clone(wifis || []); | ||
|
|
||
| // Remove invalid entries | ||
| for (local i = _wifis.len() - 1; i >= 0; i--) { |
There was a problem hiding this comment.
I probably wouldn't check if they're valid. Would just assume they are and let it fail if they're not.
There was a problem hiding this comment.
"let it fail" could mean an unhandled exception which causes the device to reboot and then repeatedly fail to connect
| ### onConnect(*callback*) | ||
|
|
||
| This function sets a callback to be trigger whenever the device connects. | ||
| Sets a callback to be trigger whenever the device connects. |
There was a problem hiding this comment.
Make sure to document that it will also be called if connect() is called when the device is already connected.
| // `logger` is a logger object with `.log()` and `.error()` methods | ||
| constructor(cm, flags = null, logger = server) { | ||
| _cm = cm; | ||
| _logger = logger; |
There was a problem hiding this comment.
You shouldn't log unless the user has set a debug flag and if they have the logs should be prepended with: "WifiQueue:"
| }) | ||
| .onFail(function() { | ||
| logger.error("WifiQueue failed to connect"); | ||
| }); |
There was a problem hiding this comment.
This semi-colon shouldn't be here.
Also, you haven't defined logger but have used it.
jayeheffernan
force-pushed
the
master
branch
from
081fb32 to
bc1b70f
Compare
jayeheffernan
changed the title
|
@blindman2k I've reviewed and think it's ready to merged. |
2 participants
This pull request implements the following optimised connection flow:
wifiListfirst if there are multiple candidates.wifiList.imp.scanwifinetworks()once to get a list of visible, open networks. Try to connect to each in turn, as long as it remains visible at the time of attempting to connect.onFailcallback.@dbns97 please review