search

eclipse / paho.mqtt.java

Eclipse Paho Java MQTT client library. Paho is an Eclipse IoT project.
https://eclipse.org/paho
Other
2.12k stars 883 forks source link

MQTTv5 - Could not get return codes from Publish Message flows #520

Closed jpwsutton closed 6 years ago

jpwsutton commented 6 years ago

Please fill out the form below before submitting, thank you!

If this is a bug regarding the Android Service, please raise the bug here instead: https://github.com/eclipse/paho.mqtt.android/issues/new

Whilst getReasonCodes() worked for SubAck and UnSubAck messages, it was not working for any of the ACKs in the publish flow.

The reason for this was because when adding an optimisation for the ACK packets from the spec that assumes a 0 return code if the message is a certain length, it meant that the default RC of 0 was not being set.

Whilst fixing this, I've noticed that the Token API is a little messy. I could leave the existing way to access it as deliveryToken.getResponse().getReasonCodes(). Or change it to something a bit more user friendly like:

deliveryToken.getReasonCodes().

Does anyone have a preference?

dennynguyen commented 6 years ago

I'm still getting getReasonCodes() returning 0 for all cases in QoS=1 and QoS=2. I have confirmed the server is sending the correct reason code on the PUBACK and PUBREC's, but I'm not able to see them in Paho.

jpwsutton commented 6 years ago

Hi @dennynguyen , I've finally figured out what was going wrong. It's taken me a while to get to the bottom of this, mostly because everything I've fixed so far has in some part been responsible, however I found the main culprit just now. One of the newer additions to the v5 spec allowed for shortening of publish Ack packets when the return code was 0, after I ended up hand-decoding the PubRec that I got back from Messagesight, I found that the logic that read the return code, was only working when the message had a defined property as well as the RC, but for messages that just had the RC with no properties, the RC was being skipped and set to 0. This probably explains how I missed this in earlier testing, as I was putting properties in everything to test that logic at the same time, so I never hit this test case.

The latest snapshot build should now resolve this.