hsaturn / TinyMqtt

ESP 8266 / 32 / WROOM Small footprint Mqtt Broker and Client
GNU General Public License v3.0
198 stars 41 forks source link

Mosquitto dislikes the default empty client_id ("Client <unknown> disconnected due to protocol error.") #53

Closed wdoekes closed 1 year ago

wdoekes commented 1 year ago

When trying TinyMqtt I got the following errors in my Mosquitto logs:

2022-12-14T14:12:45+0000 (1671027165): New connection from x.x.x.x:55797 on port 1883.
2022-12-14T14:12:46+0000 (1671027166): Sending CONNACK to x.x.x.x (0, 2)
2022-12-14T14:12:46+0000 (1671027166): Client <unknown> disconnected due to protocol error.

It turns out it was because of this:

CONNECT

image

ACK (with disconnect reason)

image

This was because I did not set MqttClient.id("something"); -- the example did not suggest so.


Possible fix:

--- TinyMqtt/src/TinyMqtt.h.orig    2022-12-14 15:48:05.244851788 +0100
+++ TinyMqtt/src/TinyMqtt.h 2022-12-14 15:48:34.916688801 +0100
@@ -35,6 +35,9 @@
 #include <string>
 #include "StringIndexer.h"

+// Default to a non-empty client identifier because some brokers reject those.
+#define TINY_MQTT_DEFAULT_CLIENT_ID "TinyMqtt"
+
 #if TINY_MQTT_DEBUG
 #include <TinyStreaming.h>
 #include <TinyConsole.h>    // https://github.com/hsaturn/TinyConsole
@@ -174,16 +177,16 @@ class MqttClient
   public:
     /** Constructor. Broker is the adress of a local broker if not null
         If you want to connect elsewhere, leave broker null and use connect() **/
-    MqttClient(MqttBroker* broker = nullptr, const std::string& id="");
+    MqttClient(MqttBroker* broker=nullptr, const std::string& id=TINY_MQTT_DEFAULT_CLIENT_ID);
     MqttClient(const std::string& id) : MqttClient(nullptr, id){}

     ~MqttClient();

     void connect(MqttBroker* local_broker);
-    void connect(std::string broker, uint16_t port, uint16_t keep_alive = 10);
+    void connect(std::string broker, uint16_t port, uint16_t keep_alive=10);

     // TODO it seems that connected returns true in tcp mode even if
-    // no negociation occured
+    // no negotiation occurred
     bool connected()
     {
       return (local_broker!=nullptr and client==nullptr) or (client and client->connected());
@@ -195,7 +198,7 @@ class MqttClient
     }

     const std::string& id() const { return clientId; }
-    void id(std::string& new_id) { clientId = new_id; }
+    void id(const std::string& new_id) { clientId = new_id; }

     /** Should be called in main loop() */
     void loop();
hsaturn commented 1 year ago

Pushed, and release as verison 0.9.7. Thanks