From 73f5be4e5c6ed2a9b98f81d5cf00d77bb3a2e935 Mon Sep 17 00:00:00 2001 From: Matthijs Kooijman Date: Thu, 2 Jul 2015 12:59:46 +0200 Subject: [PATCH 01/11] Use const pointers for payload The payload data is never modified by the library, so using const is possible without any further changes. Using const allows using a string literal, or String::c_str() as the payload. --- Adafruit_MQTT.cpp | 6 +++--- Adafruit_MQTT.h | 6 +++--- 2 files changed, 6 insertions(+), 6 deletions(-) diff --git a/Adafruit_MQTT.cpp b/Adafruit_MQTT.cpp index 5d98279..d7963dc 100644 --- a/Adafruit_MQTT.cpp +++ b/Adafruit_MQTT.cpp @@ -140,7 +140,7 @@ int8_t Adafruit_MQTT::connect() { return 0; } -bool Adafruit_MQTT::publish(const char *topic, char *data, uint8_t qos) { +bool Adafruit_MQTT::publish(const char *topic, const char *data, uint8_t qos) { // Construct and send publish packet. uint8_t len = publishPacket(buffer, topic, data, qos); if (!sendPacket(buffer, len)) @@ -294,7 +294,7 @@ uint8_t Adafruit_MQTT::connectPacket(uint8_t *packet) { } uint8_t Adafruit_MQTT::publishPacket(uint8_t *packet, const char *topic, - char *data, uint8_t qos) { + const char *data, uint8_t qos) { uint8_t *p = packet; uint16_t len; @@ -375,7 +375,7 @@ bool Adafruit_MQTT_Publish::publish(uint32_t i) { return mqtt->publish(topic, payload, qos); } -bool Adafruit_MQTT_Publish::publish(char *payload) { +bool Adafruit_MQTT_Publish::publish(const char *payload) { return mqtt->publish(topic, payload, qos); } diff --git a/Adafruit_MQTT.h b/Adafruit_MQTT.h index c5c76aa..7d89801 100644 --- a/Adafruit_MQTT.h +++ b/Adafruit_MQTT.h @@ -116,7 +116,7 @@ class Adafruit_MQTT { // Publish a message to a topic using the specified QoS level. Returns true // if the message was published, false otherwise. - bool publish(const char *topic, char *payload, uint8_t qos); + bool publish(const char *topic, const char *payload, uint8_t qos); // Add a subscription to receive messages for a topic. Returns true if the // subscription could be added, false otherwise. @@ -161,7 +161,7 @@ class Adafruit_MQTT { // Functions to generate MQTT packets. uint8_t connectPacket(uint8_t *packet); - uint8_t publishPacket(uint8_t *packet, const char *topic, char *payload, uint8_t qos); + uint8_t publishPacket(uint8_t *packet, const char *topic, const char *payload, uint8_t qos); uint8_t subscribePacket(uint8_t *packet, const char *topic, uint8_t qos); uint8_t pingPacket(uint8_t *packet); }; @@ -171,7 +171,7 @@ class Adafruit_MQTT_Publish { public: Adafruit_MQTT_Publish(Adafruit_MQTT *mqttserver, const char *feed, uint8_t qos = 0); - bool publish(char *s); + bool publish(const char *s); bool publish(double f, uint8_t precision=2); // Precision controls the minimum number of digits after decimal. // This might be ignored and a higher precision value sent. bool publish(int32_t i); From 6879df86f68658e9a5bd941a8b7cea2543e36cb2 Mon Sep 17 00:00:00 2001 From: Matthijs Kooijman Date: Thu, 2 Jul 2015 13:01:05 +0200 Subject: [PATCH 02/11] Add Adfruit_MQTT::connectErrorString() method This easily allows printing error messages without having to hard-code return values in the sketch and makes the example sketches a bit more concise. --- Adafruit_MQTT.cpp | 13 +++++++++++++ Adafruit_MQTT.h | 8 ++++++++ examples/mqtt_cc3k/mqtt_cc3k.ino | 13 ++----------- examples/mqtt_esp8266/mqtt_esp8266.ino | 12 +----------- examples/mqtt_fona/mqtt_fona.ino | 13 +------------ 5 files changed, 25 insertions(+), 34 deletions(-) diff --git a/Adafruit_MQTT.cpp b/Adafruit_MQTT.cpp index d7963dc..fc415c6 100644 --- a/Adafruit_MQTT.cpp +++ b/Adafruit_MQTT.cpp @@ -140,6 +140,19 @@ int8_t Adafruit_MQTT::connect() { return 0; } +const __FlashStringHelper* Adafruit_MQTT::connectErrorString(int8_t code) { + switch (code) { + case 1: return F("Wrong protocol"); + case 2: return F("ID rejected"); + case 3: return F("Server unavail"); + case 4: return F("Bad user/pass"); + case 5: return F("Not authed"); + case 6: return F("Failed to subscribe"); + case -1: return F("Connection failed"); + default: return F("Unknown error"); + } +} + bool Adafruit_MQTT::publish(const char *topic, const char *data, uint8_t qos) { // Construct and send publish packet. uint8_t len = publishPacket(buffer, topic, data, qos); diff --git a/Adafruit_MQTT.h b/Adafruit_MQTT.h index 7d89801..909ee58 100644 --- a/Adafruit_MQTT.h +++ b/Adafruit_MQTT.h @@ -105,8 +105,16 @@ class Adafruit_MQTT { // 4 = Bad username or password // 5 = Not authenticated // 6 = Failed to subscribe + // Use connectErrorString() to get a printable string version of the + // error. int8_t connect(); + // Return a printable string version of the error code returned by + // connect(). This returns a __FlashStringHelper*, which points to a + // string stored in flash, but can be directly passed to e.g. + // Serial.println without any further processing. + const __FlashStringHelper* connectErrorString(int8_t code); + // Disconnect from the MQTT server. Returns true if disconnected, false // otherwise. virtual bool disconnect() = 0; // Subclasses need to fill this in! diff --git a/examples/mqtt_cc3k/mqtt_cc3k.ino b/examples/mqtt_cc3k/mqtt_cc3k.ino index d2490f4..ab04d64 100644 --- a/examples/mqtt_cc3k/mqtt_cc3k.ino +++ b/examples/mqtt_cc3k/mqtt_cc3k.ino @@ -152,18 +152,9 @@ void MQTT_connect() { Serial.print("Connecting to MQTT... "); while ((ret = mqtt.connect()) != 0) { // connect will return 0 for connected - switch (ret) { - case 1: Serial.println("Wrong protocol"); break; - case 2: Serial.println("ID rejected"); break; - case 3: Serial.println("Server unavailable"); break; - case 4: Serial.println("Bad user/password"); break; - case 5: Serial.println("Not authenticated"); break; - case 6: Serial.println("Failed to subscribe"); break; - default: - Serial.println(F("Connection failed")); + Serial.println(mqtt.connectErrorString(ret)); + if (ret < 0) CC3000connect(WLAN_SSID, WLAN_PASS, WLAN_SECURITY); // y0w, lets connect to wifi again - break; - } Serial.println("Retrying MQTT connection in 5 seconds..."); mqtt.disconnect(); delay(5000); // wait 5 seconds diff --git a/examples/mqtt_esp8266/mqtt_esp8266.ino b/examples/mqtt_esp8266/mqtt_esp8266.ino index 5a7d217..5faccf0 100644 --- a/examples/mqtt_esp8266/mqtt_esp8266.ino +++ b/examples/mqtt_esp8266/mqtt_esp8266.ino @@ -137,17 +137,7 @@ void MQTT_connect() { Serial.print("Connecting to MQTT... "); while ((ret = mqtt.connect()) != 0) { // connect will return 0 for connected - switch (ret) { - case 1: Serial.println("Wrong protocol"); break; - case 2: Serial.println("ID rejected"); break; - case 3: Serial.println("Server unavailable"); break; - case 4: Serial.println("Bad user/password"); break; - case 5: Serial.println("Not authenticated"); break; - case 6: Serial.println("Failed to subscribe"); break; - default: Serial.print("Couldn't connect to server, code: "); - Serial.println(ret); - break; - } + Serial.println(mqtt.connectErrorString(ret)); Serial.println("Retrying MQTT connection in 5 seconds..."); mqtt.disconnect(); delay(5000); // wait 5 seconds diff --git a/examples/mqtt_fona/mqtt_fona.ino b/examples/mqtt_fona/mqtt_fona.ino index fd95690..1474fc4 100644 --- a/examples/mqtt_fona/mqtt_fona.ino +++ b/examples/mqtt_fona/mqtt_fona.ino @@ -122,18 +122,7 @@ void loop() { Serial.println(F("Connecting to MQTT...")); int8_t ret, retries = 5; while (retries && (ret = mqtt.connect()) != 0) { - switch (ret) { - case 1: Serial.println(F("Wrong protocol")); break; - case 2: Serial.println(F("ID rejected")); break; - case 3: Serial.println(F("Server unavail")); break; - case 4: Serial.println(F("Bad user/pass")); break; - case 5: Serial.println(F("Not authed")); break; - case 6: Serial.println(F("Failed to subscribe")); break; - default: { - Serial.println(F("Connection failed")); - break; - } - } + Serial.println(mqtt.connectErrorString(ret)); Serial.println(F("Retrying MQTT connection")); retries--; if (retries == 0) halt("Resetting system"); From 564e87ff361a49d08795f5439e8ae90fd6427bca Mon Sep 17 00:00:00 2001 From: Matthijs Kooijman Date: Thu, 2 Jul 2015 13:26:56 +0200 Subject: [PATCH 03/11] Fix indentation in Adafruit_MQTT::subscribe --- Adafruit_MQTT.cpp | 14 +++++++------- 1 file changed, 7 insertions(+), 7 deletions(-) diff --git a/Adafruit_MQTT.cpp b/Adafruit_MQTT.cpp index fc415c6..b0dbb16 100644 --- a/Adafruit_MQTT.cpp +++ b/Adafruit_MQTT.cpp @@ -180,13 +180,13 @@ bool Adafruit_MQTT::subscribe(Adafruit_MQTT_Subscribe *sub) { } } if (i==MAXSUBSCRIPTIONS) { // add to subscriptionlist - for (i=0; i Date: Thu, 2 Jul 2015 13:28:38 +0200 Subject: [PATCH 04/11] Fix Adafruit_MQTT:subscribe() This method contained a loop that checked for an existing subscription, but it only printed a debug message if so. Now, it immediately returns true if the subscription is already registered. When this method succesfully adds a subscription, it was documented to return true, but the actual code did not return anything in this case, resulting in a compiler warning. In practice, on AVR, the value of the first argument would be returned, which likely evaluates as true, so it is likely it actually seemed to work fine. --- Adafruit_MQTT.cpp | 11 +++++------ Adafruit_MQTT.h | 2 +- 2 files changed, 6 insertions(+), 7 deletions(-) diff --git a/Adafruit_MQTT.cpp b/Adafruit_MQTT.cpp index b0dbb16..557ee06 100644 --- a/Adafruit_MQTT.cpp +++ b/Adafruit_MQTT.cpp @@ -176,7 +176,7 @@ bool Adafruit_MQTT::subscribe(Adafruit_MQTT_Subscribe *sub) { for (i=0; i Date: Thu, 2 Jul 2015 13:33:42 +0200 Subject: [PATCH 05/11] Clarify that subscribe() must be called before connect() --- Adafruit_MQTT.h | 2 ++ 1 file changed, 2 insertions(+) diff --git a/Adafruit_MQTT.h b/Adafruit_MQTT.h index dc703bc..a16c576 100644 --- a/Adafruit_MQTT.h +++ b/Adafruit_MQTT.h @@ -128,6 +128,8 @@ class Adafruit_MQTT { // Add a subscription to receive messages for a topic. Returns true if the // subscription could be added or was already present, false otherwise. + // Must be called before connect(), subscribing after the connection + // is made is not currently supported. bool subscribe(Adafruit_MQTT_Subscribe *sub); // Check if any subscriptions have new messages. Will return a reference to From 05b145146ae5e99ef48341a9971f60d7ec4ee8cb Mon Sep 17 00:00:00 2001 From: Matthijs Kooijman Date: Thu, 2 Jul 2015 13:39:33 +0200 Subject: [PATCH 06/11] Comment out stringprint(), it is not used This fixes a compiler warning. --- Adafruit_MQTT.cpp | 2 ++ 1 file changed, 2 insertions(+) diff --git a/Adafruit_MQTT.cpp b/Adafruit_MQTT.cpp index 557ee06..87fea10 100644 --- a/Adafruit_MQTT.cpp +++ b/Adafruit_MQTT.cpp @@ -38,6 +38,7 @@ void printBuffer(uint8_t *buffer, uint8_t len) { DEBUG_PRINTER.println(); } +/* Not used now, but might be useful in the future static uint8_t *stringprint(uint8_t *p, char *s) { uint16_t len = strlen(s); p[0] = len >> 8; p++; @@ -45,6 +46,7 @@ static uint8_t *stringprint(uint8_t *p, char *s) { memcpy(p, s, len); return p+len; } +*/ static uint8_t *stringprint_P(uint8_t *p, const char *s, uint16_t maxlen=0) { // If maxlen is specified (has a non-zero value) then use it as the maximum From 565d9afd366e6475a3531f24cb496520eb8f96a3 Mon Sep 17 00:00:00 2001 From: Matthijs Kooijman Date: Thu, 2 Jul 2015 13:43:10 +0200 Subject: [PATCH 07/11] Add parenthesis to fix compiler warning Gcc warns when using = where == seems logical, and adding parenthesis tells gcc you really mean =. --- examples/mqtt_cc3k/mqtt_cc3k.ino | 2 +- examples/mqtt_esp8266/mqtt_esp8266.ino | 2 +- examples/mqtt_fona/mqtt_fona.ino | 2 +- 3 files changed, 3 insertions(+), 3 deletions(-) diff --git a/examples/mqtt_cc3k/mqtt_cc3k.ino b/examples/mqtt_cc3k/mqtt_cc3k.ino index ab04d64..e9731a2 100644 --- a/examples/mqtt_cc3k/mqtt_cc3k.ino +++ b/examples/mqtt_cc3k/mqtt_cc3k.ino @@ -121,7 +121,7 @@ void loop() { // this is our 'wait for incoming subscription packets' busy subloop Adafruit_MQTT_Subscribe *subscription; - while (subscription = mqtt.readSubscription(1000)) { + while ((subscription = mqtt.readSubscription(1000))) { if (subscription == &onoffbutton) { Serial.print(F("Got: ")); Serial.println((char *)onoffbutton.lastread); diff --git a/examples/mqtt_esp8266/mqtt_esp8266.ino b/examples/mqtt_esp8266/mqtt_esp8266.ino index 5faccf0..528f002 100644 --- a/examples/mqtt_esp8266/mqtt_esp8266.ino +++ b/examples/mqtt_esp8266/mqtt_esp8266.ino @@ -104,7 +104,7 @@ void loop() { // this is our 'wait for incoming subscription packets' busy subloop Adafruit_MQTT_Subscribe *subscription; - while (subscription = mqtt.readSubscription(1000)) { + while ((subscription = mqtt.readSubscription(1000))) { if (subscription == &onoffbutton) { Serial.print(F("Got: ")); Serial.println((char *)onoffbutton.lastread); diff --git a/examples/mqtt_fona/mqtt_fona.ino b/examples/mqtt_fona/mqtt_fona.ino index 1474fc4..e14dcf6 100644 --- a/examples/mqtt_fona/mqtt_fona.ino +++ b/examples/mqtt_fona/mqtt_fona.ino @@ -144,7 +144,7 @@ void loop() { // this is our 'wait for incoming subscription packets' busy subloop Adafruit_MQTT_Subscribe *subscription; - while (subscription = mqtt.readSubscription(5000)) { + while ((subscription = mqtt.readSubscription(5000))) { if (subscription == &onoffbutton) { Serial.print(F("Got: ")); Serial.println((char *)onoffbutton.lastread); From 22c3533745cb2dececefacb6bdafcca9868f24a5 Mon Sep 17 00:00:00 2001 From: Matthijs Kooijman Date: Thu, 2 Jul 2015 18:02:45 +0200 Subject: [PATCH 08/11] Fix type of Adafruit_MQTT_Subscribe::lastread The lastread array stores the data bytes of the most recently read received packet. It was previously declared as an array of pointers to uint8_t, instead of an array of uint8_t, which obviously was not the intention. Because the examples explicitely cast to (char*) for printing (because Print::println(uint8_t[]) is not defined) and a pointer is at least as big as uint8_t and memcpy uses void*, this problem did not show up before. --- Adafruit_MQTT.h | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/Adafruit_MQTT.h b/Adafruit_MQTT.h index a16c576..82e5bc1 100644 --- a/Adafruit_MQTT.h +++ b/Adafruit_MQTT.h @@ -202,7 +202,7 @@ class Adafruit_MQTT_Subscribe { const char *topic; uint8_t qos; - uint8_t * lastread[SUBSCRIPTIONDATALEN]; + uint8_t lastread[SUBSCRIPTIONDATALEN]; private: Adafruit_MQTT *mqtt; }; From 22b77ecb68c4897c7044c33032a79b3666d2b7ea Mon Sep 17 00:00:00 2001 From: Matthijs Kooijman Date: Thu, 2 Jul 2015 18:07:33 +0200 Subject: [PATCH 09/11] Add Adafruit_MQTT_Subscribe::datalen This stores the number of valid bytes in lastread. Having access to this information allows transmitting binary data as well, containing embedded nul bytes. --- Adafruit_MQTT.cpp | 1 + Adafruit_MQTT.h | 3 +++ 2 files changed, 4 insertions(+) diff --git a/Adafruit_MQTT.cpp b/Adafruit_MQTT.cpp index 87fea10..443fc69 100644 --- a/Adafruit_MQTT.cpp +++ b/Adafruit_MQTT.cpp @@ -234,6 +234,7 @@ Adafruit_MQTT_Subscribe *Adafruit_MQTT::readSubscription(int16_t timeout) { } // extract out just the data, into the subscription object itself memcpy(subscriptions[i]->lastread, buffer+4+topiclen, datalen); + subscriptions[i]->datalen = datalen; DEBUG_PRINT(F("Data len: ")); DEBUG_PRINTLN(datalen); DEBUG_PRINT(F("Data: ")); DEBUG_PRINTLN((char *)subscriptions[i]->lastread); diff --git a/Adafruit_MQTT.h b/Adafruit_MQTT.h index 82e5bc1..0fe5ed4 100644 --- a/Adafruit_MQTT.h +++ b/Adafruit_MQTT.h @@ -203,6 +203,9 @@ class Adafruit_MQTT_Subscribe { uint8_t qos; uint8_t lastread[SUBSCRIPTIONDATALEN]; + // Number valid bytes in lastread. Limited to SUBSCRIPTIONDATALEN-1 to + // ensure nul terminating lastread. + uint8_t datalen; private: Adafruit_MQTT *mqtt; }; From 5d101b8a7b0d329cee80e2599854578d311cd657 Mon Sep 17 00:00:00 2001 From: Matthijs Kooijman Date: Fri, 24 Jul 2015 11:29:22 +0200 Subject: [PATCH 10/11] Allow passing __FlashStringHelper* in addition to char* everywhere Arduino supplies the F() macro that allows easily putting strings in PROGMEM and marking them with a custom type. Essentially, a __FlashStringHelper* is just the same char*, but cast to a different type, which allows for example Serial.print() to automatically handle both PROGMEM and normal strings. By letting this library accept __FlashStringHelper* as well, you can use easily use the F() macro and reduce the chance of mixing up normal and PROGMEM pointers. --- Adafruit_MQTT.cpp | 28 ++++++++++++++++++++++++++++ Adafruit_MQTT.h | 9 +++++++++ 2 files changed, 37 insertions(+) diff --git a/Adafruit_MQTT.cpp b/Adafruit_MQTT.cpp index 443fc69..5558a65 100644 --- a/Adafruit_MQTT.cpp +++ b/Adafruit_MQTT.cpp @@ -83,6 +83,20 @@ Adafruit_MQTT::Adafruit_MQTT(const char *server, uint16_t port, const char *cid, } } +Adafruit_MQTT::Adafruit_MQTT(const __FlashStringHelper *server, uint16_t port, const __FlashStringHelper *cid, + const __FlashStringHelper *user, const __FlashStringHelper *pass) { + servername = (const char *)server; + portnum = port; + clientid = (const char *)cid; + username = (const char *)user; + password = (const char *)pass; + + for (uint8_t i=0; i Date: Mon, 27 Jul 2015 22:42:15 +0200 Subject: [PATCH 11/11] Default to qos = 0 in Adafruit_MQTT::publish() This was already the case for the Adafruit_MQTT_Publish constructor, so this makes things a bit more consistent when using the publish() method directly. --- Adafruit_MQTT.h | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/Adafruit_MQTT.h b/Adafruit_MQTT.h index d06ce7a..e18110e 100644 --- a/Adafruit_MQTT.h +++ b/Adafruit_MQTT.h @@ -128,8 +128,8 @@ class Adafruit_MQTT { // if the message was published, false otherwise. // The topic must be stored in PROGMEM. It can either be a // char*, or a __FlashStringHelper* (the result of the F() macro). - bool publish(const char *topic, const char *payload, uint8_t qos); - bool publish(const __FlashStringHelper *topic, const char *payload, uint8_t qos) { + bool publish(const char *topic, const char *payload, uint8_t qos = 0); + bool publish(const __FlashStringHelper *topic, const char *payload, uint8_t qos = 0) { return publish((const char *)topic, payload, qos); }