Update client.connect() use

[ssilverman]

Summary of the two changes:

1. Fix checking client.connect() for an error; only '1' is success. Implicit Boolean conversion will turn non-zero errors into true.
2. Add attempt to connect to port 443 on the gateway because some gateways might not have port 80 open.

[ssilverman]

EthernetClient::connect() reference: https://www.arduino.cc/reference/en/libraries/ethernet/client.connect/

[JAndrassy]

the documentation is wrong. all examples have if (client.connect()) and all libraries are implemented that way. everybody uses it as if it has a bool. no library can return anything else that 0 or 'true'

the example can't fit all networks. the user will just have to adopt it to their network. many Ethernet libraries don't support secure layer

[ssilverman]

Mine does; I follow that API spec.
See here: https://github.com/ssilverman/QNEthernet/blob/master/src/QNEthernetClient.cpp
and search for "ConnectReturns::".

This is the big problem with this API spec and one of several reasons I don't love it. Inconsistency and weird choices abound. A Boolean-returning API is a little easier to work with, but then there needs to be a way to find the last error that occurred, and, unfortunately, there's no such standard for this. I've been thinking about how to do this, though. Currently, however, the only way to tell the difference between "timeout", "bad server", or "success" connection attempts is with those error codes.

So for this case, it's actually more correct to check for "==1" because even if an API function returns a bool, bools get implicitly converted (according to the C++ spec) to 1 for true and zero for false. In other words, checking for '1' will always work, but checking for true won't always work, especially for those libraries that follow the Arduino spec.

Related note from my README about the inconsistency in the Arduino examples related to EthernetClient:

The examples at https://www.arduino.cc/reference/en/libraries/ethernet/server.accept/ and https://www.arduino.cc/reference/en/libraries/ethernet/if-ethernetclient/ directly contradict each other with regard to what operator bool() means in EthernetClient. The first example uses it as "already connected", while the second uses it as "available to connect". "Connected" is the chosen concept, but different from connected() in that it doesn't check for unread data.

[JAndrassy]

that is the thing with the established API. we can't fix it. we have to live with it

[ssilverman]

that is the thing with the established API. we can't fix it. we have to live with it

It sounds like you're agreeing with my point that the API is what it is? This means we can't treat EthernetClient::connect() as a Boolean. An API, according to the "established API", as you put it, can return a negative value if there's, say, a timeout or host lookup error. These will get converted to true and therefore "success" if treated as a Boolean.

It's the same deal with #3 that was accepted. Ethernet.hostByName() returns an int.

[JAndrassy]

everybody does treat it as a boolean because of the official examples. I don't recommend going against it.
it is simpler to changes the documentation.

[ssilverman]

everybody does treat it as a boolean because of the official examples. I don't recommend going against it

The official examples are wrong. In any case, comparing using "==1" isn't wrong. Similar to PR #3 that was accepted. (See the Ethernet.hostByName() check.)

[ssilverman]

everybody does treat it as a boolean because of the official examples. I don't recommend going against it. it is simpler to changes the documentation.

Right, but the documentation "is what is" right now (see your post here: #4 (comment)). Here's the spec: https://www.arduino.cc/reference/en/libraries/ethernet/client.connect/

So are you proposing to use the documented spec or the examples that disagree with that spec? I've chosen to go with the documented spec for my API.

[JAndrassy]

hostByName is optional advanced function, it is not used so frequently and is not in the examples. most implementations return 1 or 0. only a few have other return values.

I think I will initiate the change of the doc for connect

[ssilverman]

I'll add: the examples were probably made without understanding how integers get transformed into Booleans in C++, so I'd personally use the spec.

hostByName is optional advanced function, it is not used so frequently and is not in the examples. most implementations return 1 or 0. only a few have other return values.

I think I will initiate the change of the doc for connect

That would be great, but if that changes, how does one tell the difference between a timeout, bad host lookup, some other error, or success? There needs to be either a reference/pointer argument where the error gets stored or some sort of "get last error" function. Otherwise, the API can't be used at all for robust applications. A simple Boolean return doesn't work.

[JAndrassy]

so it is Arduino and thing should be simple at top level.
advanced users can do hostByName first to separate DNS part from connect.

[ssilverman]

so it is Arduino and thing should be simple at top level. advanced users can do hostByName first to separate DNS part from connect.

True. What about a timeout?

Update: Let me answer myself: Check whether connected.
Update 2: For the record, I prefer a simpler API.

[JAndrassy]

so there is only port refused or timeout for not secured connection. port refused happens immediately,

[JAndrassy]

for secured connection esp8266 has getLastSSLError