commschamp / cc.mqttsn.libs

CommsChampion Ecosystem MQTT-SN client / gateway libraries and applications
Mozilla Public License 2.0
63 stars 16 forks source link

Fix MsgId field for QOS -1 and QOS 0 #7

Closed a1lu closed 4 years ago

a1lu commented 4 years ago

Section 5.4.12 of MQTT-SN spec states the following for the message ID:

MsgId: same meaning as the MQTT “Message ID”; only relevant in case of QoS levels 1 and 2, otherwisecoded 0x0000.

This PR fixes this.

arobenko commented 4 years ago

Thanks, I'm currently working on some code cleanup to allow usage of latest COMMS library (#6). I will slip you update in within next couple of days.

arobenko commented 4 years ago

Fixed on "develop".

arobenko commented 4 years ago

Should be fixed in v0.9 release, please reopen if problem persists.

arobenko commented 4 years ago

Should be fixed in v0.9 release, please reopen if problem persists.

a1lu commented 4 years ago

Could it be that you missed allocMsgId() in publish()?

arobenko commented 4 years ago

Hi @a1lu, Your link points to the location where allocMsgId() is called for the variant of qos=0, however line 793 invokes doPublishId() for other qos values. The doPublishId() allocates new msgId for the first message (see line 2289) and retains the old one for the messages being resent (dup ones).

Does it answer your question?

a1lu commented 4 years ago

Hi @arobenko

Your link points to the location where allocMsgId() is called for the variant of qos=0

Exactly. According to the spec the message ID for qos 0 and qos -1 must be 0x0000.

arobenko commented 4 years ago

@a1lu, sorry it looks I misunderstood you and didn't review the context of your comment properly. I thought you meant the call to allocMsgId() is missing.

I see what you mean now. I'll do the correction release in about 1 week time. Thanks.

a1lu commented 4 years ago

Hi @arobenko , sorry for the bad description. Thanks for the update.