Fix generation of packet identifier.

Build/MQTTnet.nuspec

@@ -12,6 +12,7 @@
     <description>MQTTnet is a high performance .NET library for MQTT based communication. It provides a MQTT client and a MQTT server (broker).</description>
     <releaseNotes>* [Core] Fixed some still thread blocking parts in the code (thanks to @kpreisser).
 * [Core] Updated 3rd-Party packages.
+* [Core] Fixed wrong packet identifier calculation (thanks to @benpittoors).
 * [Core] Fixed an issue when reading the body of a package from a disconnected sender (thanks to @kpreisser).
 * [Core] Fixed wrong parsing of the body length (thanks to @kpreisser).
 * [Client] The client interfaces are now implementing _IDisposable_.

Frameworks/MQTTnet.NetStandard/Client/MqttClient.cs

@@ -14,13 +14,13 @@ namespace MQTTnet.Client
 {
     public class MqttClient : IMqttClient
     {
+        private readonly MqttPacketIdentifierProvider _packetIdentifierProvider = new MqttPacketIdentifierProvider();
         private readonly IMqttClientAdapterFactory _adapterFactory;
         private readonly MqttPacketDispatcher _packetDispatcher;
         private readonly IMqttNetLogger _logger;
 
         private IMqttClientOptions _options;
         private bool _isReceivingPackets;
-        private int _latestPacketIdentifier;
         private CancellationTokenSource _cancellationTokenSource;
         private IMqttChannelAdapter _adapter;
 
@@ -49,7 +49,7 @@ namespace MQTTnet.Client
             {
                 _options = options;
                 _cancellationTokenSource = new CancellationTokenSource();
-                _latestPacketIdentifier = 0;
+                _packetIdentifierProvider.Reset();
                 _packetDispatcher.Reset();
 
                 _adapter = _adapterFactory.CreateClientAdapter(options.ChannelOptions, _logger);
@@ -106,7 +106,7 @@ namespace MQTTnet.Client
 
             var subscribePacket = new MqttSubscribePacket
             {
-                PacketIdentifier = GetNewPacketIdentifier(),
+                PacketIdentifier = _packetIdentifierProvider.GetNewPacketIdentifier(),
                 TopicFilters = topicFilters.ToList()
             };
 
@@ -128,7 +128,7 @@ namespace MQTTnet.Client
 
             var unsubscribePacket = new MqttUnsubscribePacket
             {
-                PacketIdentifier = GetNewPacketIdentifier(),
+                PacketIdentifier = _packetIdentifierProvider.GetNewPacketIdentifier(),
                 TopicFilters = topicFilters.ToList()
             };
 
@@ -156,7 +156,7 @@ namespace MQTTnet.Client
                         {
                             foreach (var publishPacket in qosGroup)
                             {
-                                publishPacket.PacketIdentifier = GetNewPacketIdentifier();
+                                publishPacket.PacketIdentifier = _packetIdentifierProvider.GetNewPacketIdentifier();
                                 await SendAndReceiveAsync<MqttPubAckPacket>(publishPacket).ConfigureAwait(false);
                             }
 
@@ -166,7 +166,7 @@ namespace MQTTnet.Client
                         {
                             foreach (var publishPacket in qosGroup)
                             {
-                                publishPacket.PacketIdentifier = GetNewPacketIdentifier();
+                                publishPacket.PacketIdentifier = _packetIdentifierProvider.GetNewPacketIdentifier();
                                 var pubRecPacket = await SendAndReceiveAsync<MqttPubRecPacket>(publishPacket).ConfigureAwait(false);
                                 await SendAndReceiveAsync<MqttPubCompPacket>(pubRecPacket.CreateResponse<MqttPubRelPacket>()).ConfigureAwait(false);
                             }
@@ -340,11 +340,6 @@ namespace MQTTnet.Client
             return (TResponsePacket)await packetAwaiter.ConfigureAwait(false);
         }
 
-        private ushort GetNewPacketIdentifier()
-        {
-            return (ushort)Interlocked.Increment(ref _latestPacketIdentifier);
-        }
-
         private async Task SendKeepAliveMessagesAsync(CancellationToken cancellationToken)
         {
             _logger.Info<MqttClient>("Start sending keep alive packets.");

Frameworks/MQTTnet.NetStandard/Client/MqttPacketIdentifierProvider.cs

@@ -0,0 +1,32 @@
+namespace MQTTnet.Client
+{
+    public class MqttPacketIdentifierProvider
+    {
+        private readonly object _syncRoot = new object();
+        private ushort _value;
+
+        public void Reset()
+        {
+            lock (_syncRoot)
+            {
+                _value = 0;
+            }
+        }
+
+        public ushort GetNewPacketIdentifier()
+        {
+            lock (_syncRoot)
+            {
+                _value++;
+
+                if (_value == 0)
+                {
+                    // As per official MQTT documentation the package identifier should never be 0.
+                    _value = 1;
+                }
+
+                return _value;
+            }
+        }
+    }
+}

Tests/MQTTnet.Core.Tests/MqttPacketIdentifierProviderTests.cs

@@ -0,0 +1,32 @@
+using Microsoft.VisualStudio.TestTools .UnitTesting;
+using MQTTnet.Client;
+
+namespace MQTTnet.Core.Tests
+{
+    [TestClass]
+    public class MqttPacketIdentifierProviderTests
+    {
+        [TestMethod]
+        public void Reset()
+        {
+            var p = new MqttPacketIdentifierProvider();
+            Assert.AreEqual(1, p.GetNewPacketIdentifier());
+            Assert.AreEqual(2, p.GetNewPacketIdentifier());
+            p.Reset();
+            Assert.AreEqual(1, p.GetNewPacketIdentifier());
+        }
+
+        [TestMethod]
+        public void ReachBoundaries()
+        {
+            var p = new MqttPacketIdentifierProvider();
+
+            for (ushort i = 0; i < ushort.MaxValue; i++)
+            {
+                Assert.AreEqual(i + 1, p.GetNewPacketIdentifier());
+            }
+
+            Assert.AreEqual(1, p.GetNewPacketIdentifier());
+        }
+    }
+}