Improve subacks

Wiebe Cazemier
1 parent ee79271a
Showing 3 changed files with 20 additions and 23 deletions
mqttpacket.cpp
types.cpp
types.h
@@ -69,7 +69,8 @@ MqttPacket::MqttPacket(const SubAck &amp;subAck) :
     }
     std::vector<char> returnList;
-    for (SubAckReturnCodes code : subAck.responses)
+    returnList.reserve(subAck.responses.size());
+    for (ReasonCodes code : subAck.responses)
     {
         returnList.push_back(static_cast<char>(code));
     }
@@ -605,7 +606,7 @@ void MqttPacket::handleSubscribe()
     if (firstByteFirstNibble != 2)
         throw ProtocolError("First LSB of first byte is wrong value for subscribe packet.");
-    uint16_t packet_id = readTwoBytesToUInt16();
+    const uint16_t packet_id = readTwoBytesToUInt16();
     if (packet_id == 0)
     {
@@ -637,7 +638,7 @@ void MqttPacket::handleSubscribe()
     Authentication &authentication = *ThreadGlobals::getAuth();
-    std::list<char> subs_reponse_codes;
+    std::list<ReasonCodes> subs_reponse_codes;
     while (remainingAfterPos() > 0)
     {
         uint16_t topicLength = readTwoBytesToUInt16();
@@ -649,7 +650,7 @@ void MqttPacket::handleSubscribe()
         if (!isValidSubscribePath(topic))
             throw ProtocolError(formatString("Invalid subscribe path: %s", topic.c_str()));
-        char qos = readByte();
+        uint8_t qos = readByte();
         if (qos > 2)
             throw ProtocolError("QoS is greater than 2, and/or reserved bytes in QoS field are not 0.");
@@ -660,16 +661,14 @@ void MqttPacket::handleSubscribe()
         {
             logger->logf(LOG_SUBSCRIBE, "Client '%s' subscribed to '%s' QoS %d", sender->repr().c_str(), topic.c_str(), qos);
             sender->getThreadData()->getSubscriptionStore()->addSubscription(sender, topic, subtopics, qos);
-            subs_reponse_codes.push_back(qos);
+            subs_reponse_codes.push_back(static_cast<ReasonCodes>(qos));
         }
         else
         {
             logger->logf(LOG_SUBSCRIBE, "Client '%s' subscribe to '%s' denied or failed.", sender->repr().c_str(), topic.c_str());
-            // We can't not send an ack, because if there are multiple subscribes, you send fewer acks back, losing sync.
-            char return_code = qos;
-            if (sender->getProtocolVersion() >= ProtocolVersion::Mqtt311)
-                return_code = static_cast<char>(SubAckReturnCodes::Fail);
+            // We can't not send an ack, because if there are multiple subscribes, you'd send fewer acks back, losing sync.
+            ReasonCodes return_code = sender->getProtocolVersion() >= ProtocolVersion::Mqtt311 ? ReasonCodes::NotAuthorized : static_cast<ReasonCodes>(qos);
             subs_reponse_codes.push_back(return_code);
         }
     }
@@ -82,15 +82,21 @@ size_t ConnAck::getLengthWithoutFixedHeader() const
     return result;
 }
-SubAck::SubAck(const ProtocolVersion protVersion, uint16_t packet_id, const std::list<char> &subs_qos_reponses) :
+SubAck::SubAck(const ProtocolVersion protVersion, uint16_t packet_id, const std::list<ReasonCodes> &subs_qos_reponses) :
     protocol_version(protVersion),
     packet_id(packet_id)
 {
     assert(!subs_qos_reponses.empty());
-    for (char ack_code : subs_qos_reponses)
+    for (const ReasonCodes ack_code : subs_qos_reponses)
     {
-        responses.push_back(static_cast<SubAckReturnCodes>(ack_code));
+        assert(protVersion >= ProtocolVersion::Mqtt311 || ack_code <= ReasonCodes::GrantedQoS2);
+
+        ReasonCodes _ack_code = ack_code;
+        if (protVersion < ProtocolVersion::Mqtt5 && ack_code >= ReasonCodes::UnspecifiedError)
+            _ack_code = ReasonCodes::UnspecifiedError; // Equals Mqtt 3.1.1 'suback failure'
+
+        responses.push_back(static_cast<ReasonCodes>(_ack_code));
     }
 }
@@ -164,23 +164,15 @@ public:
     size_t getLengthWithoutFixedHeader() const;
 };
-enum class SubAckReturnCodes
-{
-    MaxQoS0 = 0,
-    MaxQoS1 = 1,
-    MaxQoS2 = 2,
-    Fail = 0x80
-};
-
 class SubAck
 {
 public:
     const ProtocolVersion protocol_version;
-    uint16_t packet_id;
-    std::list<SubAckReturnCodes> responses;
+    const uint16_t packet_id;
+    std::list<ReasonCodes> responses;
     std::shared_ptr<Mqtt5PropertyBuilder> propertyBuilder;
-    SubAck(const ProtocolVersion protVersion, uint16_t packet_id, const std::list<char> &subs_qos_reponses);
+    SubAck(const ProtocolVersion protVersion, uint16_t packet_id, const std::list<ReasonCodes> &subs_qos_reponses);
     size_t getLengthWithoutFixedHeader() const;
 };