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

ESP32/network fixes #2325

Merged
merged 6 commits into from
May 5, 2021
Merged

ESP32/network fixes #2325

merged 6 commits into from
May 5, 2021

Conversation

mikee47
Copy link
Contributor

@mikee47 mikee47 commented May 1, 2021

  • Basic_WebSkeletonApp goes pop with ESP32 due to ESP_ERROR_CHECK.
  • Don't start webserver until gotIP onReady

Don't use ESP_ERROR_CHECK so much...
I guess calling these when interface isn't initialised is a genuine logic error.. but could be annoying

  • Fix Ssl::Options::toString()

  • Change Timer to SimpleTimer for HttpClient
    Uses a little less RAM.

  • Increase ESP32 timer task stack size
    Bugchecks during HostTests

@avr39-ripe
Copy link
Contributor

Don't think that we can start web server AFTER got IP, we need this also for station to configure wifi.

@mikee47
Copy link
Contributor Author

mikee47 commented May 1, 2021

Don't think that we can start web server AFTER got IP, we need this also for station to configure wifi.

Thanks for pointing this out. Wrong soluation! The problem in this (and other samples) is the call to getIP() in startWebServer which clearly must fail - this crashes the ESP32 (though now mitigated by removal of some ESP_CHECKs). I'll take another look at this.

@mikee47
Copy link
Contributor Author

mikee47 commented May 1, 2021

Actually other sames start webservers after GotIP() so the problem is more widespread.

I've done a brief test on the Esp8266, and the IP is allocated after leaving init(), then a couple of mode change notifications (to Wifi event handler) before our ready handler is called - that looks like the best place to startup web servers.

129858 mount() returned 0 (Success)
129996 File system initialised
sleep disable
142933 open('.therm.conf'): NOT_FOUND (-11002)
143110 File '.therm.conf' open error: NOT_FOUND
143853 Station configuration is: PleaseEnterSSID
145334 WifiAccessPoint.enable()
mode : sta(dc:4f:22:1c:26:66) + softAP(de:4f:22:1c:26:66)
add if0
add if1
dhcp server start:(ip:192.168.4.1,mask:255.255.255.0,gw:192.168.4.1)
bcn 100
162955 event 8

163099 Old mode: 2, new mode: 3
165172 event 8

166479 Old mode: 3, new mode: 3
169415 onReady()
scandone
no PleaseEnterSSID found, reconnect after 1s
3004981 event 1

3005127 disconnect from ssid PleaseEnterSSID, reason 201

3005360 DISCONNECT - SSID: PleaseEnterSSID, REASON: No AP found

reconnect

@slaff slaff added this to the 4.3.1 milestone May 2, 2021
I guess calling these when interface isn't initialised is a genuine logic error.. but could be annoying
Bugchecks during HostTests
Do it from `onReady()` handler. All other seem OK.
@slaff
Copy link
Contributor

slaff commented May 4, 2021

@mikee47 Ready with this PR? Did you test the changes on a real Esp32?

@mikee47
Copy link
Contributor Author

mikee47 commented May 4, 2021

Just re-tested with Basic_IFS sample (which is where all this started) and all good. Moved the startWebServer call into init() and it doesn't go pop :-)

@mikee47
Copy link
Contributor Author

mikee47 commented May 4, 2021

This is on ESP32. All done!

@slaff slaff removed the 3 - Review label May 5, 2021
@slaff slaff merged commit dc0747c into SmingHub:develop May 5, 2021
@mikee47 mikee47 deleted the fix/esp32-network branch May 5, 2021 11:06
@slaff slaff mentioned this pull request May 5, 2021
5 tasks
slaff pushed a commit that referenced this pull request Sep 27, 2021
Don't start webserver until onReady
Don't use ESP_ERROR_CHECK so much...
Fix Ssl::Options::toString()
Change Timer to SimpleTimer for HttpClient. Uses a little less RAM.
Increase ESP32 timer task stack size
Bugchecks during HostTests.
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.

3 participants