tests modified accordingly to unique_ptrs

This change needs some modification in EpoxyDuino that are not yet accepted.

.github/workflows/aunit.yml

@@ -9,7 +9,7 @@ on: [push]
 jobs:
   build:
 
-    runs-on: ubuntu-18.04
+    runs-on: ubuntu-20.04
 
     steps:
     - uses: actions/checkout@v2

examples/advanced/mqtt_class_binder/mqtt_class_binder.ino

@@ -30,7 +30,7 @@
   *      - accepts external clients
   *      - MqttClassBinder allows to mix together many mqtt sources
   *
-  * cons - Takes more memory (48 more bytes for the one MqttClassBinder<Class>
+  * cons - Takes more memory (24 more bytes for the one MqttClassBinder<Class>
   *      - a bit hard to understand
   *
   */

library.properties

@@ -1,5 +1,5 @@
 name=TinyMqtt
-version=0.9.10
+version=0.9.11
 author=Francois BIOT, HSaturn, <hsaturn@gmail.com>
 maintainer=Francois BIOT, HSaturn, <hsaturn@gmail.com>
 sentence=A tiny broker and client library for MQTT messaging.

src/TinyMqtt.cpp

@@ -17,68 +17,55 @@ int TinyMqtt::debug=2;
 
 MqttBroker::MqttBroker(uint16_t port)
 {
-  server = new TcpServer(port);
+  server = std::unique_ptr<TcpServer>(new TcpServer(port));
 #ifdef TINY_MQTT_ASYNC
   server->onClient(onClient, this);
 #endif
 }
 
-MqttBroker::~MqttBroker()
-{
-  while(clients.size())
-  {
-    delete clients[0];
-  }
-  delete server;
-}
-
 // private constructor used by broker only
 MqttClient::MqttClient(MqttBroker* local_broker, TcpClient* new_client)
-  : local_broker(local_broker)
 {
+  connect(local_broker);
   debug("MqttClient private with broker");
 #ifdef TINY_MQTT_ASYNC
-  client = new_client;
+  tcp_client = new_client;
-  client->onData(onData, this);
+  tcp_client->onData(onData, this);
   // client->onConnect() TODO
   // client->onDisconnect() TODO
 #else
-  client = new WiFiClient(*new_client);
+  tcp_client.reset(new WiFiClient(*new_client));
-#endif
-#ifdef EPOXY_DUINO
-  alive = millis()+500000;
-#else
-  alive = millis()+5000;  // TODO MAGIC client expires after 5s if no CONNECT msg
 #endif
+  alive = millis()+5000;
 }
 
 MqttClient::MqttClient(MqttBroker* local_broker, const std::string& id)
   : local_broker(local_broker), clientId(id)
 {
-    client = nullptr;
+  alive = 0;
 
-    if (local_broker) local_broker->addClient(this);
+  if (local_broker) local_broker->addClient(this);
 }
 
 MqttClient::~MqttClient()
 {
   close();
-  delete client;
+  debug("*** MqttClient delete()");
 }
 
 void MqttClient::close(bool bSendDisconnect)
 {
   debug("close " << id().c_str());
-  mqtt_connected = false;
+  mqtt_flags &= ~FlagConnected;
-  if (client)  // connected to a remote broker
+  if (tcp_client)  // connected to a remote broker
   {
-    if (bSendDisconnect and client->connected())
+    if (bSendDisconnect and tcp_client->connected())
     {
       message.create(MqttMessage::Type::Disconnect);
       message.hexdump("close");
       message.sendTo(this);
     }
-    client->stop();
+    tcp_client->stop();
   }
 
   if (local_broker)
@@ -91,8 +78,10 @@ void MqttClient::close(bool bSendDisconnect)
 void MqttClient::connect(MqttBroker* local)
 {
   debug("MqttClient::connect_local");
+  alive = 0;
   close();
   local_broker = local;
+  clientAlive();
 }
 
 void MqttClient::connect(std::string broker, uint16_t port, uint16_t ka)
@@ -100,18 +89,17 @@ void MqttClient::connect(std::string broker, uint16_t port, uint16_t ka)
   debug("MqttClient::connect_to_host " << broker << ':' << port);
   keep_alive = ka;
   close();
-  if (client) delete client;
+  tcp_client.reset(new TcpClient);
-  client = new TcpClient;
 
 #ifdef TINY_MQTT_ASYNC
-  client->onData(onData, this);
+  tcp_client->onData(onData, this);
-  client->onConnect(onConnect, this);
+  tcp_client->onConnect(onConnect, this);
-  client->connect(broker.c_str(), port, ka);
+  tcp_client->connect(broker.c_str(), port, ka);
 #else
-  if (client->connect(broker.c_str(), port))
+  if (tcp_client->connect(broker.c_str(), port))
   {
     debug("link established");
-    onConnect(this, client);
+    onConnect(this, tcp_client.get());
   }
   else
   {
@@ -120,40 +108,23 @@ void MqttClient::connect(std::string broker, uint16_t port, uint16_t ka)
 #endif
 }
 
-void MqttBroker::addClient(MqttClient* client)
+void MqttBroker::addClient(TcpClient* client)
 {
   debug("MqttBroker::addClient");
-  clients.push_back(client);
+  clients.insert(std::unique_ptr<MqttClient>(new MqttClient(this, client)));
 }
 
 void MqttBroker::connect(const std::string& host, uint16_t port)
 {
   debug("MqttBroker::connect");
-  if (broker == nullptr) broker = new MqttClient;
+  if (remote_broker == nullptr) remote_broker = new MqttClient;
-  broker->connect(host, port);
+  remote_broker->connect(host, port);
-  broker->local_broker = this;  // Because connect removed the link
+  remote_broker->local_broker = this;  // Because connect removed the link
 }
 
 void MqttBroker::removeClient(MqttClient* remove)
 {
-  debug("removeClient");
+  local_clients.erase(remove);
-  for(auto it=clients.begin(); it!=clients.end(); it++)
-  {
-    auto client=*it;
-    if (client==remove)
-    {
-      // TODO if this broker is connected to an external broker
-      // we have to unsubscribe remove's topics.
-      // (but doing this, check that other clients are not subscribed...)
-      // Unless -> we could receive useless messages
-      //        -> we are using (memory) one IndexedString plus its string for nothing.
-      debug("Remove " << clients.size());
-      clients.erase(it);
-      debug("Client removed " << clients.size());
-      return;
-    }
-  }
-  debug(red << "Error cannot remove client");  // TODO should not occur
 }
 
 void MqttBroker::onClient(void* broker_ptr, TcpClient* client)
@@ -161,7 +132,7 @@ void MqttBroker::onClient(void* broker_ptr, TcpClient* client)
   debug("MqttBroker::onClient");
   MqttBroker* broker = static_cast<MqttBroker*>(broker_ptr);
 
-  broker->addClient(new MqttClient(broker, client));
+  broker->addClient(client);
   debug("New client");
 }
 
@@ -175,39 +146,33 @@ void MqttBroker::loop()
     onClient(this, &client);
   }
 #endif
-  if (broker)
+  if (remote_broker)
   {
     // TODO should monitor broker's activity.
     // 1 When broker disconnect and reconnect we have to re-subscribe
-    broker->loop();
+    remote_broker->loop();
   }
 
-
+  // 200 bytes shorter than for(auto& client: clients) !
-  // for(auto it=clients.begin(); it!=clients.end(); it++)
+  for(auto it=clients.begin(); it!=clients.end(); it++)
-  // use index because size can change during the loop
-  for(size_t i=0; i<clients.size(); i++)
   {
-    auto client = clients[i];
+    it->get()->loop();
-    if (client->connected())
+    if (not it->get()->connected())
     {
-      client->loop();
+      clients.erase(it);
-    }
-    else
-    {
-      debug("Client " << client->id().c_str() << "  Disconnected, local_broker=" << (dbg_ptr)client->local_broker);
-      // Note: deleting a client not added by the broker itself will probably crash later.
-      delete client;
       break;
     }
   }
+  for(const auto& client: local_clients)
+    client->loop();
 }
 
 MqttError MqttBroker::subscribe(const Topic& topic, uint8_t qos)
 {
   debug("MqttBroker::subscribe");
-  if (broker && broker->connected())
+  if (remote_broker && remote_broker->connected())
   {
-    return broker->subscribe(topic, qos);
+    return remote_broker->subscribe(topic, qos);
   }
   return MqttNowhereToSend;
 }
@@ -217,24 +182,23 @@ MqttError MqttBroker::publish(const MqttClient* source, const Topic& topic, Mqtt
   MqttError retval = MqttOk;
 
   debug("MqttBroker::publish");
-  int i=0;
+  int clt_num = 0;
-  for(auto client: clients)
+  for(auto& client: clients)
   {
-    i++;
+    debug (" broker:" << (remote_broker && remote_broker->connected() ? "linked" : "alone")
-#if TINY_MQTT_DEBUG
+        << "  srce=" << (source->isLocal() ? "loc" : "rem") << " clt#" << ++clt_num
-    Console << __LINE__ << " broker:" << (broker && broker->connected() ? "linked" : "alone") <<
+        << ", local=" << client->isLocal() << ", con=" << client->connected());
-       "  srce=" << (source->isLocal() ? "loc" : "rem") << " clt#" << i << ", local=" << client->isLocal() << ", con=" << client->connected() << endl;
+
-#endif
     bool doit = false;
-    if (broker && broker->connected())  // this (MqttBroker) is connected (to a external broker)
+    if (remote_broker && remote_broker->connected())  // this (MqttBroker) is connected (to a external broker)
     {
       // ext_broker -> clients or clients -> ext_broker
-      if (source == broker)  // external broker -> internal clients
+      if (source == remote_broker)  // external broker -> internal clients
         doit = true;
       else                  // external clients -> this broker
       {
         // As this broker is connected to another broker, simply forward the msg
-        MqttError ret = broker->publishIfSubscribed(topic, msg);
+        MqttError ret = remote_broker->publishIfSubscribed(topic, msg);
         if (ret != MqttOk) retval = ret;
       }
     }
@@ -242,9 +206,8 @@ MqttError MqttBroker::publish(const MqttClient* source, const Topic& topic, Mqtt
     {
       doit = true;
     }
-#if TINY_MQTT_DEBUG
+
-    Console << ", doit=" << doit << ' ';
+    debug(" doit=" << doit << ' ');
-#endif
 
     if (doit) retval = client->publishIfSubscribed(topic, msg);
     debug("");
@@ -268,16 +231,12 @@ void MqttMessage::getString(const char* &buff, uint16_t& len)
   buff+=2;
 }
 
-void MqttClient::clientAlive(uint32_t more_seconds)
+void MqttClient::clientAlive()
 {
   debug("MqttClient::clientAlive");
   if (keep_alive)
   {
-#ifdef EPOXY_DUINO
+    alive=millis()+1000*(keep_alive+(local_broker ? TINY_MQTT_CLIENT_ALIVE_TOLERANCE : 0));
-    alive=millis()+500000+0*more_seconds;
-#else
-    alive=millis()+1000*(keep_alive+more_seconds);
-#endif
   }
   else
     alive=0;
@@ -285,29 +244,30 @@ void MqttClient::clientAlive(uint32_t more_seconds)
 
 void MqttClient::loop()
 {
-  if (alive && (millis() > alive))
+  if (alive && (millis() >= alive))
   {
     if (local_broker)
     {
-      debug(red << "timeout client");
+      Serial << "timeout client " << clientId << endl;
       close();
       debug(red << "closed");
     }
-    else if (client && client->connected())
+    else if (tcp_client && tcp_client->connected())
     {
       debug("pingreq");
       uint16_t pingreq = MqttMessage::Type::PingReq;
-      client->write((const char*)(&pingreq), 2);
+
-      clientAlive(0);
+      tcp_client->write((const char*)(&pingreq), 2);
+      clientAlive();
 
       // TODO when many MqttClient passes through a local broker
       // there is no need to send one PingReq per instance.
     }
   }
 #ifndef TINY_MQTT_ASYNC
-  while(client && client->available()>0)
+  while(tcp_client && tcp_client->available()>0)
   {
-    message.incoming(client->read());
+    message.incoming(tcp_client->read());
     if (message.type())
     {
       processMessage(&message);
@@ -334,7 +294,7 @@ void MqttClient::onConnect(void *mqttclient_ptr, TcpClient*)
   msg.reset();
   debug("cnx: mqtt sent " << (dbg_ptr)mqtt->local_broker);
 
-  mqtt->clientAlive(0);
+  mqtt->clientAlive();
 }
 
 #ifdef TINY_MQTT_ASYNC
@@ -366,7 +326,7 @@ void MqttClient::resubscribe()
     msg.add(0);
     msg.add(0);
 
-    for(auto topic: subscriptions)
+    for(const auto& topic: subscriptions)
     {
       msg.add(topic);
       msg.add(0);    // TODO qos
@@ -441,13 +401,14 @@ void MqttClient::processMessage(MqttMessage* mesg)
   switch(mesg->type())
   {
     case MqttMessage::Type::Connect:
-      if (mqtt_connected)
+      if (mqtt_flags & FlagConnected)
       {
         debug("already connected");
         break;
       }
       payload = header+10;
-      mqtt_flags = header[7];
+      // Todo should check that reserved == 0 (spec)
+      mqtt_flags = header[7] & ~FlagConnected;
       keep_alive = MqttMessage::getSize(header+8);
       if (strncmp("MQTT", header+2,4))
       {
@@ -487,11 +448,10 @@ void MqttClient::processMessage(MqttMessage* mesg)
         payload += len;
       }
 
-      #if TINY_MQTT_DEBUG
+      debug(yellow << "Client " << clientId << " connected : keep alive=" << keep_alive << '.' << white);
-        Console << yellow << "Client " << clientId << " connected : keep alive=" << keep_alive << '.' << white << endl;
+
-      #endif
       bclose = false;
-      mqtt_connected=true;
+      mqtt_flags |= FlagConnected;
       {
         MqttMessage msg(MqttMessage::Type::ConnAck);
         msg.add(0);  // Session present (not implemented)
@@ -501,14 +461,14 @@ void MqttClient::processMessage(MqttMessage* mesg)
       break;
 
     case MqttMessage::Type::ConnAck:
-      mqtt_connected = true;
+      mqtt_flags |= FlagConnected;
       bclose = false;
       resubscribe();
       break;
 
     case MqttMessage::Type::SubAck:
     case MqttMessage::Type::PubAck:
-      if (!mqtt_connected) break;
+      if (not (mqtt_flags & FlagConnected)) break;
       // Ignore acks
       bclose = false;
       break;
@@ -519,12 +479,12 @@ void MqttClient::processMessage(MqttMessage* mesg)
       break;
 
     case MqttMessage::Type::PingReq:
-      if (!mqtt_connected) break;
+      if (not (mqtt_flags & FlagConnected)) break;
-      if (client)
+      if (tcp_client)
       {
         uint16_t pingreq = MqttMessage::Type::PingResp;
         debug(cyan << "Ping response to client ");
-        client->write((const char*)(&pingreq), 2);
+        tcp_client->write((const char*)(&pingreq), 2);
         bclose = false;
       }
       else
@@ -536,7 +496,7 @@ void MqttClient::processMessage(MqttMessage* mesg)
     case MqttMessage::Type::Subscribe:
     case MqttMessage::Type::UnSubscribe:
       {
-        if (!mqtt_connected) break;
+        if (not (mqtt_flags & FlagConnected)) break;
         payload = header+2;
 
         debug("un/subscribe loop");
@@ -580,31 +540,27 @@ void MqttClient::processMessage(MqttMessage* mesg)
       break;
 
     case MqttMessage::Type::UnSuback:
-      if (!mqtt_connected) break;
+      if (not (mqtt_flags & FlagConnected)) break;
       bclose = false;
       break;
 
     case MqttMessage::Type::Publish:
-      #if TINY_MQTT_DEBUG
+      debug("publish " << (mqtt_flags & FlagConnected) << '/' << (long) tcp_client.get());
-        Console << "publish " << mqtt_connected << '/' << (long) client << endl;
+      if ((mqtt_flags & FlagConnected) or tcp_client == nullptr)
-      #endif
-      if (mqtt_connected or client == nullptr)
       {
         uint8_t qos = mesg->flags();
         payload = header;
         mesg->getString(payload, len);
         Topic published(payload, len);
         payload += len;
-        #if TINY_MQTT_DEBUG
+        debug("Received Publish (" << published.str().c_str() << ") size=" << (int)len);
-          Console << "Received Publish (" << published.str().c_str() << ") size=" << (int)len << endl;
-        #endif
         // << '(' << std::string(payload, len).c_str() << ')'  << " msglen=" << mesg->length() << endl;
         if (qos) payload+=2;  // ignore packet identifier if any
         len=mesg->end()-payload;
         // TODO reset DUP
         // TODO reset RETAIN
 
-        if (local_broker==nullptr or client==nullptr)  // internal MqttClient receives publish
+        if (local_broker==nullptr or tcp_client==nullptr)  // internal MqttClient receives publish
         {
           #if TINY_MQTT_DEBUG
             if (TinyMqtt::debug >= 2)
@@ -629,8 +585,8 @@ void MqttClient::processMessage(MqttMessage* mesg)
 
     case MqttMessage::Type::Disconnect:
       // TODO should discard any will msg
-      if (!mqtt_connected) break;
+      if (not (mqtt_flags & FlagConnected)) break;
-      mqtt_connected = false;
+      mqtt_flags &= ~FlagConnected;
       close(false);
       bclose=false;
       break;
@@ -651,7 +607,7 @@ void MqttClient::processMessage(MqttMessage* mesg)
   }
   else
   {
-    clientAlive(local_broker ? 5 : 0);
+    clientAlive();
   }
 }
 
@@ -730,7 +686,7 @@ MqttError MqttClient::publish(const Topic& topic, const char* payload, size_t pa
   {
     return local_broker->publish(this, topic, msg);
   }
-  else if (client)
+  else if (tcp_client)
     return msg.sendTo(this);
   else
     return MqttNowhereToSend;
@@ -744,15 +700,12 @@ MqttError MqttClient::publishIfSubscribed(const Topic& topic, MqttMessage& msg)
   debug("mqttclient publishIfSubscribed " << topic.c_str() << ' ' << subscriptions.size());
   if (isSubscribedTo(topic))
   {
-    if (client)
+    if (tcp_client)
       retval = msg.sendTo(this);
     else
     {
       processMessage(&msg);
-
+      debug("Should call the callback ?");
-      #if TINY_MQTT_DEBUG
-        Console << "Should call the callback ?\n";
-      #endif
       // callback(this, topic, nullptr, 0);  // TODO Payload
     }
   }

src/TinyMqtt.h

@@ -4,6 +4,10 @@
 #ifndef TINY_MQTT_DEBUG
 #define TINY_MQTT_DEBUG 0
 #endif
+#ifndef TINY_MQTT_DEFAULT_ALIVE
+#define TINY_MQTT_DEFAULT_ALIVE 10
+#endif
+#define TINY_MQTT_CLIENT_ALIVE_TOLERANCE 5
 
 // TODO Should add a AUnit with both TINY_MQTT_ASYNC and not TINY_MQTT_ASYNC
 // #define TINY_MQTT_ASYNC  // Uncomment this to use ESPAsyncTCP instead of normal cnx
@@ -33,7 +37,7 @@
   #include <rpcWiFi.h>
 #endif
 
-#include <vector>
+#include <memory>
 #include <set>
 #include <string>
 #include "StringIndexer.h"
@@ -48,7 +52,7 @@
     static int debug;
   };
 
-  #define debug(what) { if (TinyMqtt::debug>=1) Console << (int)__LINE__ << ' ' << what << TinyConsole::white << endl; delay(100); }
+  #define debug(what) { if (TinyMqtt::debug>=1) Console << (int)__LINE__ << ' ' << what << TinyConsole::white << endl; delay(10); }
 #else
   #define debug(what) {}
 #endif
@@ -173,7 +177,9 @@ class MqttClient
     FlagWillQos = 16 | 8,  // unsupported
     FlagWill = 4,          // unsupported
     FlagCleanSession = 2,  // unsupported
-    FlagReserved = 1
+
+    FlagReserved = 1,      // use reserved as connected (save 1 byte)
+    FlagConnected = 1
   };
   public:
 
@@ -187,18 +193,19 @@ class MqttClient
     ~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 = TINY_MQTT_DEFAULT_ALIVE);
 
     // TODO it seems that connected returns true in tcp mode even if
     // no negociation occurred
     bool connected()
     {
-      return (local_broker!=nullptr and client==nullptr) or (client and client->connected());
+      return (local_broker!=nullptr and tcp_client==nullptr)
+           or (tcp_client and tcp_client->connected());
     }
 
     void write(const char* buf, size_t length)
     {
-      if (client) client->write(buf, length);
+      if (tcp_client) tcp_client->write(buf, length);
     }
 
     const std::string& id() const { return clientId; }
@@ -229,7 +236,7 @@ class MqttClient
 
     // connected to local broker
     // TODO seems to be useless
-    bool isLocal() const { return client == nullptr; }
+    bool isLocal() const { return tcp_client == nullptr; }
 
     void dump(std::string indent="")
     {
@@ -238,9 +245,9 @@ class MqttClient
         uint32_t ms=millis();
         Console << indent << "+-- " << '\'' << clientId.c_str() << "' " << (connected() ? " ON " : " OFF");
         Console << ", alive=" << alive << '/' << ms << ", ka=" << keep_alive << ' ';
-        if (client)
+        if (tcp_client)
         {
-          if (client->connected())
+          if (tcp_client->connected())
             Console << TinyConsole::green << "connected";
           else
             Console << TinyConsole::red << "disconnected";
@@ -282,21 +289,22 @@ class MqttClient
     // republish a received publish if topic matches any in subscriptions
     MqttError publishIfSubscribed(const Topic& topic, MqttMessage& msg);
 
-    void clientAlive(uint32_t more_seconds);
+    void clientAlive();
     void processMessage(MqttMessage* message);
 
-    bool mqtt_connected = false;
+    char mqtt_flags = 0;
-    char mqtt_flags;
+    uint16_t keep_alive = 30;
-    uint32_t keep_alive = 30;
+    // for client connected to remote broker, PingReq is sent when millis() >= alive
-    uint32_t alive;
+    // for a client managed by a broker, disconnect it if millis() >= alive
+    uint32_t alive; // PingReq if millis() > alive,
     MqttMessage message;
 
     // connection to local broker, or link to the parent
     // when MqttBroker uses MqttClient for each external connexion
     MqttBroker* local_broker=nullptr;
 
-    TcpClient* client=nullptr;    // connection to remote broker
+    std::unique_ptr<TcpClient> tcp_client;    // connection to remote broker
-    std::set<Topic>  subscriptions;
+    std::set<Topic> subscriptions;
     std::string clientId;
     CallBack callback = nullptr;
 };
@@ -312,7 +320,6 @@ class MqttBroker
   public:
     // TODO limit max number of clients
     MqttBroker(uint16_t port);
-    ~MqttBroker();
 
     void begin() { server->begin(); }
     void loop();
@@ -320,15 +327,21 @@ class MqttBroker
     void connect(const std::string& host, uint16_t port=1883);
     bool connected() const { return state == Connected; }
 
-    size_t clientsCount() const { return clients.size(); }
 
     void dump(std::string indent="")
     {
-      for(auto client: clients)
+      for(const auto& client: clients)
         client->dump(indent);
     }
 
-    const std::vector<MqttClient*>  getClients() const { return clients; }
+    using Clients = std::set<std::unique_ptr<MqttClient>>;
+    using LocalClients = std::set<MqttClient*>;
+
+    const Clients& getClients() const { return clients; }
+    const LocalClients& getLocalClients() const { return local_clients; }
+
+    size_t clientsCount() const { return clients.size(); }
+    size_t localClientsCount() const { return local_clients.size(); }
 
   private:
     friend class MqttClient;
@@ -345,19 +358,21 @@ class MqttBroker
 
     MqttError subscribe(const Topic& topic, uint8_t qos);
 
-    // For clients that are added not by the broker itself (local clients)
+    void addClient(MqttClient* local) { local_clients.insert(local); }
-    void addClient(MqttClient* client);
+    void addClient(TcpClient* client);
-    void removeClient(MqttClient* client);
+
+    void removeClient(MqttClient* local);
 
     bool compareString(const char* good, const char* str, uint8_t str_len) const;
-    std::vector<MqttClient*>  clients;
+    Clients clients;
+    LocalClients local_clients;
 
   private:
-    TcpServer* server = nullptr;
+    std::unique_ptr<TcpServer> server;
 
     const char* auth_user = "guest";
     const char* auth_password = "guest";
-    MqttClient* broker = nullptr;
+    MqttClient* remote_broker = nullptr;
 
     State state = Disconnected;
 };

src/make_unique.inc

@@ -0,0 +1,142 @@
+// Implementation of C++14's make_unique for C++11 compilers.
+//
+// This has been tested with:
+// - MSVC 11.0 (Visual Studio 2012)
+// - gcc 4.6.3
+// - Xcode 4.4 (with clang "4.0")
+//
+// It is based off an implementation proposed by Stephan T. Lavavej for
+// inclusion in the C++14 standard:
+//    http://isocpp.org/files/papers/N3656.txt
+// Where appropriate, it borrows the use of MSVC's _VARIADIC_EXPAND_0X macro
+// machinery to compensate for lack of variadic templates.
+//
+// This file injects make_unique into the std namespace, which I acknowledge is
+// technically forbidden ([C++11: 17.6.4.2.2.1/1]), but is necessary in order
+// to have syntax compatibility with C++14.
+//
+// I perform compiler version checking for MSVC, gcc, and clang to ensure that
+// we don't add make_unique if it is already there (instead, we include
+// <memory> to get the compiler-provided one). You can override the compiler
+// version checking by defining the symbol COMPILER_SUPPORTS_MAKE_UNIQUE.
+//
+//
+// ===============================================================================
+// This file is released into the public domain. See LICENCE for more information.
+// ===============================================================================
+
+#pragma once
+
+// If user hasn't specified COMPILER_SUPPORTS_MAKE_UNIQUE then try to figure out
+// based on compiler version if std::make_unique is provided.
+#if !defined(COMPILER_SUPPORTS_MAKE_UNIQUE)
+   #if defined(_MSC_VER)
+      // std::make_unique was added in MSVC 12.0
+      #if _MSC_VER >= 1800 // MSVC 12.0 (Visual Studio 2013)
+         #define COMPILER_SUPPORTS_MAKE_UNIQUE
+      #endif
+   #elif defined(__clang__)
+      // std::make_unique was added in clang 3.4, but not until Xcode 6.
+      // Annoyingly, Apple makes the clang version defines match the version
+      // of Xcode, not the version of clang.
+      #define CLANG_VERSION (__clang_major__ * 10000 + __clang_minor__ * 100 + __clang_patchlevel__)
+      #if defined(__APPLE__) && CLANG_VERSION >= 60000
+         #define COMPILER_SUPPORTS_MAKE_UNIQUE
+      #elif !defined(__APPLE__) && CLANG_VERSION >= 30400 
+         #define COMPILER_SUPPORTS_MAKE_UNIQUE
+      #endif
+   #elif defined(__GNUC__)
+      // std::make_unique was added in gcc 4.9, for standards versions greater
+      // than -std=c++11.
+      #define GCC_VERSION (__GNUC__ * 10000 + __GNUC_MINOR__ * 100 + __GNUC_PATCHLEVEL__)
+      #if GCC_VERSION >= 40900 && __cplusplus > 201103L
+         #define COMPILER_SUPPORTS_MAKE_UNIQUE
+      #endif
+   #endif
+#endif
+
+#if defined(COMPILER_SUPPORTS_MAKE_UNIQUE)
+
+// If the compiler supports std::make_unique, then pull in <memory> to get it.
+#include <memory>
+
+#else
+
+// Otherwise, the compiler doesn't provide it, so implement it ourselves.
+
+#include <cstddef>
+#include <memory>
+#include <type_traits>
+#include <utility>
+
+namespace std {
+
+template<class _Ty> struct _Unique_if {
+   typedef unique_ptr<_Ty> _Single_object;
+};
+
+template<class _Ty> struct _Unique_if<_Ty[]> {
+   typedef unique_ptr<_Ty[]> _Unknown_bound;
+};
+
+template<class _Ty, size_t N> struct _Unique_if<_Ty[N]> {
+   typedef void _Known_bound;
+};
+
+//
+// template< class T, class... Args >
+// unique_ptr<T> make_unique( Args&&... args);
+//
+
+#if defined(_MSC_VER) && (_MSC_VER < 1800)
+
+// Macro machinery because MSVC 11.0 doesn't support variadic templates.
+// The _VARIADIC_EXPAND_0X stuff is defined in <xstddef>
+#define _MAKE_UNIQUE( \
+   TEMPLATE_LIST, PADDING_LIST, LIST, COMMA, X1, X2, X3, X4) \
+   template<class _Ty COMMA LIST(_CLASS_TYPE)> inline \
+      typename _Unique_if<_Ty>::_Single_object make_unique(LIST(_TYPE_REFREF_ARG)) \
+   { \
+      return unique_ptr<_Ty>(new _Ty(LIST(_FORWARD_ARG))); \
+   } \
+
+_VARIADIC_EXPAND_0X(_MAKE_UNIQUE, , , , )
+#undef _MAKE_UNIQUE
+
+#else // not MSVC 11.0 or earlier
+
+template<class _Ty, class... Args>
+   typename _Unique_if<_Ty>::_Single_object
+   make_unique(Args&&... args) {
+      return unique_ptr<_Ty>(new _Ty(std::forward<Args>(args)...));
+   }
+
+#endif
+
+// template< class T >
+// unique_ptr<T> make_unique( std::size_t size );
+
+template<class _Ty>
+   typename _Unique_if<_Ty>::_Unknown_bound
+   make_unique(size_t n) {
+      typedef typename remove_extent<_Ty>::type U;
+      return unique_ptr<_Ty>(new U[n]());
+   }
+
+// template< class T, class... Args >
+// /* unspecified */ make_unique( Args&&... args ) = delete;
+
+// MSVC 11.0 doesn't support deleted functions, so the best we can do
+// is simply not define the function.
+#if !(defined(_MSC_VER) && (_MSC_VER < 1800))
+
+template<class T, class... Args>
+   typename _Unique_if<T>::_Known_bound
+   make_unique(Args&&...) = delete;
+
+#endif
+
+} // namespace std
+
+#endif // !COMPILER_SUPPORTS_MAKE_UNIQUE
+

tests/Makefile

@@ -1,6 +1,8 @@
+SUB=n
+
 tests:
 	set -e; \
-	for i in *-tests/Makefile; do \
+	for i in ${SUB}*-tests/Makefile; do \
 		echo '==== Making:' $$(dirname $$i); \
 		$(MAKE) -C $$(dirname $$i) -j; \
 	done
@@ -15,14 +17,14 @@ runtests: debugtest
 	$(MAKE) clean
 	$(MAKE) tests
 	set -e; \
-	for i in *-tests/Makefile; do \
+	for i in ${SUB}*-tests/Makefile; do \
 		echo '==== Running:' $$(dirname $$i); \
 		$$(dirname $$i)/$$(dirname $$i).out; \
 	done
 
 clean:
 	set -e; \
-	for i in *-tests/Makefile; do \
+	for i in ${SUB}*-tests/Makefile; do \
 		echo '==== Cleaning:' $$(dirname $$i); \
 		$(MAKE) -C $$(dirname $$i) clean; \
 	done

tests/classbind-tests/Makefile

@@ -0,0 +1,13 @@
+# See https://github.com/bxparks/EpoxyDuino for documentation about this
+# Makefile to compile and run Arduino programs natively on Linux or MacOS.
+
+EXTRA_CXXFLAGS=-g3 -O0 -DTINY_MQTT_TESTS
+
+# Remove flto flag from EpoxyDuino (too many <optimized out>)
+CXXFLAGS = -Wextra -Wall -std=gnu++11 -fno-exceptions -fno-threadsafe-statics
+
+APP_NAME := classbind-tests
+ARDUINO_LIBS := AUnit AceCommon AceTime TinyMqtt EspMock ESP8266WiFi  ESPAsyncTCP TinyConsole
+ARDUINO_LIB_DIRS := ../../../EspMock/libraries
+EPOXY_CORE := EPOXY_CORE_ESP8266
+include ../../../EpoxyDuino/EpoxyDuino.mk

tests/local-tests/Makefile

@@ -1,7 +1,7 @@
 # See https://github.com/bxparks/EpoxyDuino for documentation about this
 # Makefile to compile and run Arduino programs natively on Linux or MacOS.
 
-EXTRA_CXXFLAGS=-g3 -O0
+EXTRA_CXXFLAGS=-g3 -O0 -DTINY_MQTT_DEFAULT_ALIVE=1
 
 # Remove flto flag from EpoxyDuino (too many <optimized out>)
 CXXFLAGS = -Wextra -Wall -std=gnu++11 -fno-exceptions -fno-threadsafe-statics

tests/local-tests/local-tests.ino

@@ -14,7 +14,6 @@
 
 using namespace std;
 
-MqttBroker broker(1883);
 
 std::map<std::string, std::map<Topic, int>>  published;    // map[client_id] => map[topic] = count
 
@@ -31,29 +30,48 @@ void onPublish(const MqttClient* srce, const Topic& topic, const char* payload,
 
 test(local_client_should_unregister_when_destroyed)
 {
-  assertEqual(broker.clientsCount(), (size_t)0);
+  MqttBroker broker(1883);
+  assertEqual(broker.localClientsCount(), (size_t)0);
   {
-    assertEqual(broker.clientsCount(), (size_t)0);  // Ensure client is not yet connected
+    assertEqual(broker.localClientsCount(), (size_t)0);  // Ensure client is not yet connected
     MqttClient client(&broker);
-    assertEqual(broker.clientsCount(), (size_t)1);  // Ensure client is now connected
+    assertEqual(broker.localClientsCount(), (size_t)1);  // Ensure client is now connected
   }
-  assertEqual(broker.clientsCount(), (size_t)0);
+  assertEqual(broker.localClientsCount(), (size_t)0);
+}
+
+test(local_client_alive)
+{
+  set_millis(0);
+  MqttBroker broker(1883);
+  MqttClient client(&broker);
+
+  broker.loop();
+  assertEqual(broker.localClientsCount(), (size_t)1);  // Ensure client is now connected
+
+  add_millis(TINY_MQTT_DEFAULT_ALIVE*1000/2);
+  broker.loop();
+  assertEqual(broker.localClientsCount(), (size_t)1);  // Ensure client is still connected
+
+  add_seconds(TINY_MQTT_DEFAULT_ALIVE*5);
+  broker.loop();
+  assertEqual(broker.localClientsCount(), (size_t)1);  // Ensure client is still connected
 }
 
 #if 0
 test(local_connect)
 {
-  assertEqual(broker.clientsCount(), (size_t)0);
+  assertEqual(broker.localClientsCount(), (size_t)0);
 
   MqttClient client;
   assertTrue(client.connected());
-  assertEqual(broker.clientsCount(), (size_t)1);
+  assertEqual(broker.localClientsCount(), (size_t)1);
 }
 
 test(local_publish_should_be_dispatched)
 {
   published.clear();
-  assertEqual(broker.clientsCount(), (size_t)0);
+  assertEqual(broker.localClientsCount(), (size_t)0);
 
   MqttClient subscriber;
   subscriber.subscribe("a/b");
@@ -73,7 +91,7 @@ test(local_publish_should_be_dispatched)
 test(local_publish_should_be_dispatched_to_local_clients)
 {
   published.clear();
-  assertEqual(broker.clientsCount(), (size_t)0);
+  assertEqual(broker.localClientsCount(), (size_t)0);
 
   MqttClient subscriber_a("A");
   subscriber_a.setCallback(onPublish);
@@ -98,7 +116,7 @@ test(local_publish_should_be_dispatched_to_local_clients)
 test(local_unsubscribe)
 {
   published.clear();
-  assertEqual(broker.clientsCount(), (size_t)0);
+  assertEqual(broker.localClientsCount(), (size_t)0);
 
   MqttClient subscriber;
   subscriber.setCallback(onPublish);
@@ -118,7 +136,7 @@ test(local_unsubscribe)
 test(local_nocallback_when_destroyed)
 {
   published.clear();
-  assertEqual(broker.clientsCount(), (size_t)0);
+  assertEqual(broker.localClientsCount(), (size_t)0);
 
   MqttClient publisher;
   {

tests/network-tests/network-tests.ino

@@ -142,6 +142,55 @@ test(suback)
   assertEqual(MqttClient::counters[MqttMessage::Type::SubAck], 1);
 }
 
+uint32_t getClientKeepAlive(MqttBroker& broker)
+{
+  if (broker.getClients().size() == 1)
+    for (auto& it : broker.getClients())
+      return it->keepAlive();
+
+  return 9999;
+}
+
+test(network_client_alive)
+{
+  const uint32_t keep_alive=1;
+  start_servers(2, true);
+  assertEqual(WiFi.status(), WL_CONNECTED);
+  set_millis(0); // Enter simulated time
+
+  MqttBroker broker(1883);
+  broker.begin();
+  IPAddress broker_ip = WiFi.localIP();
+
+  ESP8266WiFiClass::selectInstance(2);
+  MqttClient client;
+  client.connect(broker_ip.toString().c_str(), 1883, keep_alive);
+  broker.loop();
+  client.loop();
+
+  assertTrue(broker.clientsCount() == 1);
+  assertTrue(client.connected());
+
+  uint32_t ka = getClientKeepAlive(broker);
+  assertEqual(ka, keep_alive);
+  assertEqual(broker.clientsCount(), (size_t)1);
+
+  // All is going well if we call client.loop()
+  // The client is able to send PingReq to the broker
+  add_seconds(keep_alive);
+  client.loop();
+  broker.loop();
+  assertEqual(broker.clientsCount(), (size_t)1);
+
+  // Now simulate that the client is frozen for
+  // a too long time
+  add_seconds(TINY_MQTT_CLIENT_ALIVE_TOLERANCE*2);
+  broker.loop();
+  assertEqual(broker.clientsCount(), (size_t)0);
+
+  set_real_time();
+}
+
 test(network_client_keep_alive_high)
 {
   const uint32_t keep_alive=1000;
@@ -172,9 +221,8 @@ test(network_client_keep_alive_high)
   uint32_t sz = broker.getClients().size();
   assertEqual(sz , (uint32_t)1);
 
-  uint32_t ka = broker.getClients()[0]->keepAlive();
+  uint32_t ka = getClientKeepAlive(broker);
   assertEqual(ka, keep_alive);
-
 }
 
 test(network_client_to_broker_connexion)