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

calling ESP8266WiFiMesh::deactivateAP() doesn't fully disable wifi and corrupts SSID #5071

Closed
rorosaurus opened this issue Aug 22, 2018 · 11 comments
Assignees
Labels
waiting for feedback Waiting on additional info. If it's not received, the issue may be closed.
Milestone

Comments

@rorosaurus
Copy link

Basic Infos

  • [x ] This issue complies with the issue POLICY doc.
  • [ x] I have read the documentation at readthedocs and the issue is not addressed there.
  • [x ] I have tested that the issue is present in current master branch (aka latest git).
  • [ x] I have searched the issue tracker for a similar issue.
  • [ x] If there is a stack dump, I have decoded it.
  • [ x] I have filled out all fields below.

Calling ESP8266WiFiMesh::deactivateAP() in the example HelloMesh program produces a Wifi network with this result, after refreshing wifi on my phone:

screenshot_20180822-104301__01

I would have expected the wifi network to be shut off completely, not remove password, SSID, etc.

@rorosaurus
Copy link
Author

Maybe this is expected behavior? But I didn't expect to need to call WiFi.mode(WIFI_OFF); after calling ESP8266WiFiMesh::deactivateAP()

@devyte
Copy link
Collaborator

devyte commented Aug 22, 2018

CC @aerlon

@aerlon
Copy link
Contributor

aerlon commented Aug 24, 2018

Interesting. Neither my phone, nor my laptop can detect the WiFi once ESP8266WiFiMesh::deactivateAP() is called, but other ESP8266 nodes are able to do it. It seems as though an AP with an empty SSID is created. The intended result is to completely shut down the AP, as you guessed, so this is a bug.

This problem is caused by the use of WiFi.mode(WIFI_AP_STA); as the default WiFi mode in the library. It seems that the node will automatically set up an AP in this WiFi mode, even though WiFi.softAP() is never called. You can try this yourself by adding delay(30000); after meshNode.begin(); in HelloMesh, an AP will be created even though we have never called activateAP().

@devyte Is it expected behaviour that just having the ESP8266 enter WiFi.mode(WIFI_AP_STA); will cause an AP to be created (even if we never call WiFi.softAP()), or is this a bug in the core WiFi code?

If this is not a bug in the core code, the default WiFi mode of the mesh library will have to change. This should be possible, but I will need to do some testing.

@rorosaurus ESP8266WiFiMesh::deactivateAP() requires a slight change to fix the bug you found. Changing WiFi.softAPdisconnect(); to WiFi.softAPdisconnect(true); in the method seems to fix it, at least till the node re-enters the WIFI_AP_STA mode. We cannot use WiFi.mode(WIFI_OFF); in ESP8266WiFiMesh::deactivateAP() because deactivating the node AP would then also disconnect any active WiFi station the node may have. (you can of course use WiFi.mode(WIFI_OFF); as a work around for now, I'll include the fix for this bug in my intermediate release which should be ready within a week)

@devyte
Copy link
Collaborator

devyte commented Aug 25, 2018

My guess is that of you change mode to ap_sta, then an ap will be brought up with whatever parameters possible. I would expect some default if nothing has been set up, but I'm just guessing.
My own policy is to always specify every parameter myself, and not rely on sdk defaults or whatever. That means that I do something along the lines of:
If newmode is sta or sta_ap
Set up sta with my parameters
If newmode is ap or sta_ap
Set up ap with my parameters
Change mode to newmode

(Sorry for lousy pseudo code, I'm travelling with just my phone).

@aerlon
Copy link
Contributor

aerlon commented Aug 25, 2018

Yes, after reviewing the core source code it seems as though WiFi.mode() is always in charge of actually enabling and disabling the AP. All other WiFi methods dealing with this functionality just wrap the WiFi.mode() method in different ways. I'll modify the mesh library to take this into account.

devyte pushed a commit that referenced this issue Sep 24, 2018
* - Add assert in HelloMesh.ino for invalid transmission status.
- Make uint64ToString and stringToUint64 methods into stand-alone type conversion functions.
- Add getters and setters for requestHandler and responseHandler.
- Polish HelloMesh.ino code by adding networkIndex as networkFilter loop variable and switching networkFilter definition position.
- Add initial WiFi.disconnect() in HelloMesh.ino setup() function to ensure smooth WiFi operation.
- Add latestTransmissionSuccessful() convenience method.
- Change default WiFi mode to WIFI_STA and improve handling of WiFi mode (fixes issue #5071).
- Add checks to methods that change AP properties to avoid unnecessary AP restarts.
- Add getter for ESP8266WiFiMesh SSID and getters and setters for ESP8266WiFiMesh settings related to hidden SSID usage, max station connections allowed per AP and WiFi timeouts.
- Make waitForClientTransmission method use more accurate timekeeping.
- Improve type usage.
- Improve comments.
- Update README.md, keywords.txt and library.properties.

* Make getter and setter order consistent throughout code.

* - Fix active AP getting turned off when calling begin().
- Fix crash bug due to WiFiServer duplication when using the ESP8266WiFiMesh copy constructor with the AP controller as argument, under certain circumstances.

* - Move non performance-sensitive Strings to flash memory to save RAM.

- Add comments explaining F(), FPSTR() and PROGMEM.

- Fix README.md formatting.

* Remove the uint64ToString and stringToUint64 methods from the ESP8266WiFiMesh class since they are now stand-alone functions in the TypeConversionFunctions files.

* Change the minimum valid argument value of the setMaxAPStations method to 0, since this value is also supported by the ESP8266.

* Fix compiler warning.
@aerlon
Copy link
Contributor

aerlon commented Sep 25, 2018

@rorosaurus There were a few more complications than expected, but now the intermediate mesh update is done and merged to master. Among other things it should fix all the bugs we have discussed and also allow you to choose the number of stations that can connect to each AP (between 0 and 8).

Would it be possible for you to verify that the AP deactivation problem is fixed in the latest master and, if true, close this issue? That would help keep the number of open issues down.

@aerlon
Copy link
Contributor

aerlon commented Sep 25, 2018

I should probably add that uint64ToString and stringToUint64 are now stand-alone functions, so if your code used the old class methods you will probably need to #include <TypeConversionFunctions.h> and change the function call signature a bit. The updated HelloMesh example can be used as a reference for this.

@devyte devyte added this to the 2.7.0 milestone Nov 9, 2019
@devyte devyte self-assigned this Nov 9, 2019
@devyte
Copy link
Collaborator

devyte commented Nov 10, 2019

Reference: #6280

@devyte devyte modified the milestones: 2.7.0, 2.7.1 Apr 23, 2020
@devyte devyte modified the milestones: 2.7.1, 2.7.2 May 11, 2020
@devyte
Copy link
Collaborator

devyte commented Jul 7, 2020

Still not done with this, pushing back to 3.

@devyte devyte modified the milestones: 2.7.2, 3.0.0 Jul 7, 2020
@d-a-v
Copy link
Collaborator

d-a-v commented Mar 31, 2021

Does merged #6280 address this issue ?

@devyte devyte added the waiting for feedback Waiting on additional info. If it's not received, the issue may be closed. label Apr 3, 2021
@aerlon
Copy link
Contributor

aerlon commented Apr 10, 2021

Mesh version 2.2 should address everything mentioned here. The issue can be closed.

@d-a-v d-a-v closed this as completed Apr 10, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
waiting for feedback Waiting on additional info. If it's not received, the issue may be closed.
Projects
None yet
Development

No branches or pull requests

4 participants