From db003d30d1c167f8029e746069e801026d3ee1be Mon Sep 17 00:00:00 2001 From: xdylanm Date: Fri, 14 May 2021 08:09:07 -0700 Subject: [PATCH] Addressing comments from PR #193 with merge from PR#166. Clang-format, version ticked to 2.3.0, replaced duplicate topicOffsetFromLength with packetAdditionalLen. --- Adafruit_MQTT.cpp | 62 ++++++++++++++++------------------------ Adafruit_MQTT_Client.cpp | 2 +- library.properties | 2 +- 3 files changed, 26 insertions(+), 40 deletions(-) diff --git a/Adafruit_MQTT.cpp b/Adafruit_MQTT.cpp index 4c4dd96..a38ec80 100644 --- a/Adafruit_MQTT.cpp +++ b/Adafruit_MQTT.cpp @@ -96,6 +96,22 @@ static uint8_t *stringprint(uint8_t *p, const char *s, uint16_t maxlen = 0) { return p + len; } +// packetAdditionalLen is a helper function used to figure out +// how bigger the payload needs to be in order to account for +// its variable length field. As per +// http://docs.oasis-open.org/mqtt/mqtt/v3.1.1/os/mqtt-v3.1.1-os.html#_Table_2.4_Size +// See also readFullPacket +static uint16_t packetAdditionalLen(uint32_t currLen) { + /* Increase length field based on current length */ + if (currLen < 128) // 7-bits + return 0; + if (currLen < 16384) // 14-bits + return 1; + if (currLen < 2097152) // 21-bits + return 2; + return 3; +} + // Adafruit_MQTT Definition //////////////////////////////////////////////////// Adafruit_MQTT::Adafruit_MQTT(const char *server, uint16_t port, const char *cid, @@ -267,7 +283,8 @@ uint16_t Adafruit_MQTT::readFullPacket(uint8_t *buffer, uint16_t maxsize, DEBUG_PRINT(F("Packet Length:\t")); DEBUG_PRINTLN(value); - if (value > (maxsize - (pbuff - buffer) - 1)) { + // maxsize is limited to 65536 by 16-bit unsigned + if (value > uint32_t(maxsize - (pbuff - buffer) - 1)) { DEBUG_PRINTLN(F("Packet too big for buffer")); rlen = readPacket(pbuff, (maxsize - (pbuff - buffer) - 1), timeout); } else { @@ -474,22 +491,6 @@ Adafruit_MQTT_Subscribe *Adafruit_MQTT::readSubscription(int16_t timeout) { return handleSubscriptionPacket(len); } -namespace { - -uint16_t topicOffsetFromLength(uint16_t const len) -{ - if (len < 128) { // 7 bits (+1 continuation bit) - return 0; - } else if (len < 16384) { // 14 bits (+2 continuation bits) - return 1; - } else if (len < 2097152) { // 21 bits - return 2; - } - return 3; -} - -} // anon - Adafruit_MQTT_Subscribe *Adafruit_MQTT::handleSubscriptionPacket(uint16_t len) { uint16_t i, topiclen, datalen; @@ -508,9 +509,9 @@ Adafruit_MQTT_Subscribe *Adafruit_MQTT::handleSubscriptionPacket(uint16_t len) { } // Parse out length of packet. - uint16_t const topicoffset = topicOffsetFromLength(len); + uint16_t const topicoffset = packetAdditionalLen(len); uint16_t const topicstart = topicoffset + 4; - topiclen = buffer[3+topicoffset]; + topiclen = buffer[3 + topicoffset]; DEBUG_PRINT(F("Looking for subscription len ")); DEBUG_PRINTLN(topiclen); @@ -523,8 +524,8 @@ Adafruit_MQTT_Subscribe *Adafruit_MQTT::handleSubscriptionPacket(uint16_t len) { continue; // Stop if the subscription topic matches the received topic. Be careful // to make comparison case insensitive. - if (strncasecmp((char *)buffer + topicstart, subscriptions[i]->topic, topiclen) == - 0) { + if (strncasecmp((char *)buffer + topicstart, subscriptions[i]->topic, + topiclen) == 0) { DEBUG_PRINT(F("Found sub #")); DEBUG_PRINTLN(i); break; @@ -552,8 +553,8 @@ Adafruit_MQTT_Subscribe *Adafruit_MQTT::handleSubscriptionPacket(uint16_t len) { datalen = SUBSCRIPTIONDATALEN - 1; // cut it off } // extract out just the data, into the subscription object itself - memmove(subscriptions[i]->lastread, buffer + topicstart + topiclen + packet_id_len, - datalen); + memmove(subscriptions[i]->lastread, + buffer + topicstart + topiclen + packet_id_len, datalen); subscriptions[i]->datalen = datalen; DEBUG_PRINT(F("Data len: ")); DEBUG_PRINTLN(datalen); @@ -687,21 +688,6 @@ uint8_t Adafruit_MQTT::connectPacket(uint8_t *packet) { return len; } -// packetAdditionalLen is a helper function used to figure out -// how bigger the payload needs to be in order to account for -// its variable length field. As per -// http://docs.oasis-open.org/mqtt/mqtt/v3.1.1/os/mqtt-v3.1.1-os.html#_Table_2.4_Size -static uint16_t packetAdditionalLen(uint16_t currLen) { - /* Increase length field based on current length */ - if (currLen < 128) - return 0; - if (currLen < 16384) - return 1; - if (currLen < 2097151) - return 2; - return 3; -} - // as per // http://docs.oasis-open.org/mqtt/mqtt/v3.1.1/os/mqtt-v3.1.1-os.html#_Toc398718040 uint16_t Adafruit_MQTT::publishPacket(uint8_t *packet, const char *topic, diff --git a/Adafruit_MQTT_Client.cpp b/Adafruit_MQTT_Client.cpp index 2720ea9..9aa136c 100644 --- a/Adafruit_MQTT_Client.cpp +++ b/Adafruit_MQTT_Client.cpp @@ -95,7 +95,7 @@ bool Adafruit_MQTT_Client::sendPacket(uint8_t *buffer, uint16_t len) { DEBUG_PRINTLN(ret); len -= ret; offset += ret; - + if (ret != sendlen) { DEBUG_PRINTLN("Failed to send packet."); return false; diff --git a/library.properties b/library.properties index 230e502..d544748 100644 --- a/library.properties +++ b/library.properties @@ -1,5 +1,5 @@ name=Adafruit MQTT Library -version=2.2.0 +version=2.3.0 author=Adafruit maintainer=Adafruit sentence=MQTT library that supports the FONA, ESP8266, Yun, and generic Arduino Client hardware.