Fix crash at end of unit tests

.github/workflows/aunit.yml

@@ -0,0 +1,28 @@
+# See https://docs.github.com/en/actions/guides for documentation about GitHub
+# Actions.
+
+name: AUnit Tests
+
+# Run on all branches.
+on: [push]
+
+jobs:
+  build:
+
+    runs-on: ubuntu-18.04
+
+    steps:
+    - uses: actions/checkout@v2
+
+    - name: Setup
+      run: |
+        cd ..
+        git clone https://github.com/bxparks/EpoxyDuino
+        git clone https://github.com/bxparks/AceRoutine
+        git clone https://github.com/bxparks/AUnit
+        git clone https://github.com/bxparks/AceCommon
+        git clone https://github.com/hsaturn/EspMock
+    - name: Verify tests
+      run: |
+        make -C tests
+        make -C tests runtests

.github/workflows/superlinter.yml

@@ -0,0 +1,26 @@
+name: Super-Linter
+
+# Run this workflow every time a new commit pushed to your repository
+#
+on: push
+
+jobs:
+  # Set the job key. The key is displayed as the job name
+  # when a job name is not provided
+  super-lint:
+    # Name the Job
+    name: Lint code base
+    # Set the type of machine to run on
+    runs-on: ubuntu-latest
+
+    steps:
+      # Checks out a copy of your repository on the ubuntu-latest machine
+      - name: Checkout code
+        uses: actions/checkout@v2
+
+      # Runs the Super-Linter action
+      - name: Run Super-Linter
+        uses: github/super-linter@v3
+        env:
+          DEFAULT_BRANCH: main
+          GITHUB_TOKEN: ${{ secrets.GITHUB_TOKEN }}

.gitignore

@@ -1,2 +1,5 @@
 *~
 src/my_credentials.h
+*.o
+*.swp
+*.out

README.md

@@ -1,10 +1,10 @@
 # TinyMqtt
 
-![](https://img.shields.io/github/v/release/hsaturn/TinyMqtt)
+![Release](https://img.shields.io/github/v/release/hsaturn/TinyMqtt)
-![](https://img.shields.io/github/issues/hsaturn/TinyMqtt)
+![Issues](https://img.shields.io/github/issues/hsaturn/TinyMqtt)
-![](https://img.shields.io/badge/platform-ESP8266-green)
+![Esp8266](https://img.shields.io/badge/platform-ESP8266-green)
-![](https://img.shields.io/github/license/hsaturn/TinyMqtt)
+![Gpl 3.0](https://img.shields.io/github/license/hsaturn/TinyMqtt)
-![](https://img.shields.io/badge/Mqtt-%203.1.1-yellow)
+![Mqtt 3.1.1](https://img.shields.io/badge/Mqtt-%203.1.1-yellow)
 
 ESP 8266 is a small, fast and capable Mqtt Broker and Client
 
@@ -26,6 +26,7 @@ ESP 8266 is a small, fast and capable Mqtt Broker and Client
 * Implement zeroconf mode (needs async)
 * Add a max_clients in MqttBroker. Used with zeroconf, there will be
 no need for having tons of clients (also RAM is the problem with many clients)
+* Why not a 'global' TinyMqtt::loop() instead of having to call loop for all broker/clients instances
 * Test what is the real max number of clients for broker. As far as I saw, 1k is needed per client which would make more than 30 clients critical.
 * ~~MqttMessage uses a buffer 256 bytes which is usually far than needed.~~
 * ~~MqttClient does not support more than one subscription at time~~

examples/tinymqtt-test/tinymqtt-test.ino

@@ -16,11 +16,6 @@
   * cons - Takes more memory
 	*      - a bit hard to understand
 	*
-  * This sounds crazy: a mqtt mqtt that do not need a broker !
-  *  The use case arise when one ESP wants to publish topics and subscribe to them at the same time.
-  *  Without broker, the ESP won't react to its own topics.
-  *  
-  *  TinyMqtt mqtt allows this use case to work.
   */
 
 #include <my_credentials.h>
@@ -28,18 +23,26 @@
 std::string topic="sensor/temperature";
 
 void onPublish(const MqttClient* srce, const Topic& topic, const char* payload, size_t length)
-{ Serial << "--> " << srce->id().c_str() << ": ======> received " << topic.c_str() << endl; }
+{
+	Serial << "--> " << srce->id().c_str() << ": ======> received " << topic.c_str();
+	if (payload) Serial << ", payload[" << length << "]=[";
+	while(length--)
+	{
+		const char c=*payload++;
+		if (c!=10 and c!=13 and c <32) Serial << '?';
+		Serial << *payload++;
+	}
+	Serial<< endl;
+}
 
 std::map<std::string, MqttClient*> clients;
 std::map<std::string, MqttBroker*> brokers;
 
-
 void setup()
 {
   Serial.begin(115200);
 	delay(500);
 	Serial << endl << endl << endl
-	<< "Demo started. Type help for more..." << endl
 	<< "Connecting to '" << ssid << "' ";
 
   WiFi.mode(WIFI_STA);
@@ -49,6 +52,7 @@ void setup()
 	{ Serial << '-'; delay(500); }
 
   Serial << "Connected to " << ssid << "IP address: " << WiFi.localIP() << endl;
+	Serial << "Type help for more..." << endl;
 
 	MqttBroker* broker = new MqttBroker(1883);
 	broker->begin();
@@ -92,7 +96,6 @@ std::string getip(std::string& str, const char* if_empty=nullptr, char sep=' ')
 	std::string addr=getword(str, if_empty, sep);
 	std::string ip=addr;
 	std::vector<std::string> build;
-	bool ok=true;
 	while(ip.length())
 	{
 		std::string b=getword(ip,nullptr,'.');

library.json

@@ -6,7 +6,7 @@
         "type": "git",
         "url": "https://github.com/hsaturn/TinyMqtt.git"
     },
-    "version": "0.6.0",
+    "version": "0.7.1",
     "exclude": "",
     "examples": "examples/*/*.ino",
     "frameworks": "arduino",

library.properties

@@ -1,5 +1,5 @@
 name=TinyMqtt
-version=0.6.0
+version=0.7.1
 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/StringIndexer.h

@@ -15,6 +15,16 @@ class StringIndexer
 		std::string str;
 		uint8_t used=0;
 		friend class StringIndexer;
+
+		#if EPOXY_DUINO
+		public:
+		// Workaround to avoid coredump in Indexer::release
+		// when destroying a Topic after the deletion of
+		// StringIndexer::strings map (which can occurs only with AUnit,
+		// never in the ESP itself, because ESP never ends)
+		// (I hate static vars)
+		~StringCounter() { used=255; }
+		#endif
 	};
 	public:
 		using index_t=uint8_t;
@@ -104,7 +114,7 @@ class IndexedString
 
 		const std::string& str() const { return StringIndexer::str(index); }
 
-		const StringIndexer::index_t getIndex() const { return index; }
+		const StringIndexer::index_t& getIndex() const { return index; }
 
 	private:
 		StringIndexer::index_t index;

src/TinyMqtt.cpp

@@ -29,8 +29,8 @@ MqttClient::MqttClient(MqttBroker* parent, WiFiClient& new_client)
 	alive = millis()+5000;	// client expires after 5s if no CONNECT msg
 }
 
-MqttClient::MqttClient(MqttBroker* parent)
+MqttClient::MqttClient(MqttBroker* parent, const std::string& id)
-	: parent(parent)
+	: parent(parent), clientId(id)
 {
 		client = nullptr;
 
@@ -43,12 +43,17 @@ MqttClient::~MqttClient()
 	delete client;
 }
 
-void MqttClient::close()
+void MqttClient::close(bool bSendDisconnect)
 {
 	debug("close " << id().c_str());
 	mqtt_connected = false;
 	if (client)
 	{
+		if (bSendDisconnect and client->connected())
+		{
+			message.create(MqttMessage::Type::Disconnect);
+			message.sendTo(this);
+		}
 		client->stop();
 	}
 
@@ -120,7 +125,7 @@ void MqttBroker::loop()
 
   // for(auto it=clients.begin(); it!=clients.end(); it++)
 	// use index because size can change during the loop
-	for(int i=0; i<clients.size(); i++)
+	for(size_t i=0; i<clients.size(); i++)
 	{
 		auto client = clients[i];
     if (client->connected())
@@ -263,24 +268,42 @@ MqttError MqttClient::subscribe(Topic topic, uint8_t qos)
 
  	subscriptions.insert(topic);
 
-	if (parent==nullptr) // remote broker ?
+	if (parent==nullptr) // remote broker
 	{
-		debug("remote subscribe");
+		return sendTopic(topic, MqttMessage::Type::Subscribe, qos);
-		MqttMessage msg(MqttMessage::Type::Subscribe, 2);
-
-		// TODO manage packet identifier
-		msg.add(0);
-		msg.add(0);
-		
-		msg.add(topic);
-		msg.add(qos);
-		ret = msg.sendTo(this);
-		
-		// TODO we should wait (state machine) for SUBACK
 	}
 	return ret;
 }
 
+MqttError MqttClient::unsubscribe(Topic topic)
+{
+	auto it=subscriptions.find(topic);
+	if (it != subscriptions.end())
+	{
+		subscriptions.erase(it);
+		if (parent==nullptr) // remote broker
+		{
+			return sendTopic(topic, MqttMessage::Type::UnSubscribe, 0);
+		}
+	}
+	return MqttOk;
+}
+
+MqttError MqttClient::sendTopic(const Topic& topic, MqttMessage::Type type, uint8_t qos)
+{
+	MqttMessage msg(type, 2);
+
+	// TODO manage packet identifier
+	msg.add(0);
+	msg.add(0);
+
+	msg.add(topic);
+	msg.add(qos);
+
+	// TODO instead we should wait (state machine) for SUBACK / UNSUBACK ?
+	return msg.sendTo(this);
+}
+
 long MqttClient::counter=0;
 
 void MqttClient::processMessage()
@@ -447,11 +470,21 @@ if (message.type() != MqttMessage::Type::PingReq && message.type() != MqttMessag
 				{
 					callback(this, published, nullptr, 0);	// TODO send the real payload
 				}
-				// TODO should send PUBACK
+				message.create(MqttMessage::Type::PubAck);
+				// TODO re-add packet identifier if any
+				message.sendTo(this);
 				bclose = false;
 			}
 			break;
 
+		case MqttMessage::Type::Disconnect:
+			// TODO should discard any will message
+			if (!mqtt_connected) break;
+			mqtt_connected = false;
+			close(false);
+			bclose=false;
+			break;
+
 		default:
 			bclose=true;
 			break;
@@ -602,7 +635,7 @@ void MqttMessage::encodeLength(char* msb, int length)
 
 MqttError MqttMessage::sendTo(MqttClient* client)
 {
-	if (buffer.size()>2)
+	if (buffer.size())
 	{
 		debug("sending " << buffer.size() << " bytes");
 		encodeLength(&buffer[1], buffer.size()-2);

src/TinyMqtt.h

@@ -49,6 +49,7 @@ class MqttMessage
 			UnSubscribe = 0xA0,
 			PingReq     = 0xC0,
 			PingResp    = 0xD0,
+			Disconnect  = 0xE0
 		};
 		enum State
 		{
@@ -119,8 +120,8 @@ class MqttClient
 		FlagReserved = 1
 	};
 	public:
-		MqttClient(MqttBroker*);
+		MqttClient(MqttBroker* brk = nullptr, const std::string& id="");
-		MqttClient() : MqttClient(nullptr) {};
+		MqttClient(const std::string& id) : MqttClient(nullptr, id){}
 
 		~MqttClient();
 
@@ -137,7 +138,7 @@ class MqttClient
 		void id(std::string& new_id) { clientId = new_id; }
 
 		void loop();
-		void close();
+		void close(bool bSendDisconnect=true);
 		void setCallback(CallBack fun) {callback=fun; };
 
 		// Publish from client to the world
@@ -147,7 +148,7 @@ class MqttClient
 		MqttError publish(const Topic& t) { return publish(t, nullptr, 0);};
 
 		MqttError subscribe(Topic topic, uint8_t qos=0);
-		MqttError unsubscribe(Topic& topic);
+		MqttError unsubscribe(Topic topic);
 
 		// connected to local broker
 		// TODO seems to be useless
@@ -156,10 +157,9 @@ class MqttClient
 		void dump()
 		{
 			uint32_t ms=millis();
-			Serial << "MqttClient (" << clientId.c_str() << ") p=" << (int32_t) parent
+			Serial << "MqttClient (" << clientId.c_str() << ") " << (connected() ? " ON " : " OFF");
-				<< " c=" << (int32_t)client << (connected() ? " ON " : " OFF"); 
+			Serial << ", alive=" << alive << '/' << ms << ", ka=" << keep_alive;
-			Serial << ", alive=" << (uint32_t)alive << '/' << ms << ", ka=" << keep_alive;
+			Serial << (client && client->connected() ? "" : "dis") << "connected";
-			Serial << " cnx " << (client && client->connected());
 		  message.hexdump("entrant msg");
 			bool c=false;
 			Serial << " [";
@@ -176,6 +176,7 @@ class MqttClient
 		static long counter;	// Number of messages sent
 
 	private:
+		MqttError sendTopic(const Topic& topic, MqttMessage::Type type, uint8_t qos);
 		void resubscribe();
 
 		friend class MqttBroker;
@@ -219,11 +220,13 @@ class MqttBroker
 		void begin() { server.begin(); }
 		void loop();
 
-		uint8_t port() const { return server.port(); }
+		uint16_t port() const { return server.port(); }
 
-		void connect(std::string host, uint32_t port=1883);
+		void connect(std::string host, uint16_t port=1883);
 		bool connected() const { return state == Connected; }
 
+		size_t clientsCount() const { return clients.size(); }
+
 		void dump()
 		{
 			Serial << clients.size() << " client/s" << endl;

tests/Makefile

@@ -0,0 +1,20 @@
+tests:
+	set -e; \
+	for i in *-tests/Makefile; do \
+		echo '==== Making:' $$(dirname $$i); \
+		$(MAKE) -C $$(dirname $$i) -j; \
+	done
+
+runtests:
+	set -e; \
+	for i in *-tests/Makefile; do \
+		echo '==== Running:' $$(dirname $$i); \
+		$$(dirname $$i)/$$(dirname $$i).out; \
+	done
+
+clean:
+	set -e; \
+	for i in *-tests/Makefile; do \
+		echo '==== Cleaning:' $$(dirname $$i); \
+		$(MAKE) -C $$(dirname $$i) clean; \
+	done

tests/local-tests/Makefile

@@ -0,0 +1,6 @@
+# See https://github.com/bxparks/EpoxyDuino for documentation about this
+# Makefile to compile and run Arduino programs natively on Linux or MacOS.
+
+APP_NAME := local-tests
+ARDUINO_LIBS := AUnit AceCommon AceTime TinyMqtt EspMock
+include ../../../EpoxyDuino/EpoxyDuino.mk

tests/local-tests/local-tests.ino

@@ -0,0 +1,146 @@
+#include <AUnit.h>
+#include <TinyMqtt.h>
+#include <map>
+
+/**
+	* TinyMqtt local unit tests.
+	*
+	* Clients are connected to pseudo remote broker
+	* The remote will be 127.0.0.1:1883
+	* We are using 127.0.0.1 because this is simpler to test with a single ESP
+	* Also, this will allow to mock and thus run Action on github
+	**/
+
+using namespace std;
+
+MqttBroker broker(1883);
+
+std::map<std::string, std::map<Topic, int>>	published;		// map[client_id] => map[topic] = count
+
+void onPublish(const MqttClient* srce, const Topic& topic, const char* payload, size_t length)
+{
+	if (srce)
+		published[srce->id()][topic]++;
+}
+
+test(local_client_should_unregister_when_destroyed)
+{
+	assertEqual(broker.clientsCount(), (size_t)0);
+	{
+		MqttClient client;
+		assertEqual(broker.clientsCount(), (size_t)0);	// Ensure client is not yet connected
+		client.connect("127.0.0.1", 1883);
+		assertEqual(broker.clientsCount(), (size_t)1);	// Ensure client is now connected
+	}
+	assertEqual(broker.clientsCount(), (size_t)0);
+}
+
+#if 0
+test(local_connect)
+{
+	assertEqual(broker.clientsCount(), (size_t)0);
+
+	MqttClient client;
+	assertTrue(client.connected());
+	assertEqual(broker.clientsCount(), (size_t)1);
+}
+
+test(local_publish_should_be_dispatched)
+{
+	published.clear();
+	assertEqual(broker.clientsCount(), (size_t)0);
+
+	MqttClient subscriber;
+	subscriber.subscribe("a/b");
+	subscriber.subscribe("a/c");
+	subscriber.setCallback(onPublish);
+
+	MqttClient publisher;
+	publisher.publish("a/b");
+	publisher.publish("a/c");
+	publisher.publish("a/c");
+
+	assertEqual(published.size(), (size_t)1);	// 1 client has received something
+	assertTrue(published[""]["a/b"] == 1);
+	assertTrue(published[""]["a/c"] == 2);
+}
+
+test(local_publish_should_be_dispatched_to_local_clients)
+{
+	published.clear();
+	assertEqual(broker.clientsCount(), (size_t)0);
+
+	MqttClient subscriber_a("A");
+	subscriber_a.setCallback(onPublish);
+	subscriber_a.subscribe("a/b");
+	subscriber_a.subscribe("a/c");
+
+	MqttClient subscriber_b("B");
+	subscriber_b.setCallback(onPublish);
+	subscriber_b.subscribe("a/b");
+
+	MqttClient publisher;
+	publisher.publish("a/b");
+	publisher.publish("a/c");
+
+	assertEqual(published.size(), (size_t)2);	// 2 clients have received something
+	assertTrue(published["A"]["a/b"] == 1);
+	assertTrue(published["A"]["a/c"] == 1);
+	assertTrue(published["B"]["a/b"] == 1);
+	assertTrue(published["B"]["a/c"] == 0);
+}
+
+test(local_unsubscribe)
+{
+	published.clear();
+	assertEqual(broker.clientsCount(), (size_t)0);
+
+	MqttClient subscriber;
+	subscriber.setCallback(onPublish);
+	subscriber.subscribe("a/b");
+
+	MqttClient publisher;
+	publisher.publish("a/b");
+
+	subscriber.unsubscribe("a/b");
+
+	publisher.publish("a/b");
+	publisher.publish("a/b");
+
+	assertTrue(published[""]["a/b"] == 1);	// Only one publish has been received
+}
+
+test(local_nocallback_when_destroyed)
+{
+	published.clear();
+	assertEqual(broker.clientsCount(), (size_t)0);
+
+	MqttClient publisher;
+	{
+		MqttClient subscriber;
+		subscriber.setCallback(onPublish);
+		subscriber.subscribe("a/b");
+		publisher.publish("a/b");
+	}
+
+	publisher.publish("a/b");
+
+	assertEqual(published.size(), (size_t)1);	// Only one publish has been received
+}
+#endif
+
+//----------------------------------------------------------------------------
+// setup() and loop()
+void setup() {
+	delay(1000);
+	Serial.begin(115200);
+	while(!Serial);
+
+	Serial.println("=============[ NO WIFI CONNECTION TinyMqtt TESTS ]========================");
+}
+
+void loop() {
+	aunit::TestRunner::run();
+
+	if (Serial.available()) ESP.reset();
+}

tests/nowifi-tests/Makefile

@@ -0,0 +1,6 @@
+# See https://github.com/bxparks/EpoxyDuino for documentation about this
+# Makefile to compile and run Arduino programs natively on Linux or MacOS.
+
+APP_NAME := nowifi-tests
+ARDUINO_LIBS := AUnit AceCommon AceTime TinyMqtt EspMock
+include ../../../EpoxyDuino/EpoxyDuino.mk

tests/nowifi-tests/nowifi-tests.ino

@@ -0,0 +1,141 @@
+#include <AUnit.h>
+#include <TinyMqtt.h>
+#include <map>
+
+/**
+  * TinyMqtt nowifi unit tests.
+	*
+	* No wifi connection unit tests.
+    * Checks with a local broker. Clients must connect to the local client
+	**/
+
+using namespace std;
+
+MqttBroker broker(1883);
+
+std::map<std::string, std::map<Topic, int>>	published;		// map[client_id] => map[topic] = count
+
+void onPublish(const MqttClient* srce, const Topic& topic, const char* payload, size_t length)
+{
+	if (srce)
+		published[srce->id()][topic]++;
+}
+
+test(nowifi_client_should_unregister_when_destroyed)
+{
+	assertEqual(broker.clientsCount(), (size_t)0);
+	{
+		MqttClient client(&broker);
+		assertEqual(broker.clientsCount(), (size_t)1);
+	}
+	assertEqual(broker.clientsCount(), (size_t)0);
+}
+
+test(nowifi_connect)
+{
+	assertEqual(broker.clientsCount(), (size_t)0);
+
+	MqttClient client(&broker);
+	assertTrue(client.connected());
+	assertEqual(broker.clientsCount(), (size_t)1);
+}
+
+test(nowifi_publish_should_be_dispatched)
+{
+	published.clear();
+	assertEqual(broker.clientsCount(), (size_t)0);
+
+	MqttClient subscriber(&broker);
+	subscriber.subscribe("a/b");
+	subscriber.subscribe("a/c");
+	subscriber.setCallback(onPublish);
+
+	MqttClient publisher(&broker);
+	publisher.publish("a/b");
+	publisher.publish("a/c");
+	publisher.publish("a/c");
+
+	assertEqual(published.size(), (size_t)1);	// 1 client has received something
+	assertTrue(published[""]["a/b"] == 1);
+	assertTrue(published[""]["a/c"] == 2);
+}
+
+test(nowifi_publish_should_be_dispatched_to_nowifi_clients)
+{
+	published.clear();
+	assertEqual(broker.clientsCount(), (size_t)0);
+
+	MqttClient subscriber_a(&broker, "A");
+	subscriber_a.setCallback(onPublish);
+	subscriber_a.subscribe("a/b");
+	subscriber_a.subscribe("a/c");
+
+	MqttClient subscriber_b(&broker, "B");
+	subscriber_b.setCallback(onPublish);
+	subscriber_b.subscribe("a/b");
+
+	MqttClient publisher(&broker);
+	publisher.publish("a/b");
+	publisher.publish("a/c");
+
+	assertEqual(published.size(), (size_t)2);	// 2 clients have received something
+	assertTrue(published["A"]["a/b"] == 1);
+	assertTrue(published["A"]["a/c"] == 1);
+	assertTrue(published["B"]["a/b"] == 1);
+	assertTrue(published["B"]["a/c"] == 0);
+}
+
+test(nowifi_unsubscribe)
+{
+	published.clear();
+	assertEqual(broker.clientsCount(), (size_t)0);
+
+	MqttClient subscriber(&broker);
+	subscriber.setCallback(onPublish);
+	subscriber.subscribe("a/b");
+
+	MqttClient publisher(&broker);
+	publisher.publish("a/b");
+
+	subscriber.unsubscribe("a/b");
+
+	publisher.publish("a/b");
+	publisher.publish("a/b");
+
+	assertTrue(published[""]["a/b"] == 1);	// Only one publish has been received
+}
+
+test(nowifi_nocallback_when_destroyed)
+{
+	published.clear();
+	assertEqual(broker.clientsCount(), (size_t)0);
+
+	MqttClient publisher(&broker);
+
+	{
+		MqttClient subscriber(&broker);
+		subscriber.setCallback(onPublish);
+		subscriber.subscribe("a/b");
+		publisher.publish("a/b");
+	}
+
+	publisher.publish("a/b");
+
+	assertEqual(published.size(), (size_t)1);	// Only one publish has been received
+}
+
+//----------------------------------------------------------------------------
+// setup() and loop()
+void setup() {
+	delay(1000);
+	Serial.begin(115200);
+	while(!Serial);
+
+	Serial.println("=============[ NO WIFI CONNECTION TinyMqtt TESTS ]========================");
+}
+
+void loop() {
+	aunit::TestRunner::run();
+
+	if (Serial.available()) ESP.reset();
+}

tests/result.json

@@ -0,0 +1,6 @@
+{
+    "schemaVersion" : 1,
+    "label" : "tests",
+    "message" : "Message content",
+    "color": "red"
+}

tests/result.yaml

@@ -0,0 +1,2 @@
+result: 1
+insert: "passed"