[PubSubClient3] Speedup publish large messages using buffer

[TD-er]

As discussed here: #29 (comment)

Not yet tested, as I really need to get some sleep now...

My biggest concern right now is whether writeNextMsgId might also need to call flushBuffer() first.
But as I said, can't think clearly right now so if you can have a look @hmueller01

[hmueller01]

I think this can't be implemented like this.

size_t PubSubClient::write(const uint8_t* buffer, size_t size) {
    const size_t rc = appendBuffer(buffer, size);
    if (rc != 0) {
        lastOutActivity = millis();
    }
    return rc;
}

We can't set lastOutActivity here, as appendBuffer() does not necessarily write to the the broker. And we need this to ping the broker in time ...

pubsubclient3/src/PubSubClient.cpp

Line 470 in 67eba23

if (keepAliveMillis && ((t - lastInActivity > this->keepAliveMillis) || (t - lastOutActivity > this->keepAliveMillis))) {

But setting lastOutActivity in flushBuffer() only, also won't work in all cases (e.g. if the application takes to long to fill all the needed data), as here

pubsubclient3/src/PubSubClient.cpp

Line 478 in 67eba23

this->buffer[0] = MQTTPINGREQ;

we will overwrite the buffer to send the MQTTPINGREQ.
Maybe we can just flushBuffer() here, check the rc and only if it's 0 do the MQTTPINGREQ.

To your writeNextMsgId() concern. I don't think there is a problem. writeNextMsgId() is used in endPublish() there you did the flushBuffer() before and in subscribe() / unsubscribe() - don't care ...

[TD-er]

Well since appendBuffer may call flushBuffer, which does actual communications (and thus should set the lastOutActivity), I agree that appendBuffer itself and thus also these write calls should not set this lastOutActivity .

[TD-er]

I was thinking... maybe all _client.write calls should use this appendBuffer, just to make sure there isn't any risk of mixing up different calls.

[hmueller01]

This was my first thought too. But then more things need to be rewritten ...

[hmueller01]

And yes, at least here

pubsubclient3/src/PubSubClient.cpp

Line 549 in 67eba23

rc += _client->write((uint8_t)pgm_read_byte_near(payload + i));

we should use it as well to solve #36 ...

[hmueller01]

@TD-er Did some implementations mentioned above. Looks quite promising. If you have time you may have a look over it.

[hmueller01]

@TD-er We could merge appendBuffer(uint8_t data)

pubsubclient3/src/PubSubClient.cpp

Line 768 in 5b2c586

size_t PubSubClient::appendBuffer(uint8_t data) {

into

pubsubclient3/src/PubSubClient.cpp

Lines 644 to 646 in 5b2c586

	size_t PubSubClient::write(uint8_t data) {
	return appendBuffer(data);
	}

and remove appendBuffer(uint8_t data). What do you think? Might be less clear ...

[hmueller01]

This PR fixes #29.

[hmueller01]

This fixes #36

[hmueller01]

@TD-er If you could take a look over it, I'd like to merge this. Tested it for a few weeks on a device of mine (but only uses publish_P()).