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 support - crashs #38

Closed
GIPdA opened this issue Aug 2, 2019 · 8 comments
Closed

ESP32 support - crashs #38

GIPdA opened this issue Aug 2, 2019 · 8 comments

Comments

@GIPdA
Copy link

GIPdA commented Aug 2, 2019

Adding ESP32 support, I forked here: https://github.com/GIPdA/TinyUPnP/tree/esp32Support

It is working most of the time, when it doesn't crash.
It can crash and reboot several times (variable, 1 to 5 in my few tests, always in the same spot) and eventually it make it through the setup and runs the loop without crashing.

Here is the crash report:
crash.txt

I guess it is probably a heap/stack overflow.
The fact that each readStringUntil returns a String without any size check isn't very safe. In my case, it can buffer up to 4k characters (shouldn't run out of space with the simple example on the ESP32 though, but still).

I monitored the free heap size before crash (ESP.getFreeHeap()) and there were still plenty of space (~ 260000 bytes). Seems unlikely that memory fragmentation could be an issue here?

Also, the Arduino ESP32 is using FreeRTOS, so it might be related to that. Not sure exactly how stack & heap is handled under the hood (in a task), and the crash report isn't very helpful, I find.

Finally, enabling debug prints makes it crash very fast, every time. Certainly a memory issue. Need to comment most of the debugs to avoid the crash.

@ofekp
Copy link
Owner

ofekp commented Aug 3, 2019

Hey @GIPdA

First of all, you don't need to comment out debugs.
Simply remove the #define UPNP_DEBUG from the .h file and you should be all set.

Most likely this is a memory issue, as you said.
I will need to get myself an ESP32 to check what is going on, this is very weird though since ESP32 are generally stronger than ESP8266.

ESP8266 has ~82 KB
ESP32 has ~520 KB and even has two cores

Are you running the branch esp32Support?
Why did you remove controlURLFound = true; in this line please?

After you compile in Arduino IDE (I assume) what is it saying your Sketch is using in regards to memory?

for example:

Sketch uses 341972 bytes (68%) of program storage space. Maximum is 499696 bytes.
Global variables use 39788 bytes (48%) of dynamic memory, leaving 42132 bytes for local variables. Maximum is 81920 bytes.

Thanks.

@GIPdA
Copy link
Author

GIPdA commented Aug 3, 2019

I did comment out the #define UPNP_DEBUG, what I meant was if I want the debug, I can't have all of it or it crashes.

The sketch uses 772334 bytes (58%), max is 1310720. RAM use is 51568 bytes (15%) out of 327680.

I tested a bit more, and actually the issue comes from WiFiClient::read(unsigned char*, unsigned int) at esp32/hardware/esp32/1.0.2/libraries/WiFi/src/WiFiClient.cpp line 407, _rxBuffer->read(buf, size);: _rxBuffer is actually null...
So I guess not a memory issue as I thought. Could it be that the WiFiClient is closed in the middle of the read of something?

Are you running the branch esp32Support?

Yes.

Why did you remove controlURLFound = true; in this line please?

Warning for unused variable. I probably should have let that be and remove it from the commit now that I think of it. Can do before a pull request if you want.

@yankobogdan
Copy link

Hi, to all. Sorry guys but examples provided in the library only for esp8266, I've tried to change it to esp32 compatible, it's compiled but magic with ports not happen. Can you provide pls archive with esp32 example/add esp32 examples to library to make it useful for me? I've already tested on esp8266 and it's work great. Thanks, a lot.

@GIPdA
Copy link
Author

GIPdA commented Sep 25, 2019

Hello,
I used it successfully. Does it crash and reboot, report an error, or nothing? You may want to add some debug to check.
I will push my example for ESP32 this evening if you want to test (in 9-10h from now).

@GIPdA
Copy link
Author

GIPdA commented Sep 25, 2019

Here you go : https://github.com/GIPdA/TinyUPnP/tree/esp32Support/examples/SimpleServerEsp32
I did not had the time to test it again, but it should work, I used it for my tests. Otherwise, let me know.
Be sure to use the esp32Support branch.

@yankobogdan
Copy link

yankobogdan commented Sep 26, 2019 via email

@ofekp
Copy link
Owner

ofekp commented Oct 10, 2019

After I'll get #42 merged, I will close this issue.
@GIPdA thank you for this contribution, also, I am sorry for not seeing this sooner.

Edit: I closed pull request 41 and opened #42 instead with a local branch.

@ofekp
Copy link
Owner

ofekp commented Oct 14, 2019

I merged #42 so I am closing this issue.
Thank you both so much :)
@GIPdA your name will linger on under Special Thanks in the README. This was a great contribution to everyone using this library.

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

No branches or pull requests

3 participants