Adafruit_MQTT_Library
Adafruit_MQTT_Library copied to clipboard
adafruit
MQTT with QOS 1 or 2 forbids a packet ID of 0.
- Describe the scope of your change--i.e. what the change does and what parts of the code were modified. This will help us understand any risks of integrating the code.
The change is to the packet numbering, which now starts at 1 and wraps around to 1 (rather than 0) when overflowing 16 bits. This is because the MQTT standard requires nonzero packet IDs for QOS=1 and QOS=2. In the previous version, if a client is created, then connected, then a QOS=1 subscription packet is sent with packet ID 0 is sent, and my server (mosquitto) disconnects. Re-connecting and trying again works because the packet ID has incremented to 1, which is legal.
- Describe any known limitations with your change. For example if the change doesn't apply to a supported platform of the library please mention it.
I don't think there are any.
- Please run any tests or examples that can exercise your modified code. We strive to not break users of the code and running tests/examples helps with this process.
I am using this code in a project and have tried with pub/sub; seems to work now.
@dlizotte-uwo are you available to make the requested changes above?
Hi sorry @brentru - day job is madness. I can probably fix this weekend if that's still useful.
@dlizotte-uwo It's still useful! I'll keep this issue open for when you're ready.
I think based on what fipwmaqzufheoxq92ebc says and based on https://github.com/adafruit/Adafruit_MQTT_Library/pull/199 that this is still needed and checks are in the right place. I think it's good to keep 1 <= packet_id_counter <= 65535 an invariant in this code.
I'm digesting, I'll come back to you tomorrow. I hadn't before appreciated the packet Id of server initiated messages and client initiated messages could have the same packet Id at the same time. I'd never really examined the protocol in such depth before. Just verifying where we check and use packet Id's, etc.
On Mon, 21 Aug 2023, 18:46 Brent Rubell, @.***> wrote:
@tyeth https://github.com/tyeth Before I look it over, do you have thoughts on this PR?
— Reply to this email directly, view it on GitHub https://github.com/adafruit/Adafruit_MQTT_Library/pull/190#issuecomment-1686767116, or unsubscribe https://github.com/notifications/unsubscribe-auth/ABTBZ47HMK3XOOZGRAN6G6LXWONHJANCNFSM42JQGOUQ . You are receiving this because you were mentioned.Message ID: @.***>
@brentru I think we might be okay, I'm fine with where the checks are, and the use of packet id seems like we wont run into conflicts. (Worth rechecking when we support QOS2 or mqtt5)