WiFiClient setTimeout is documented to accept milliseconds (sane) but value is used as seconds directly (loss of resolution is not sane). Protoype expects u32 instead of f32/f64.

[emilfihlman]

WiFiClient setTimeout is documented to accept milliseconds (sane) but value is used as seconds directly (loss of resolution is not sane). Protoype expects u32 instead of f32/f64.

Documentation (inheritance) on value being milliseconds, not seconds
https://www.arduino.cc/reference/en/language/functions/communication/stream/streamsettimeout/

/path/to/sketch.ino: In function 'void loop()':
sketch:614:31: error: invalid conversion from 'const char*' to 'uint32_t {aka unsigned int}' [-fpermissive]
connection.setTimeout("asd");
^
In file included from /path/to/arduino/.arduino15/packages/esp32/hardware/esp32/1.0.4/libraries/WiFi/src/WiFi.h:37:0,
from /path/to/sketch.ino:5:
/path/to/arduino/.arduino15/packages/esp32/hardware/esp32/1.0.4/libraries/WiFi/src/WiFiClient.h:91:9: note: initializing argument 1 of 'virtual int WiFiClient::setTimeout(uint32_t)'
int setTimeout(uint32_t seconds);

On Linux dell 4.19.0-6-amd64 #1 SMP Debian 4.19.67-2+deb10u2 (2019-11-11) x86_64 GNU/Linux
Arduino is 1.8.11 Hourly Build 2020/02/05 07:25

Reproduction:
u32 t;
u8 buf[1024];
WiFiClient connection;
connection.connect(server, port);
connection.setTimeout(2);
t=millis();
Serial.print(t);
connection.readBytes(buf, 1024);
//Send say a single character from your server
Serial.print(millis()-t);

[lbernstone]

While the choice to use seconds rather than msec is certainly debatable, WiFiClient is not a stream object, and setTimeout is not even available in the arduino version of the library. This is not a bug, and the header file clearly defines the parameter as seconds.

[MarkusAD]

It's a timeout for cryin' out loud. How accurate does it need to be? Don't waste any more of my memory for your bloody accurate timeout.

[emilfihlman]

@lbernstone alrighty, thanks for clearing it up. In the interest of adhering to common and expected behaviour in the Arduino flavour, I believe it should be changed to milliseconds (or possibly micros, nanos, whatever the best timebase is). Keeping it at a second is both unnecessary and silly, both decreasing resolution and making the lib harded to use, we are talking about an embedded device here afterall.

@markyad that's a pretty toxic, unhelpful and not relevant comment. It accepts an u32, are you expecting to wait for max 136 years? Wouldn't perhaps 50 days be a better max timeout?

[MarkusAD]

@emilfihlman Tempus fugit, time-tripper. If a client times out in 6 seconds instead of 5.6 why would anyone care? Besides, how things work around here doesn't involve sitting back and saying "this doesnt work the way I like, fix it". YOU fix it the way you think it should be and submit it as a pull request and the developers will either accept it or not. Now get busy, there is no core issue here.

[MarkusAD]

@emilfihlman quoting your original post:

sketch:614:31: error: invalid conversion from 'const char*' to 'uint32_t {aka unsigned int}' [-fpermissive]
connection.setTimeout("asd");

You tried passing it a character array? WTF is that idiocy?

Protoype expects u32 instead of f32/f64.

Look through the core header files and spot where those short variable types are used, they aren't. Its uint32_t btw. Why would you want to make it larger still by going to a 64 bit type? I could see making it uint16_t but thats not what you posted. No one said you couldn't pass it a uint8_t or better the way it is, did you even try that?

[MarkusAD]

@markyad that's a pretty toxic, unhelpful and not relevant comment. It accepts an u32, are you expecting to wait for max 136 years? Wouldn't perhaps 50 days be a better max timeout?

What you'll find is that the esp32 is fastest when it has to deal with 4-byte (uint32_t) aligned data types. Not that it matters for a function thats mostly used in setup() but thats why its coded the way it is.

Again, feel free to submit your pull request with why you think a sub-second timeout is needed and let the developers decide. @lbernstone already pointed out that there is no setTimeout() in the arduino library so its not a matter of "arduino flavor". esp32 is not an arduino in case you havent realized that yet, and a subsecond timeout for this routine amounts to unneeded fluff (IMHO).

Awaiting your pull request. Good luck.

[MarkusAD]

Get busy @emilfihlman

[emilfihlman]

@markyad do you not understand how automatic type coercion works or are you how angry now? Can you not figure out that to get the required information easily, you must give the function shit to make the compiler give you the actual datatypes? Not to mention that it sounded so insane to me to expect a second resolution that I offered to accept floats and a mistake in type selection.

And while you might not care, how can you ever make a decision what matters to others? Millisecond resolution is the expected and standard resolution and a departure from that to a smaller resolution is just stupid.

I'm honestly pretty disappointed that you represented this project at all. Please don't waste everyone's time by participating since clearly you are only about being an ass towards others.

[MarkusAD]

I dont represent this project I represent myself. Get to coding your pull request and STFU already.

[lbernstone]

A pull request would need to define a new function for this, as redefining it would have bad consequences for current users.

[emilfihlman]

@lbernstone we could redefine it to accept an f32 (a choice I'm not super thrilled about), which would prevent breaking existing applications. Making a new function is easy enough, though, but still gives a bad taste.

[emilfihlman]

Looking at the code, it's clearly a developer error in using seconds where milliseconds would have been correct. Everywhere else timeouts are in milliseconds, not to mention that WiFiClient write operates on Stream objects (

arduino-esp32/libraries/WiFi/src/WiFiClient.cpp

Line 413 in 109ba7a

size_t WiFiClient::write(Stream &stream)

), so it's even easier to see that it's developer error.

I recommend we simply change timeout to milliseconds as it has always been intended to be, and update any documentation to match. In this instance any "breaking" of existing applications is acceptable, since timeouts a) cannot be used for accurate timekeeping anyhow so client applications must address timing issues anyhow and b) documentation is easy to look up.

I'll try to write up a pull request this weekend but everyone else is of course welcome to do that in my place.

[stale]

[STALE_SET] This issue has been automatically marked as stale because it has not had recent activity. It will be closed in 14 days if no further activity occurs. Thank you for your contributions.

[presslab-us]

I came here trying to understand why setTimeout() was in seconds. It clearly looks like a bug that was 'covered up' so to speak. I'd like the additional resolution because I'm dealing with the default 5 second task watchdog timeout.

It's too bad some of the contributors here have such an attitude, but that's open source for you. I don't really want to be the one to deal with them, but if @emilfihlman were to do a pull request myself and I'm sure others would appreciate it.

[stale]

[STALE_CLR] This issue has been removed from the stale queue. Please ensure activity to keep it openin the future.

[MarkusAD]

He's not going to do anything because he doesn't know how. I have no interest in coding a rework for this because 1. I see no need for less than second resolution. 2. Im not dealing with any breaking changes that reworking it would cause.

Also, If you're getting into task watchdog timeouts, I'd consider re-looking at what my own code was doing rather than trying to work around it with core routines. Task watchdog timeouts are a bad sign that your task is starving the system for cpu time.

[MarkusAD]

Time to rewrite your code, not the core. lmao.

[presslab-us]

Sigh, you're not really worth a reply.

The timeouts are occurring inside WiFiClient as the default tx timeouts are 10*1000000us, or 10 seconds. This is twice the 5 seconds given in the default arduino-esp32 task watchdog, and they are obviously not designed to work together. This is something else that needs work, but that's another issue.

The reason for the finer granularity is to give as much time as possible for a WiFiClient timeout, as I don't need a whole second of overhead for my own code. With 1 second granularity I can't make it smaller than this. lmao.

[MarkusAD]

So stop relying on others and get to coding. How do you think we got to where we are?

[MarkusAD]

I doubt this PR will be accepted at all to tell you the truth, especially after all this.

[MarkusAD]

Funny how this issue seems to only bother you two, and not the 4.7K others who use this repo.

[presslab-us]

Why would I get to coding when people like you will reject the PR? I already stated I would not do a pull request because of your attitude.

You're talking out of both sides of your face, but I doubt you will ever have the introspection to realize this.

[MarkusAD]

I dont have a thing to do at all with accepting PR's, thats your first misconception.

[MarkusAD]

Stop wasting my time and go fix your broken code. Mine works, yours can too.

[presslab-us]

I'm not really sure what your problem is, nor do I really care to know.

As you seem to have a pattern of unhelpful behavior I think it would be best if the maintainers limited your access, otherwise it sets a bad example for this project.

[stale]

[STALE_SET] This issue has been automatically marked as stale because it has not had recent activity. It will be closed in 14 days if no further activity occurs. Thank you for your contributions.

[stale]

[STALE_DEL] This stale issue has been automatically closed. Thank you for your contributions.

[jarda195]

WiFiClien.cpp

int WiFiClient::setTimeout(uint32_t seconds)
{
Client::setTimeout(seconds * 1000); -> modify Client::setTimeout(seconds); //working
struct timeval tv;
tv.tv_sec = seconds;
tv.tv_usec = 0;
if(setSocketOption(SO_RCVTIMEO, (char *)&tv, sizeof(struct timeval)) < 0) {
return -1;
}
return setSocketOption(SO_SNDTIMEO, (char *)&tv, sizeof(struct timeval));
}

just remove 1000

[jordam]

The behavior shown in this thread is disgraceful, it violates the GitHub community guidelines, and I agree that espressif would be wise to limit access to the repo when behavior is this abrasive. The back and forth shown here does not encourage other programmers new to the platform to contribute. It only hurts the group as a whole.

This is an important issue, it should be fixed and is noncompliant with nearly every expectation of how a timeout operates on a microcontroller. Saying you don't need a timeout less than a full second is just silly.

[TheDevCactus]

🍿

[DriekdeGadgetfreak]

I'm moving from ESP8266 tot ESP32. setTimeout argument for a WiFiClient changes from millis tot seconds. Took me a lot of time to figure this out.

[Yacubane]

Can we reopen this issue?

[JoeEarlam]

Imagine being such a terrible person you want a sub-second timeout and parity with other cores, gosh.

[JAndrassy]

@JoeEarlam some problem?