Merge pull request emqx#111 from KonstantinRitt/sub_unsub

Simplify (un)subscribe (changes API)
gilbertwong96 · Aug 21, 2017 · 20ed522 · 20ed522
2 parents f4334fd + a532eaf
commit 20ed522
Show file tree

Hide file tree

Showing 5 changed files with 14 additions and 31 deletions.
diff --git a/src/mqtt/qmqtt_client.cpp b/src/mqtt/qmqtt_client.cpp
@@ -293,10 +293,10 @@ quint16 QMQTT::Client::publish(const Message& message)
     return d->publish(message);
 }
 
-quint16 QMQTT::Client::subscribe(const QString& topic, const quint8 qos)
+void QMQTT::Client::subscribe(const QString& topic, const quint8 qos)
 {
     Q_D(Client);
-    return d->subscribe(topic, qos);
+    d->subscribe(topic, qos);
 }
 
 void QMQTT::Client::unsubscribe(const QString& topic)

diff --git a/src/mqtt/qmqtt_client.h b/src/mqtt/qmqtt_client.h
@@ -187,7 +187,7 @@ public slots:
     void connectToHost();
     void disconnectFromHost();
 
-    quint16 subscribe(const QString& topic, const quint8 qos);
+    void subscribe(const QString& topic, const quint8 qos = 0);
     void unsubscribe(const QString& topic);
 
     quint16 publish(const Message& message);
@@ -197,7 +197,7 @@ public slots:
     void disconnected();
     void error(const QMQTT::ClientError error);
 
-    void subscribed(const QString& topic, const quint8 qos);
+    void subscribed(const QString& topic, const quint8 qos = 0);
     void unsubscribed(const QString& topic);
     void published(const quint16 msgid, const quint8 qos);
     void received(const QMQTT::Message& message);

diff --git a/src/mqtt/qmqtt_client_p.cpp b/src/mqtt/qmqtt_client_p.cpp
@@ -341,11 +341,10 @@ void QMQTT::ClientPrivate::puback(const quint8 type, const quint16 msgid)
     sendPuback(type, msgid);
 }
 
-quint16 QMQTT::ClientPrivate::subscribe(const QString& topic, const quint8 qos)
+void QMQTT::ClientPrivate::subscribe(const QString& topic, const quint8 qos)
 {
     quint16 msgid = sendSubscribe(topic, qos);
     _midToTopic[msgid] = topic;
-    return msgid;
 }
 
 void QMQTT::ClientPrivate::unsubscribe(const QString& topic)
@@ -359,7 +358,7 @@ void QMQTT::ClientPrivate::onNetworkDisconnected()
     Q_Q(Client);
 
     stopKeepAlive();
-    clearMidToTopic();
+    _midToTopic.clear();
     emit q->disconnected();
 }
 
@@ -405,12 +404,14 @@ void QMQTT::ClientPrivate::onNetworkReceived(const QMQTT::Frame& frm)
         break;
     case SUBACK:
         mid = frame.readInt();
+        topic = _midToTopic.take(mid);
         qos = frame.readChar();
-        handleSuback(midToTopic(mid), qos);
+        handleSuback(topic, qos);
         break;
     case UNSUBACK:
         mid = frame.readInt();
-        handleUnsuback(midToTopic(mid));
+        topic = _midToTopic.take(mid);
+        handleUnsuback(topic);
         break;
     case PINGRESP:
         handlePingresp();
@@ -480,22 +481,6 @@ void QMQTT::ClientPrivate::handleUnsuback(const QString &topic) {
     emit q->unsubscribed(topic);
 }
 
-QString QMQTT::ClientPrivate::midToTopic(const quint16 mid)
-{
-    QString result;
-    QHash<quint16, QString>::Iterator it = _midToTopic.find(mid);
-    if (it != _midToTopic.end()) {
-        result = it.value();
-        _midToTopic.erase(it);
-    }
-    return result;
-}
-
-void QMQTT::ClientPrivate::clearMidToTopic()
-{
-    _midToTopic.clear();
-}
-
 bool QMQTT::ClientPrivate::autoReconnect() const
 {
     return _network->autoReconnect();

diff --git a/src/mqtt/qmqtt_client_p.h b/src/mqtt/qmqtt_client_p.h
@@ -95,7 +95,7 @@ class ClientPrivate
     void onNetworkDisconnected();
     quint16 publish(const Message& message);
     void puback(const quint8 type, const quint16 msgid);
-    quint16 subscribe(const QString& topic, const quint8 qos);
+    void subscribe(const QString& topic, const quint8 qos);
     void unsubscribe(const QString& topic);
     void onNetworkReceived(const QMQTT::Frame& frame);
     void handleConnack(const quint8 ack);
@@ -105,8 +105,6 @@ class ClientPrivate
     void handleUnsuback(const QString& topic);
     void handlePubcomp(const Message& message);
     void handlePingresp();
-    QString midToTopic(const quint16 mid);
-    void clearMidToTopic();
     bool autoReconnect() const;
     void setAutoReconnect(const bool autoReconnect);
     bool autoReconnectInterval() const;

diff --git a/tests/gtest/tests/clienttest.cpp b/tests/gtest/tests/clienttest.cpp
@@ -431,11 +431,11 @@ TEST_F(ClientTest, subscribeEmitsSubscribedSignal_Test)
     EXPECT_CALL(*_networkMock, sendFrame(_));
     QSignalSpy spy(_client.data(), &QMQTT::Client::subscribed);
 
-    quint16 msgid = _client->subscribe("topic", QOS2);
+    _client->subscribe("topic", QOS2);
 
     QByteArray payLoad;
-    payLoad.append((char)(msgid >> 8)); // message ID MSB
-    payLoad.append((char)(msgid && 0xFF)); // message ID LSB
+    payLoad.append((char)0x00); // message ID MSB
+    payLoad.append((char)0x01); // message ID LSB
     payLoad.append((char)QOS2); // QOS
     QMQTT::Frame frame(SUBACK_TYPE, payLoad);
     emit _networkMock->received(frame);