WiFiClient write doesn't consider timeout

[timdah]

Basic Infos

• This issue complies with the issue POLICY doc.
• I have read the documentation at readthedocs and the issue is not addressed there.
• I have tested that the issue is present in current master branch (aka latest git).
• I have searched the issue tracker for a similar issue.
• If there is a stack dump, I have decoded it.
• I have filled out all fields below.

Platform

• Hardware: [ESP-12E]
• Development Env: [PlatformIO]
• Operating System: [Windows]

Settings in IDE

• Module: [Wemos D1 mini r2]
• Flash Size: [4MB]
• CPU Frequency: [80Mhz]
• Upload Using: [SERIAL]
• Upload Speed: [921600]

Problem Description

Seems like the timeout isn't considered for write operations in my case.
When i run the following code and enter an area with bad wifi signal the write method returns after 300-400 ms but the timeout was set to 10 ms. It's blocking the rest of the program too long.
It's weird, because the problem should be fixed since #3257

Sketch

WiFiClient* client = new WiFiClient();
client->setTimeout(10);
...
unsigned long ptime = millis();
uint16_t rc = client->write(buf+(MQTT_MAX_HEADER_SIZE-hlen),length+hlen);
Serial.print("writetime: ");
Serial.println(millis()-ptime);

[d-a-v]

#3257 is apparently for connect() and hostname resolution timeout.
You are measuring write().
What's your error code rc in that case ?

What do you expect write() to do if your link is poor,
effectively send data, or abort even if those data can be sent ?

The semantic of this setTimeout() is also to wait at most this time between effective (sub-)writes.
To explain this, in a worst case, if your link allows to send 1 byte every 5ms, and you have 100 bytes to send, then this timeout won't be of any help and overall duration will be 100*5ms.

So you would need a new "overall-timeout" API that would break the current data sending after that amount of time, knowing that it would destroy any ongoing protocol on that tcp connection.

How much is length+hlen ?

[d-a-v]

You can check on how much data you can send immediately without any waiting, by reading client->availableForWrite().

[timdah]

First of all, thanks for the quick answer 🙂

#3257 is apparently for connect() and hostname resolution timeout.

The first comment of the pull request says "This change implements timeout during connect and write." as well as the title 🤔

What do you expect write() to do if your link is poor,
effectively send data, or abort even if those data can be sent ?
The semantic of this setTimeout() is also to wait at most this time between effective (sub-)writes.
To explain this, in a worst case, if your link allows to send 1 byte every 5ms, and you have 100 bytes to send, then this timeout won't be of any help and overall duration will be 100*5ms.
So you would need a new "overall-timeout" API that would break the current data sending after that amount of time, knowing that it would destroy any ongoing protocol on that tcp connection.

I would expect that write() cancels the transmission if it is not finished within the given timeout, like the "new API" you mentioned.

How much is length+hlen ?

It's between 100-200 bytes.

You can check on how much data you can send immediately without any waiting, by reading client->availableForWrite().

Ah okay, so a workaround would be to only send the message when length+hlen <= availableForWrite() to avoid subwrites?

[d-a-v]

The first comment of the pull request says "This change implements timeout during connect and write." as well as the title 🤔

I read further the first comment 😉 and I explained how this timeout is handled in write():

The semantic of this setTimeout() is also to wait at most this time between effective (sub-)writes.

Ah okay, so a workaround would be to only send the message when length+hlen <= availableForWrite() to avoid subwrites?

Yes, or as chunks if your protocol allows it.

[timdah]

Okay i'll give it a try next week. Thanks for the help 👍

[devyte]

@d-a-v is correct. This timeout is meant for identifying a communication stall. In other words:

• as long as there is some progress in sending, it continues to do so, even if the whole sequence takes a long time.
• If there is stalling for more than timeout time, then the connection is considered dead, and sending is aborted.
• If you need finer granularity to handle other things (i.e.: you can't tolerate a long blocking operation), you should send smaller chunks at a time, at most availableForWrite(), and handle your other processing in between those sends.

Not an issue in the core, closing.