dplasa / FTPClientServer

Simple FTP Server and Client for the esp8266/esp32 with LittleFS and SPIFFS support
GNU Lesser General Public License v2.1
54 stars 29 forks source link

Fix timeout calculations #7

Closed devyte closed 4 years ago

devyte commented 4 years ago

The code that checks timeouts is flawed and reinvents the wheel. I strongly suggest replacing with polledTimeout::oneShotMs, which is known to be correct and even covered by unit tests, see core examples.

dplasa commented 4 years ago

That's a neat suggestion and I've done so! Not really sure if I used polledTimeout::oneShotMs in the very intended way but it works expected and now uses that part of the esp8266 core library.

dplasa commented 4 years ago

Please do comment on my use of the esp8266::polledTimeout::oneShotMs

devyte commented 4 years ago

At a glance, and without checking your specific logic, it seems ok. There is one case where you're taking a uint16_t for timeout value, I think it should be uint32_t like everywhere else. Fix that and you cam close this.

dplasa commented 4 years ago

It would not have made a difference as the waitFor(...) call (waiting for an FTP's server response) was never called with more than 10000ms, but I changed it anyways.