Add option 'qos2Puback' -easy merge #642
Conversation
…ges to QOS-1, resulting in a puback message. Prevents mqtt.js keeping qos-2 messages forever if incorrectly configured.
| @@ -19,6 +19,7 @@ var moscaSetting = { | |||
| { type: "https", port: 3001, bundle: true, credentials: { keyPath: SECURE_KEY, certPath: SECURE_CERT } } | |||
| ], | |||
| stats: false, | |||
| qos2Puback: false, // can set to true if using a client which will eat puback for QOS 2; e.g. mqtt.js | |||
There was a problem hiding this comment.
I'm thinking more about this, and I think this behavior should be a bit more customizable. How about adding it as a string with two options:'dropToQoS1','ignore'and'disconnect'? Maybe we can call it onQoS2publish`?
There was a problem hiding this comment.
an example of close of the connection is just above, so it's not going to take much serious thought :).
Are we worried about string comparison in such a well-called area of code?
There was a problem hiding this comment.
I would not worry about it, it should not be a hot path: QoS 2 is not really supported.
There was a problem hiding this comment.
I move the 'if' so it checks the QOS before anything else anyway. It's only here as an option to protect stupids like me who set mqtt.js to QoS2 :). (well node-red does default to QoS2 for mqtt subscription; fooled me).
…nitially 'noack', and can optionally be 'disconnect' or 'dropToQoS1' 'disconnect' will cause a client disconnect on receipt of a QoS2 message. 'dropToQoS1' will puback the message, allowing mqtt.js to work; just not at QoS2 service level.
|
take a look. not tested, but tests added (maybe travis will tell us if I screwed it up :) ). |
|
Tests are failing: https://travis-ci.org/mcollina/mosca/jobs/234047506#L1637 |
|
error in test - sent QoS1 and expected it to disconnect! fixed now hopefully; give travis another 10 minutes. |
|
@btsimonh yes. But you can just run https://github.com/mcollina/mosca/blob/master/test/server.js. |
|
for my future reference: |
|
not failing locally anymore. Love travis; had actually got the code all in the wrong place. Should be good to go now. |
|
all good now, only node4 failing at redis. |
Add option 'qos2Puback' - if set to true, will modify published qos 2 messages to qos 1 before processing.
This results in the messages getting a puback, which fools mqtt.js into not getting stuck when sending qos 2 to mosca.
It's an option because the spec does not allow for this? Probably will not suit all clients, but better than mqtt.js dying; a patch over the issue until mosca can get true QOS-2 or QOS-2 pseudo-processing.
This PR replaces the previous for ease of merging and understanding the changes.