Inconsistency in setTimeout implementation

[cziter15]

Board / hardware

Doesn't really matter, affect all ESP32 boards

Version

v2.0.3

Description

This is not really a bug to be honest. It's rather small inconsistency that can cause big troubles and might be somehow tweaked in arduino-esp32 to match other implementations of Client.

In development of my hobby IoT framework I've encountered inconsistent behavior between Arduino for ESP32 and ESP8266.

The problem is that everywhere in Arduino framework timeout passed to TCP clients is passed in milliseconds (in Arduino-esp8266 too). However, ESP32's WiFiClient doesn't follow that rule and wants seconds. This is described in header files, but users porting existing code might not notice that fact.

https://www.arduino.cc/reference/en/libraries/ethernet/client.setconnectiontimeout/
https://github.com/esp8266/Arduino/blob/master/libraries/ESP8266WiFi/src/WiFiClient.h (setTimeout method in base client class)

https://github.com/espressif/arduino-esp32/blob/master/libraries/WiFi/src/WiFiClient.h (setTimeout method)

[lbernstone]

Unfortunately, this is seriously breaking behavior for people that already have things set for seconds. Perhaps this could be implemented in stages, with a note indicating that the behavior will be deprecated at some point- temporarily put in a change so that numbers over 600 are interpreted as milliseconds, and then anything below as seconds, with a warning that the behavior will change. Then once the change has had time to soak, change over to milliseconds only.

[SuGlider]

@me-no-dev / @VojtechBartoska - I think we could do as @lbernstone suggested starting for the release of Core 2.1.0.
What do you think about it?

[petrkr]

Compared how many warnings are during compilation.. I am afraid nobody will notice just one more warning about deprecation.

[me-no-dev]

Compared how many warnings are during compilation.. I am afraid nobody will notice just one more warning about deprecation.

Warnings should be fixed (unless you mean from other libs or your code).

@SuGlider @lbernstone we can add deprecation notice now (2.0.4) question is what version to target in order to give it enough time. Also will deprecation warnings show in the IDE if no-warnings is selected in preferences?

[lbernstone]

I'd say 6-8 months for notification. It will be quite obvious once it changes. I think most people that actively set it are using timeouts less than 60 seconds, so when every connection fails, I'd expect they read the compile log or go look at the header and see that it has changed to milliseconds...

[petrkr]

Warnings should be fixed (unless you mean from other libs or your code

I agree, but lot of ArduinoIDE peoples are not developers but only glue-coders from stack overflow. And they do not understand to that black window output...

And still I do not take there are lot of warnings from other libs and code and this one will really be hidden in mass output... it should be somehow better visible. But in IDE do not know how...

What about use logger? As other debug messages take severity 'warn' and print it out for few releases by logging subsystem. So if someone will be debug why their code works 'strage' this increased log level will warn them

[lbernstone]

You can't make everybody happy. Noobs are unlikely to be setting a timeout in WiFiClient. It is a breaking change, so it is going to break things at some point, and then people will look to see what is failing. A warning in your compile regarding the piece that is breaking should be pretty clear.

[VojtechBartoska]

this relates to open Pull Request #6676