From 5f7b4537c89c7d3b61a911582742737c2e182665 Mon Sep 17 00:00:00 2001
From: hsaturn <hsaturn.gmail.com>
Date: Wed, 5 Jan 2022 02:03:42 +0100
Subject: [PATCH] Enhance PR#22

---
 src/TinyMqtt.cpp                      | 25 ++++----------
 tests/network-tests/network-tests.ino | 47 ++++++++++++++++++++++++---
 2 files changed, 50 insertions(+), 22 deletions(-)

diff --git a/src/TinyMqtt.cpp b/src/TinyMqtt.cpp
index d28e365..1547c0b 100644
--- a/src/TinyMqtt.cpp
+++ b/src/TinyMqtt.cpp
@@ -523,7 +523,7 @@ if (mesg->type() != MqttMessage::Type::PingReq && mesg->type() != MqttMessage::T
 				payload = header+2;
 				
 				debug("un/subscribe loop");
-				std::vector<char> qoss;
+				std::string qoss;
 				while(payload < mesg->end())
 				{
 					mesg->getString(payload, len);	// Topic
@@ -553,23 +553,12 @@ if (mesg->type() != MqttMessage::Type::PingReq && mesg->type() != MqttMessage::T
 				}
 				debug("end loop");
 				bclose = false;
-				// TODO SUBACK
-				if ((mesg->type() & 0XF0) == MqttMessage::Type::Subscribe)
-				{
-					sMQTTMessage msg(sMQTTMessage::Type::SubAck);
-					msg.add(header[0]);
-					msg.add(header[1]);
-					for (int i = 0; i<qoss.size(); i++)
-						msg.add(qoss[i]);
-					msg.sendTo(this);
-				}
-				else
-				{
-					sMQTTMessage msg(sMQTTMessage::Type::UnSuback);
-					msg.add(header[0]);
-					msg.add(header[1]);
-					msg.sendTo(this);
-				}
+
+        MqttMessage ack(mesg->type() == MqttMessage::Type::Subscribe ? MqttMessage::Type::SubAck : MqttMessage::Type::UnSuback);
+        ack.add(header[0]);
+        ack.add(header[1]);
+        ack.add(qoss.c_str(), qoss.size(), false);
+        ack.sendTo(this);
 			}
 			break;
 
diff --git a/tests/network-tests/network-tests.ino b/tests/network-tests/network-tests.ino
index f953114..268be3a 100644
--- a/tests/network-tests/network-tests.ino
+++ b/tests/network-tests/network-tests.ino
@@ -12,6 +12,11 @@
 
 using namespace std;
 
+String toString(const IPAddress& ip)
+{
+  return String(ip[0])+'.'+String(ip[1])+'.'+String(ip[2])+'.'+String(ip[3]);
+}
+
 MqttBroker broker(1883);
 
 std::map<std::string, std::map<Topic, int>>	published;		// map[client_id] => map[topic] = count
@@ -51,10 +56,44 @@ test(network_single_broker_begin)
   // TODO Nothing is tested here !
 }
 
+test(suback)
+{
+  start_servers(2, true);
+  assertEqual(WiFi.status(), WL_CONNECTED);
+
+  MqttBroker broker(1883);
+  broker.begin();
+  IPAddress broker_ip = WiFi.localIP();
+
+  ESP8266WiFiClass::selectInstance(2);
+  MqttClient client;
+  client.connect(broker_ip.toString().c_str(), 1883);
+  broker.loop();
+
+  assertTrue(broker.clientsCount() == 1);
+  assertTrue(client.connected());
+
+  MqttClient::counters[MqttMessage::Type::SubAck] = 0;
+  client.subscribe("a/b");
+
+  // TODO how to avoid these loops ???
+  broker.loop();
+  client.loop();
+
+  String s;
+  for(const auto& in: MqttClient::counters)
+  {
+    s = s+String(in.first);
+    s = s+':';
+    s = s+String(in.second);
+    s = s+' ';
+  }
+  assertEqual(MqttClient::counters[MqttMessage::Type::SubAck], 1);
+}
+
 test(network_client_to_broker_connexion)
 {
   start_servers(2, true);
-
   assertEqual(WiFi.status(), WL_CONNECTED);
 
   MqttBroker broker(1883);
@@ -147,7 +186,6 @@ test(network_client_should_unregister_when_destroyed)
 	assertEqual(broker.clientsCount(), (size_t)0);
 }
 
-#if 0
 
 // THESE TESTS ARE IN LOCAL MODE
 // WE HAVE TO CONVERT THEM TO WIFI MODE (pass through virtual TCP link)
@@ -277,8 +315,8 @@ test(network_hudge_payload)
   // onPublish should have filled lastPayload and lastLength
   assertEqual(payload, lastPayload);
 	assertEqual(lastLength, strlen(payload));
+	assertEqual(strcmp(payload, lastPayload), 0);
 }
-#endif
 
 //----------------------------------------------------------------------------
 // setup() and loop()
@@ -286,9 +324,10 @@ void setup() {
 	/* delay(1000);
 	Serial.begin(115200);
 	while(!Serial);
+	*/
 
 	Serial.println("=============[ FAKE NETWORK TinyMqtt TESTS       ]========================");
-*/
+
 	WiFi.mode(WIFI_STA);
 	WiFi.begin("network", "password");
 }