knolleary / pubsubclient

A client library for the Arduino Ethernet Shield that provides support for MQTT.
http://pubsubclient.knolleary.net/
MIT License
3.84k stars 1.47k forks source link

Check for _client->connected() while waiting for CONNACK #708

Open mamama1 opened 4 years ago

mamama1 commented 4 years ago

in the function

boolean PubSubClient::connect(const char *id, const char *user, const char *pass, const char* willTopic, uint8_t willQos, boolean willRetain, const char* willMessage, boolean cleanSession)

while waiting for the CONNACK message from the mqtt broker, you should also check for _client->connected() and abort if the connection has been closed by the server.

according to the mqtt protocol specification, a server is not obliged to send a CONNACK message with the corresponding error flag set to the client. your pubsub client awaits the timeout in a while loop instead of simply returning if the connection has been closed anyway.

i have solved it like this:

while (!_client->available()) {
                unsigned long t = millis();
                if (!_client->connected())
                {
                    _state = MQTT_CONNECTION_LOST;
                    _client->stop();
                    return false;
                }

                if (t-lastInActivity >= ((int32_t) MQTT_SOCKET_TIMEOUT*1000UL)) {
                    _state = MQTT_CONNECTION_TIMEOUT;
                    _client->stop();
                    return false;
                }
            }
mamama1 commented 4 years ago

erm... bump?

knolleary commented 4 years ago

If you wanted to raise a pull-request with your proposed change in, I'm happy to take a look

mamama1 commented 4 years ago

i have posted the change to your function above, unfortunately I have procrastinated on how to do pull requests, merges, branches, and such stuff for way too long now, so I unfortunately don't know how to do a pull request... :( eventually I will take a look at it but not today and not tomorrow...

izaiasemjr commented 4 years ago

@mamama1 your solution really works? you tested this in your code?

mamama1 commented 4 years ago

yes, i'm using this fix in my code.