[MQTTPINGREQ] lastOutActivity is not set when replying to ping req.

[TD-er]

Use case is a client connected to the broker without sending a lot of messages.

Typically the client will send out a keep-alive ping (MQTTPINGREQ) to the broker and the broker will reply to it with a MQTTPINGRESP
However if the broker sends out such a MQTTPINGREQ first, the client replies with a MQTTPINGRESP but doesn't set the lastOutActivity.

I assume the broker may not reply to a MQTTPINGREQ if it is sent very shortly after the last keep-alive (requested by the broker) was dealt with and acknowledged.

These kind of disconnects seem to happen mainly recently as I got quite a few reports lately where it seemed to be "fixed" by sending out more messages to keep within this keep-alive interval.
So I assume there are some MQTT brokers out there which may have implemented such a rate-limiter feature (or only now enabled by default?) on keep-alive pings.

See:

pubsubclient/src/PubSubClient.cpp

Lines 419 to 423 in 2d228f2

	} else if (type == MQTTPINGREQ) {
	this->buffer[0] = MQTTPINGRESP;
	this->buffer[1] = 0;
	_client->write(this->buffer,2);
	} else if (type == MQTTPINGRESP) {

Suggested fix:

 } else if (type == MQTTPINGREQ) { 
     this->buffer[0] = MQTTPINGRESP; 
     this->buffer[1] = 0; 
     if (_client->write(buffer,2) != 0) {
         lastOutActivity = t;
     }
 } else if (type == MQTTPINGRESP) { 

N.B. the same issue here, when MQTT_MAX_TRANSFER_SIZE is defined

pubsubclient/src/PubSubClient.cpp

Lines 581 to 603 in 2d228f2

	boolean PubSubClient::write(uint8_t header, uint8_t* buf, uint16_t length) {
	uint16_t rc;
	uint8_t hlen = buildHeader(header, buf, length);
	#ifdef MQTT_MAX_TRANSFER_SIZE
	uint8_t* writeBuf = buf+(MQTT_MAX_HEADER_SIZE-hlen);
	uint16_t bytesRemaining = length+hlen; //Match the length type
	uint8_t bytesToWrite;
	boolean result = true;
	while((bytesRemaining > 0) && result) {
	bytesToWrite = (bytesRemaining > MQTT_MAX_TRANSFER_SIZE)?MQTT_MAX_TRANSFER_SIZE:bytesRemaining;
	rc = _client->write(writeBuf,bytesToWrite);
	result = (rc == bytesToWrite);
	bytesRemaining -= rc;
	writeBuf += rc;
	}
	return result;
	#else
	rc = _client->write(buf+(MQTT_MAX_HEADER_SIZE-hlen),length+hlen);
	lastOutActivity = millis();
	return (rc == hlen+length);
	#endif
	}

Suggested fix:

    while((bytesRemaining > 0) && result) {
        delay(0);  // Prevent watchdog crashes
        bytesToWrite = (bytesRemaining > MQTT_MAX_TRANSFER_SIZE)?MQTT_MAX_TRANSFER_SIZE:bytesRemaining;
        rc = _client->write(writeBuf,bytesToWrite);
        result = (rc == bytesToWrite);
        bytesRemaining -= rc;
        writeBuf += rc;
        if (rc != 0) {
            lastOutActivity = millis();
        }
    }
    return result;