Merge pull request #2 from gnought/hotfix/no_messageid_in_qos0

MUST not contain a packet identifier in QoS 0 PUBLISH packets

.gitignore

@@ -10,8 +10,9 @@ pids
 # Directory for instrumented libs generated by jscoverage/JSCover
 lib-cov
 
-# Coverage directory used by tools like istanbul
+# Coverage directory used by tools like nyc
 coverage
+.nyc_output
 
 # Grunt intermediate storage (http://gruntjs.com/creating-plugins#storing-task-files)
 .grunt

.travis.yml

@@ -1,6 +1,5 @@
 language: node_js
 node_js:
-  - "0.10"
-  - "0.12"
-  - "4"
-  - "5"
+  - 8
+  - 10
+  - 12

package.json

@@ -6,12 +6,14 @@
   "dependencies": {},
   "devDependencies": {
     "faucet": "0.0.1",
-    "pre-commit": "^1.1.2",
-    "standard": "^5.4.1",
-    "tape": "^4.2.2"
+    "nyc": "^14.1.1",
+    "pre-commit": "^1.2.2",
+    "standard": "^13.0.2",
+    "tape": "^4.11.0"
   },
   "scripts": {
-    "test": "standard && tape test.js | faucet"
+    "test": "standard && tape test.js | faucet",
+    "coverage": "nyc --reporter=lcov tape test.js"
   },
   "repository": {
     "type": "git",

packet.js

@@ -3,12 +3,16 @@
 function Packet (original, broker) {
   this.cmd = original.cmd || 'publish'
   this.brokerId = original.brokerId || (broker && broker.id)
-  this.brokerCounter = original.brokerCounter || (broker && (++broker.counter) || 0)
+  this.brokerCounter = original.brokerCounter || (broker ? (++broker.counter) : 0)
   this.topic = original.topic
-  this.payload = original.payload || new Buffer(0)
+  this.payload = original.payload || Buffer.alloc(0)
   this.qos = original.qos || 0
   this.retain = original.retain || false
-  this.messageId = 0
+  // [MQTT-2.3.1-5]
+  if (this.qos > 0 || this.cmd !== 'publish') {
+    //  [MQTT-2.3.1-1]
+    this.messageId = 1
+  }
 }
 
 module.exports = Packet

test.js

@@ -3,16 +3,42 @@
 var test = require('tape')
 var Packet = require('./')
 
-test('Packet defaults', function (t) {
+test('Packet defaults - PUBLISH, QoS 0', function (t) {
   var instance = new Packet({})
   t.equal(instance.cmd, 'publish')
   t.equal(instance.brokerId, undefined)
   t.equal(instance.brokerCounter, 0)
   t.equal(instance.topic, undefined)
-  t.deepEqual(instance.payload, new Buffer(0))
+  t.deepEqual(instance.payload, Buffer.alloc(0))
   t.equal(instance.qos, 0)
   t.equal(instance.retain, false)
-  t.equal(instance.messageId, 0)
+  t.equal(instance.messageId, undefined)
+  t.end()
+})
+
+test('Packet defaults - PUBREL, QoS 0', function (t) {
+  var instance = new Packet({ cmd: 'pubrel' })
+  t.equal(instance.cmd, 'pubrel')
+  t.equal(instance.brokerId, undefined)
+  t.equal(instance.brokerCounter, 0)
+  t.equal(instance.topic, undefined)
+  t.deepEqual(instance.payload, Buffer.alloc(0))
+  t.equal(instance.qos, 0)
+  t.equal(instance.retain, false)
+  t.equal(instance.messageId, 1)
+  t.end()
+})
+
+test('Packet defaults - PUBLISH, QoS 1', function (t) {
+  var instance = new Packet({ qos: 1 })
+  t.equal(instance.cmd, 'publish')
+  t.equal(instance.brokerId, undefined)
+  t.equal(instance.brokerCounter, 0)
+  t.equal(instance.topic, undefined)
+  t.deepEqual(instance.payload, Buffer.alloc(0))
+  t.equal(instance.qos, 1)
+  t.equal(instance.retain, false)
+  t.equal(instance.messageId, 1)
   t.end()
 })
 
@@ -36,7 +62,7 @@ test('Packet copies over most data', function (t) {
     payload: 'world',
     qos: 2,
     retain: true,
-    messageId: 0 // this is different
+    messageId: 1 // this is different
   }
 
   t.deepEqual(instance, expected)
@@ -65,7 +91,7 @@ test('Packet fills in broker data', function (t) {
     payload: 'world',
     qos: 2,
     retain: true,
-    messageId: 0 // this is different
+    messageId: 1 // this is different
   }
 
   t.deepEqual(instance, expected)