Arduino
Arduino copied to clipboard
documentation: add description for connected() and status()
ref. the original PR and the discussion https://github.com/esp8266/Arduino/pull/4626 https://github.com/esp8266/Arduino/issues/6701
as mentioned in the https://github.com/esp8266/Arduino/issues/8327#issuecomment-933369314
(I have yet to check the readthedocs rendering)
And as #6701 already mentioned, this is somewhat different for the secure variant: https://github.com/esp8266/Arduino/blob/9d024d17fd18dffa569fa646b5326b5e8f58b0d5/libraries/ESP8266WiFi/src/WiFiClientSecureBearSSL.cpp#L259-L264 But, it's unclear whether this refers to the WiFiClient interface or the basic-socket WiFiClient class
To summarize the need for this change:
In the case where connected()
was returning true when the connection was closed by peer but data were still available for read in receive buffers, sketches found themselves allowed to write something which obviously leaded to an error when they did.
Removing the available()
test from connected()
could stop this behavior.
The downside of this change is that available()
has to be checked even if connected()
is false.
... and if this (half-closed by peer) connection is fully closed by sketch before reading remaining data, peer complains because its data will never be read. The server example in doc allows to reproduce this, the matching posix error on the peer side shows a 'Broken pipe' if I remember well.
So indeed I think this PR should also remove the available()
part of TLS's connected()
function for coherency with WiFiClient
implementation. Good catch.
But, it's unclear whether this refers to the WiFiClient interface or the basic-socket WiFiClient class
The TLS context is supposed to match the WiFiClient class.
The TLS context is supposed to match the WiFiClient class.
...true, but also note that it is used internally. can update, but it may be better suited as a separate patch
Will update with the 'connected' part with the reasoning, seems like a good place to also explain things
but also note that it is used internally.
Right!
In those places, one must check whether testing available()
must be added next to connected()
, or replace it.