ESPLwIPClient setTimeout conflicts with Stream setTimeout

[JAndrassy]

ESPLwIPClient::setTimeout conflicts with Stream::setTimeout

the classes hierarchy is Print <- Steam <- Client <- ESPLwIPClient <- WiFiClient

arduino-esp32/cores/esp32/Stream.h

Line 61 in b580bb2

void setTimeout(unsigned long timeout); // sets maximum milliseconds to wait for stream data, default is 1 second

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

Line 36 in b580bb2

virtual int setTimeout(uint32_t seconds) = 0;

[rin67630]

+1
That bug has hard consequences and prevents me to control my ESP32 devices over the network since a stream will timeout upon a manual input and
client.setTimeout(60000);
has no effect.
Please categorize it to a high priority.
Besides that, it is not admissible that ESP32 and ESP8266 behave that differently on standard methods.

[VojtechBartoska]

Hello @JAndrassy, sorry for late reply. We will take a look on this if we can fix this.

[P-R-O-C-H-Y]

Hi @JAndrassy , @rin67630
Can you provide some sketch, so I can test and fix that? :) Thanks

[JAndrassy]

so you should not change the Arduino API (Stream.h). I don't know what for is the other setTimeout but it should be removed or renamed

[P-R-O-C-H-Y]

Sure the change will be in ESPLwIPClient class or WiFiClient class.
But @rin67630 mentioned that the timeout does not have any effect. So I would like to test that to be able to fix everything at once.

[JAndrassy]

Sure the change will be in ESPLwIPClient class or WiFiClient class. But @rin67630 mentioned that the timeout does not have any effect. So I would like to test that to be able to fix everything at once.

I guess rin's problem is that it has effect on something else than expected

[P-R-O-C-H-Y]

@JAndrassy How did you compile that? I have no errors / warning during compilation and the timeout is working fine for me.

[JAndrassy]

@JAndrassy How did you compile that? I have no errors / warning during compilation and the timeout is working fine for me.

which of the two possibilities is working?

[P-R-O-C-H-Y]

Can you test WifiClient.cpp and WifiClient.h files from my fork? I'm making changes now, because the virtual cannot be there :)

link to my repo + branch with fixes

[P-R-O-C-H-Y]

Can you test WifiClient.cpp and WifiClient.h files from my fork? I'm making changes now, because the virtual cannot be there :)

link to my repo + branch with fixes

You can test changes from this PR

[JAndrassy]

sorry, I assumed you know the Arduino API Print and Stream classes as I think it is a very important part of Arduino core.

setTimeout and _timeout exist in Stream.h/cpp. _timeout is used in timed read functions readBytes, readBytesUntil, readString, readStringUntil, parseInt, parseFloat, find.
the _timeout in Stream class can't be set if setTimeout and _timeout exist in WiFiClient

[P-R-O-C-H-Y]

Can you check latest changes in linked PR? setTimeout is removed from WifiClient / WifiClientSecure.

[JAndrassy]

Stream timeout is now OK, but how is _timeout in WiFiClient set? and what is its purpose? is it for closing inactive TCP connections? I guess users would choose a very different timeout for TCP connection timeout than for Stream timed read functions. For TCP connection timeout seconds are a good scale. So if I understand this right the setTimeout in ESPLwIPClient and WiFiClent should be renamed to setConnectionTimeout(millis)? (and it should not call setTimeout of Stream)

and it would be good to know for HttpCliemt, which timeout was meant to be set

[P-R-O-C-H-Y]

Also feel free to comment and suggest changes to the PR if I missed something.

[P-R-O-C-H-Y]

the setTimeout is for read/write functions. This change will make the timeouts to be set in milliseconds to be same as Arduino Stream setTimeout. The connection timeout can be set in call Client.connect() function eg.
int connect(IPAddress ip, uint16_t port, int32_t timeout);

[podaen]

Having troubles to analyse the the stream input. I want to serial.print the count of the readed bytes in the stream.cpp but nothing is shown. If I serial.print in stream.h I got an error...

Stream.h: 104:9: error: 'Serial' was not declared in this scope
Serial.print("read bytes stream.h")

[P-R-O-C-H-Y]

Having troubles to analyse the the stream input. I want to serial.print the count of the readed bytes in the stream.cpp but nothing is shown. If I serial.print in stream.h I got an error...

Stream.h: 104:9: error: 'Serial' was not declared in this scope
Serial.print("read bytes stream.h")

Instead of Serial.print("read bytes stream.h") use log_d("read bytes stream.h") and set Core debug level to DEBUG.

[JAndrassy]

@podaen why do you hijack my issue report for an unrelated problem? please delete your comments

[podaen]

Why making a new one?? If I had that same issue, I want to see if it works for me too.

[podaen]

@P-R-O-C-H-Y Can you do this for client.h as well?

[VojtechBartoska]

Why making a new one?? If I had that same issue, I want to see if it works for me too.

Hi @podaen, up to your comment it seems you are facing different issue (correct me if I'm wrong). Due to that it's better to open an new issue if it's needed. The goal is to uncover specific problem in particular issue and solved it directly.

Other benefit of opening a new issue is providing more complex information thanks to the issue template. From a single comment it's almost impossible to just reproduce your code and at least verify the issue.

Also, if you are more experienced and as I can see you have specific request, don't hesitate to open a Pull Request. Generally it's much faster approach comparing to opened issue.

I hope you understand all parts mentioned above. We would like to help you as much as possible but please keep in mind that are resources are also limited.

Thanks.

[podaen]

Why so defendant? I admit that it was wrong to print in the stream over the hardware serial. I investigated it and it was is not the right thing to do... My attention didn't came out of nowhere, it came out of the stream and timeout. And I make some correction and I will test too!

[VojtechBartoska]

it wasn't meant to be defendant. Oki, looking forward for your feedback then! :)

[podaen]

@JAndrassy The socket will have the timeout when connecting to the wificlient. So, you can close this topic. The conflict was already checkt by you I think.

Still under investigation: The timeout is not been shared from the wificlient or the sll client to the stream.h. The client.h is the link for sharing to stream.h! I have to keep this in mind to continue tomorrow.

[JAndrassy]

@podaen I created this issue report not for some troubleshooting, but to resolve a code design mistake in the esp32 WiFi library which can't be resolved without a breaking change and a decision must be made about it by the maintainers.
your comments are out of context here. there is nothing to debug (and learn how to debug)

[VojtechBartoska]

Issue postponed to 2.1.0 milestone. More investigation and testing is needed. Thanks for understanding.

[JAndrassy]

Other libraries have client.setConnectionTimeout. It was first used in the 2.0.0 version of the Ethernet library in 2018. There it is used in client.stop() too.