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

Major Functionality Change To Improve Reliability #157

Closed
wants to merge 49 commits into from

Conversation

kentaylor
Copy link

@kentaylor kentaylor commented Apr 24, 2016

The WiFiManager library has been unreliable for me.

I think this is due to the incorrect premise that the ESP8266 should be instructed to connect to a WiFi network. This has led to the library including unneeded functionality which has the side effect of causing the ESP8266 module to intermittently fail to connect to WiFi, behave inconsistently and occasionally brick.

If this pull request is accepted the library functionality will be substantially changed and it would be included into sketches in a different way.

tablatronix and others added 25 commits March 9, 2016 11:03
…an be unique and automatically generated from the ESP8266 Chip Id.

Tries to reconnect in station mode with  whatever credentials have been saved when StartConfigPortal times out otherwise device will be without WiFi after exiting StartConfigPortal.
another ci test

ci fix

ci attempt 6

stable package json

fixe prefs

fix lib folder

debug

alalalalaalal

try with 1.6.5

try with 1.6.7

verbose build

1.6.7 again

back to 1.6.5

escaped : in url

no save pref

no save pref, one line

and back to working 1.6.5

getting all examples built

added sh include

fix sh

added build

build examples

aded debug output

going insane

beautify

added lib ArduinoJson

try again

1.6.8

get arduino json from github

back to 1.6.5
…t Wifi already does it's best to connect in the background. Calling WiFi.begin while device is already trying to connect will occasionally cause WiFi to lock up. Only call WiFi.begin after calling WiFi.disconnect. These changes are aimed at stopping WiFi.begin from being called at the wrong time.

The concept of autoconnect is also deprecated because it is trying to do what the device is already doing.  Calling this method will now block until WiFi connects. It has been changed to wait for 10 seconds then go into setup mode if connection fails. Sketch can avoid blocking call then use (WiFi.status()==WL_CONNECTED) test to see if connected yet.

Removed - Tries to reconnect in station mode with  whatever credentials have been saved when StartConfigPortal times out otherwise device will be without WiFi after exiting StartConfigPortal. - because it was wrong.
…onfiguration portal otherwise connect with existing SSID data. If a button is pushed go into configuration portal mode. Stay in configuration portal mode until user closes configuration portal or 3 minutes have passed.
…specified time has passed or user requests portal to be closed. If a successful connection is made to a network the access portal is available on the network ip address and on it's own network at two addresses. These addresses are 192.168.1.4 and the address on the other network.
…rds do not have this led.

Force to station mode on example app because if device was switched off while in access point mode it will start up next time in access point mode.
Print ip number correctly in close page
Set example app to keep access point open forever.
Turn off automatic scans for wifi.
Add current network and IP details to root page.
Removed reset button from root page as I can't think of a scenario where this is useful but url for reset functionality still works.
Make device reset in example app after returning from portal because web server can only be started once. Not sure why this is and it needs further investigation.
After setting credentials redirect to root page rather than serve a page.
Set header responses to not cache in browser for all pages.
Removed generate_204 handler. See discussion in code as to why.
…nnected to network so change to access point mode only when there is no network connection.

Provide some extra information on the root page.
Disabled page caching in the browser.
# Conflicts:
#	WiFiManager.cpp
…est to be of type boolean from previous type string.

Moved scan for WiFi networks to a seperate function. Uses the third option from https://www.eskimo.com/~scs/cclass/int/sx5.html to return an array which is messy as it requires the use of pointers, malloc and free. The function returns the number of WiFi networks it found and changes a pointer to point to an array of indexes that is used to access the WiFi networks that were found array in signal strength order.

Changed the ConfigOnSwitch example to use waitForConnectedResult method rather than poll in a loop for the WiFi status when attempting to connect to WiFi.
…on format.

The purpose of returning data in json format is to make it easy for apps to programatically configure the WiFi access point name and credentials. I expect users will find apps a convenient alternative to a browser to configure the device because usability testing has shown that people sometimes get confused by having to switch back and forward between WiFi networks .

The URLs that return json data are now /state and /scan .
Removed examples that were no longer meaningful now that the autoconnect function has been deprecated.
…ecting to a network.

Changed pin assignments to be numerical rather than by name as some ESP8266 devices use different pin mappings.
Small modifications to librarly explanation and justification.
@tzapu
Copy link
Owner

tzapu commented Apr 25, 2016

oh man, this is such a massive amount of effort into work and documenting stuff i ve put into this... thank you, very much appreciated.

i am sorry for not replying sooner, also it might be a while until i ll manage to look at it properly.

i am simply way to tied up currently with stuff and as the issues board might show, i have simply not chance of offering this the attention it deserves.

I would love to discuss about the way it works, the issues the current model has, the motivation between doing it as it is, etc and hopefully i ll get out of this problem period sooner rather than later

sorry again for the delays

@kentaylor
Copy link
Author

The reply was soon enough, thanks. I've some sympathy for the effort required. I was expecting to have network connectivity sorted in an afternoon and it has now consumed many hours over weeks. Particularly as it was failing in strange unrepeatable ways. I liked your battery powered temperature sensor too as I have a similar goal.

As it is a major change and because the claimed failure modes are intermittent and tricky to test I've provided some supporting evidence at https://github.com/kentaylor/WiFiManager#evidence-the-premise-is-wrong-and-the-problems-it-causes .

It would be great to get some more people trialing the proposed changes as my arguments need to be tested. Then when you get more time you can have confidence in whether never calling WiFi.begin() when it is already trying to auto connect in the background can improve reliability.

- Configuration portal got new look and feel
- Added a very small CSS framework based on mincss
- Added the possibility of using labels. A labels will receive the text of the corresponding placeholder
- Added option for label placement. 0 - label not displayed, 1 - label displayed before field (default), 2 - label displayed after field. Latter is useful for checkboxes
- Scan link was moved to the top
- All HTML <dl> description list elements were replaced by tables
- Removed <br> before each form parameter, CSS will take care of the spacing and custom HTML injection will be more useful
- Renamed Configure WiFi option to Configuration as WiFi is not the only option that can be configured
- Renamed Info option to WiFi Information as the original text was too short and looked strange
- Reanmed Close configuration portal option as it was too long and was displayed in two lines on smaller devices
@tablatronix
Copy link
Collaborator

tablatronix commented Nov 11, 2016

Not bad seems to work well.

@tzapu
I have some suggestions and cleanups for it if you merge I can fix it up a bit, merge it into a new branch at least for starters so the other pull requests can be built on it.

I think it is still pretty lightweight, most of the bloat can be ignored.

  • minor changes to template and verbage and UI
    • Exit Portal is actually Close or Stop Portal, shutdown etc
    • Saving flow could be more concise, simply confirm saved ( does it confirm save, or just say it saved ?Then continue, or auto meta redirect ?Device is now connected to AP SSID with IP of 10.0.0.0 etc.
  • minor document changes, cleanup of course, reformat , remove references to "new" functionality and redocument as fact.
  • remove the app references, keep json, but remove references to the app and dns name feature, not needed. Mention in readme as footnote, untested unsupported etc.
  • wrap debugging in compile definitions, so they are not in compiled code unless defined WMDEBUG enabled, else use debug constant during runtime, either works, this might already exist.
  • rework examples
  • new callbacks, portaltimeoutcallback

@tablatronix
Copy link
Collaborator

tablatronix commented Nov 11, 2016

I wonder if autoconnect could be redefined to mean start up a configuration portal if no SSID has been defined.

Good idea, would fix the need for wrapping it with connected or SSID check, and allow ship to config easier.

How about also adding a erase flash command also, toggleable option of course

@tzapu
Copy link
Owner

tzapu commented Nov 12, 2016

thank you @tablatronix and @kentaylor
i have started porting this together with a cleanup in the development branch, but as expected, goes slower than i would have wanted due to time constraints
they are all very good points and want to get to them all, but i don t want to drop the autoConnect behaviour or examples as they are as a lot of people use it like that, including myself.
docs and examples could explain all a bit more structured, as a lot stuff (start portal only if SSID is not defined, on button press config, on double reset, etc) are already possible in normal version of WIFIManger with just a bit of extra code
for example, for doing I wonder if autoconnect could be redefined to mean start up a configuration portal if no SSID has been defined , all that is needed would be similar to:

If (WiFi.SSID != "") { 
  wifiManager.autoConnect()
}

again thank you for all your points, work and observations, they are really inspiring

@kentaylor
Copy link
Author

@tablatronix said:-

Saving flow could be more concise, simply confirm saved ( does it confirm save, or just say it saved ?Then continue, or auto meta redirect ?Device is now connected to AP SSID with IP of 10.0.0.0 etc.

I'd agree that this is the weakest area of the interface but doing better requires more than

minor changes to template

@tablatronix doesn't like the app reference but the app is the only way to handle this aspect of the interaction well. The response page is returned before flash is updated which it must, because the web server and WiFi network shuts down when it tries to connect to the AP. The data returned must be bland because we haven't tried to connect to an AP yet and there are lots of reasons it might fail e.g. wrong password. The WiFi can be down for a long time, upwards of 30 seconds under some circumstances and the browsing device will usually auto switch to another network when the WiFi is down too long and/or the ESP changes WiFi channels. An IPhone will close the config portal if that occurs which minimises confusion but browsing devices all do things differently.
.
Unlike an app there is no way to control a WiFi connection from a browser. The idea at the moment is to have a URL which is valid whichever network the browsing device is on so that it doesn't look like the ESP device has failed when the browsing device fails to connect to the device URL. Once you have experience with something you can not see it like a naive user but users interpreting a failed URL as device failure was what I usually saw in user testing. This functionality could also be implemented with JavaScript. JavaScript can't query WiFi network status but could probe URL's to see if it got a response from the internet or the device and then display an appropriate message.

@tablatronix said:-

How about also adding a erase flash command also

This is already there and documented on the information page where all valid commands are shown.

@kentaylor
Copy link
Author

@tzapu the idea of autoconnect is that when an ESP device starts up a user can configure a WiFi network if the ESP can't connect to a WiFi network . This is fabulously simple to include in a sketch but I don't like it because in practice I've found more often that not the absence of the already configured WiFi network is temporary and a configuration portal isn't useful. I've taken the view that it isn't possible for the device to know the difference but an algorithm that would self correct in the case of a temporarily absent network would be to keep checking for WiFi connectivity once the configuration portal has been launched and close it automatically if WiFi becomes connected.

@tablatronix
Copy link
Collaborator

tablatronix commented Nov 12, 2016

Yes I agree with the above also, I see wifi manager as a one time or problem or changing networks only thing, not a launch when connection fails always. I doubt many people will actually us it as such.

I want the app removed because it creates a dependancy for this library and an expectation, and that is bad for oss, tzapu and contributers should not have to worry about breaking third party dependancies, but I think the json api is nice to have and good enough for users to attempt to use if they chose to. I just do not think a static url and app should be tied to it at all. Unless there is some standard local mdns way to do so , there is not that i know of.

@tablatronix
Copy link
Collaborator

tablatronix commented Nov 12, 2016

@tzapu I see 2 branches, neither has any cherry picking or merges from this that I can see, do you have milestones or an issue created for these branches so we can follow or contribute, are you planning on manually implementing these and not merging ? Trying to understand what the 2 branches are and what you propose. Its still your repo and baby, i imagine you want to maintain control over the code style etc.

oh and this lib was just mentioned on adafruits show and tell, so you might see more popularity.

@tablatronix
Copy link
Collaborator

tablatronix commented Nov 12, 2016

@kentaylor re: flash erase

This is already there and documented on the information page where all valid commands are shown.

I didn't see anything about that in your readme. How can you add a reset/clear button to the main page ?

@umranium
Copy link

@tablatronix @tzapu @kentaylor

Having an app for configuration greatly improves the user experience while using the library. The cost is of course the additional code to allow programmatic configuration (e.g. serving JSON as well as HTML). I feel in this case, and this is totally my opinion, that the gain in user experience is much higher than the cost in the additional code and resources to have the programmatic configuration APIs.

That said, perhaps @tablatronix is right, tying the WifiManager to the app closely might not be what the WifiManager community wants, especially when the app would only serve a subset of the user base.

As a compromise, how would you feel about having a list of apps instead of just this particular single app? I feel that without adding a reference to the apps in the HTML pages that the ESP device serves, many of the users would not be aware of the possibility of having a much better experience via the apps.

BTW, please try out the app. Any feedback you can give me will be highly appreciated. It's available on Google Play and the code base resides here.

@tzapu
Copy link
Owner

tzapu commented Nov 13, 2016

@tablatronix i m going through them manually because @kentaylor 's branch is heavily oriented on 'on demand' functionality and more expert uses. this is happening in the development branch
the kentaylor pull request branch is so i can mark his pull request as accepted at the end, i want to make sure contributors are given credit through all means available
where i hope to go with this is:

  • have the json api, that works with the apps (but it is totally optional)
  • have a list of apps and notable branches in the docs for people that want to experiment
  • have a better interface that you can upload to your FS (also totally optional)
  • rework the connection algorithm so it s more like ken tested and suggested (biggest point being that if it s set to auto connect, or if it connects in the background, it should do the right things, rather than force a connection)
  • rework the documentation to give a more through overview of how you can use it in the two possible ways (auto/ondemand) and to have a paragraph explaining what you can find in each example
  • ken also had a very interesting change of having a named host for the esp, which is a real domain, which could give more information and that is a very smart idea

each of these changes are potentially breakign and i would like to give people a chance to try them in the development branch as well before making it a release

my goal for this is to have it dead simple to use for everyone, paste and go. more experienced users will always find ways of changing things, but people just starting up should not need more than 3 lines of code to get started. if they want, they can explore further at any point.

also the fact that i have quite a few modules that have been working fine with power cuts and internet cuts and so on doesn't motivate me enough to have major changes to the 'philosophy' (big word, i know) behind it all :D

thanks for letting me know it was featured, had no idea :D lovely

@umranium my biggest issue with an app is that I can't properly use it on iOS, otherwise, it is a wonderful idea and I really want to give you all support needed.

@kentaylor "This is fabulously simple to include in a sketch but I don't like it because in practice I've found more often that not the absence of the already configured WiFi network is temporary and a configuration portal isn't useful."
all the more reason to not pile up features in WIFIManager, should be a simple as possible to get people online. more advanced stuff can be done with extra code in the sketch. i guess it s a ll a balance of flexibility/features/ease of use.
it s only one if to not have wifimanager kick in if your network is shaky and yo u've got a ssid set. it s another if to detect a button/erase settings/restart in config more. but not everybody has a button :P nor problems connecting.
"I think this is due to the incorrect premise that the ESP8266 should be instructed to connect to a WiFi network. " <- this is so very true, probably compounds any problems that people might be having, all the way up to making in unrelaible

thank you guys for everything, you are a wonderful community :)

@tablatronix
Copy link
Collaborator

tablatronix commented Nov 13, 2016

Sounds good, but I think the main feature of this lib is its simplicity and compactness, it should be very small, since it takes memory and is hardly ever used, which is very important. ( I really would like to the the new styles brought it immediately, the notification blocks, the big buttons, those minor css changes are very nice, and my icons of course )

I am also thinking maybe all this on demand stuff is over complicating it, all we really need is for it to detect that wifi is not working ( end user/developer decides what this means ,no ssid or no ap or no internet ping, or no ip etc ) . The simplest and most obvious solution is the following.

  • Auto start portal if no SSID previously setup, user can run this detection whenever they want.
  • Erase network config ( possibly keep other fields ) ( which forces above condition )

Thats it, real simple, even for end users, start config if no ssid, clear ssid, 2 steps and any user can provide config portal and reset mechanism, being hold button on power up, hold button for 10 seconds, cycle 3 times in 5 seconds.

Yes there are many advanced options available
on demand portal, for using to config extra fields at any time
persistance portal as a status page
but these are all easily controlled by end users with easy examples.

So we are looking at a few methods

autoconnect
configportalstart
configporttalstop
erase configportal settings ( some/all)

clean responsive UI
customizable templates all that jazz

But if you support extra fields, then ondemand needs to be available easily
If you support using status pages ( pretty cool plus ) people can use this to throw up real fast simple iot devices, eg just a temperature field, etc. ) and this is just a AP+STA setup so not really to much extra code

The saving and handing off is a definite problem, if you are looking at professional or commercial design then yes this needs to be well thought out, but if there is a standard for this then cool, use apps, but we have no idea who makes these apps, what they do whats in them.

I think that is kens approach, an app or a real url, this is how alot of iot devices allow config, my thermostat, my scale all use this method, and it to keep it available when it changes from ap to sta and networks switch and drop when restarting, typically you just use js loop and redirect to the new ip after reboot, but esp presents the problems ken addressed, wifi channel switching and all those tiny issue that cause sta+ap+scanning to cause drops in service during setup.
Then there is the is password right, can I even connect after setup, or do we bounce back to config on any failure. ( Does connecting manually begin() to the ap without a reset fix any of this? )

I am not sure there IS another solution.

@tablatronix
Copy link
Collaborator

tablatronix commented Nov 13, 2016

Just curious have you experimented with mdns for local urls as a solution ?
esp8266.local for example ?

( It seems there is a reason commercial IOT devices now use btle and wifi for setup now )

@tzapu
Copy link
Owner

tzapu commented Nov 14, 2016

intially it was based on mdns, never worked that good back then, i kept having to use the ip anyway
after dns redirects came, switched to that. i see ken isusing a specific hostname.

i shall do more tests with jsloops, there must be a way in which that will work fairly well, although not for captive portal which closes when the wifi point goes away... (it just increases the code size for a one off thingy)

@tablatronix
Copy link
Collaborator

Ken mentions some of the login portals will timeout and close while the config is saving and conx is lost. Ios etc. so maybe the real hostname keeps that page up while it switches aps not sure though seems solveable if you used a browser instead of auto launch login page. Just a thought. I do not fully understand all the changes

@Daemach
Copy link

Daemach commented Nov 14, 2016

I want to use this library to set up wifi, then redirect the user to a
public website, passing the unit's chipID + mac as a serial number so the
user can register the device, attaching it to their account for
monitoring. What's the best way to accomplish this?

On Mon, Nov 14, 2016 at 5:18 AM, Shawn A [email protected] wrote:

Ken mentions some of the login portals will timeout and close while the
config is saving and conx is lost. Ios etc. so maybe the real hostname
keeps that page up while it switches aps not sure though seems solveable if
you used a browser instead of auto launch login page. Just a thought. I do
not fully understand all the changes


You are receiving this because you are subscribed to this thread.
Reply to this email directly, view it on GitHub
#157 (comment),
or mute the thread
https://github.com/notifications/unsubscribe-auth/AL-O4Uf0YIH_3VlZpt3h5JSlvIw1Is6Sks5q-F-qgaJpZM4IOUBX
.

@tablatronix
Copy link
Collaborator

Ask questions in a new issue, this is a pull request discussion.

@tablatronix
Copy link
Collaborator

tablatronix commented Nov 16, 2016

minor annoyance.
My compiler throws this,
WiFiManager.cpp:349:23: warning: deprecated conversion from string constant to 'char*' [-Wwrite-strings]
returning strings function declared with char*, maybe change to String

and complaints about unknown escape sequence \<, might need to escape it \\<
page += F("<tr><td><a href=\"/wifisave\">/wifisave\</a></td>");

@kentaylor
Copy link
Author

@Daemach the short answer is try IOT Configurator and while @tablatronix would prefer this to be a new issue it can be interpreted as a prompt to consider the functionality WiFiManager should provide.

I realise this is left field but I think the @Daemach preference for obtaining custom application parameters from an online service is likely, over time, to become the default and accordingly consideration should be given to keeping WiFiManager minimalist and splitting out the custom parameter functionality to a separate library.

Reasoning:-

  1. Configuring WiFi is a tightly defined problem that is amenable to a universal solution.
  2. Getting a WiFi connection has to be done locally and is a separate problem to custom parameter configuration which can be done differently.
  3. Custom parameter collection and custom display is, in it's most generic form, addressed by a web server and when made more specific is akin to a web application framework
    .
  4. A web application framework type library is useful for all situations where a web interface is desired, not just WiFi set up.

@kentaylor
Copy link
Author

I've been intending to incorporate some items from this discussion in the https://github.com/kentaylor/WiFiManager version.

I liked the simplicity that could be achieved from the @tablatronix suggestion that

  • Auto start portal if no SSID previously setup, user can run this detection whenever they want.
  • Erase network config ( possibly keep other fields ) ( which forces above condition )

but when looking at how to implement it, noticed that it would conflict with the Wishlist item add multiple sets of network credentials and particularly this algorithm.

Therefore the enthusiasm for that item has waned.

@tablatronix
Copy link
Collaborator

I see, so implement wifi multi ?
hmm, yeah that does cause an issue with that reset flow , although I doubt your typical IOT appliance user actually needs this functionality, especially if it is so easy to change with wifimanager.

How often would be a device that moves around be needed or what else might a user need that for.

@kentaylor
Copy link
Author

kentaylor commented Nov 19, 2016

@tablatronix I move ESP devices among a few networks quite often and regard reconfiguring each time to be a pain. I can't think of other reasons multiple SSID's would be useful.

IOT devices aren't usually moved as often as phones or laptops but imagine the annoyance if you had to reconfigure phones and laptops each time you moved between networks and that is easier than using WiFiManager.

@Daemach
Copy link

Daemach commented Nov 19, 2016

Thanks - that was my thinking as well.

On Wed, Nov 16, 2016 at 8:15 PM, Ken Taylor [email protected]
wrote:

@Daemach https://github.com/Daemach the short answer is try IOT
Configurator http://configure.urremote.com/ and while @tablatronix
https://github.com/tablatronix would prefer this to be a new issue it
can be interpreted as a prompt to consider the functionality WiFiManager
should provide.

I realise this is left field but I think the @Daemach
https://github.com/Daemach preference for obtaining custom application
parameters from an online service is likely, over time, to become the
default and accordingly consideration should be given to keeping
WiFiManager minimalist and splitting out the custom parameter functionality
to a separate library.

Reasoning:-

  1. Configuring WiFi is a tightly defined problem that is amenable to a
    universal solution.
  2. Getting a WiFi connection has to be done locally and is a separate
    problem to custom parameter configuration which can be done differently.
  3. Custom parameter collection and custom display is, in it's most
    generic form, addressed by a web server and when made more specific is akin
    to a web application framework
    https://en.wikipedia.org/wiki/Web_framework.
  4. A web application framework type library is useful for all
    situations where a web interface is desired, not just WiFi set up.


You are receiving this because you were mentioned.
Reply to this email directly, view it on GitHub
#157 (comment),
or mute the thread
https://github.com/notifications/unsubscribe-auth/AL-O4Uw-2a0sV6QjCmCHy2QyeBjstEyRks5q-9TQgaJpZM4IOUBX
.

@tzapu
Copy link
Owner

tzapu commented Nov 19, 2016

guys, maybe another way to think of the multiple networks support is to have them in local storage on the phone ... a little bit of js, and not as convenient as auto switching from network to network, but gives you more control. i tend to also use the same device to reconfigure devices, so at least for me it would have instant access to the previous selected networks.
as opposed to ken's usage, there are people that have multiple networks in the same area and connect the device to each of them in turn, having all networks saved and connect automatically to one of them (not being 100% sure which one) would probably be quite annoying...

@kentaylor
Copy link
Author

kentaylor commented Nov 19, 2016

The @tzapu scenario does not conflict with automatic multiple network connectivity. To stick with the laptop/phone analogy, it is equivalent to clicking forget network.

Like some laptop/phones you could be more sophisticated and have a preferred network. Maybe make the first network in the array preferred and an algorithm that said use this unless the signal strength is below a threshold.

The biggest negative in my mind is the effort required to implement it. WifiMulti is a good starting point but the interface would be a pain. It has to support adding and removing APs like a laptop/phone does. Looking a bit at the Arduino library, WiFI.begin doesn't support it either so that has to be changed or else call the Espressif functions directly.

Everyone is familiar with the way phones/laptops swap networks and ultimately, in my view, are likely to expect the same from IOT devices.

@tablatronix
Copy link
Collaborator

I agree that this is way more complex i was gonna post in the actual issue for it #166

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

8 participants